From a5e5529a678b6a318823ebe0be9a57f5d6794e82 Mon Sep 17 00:00:00 2001
From: Takashi Kajinami <kajinamit@oss.nttdata.com>
Date: Wed, 10 Jul 2024 01:29:24 +0900
Subject: [PATCH] More strictly validate cpu_mode and cpu_models

The nova-compute service fails to start in case
 cpu_mode is 'custom' but cpu_models is default or empty
or
 cpu_models is configured but cpu_mode is not 'custom'

Make the deployment hard-fail in case the given values are not accepted
by nova, to make users aware of their misconfiguration.

Change-Id: Iea4fb9990981ec0dc14a0dabd9674aa3b27a7137
---
 manifests/compute/libvirt.pp              | 14 ++++++++------
 spec/classes/nova_compute_libvirt_spec.rb | 17 ++++++++++++++++-
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/manifests/compute/libvirt.pp b/manifests/compute/libvirt.pp
index 23f158aec..365c8597c 100644
--- a/manifests/compute/libvirt.pp
+++ b/manifests/compute/libvirt.pp
@@ -29,7 +29,7 @@
 #
 # [*cpu_models*]
 #   (optional) The named libvirt CPU models (see names listed in
-#   /usr/share/libvirt/cpu_map.xml). Only has effect if
+#   /usr/share/libvirt/cpu_map.xml). This option requires
 #   cpu_mode="custom" and virt_type="kvm|qemu".
 #   Defaults to []
 #
@@ -398,16 +398,18 @@ class nova::compute::libvirt (
     'libvirt/tb_cache_size':                 value => $tb_cache_size;
   }
 
-  # cpu_model param is only valid if cpu_mode=custom
-  # otherwise it should be commented out
-  if $cpu_mode_real == 'custom' and !empty($cpu_models){
+  if !empty($cpu_models) {
+    if $cpu_mode_real != 'custom' {
+      fail('$cpu_models requires that $cpu_mode is "custom"')
+    }
     $cpu_models_real = join($cpu_models, ',')
   } else {
-    if !empty($cpu_models) {
-      warning('$cpu_models requires that $cpu_mode => "custom" and will be ignored')
+    if $cpu_mode_real == 'custom' {
+      fail('$cpu_models is required when $cpu_mode is "custom"')
     }
     $cpu_models_real = $facts['os_service_default']
   }
+
   nova_config {
     'libvirt/cpu_models': value => $cpu_models_real;
   }
diff --git a/spec/classes/nova_compute_libvirt_spec.rb b/spec/classes/nova_compute_libvirt_spec.rb
index 6865e689d..a2d3e2acf 100644
--- a/spec/classes/nova_compute_libvirt_spec.rb
+++ b/spec/classes/nova_compute_libvirt_spec.rb
@@ -65,7 +65,6 @@ describe 'nova::compute::libvirt' do
           :virt_type                     => 'qemu',
           :vncserver_listen              => '0.0.0.0',
           :cpu_mode                      => 'host-passthrough',
-          :cpu_models                    => ['kvm64', 'qemu64'],
           :cpu_model_extra_flags         => 'pcid',
           :cpu_power_management          => false,
           :cpu_power_management_strategy => 'cpu_state',
@@ -165,6 +164,22 @@ describe 'nova::compute::libvirt' do
       it { is_expected.to contain_nova_config('libvirt/cpu_model_extra_flags').with_value('pcid')}
     end
 
+    context 'with non-custom cpu_node and cpu_models' do
+      let :params do
+        { :cpu_models => ['kvm64', 'qemu64'] }
+      end
+
+      it { should raise_error(Puppet::Error, /\$cpu_models requires that \$cpu_mode is "custom"/) }
+    end
+
+    context 'with custom cpu_node and no cpu_models' do
+      let :params do
+        { :cpu_mode => 'custom' }
+      end
+
+      it { should raise_error(Puppet::Error, /\$cpu_models is required when \$cpu_mode is "custom"/) }
+    end
+
     context 'with hw_machine_type set by array' do
       let :params do
         { :hw_machine_type => ['x86_64=machinetype1', 'armv7l=machinetype2'] }