XXX - experiment

Always process boot images, so that's possible to clean up the repositories

software/kvm/buildout.hash.cfg

@@ -19,7 +19,7 @@ md5sum = cca7eb0f975da97491aa7803d5bd753b
 
 [template-kvm]
 filename = instance-kvm.cfg.jinja2
-md5sum = 34b32389f9a1d5e1a93e5d57d2ae674e
+md5sum = 95bce4aa8629d10ffa500ddebecd9da6
 
 [template-kvm-cluster]
 filename = instance-kvm-cluster.cfg.jinja2.in
@@ -99,4 +99,4 @@ md5sum = e9d40162ba77472775256637a2617d14
 
 [boot-image-select-source-config]
 filename = template/boot-image-select-source-config.json.in
-md5sum = 201c9ed07d7464e9f426a3ea7309a77d
+md5sum = a209bbfb03345153dae7b2e4fad22688

software/kvm/instance-kvm.cfg.jinja2

@@ -19,19 +19,11 @@
 {% set whitelist_domains = slapparameter_dict.get('whitelist-domains', '') -%}
 {% set virtual_hard_drive_url_enabled = 'virtual-hard-drive-url' in slapparameter_dict %}
 {% set virtual_hard_drive_url_gzipped = slapparameter_dict.get('virtual-hard-drive-gzipped', False) %}
-{% set boot_image_url_list_enabled = 'boot-image-url-list' in slapparameter_dict %}
-{% if 'boot-image-url-select' in slapparameter_dict %}
-{%   set boot_image_url_select_enabled = True %}
+{% if 'boot-image-url-select' not in slapparameter_dict and ('boot-image-url-list' in slapparameter_dict or 'nbd-host' in slapparameter_dict or 'nbd2-host' in slapparameter_dict or virtual_hard_drive_url_enabled)%}
+{%   set boot_image_url_select_default = '' %}
 {% else %}
-{%   if boot_image_url_list_enabled or 'nbd-host' in slapparameter_dict or 'nbd2-host' in slapparameter_dict or boot_image_url_list_enabled or virtual_hard_drive_url_enabled %}
-{%     set boot_image_url_select_enabled = False %}
-{%   else %}
-{#     Become default if no other way is used to provide the boot image #}
-{%     set boot_image_url_select_enabled = True %}
-{%   endif %}
+{%   set boot_image_url_select_default = 'Debian Bookworm 12 netinst x86_64' %}
 {% endif %}
-# boot_image_url_list_enabled = {{ boot_image_url_list_enabled }}
-# boot_image_url_select_enabled = {{ boot_image_url_select_enabled }}
 {% set bootstrap_script_url = slapparameter_dict.get('bootstrap-script-url') -%}
 {% set cpu_max_count = dumps(slapparameter_dict.get('cpu-max-count', int(slapparameter_dict.get('cpu-count', 2)) + 1)) %}
 {% set ram_max_size = dumps(slapparameter_dict.get('ram-max-size', int(slapparameter_dict.get('ram-size', 4096)) + 512)) %}
@@ -74,16 +66,12 @@ virtual-hard-drive-url-repository = ${:srv}/virtual-hard-drive-url-repository
 virtual-hard-drive-url-var = ${:var}/virtual-hard-drive-url
 virtual-hard-drive-url-expose = ${monitor-directory:private}/virtual-hard-drive-url
 {%- endif %}
-{%- if boot_image_url_list_enabled %}
 boot-image-url-list-repository = ${:srv}/boot-image-url-list-repository
 boot-image-url-list-var = ${:var}/boot-image-url-list
 boot-image-url-list-expose = ${monitor-directory:private}/boot-image-url-list
-{%- endif %}
-{%- if boot_image_url_select_enabled %}
 boot-image-url-select-repository = ${:srv}/boot-image-url-select-repository
 boot-image-url-select-var = ${:var}/boot-image-url-select
 boot-image-url-select-expose = ${monitor-directory:private}/boot-image-url-select
-{%- endif %}
 
 [create-mac]
 recipe = slapos.cookbook:generate.mac
@@ -99,7 +87,6 @@ storage-path = ${directory:srv}/.passwd
 # VNC protocol supports passwords of 8 characters max
 bytes = 8
 
-{% if boot_image_url_select_enabled %}
 ## boot-image-url-select support BEGIN
 [empty-file-state-base-select-promise]
 <= monitor-promise-base
@@ -111,9 +98,11 @@ config-url = ${monitor-base:base-url}/private/boot-image-url-select/${:filename}
 [boot-image-url-select-source-config]
 recipe = slapos.recipe.template:jinja2
 url = {{ boot_image_select_source_config }}
-boot-image-url-select = {{ dumps(slapparameter_dict.get('boot-image-url-select', 'Debian Bookworm 12 netinst x86_64')) }}
+boot-image-url-select = {{ dumps(slapparameter_dict.get('boot-image-url-select', '')) }}
+boot-image-url-select-default = {{ dumps(boot_image_url_select_default) }}
 context =
   key boot_image_url_select :boot-image-url-select
+  key boot_image_url_select_default :boot-image-url-select-default
 output = ${directory:etc}/boot-image-url-select.json
 
 [boot-image-url-select-processed-config]
@@ -191,9 +180,7 @@ config-filename = ${boot-image-url-select-download-wrapper:md5sum-state-file}
 filename = ${boot-image-url-select-download-wrapper:error-state-filename}
 config-filename = ${boot-image-url-select-download-wrapper:error-state-file}
 ## boot-image-url-select support END
-{% endif %} {# if boot_image_url_select_enabled #}
 
-{% if boot_image_url_list_enabled %}
 ## boot-image-url-list support BEGIN
 [empty-file-state-base-list-promise]
 <= monitor-promise-base
@@ -208,7 +195,7 @@ inline =
 {%- raw %}
   {{ boot_image_url_list }}
 {% endraw -%}
-boot-image-url-list = {{ dumps(slapparameter_dict['boot-image-url-list']) }}
+boot-image-url-list = {{ dumps(slapparameter_dict.get('boot-image-url-list', '')) }}
 context =
   key boot_image_url_list :boot-image-url-list
 output = ${directory:etc}/boot-image-url-list.conf
@@ -288,7 +275,6 @@ config-filename = ${boot-image-url-list-download-wrapper:md5sum-state-file}
 filename = ${boot-image-url-list-download-wrapper:error-state-filename}
 config-filename = ${boot-image-url-list-download-wrapper:error-state-file}
 ## boot-image-url-list support END
-{% endif %} {# if boot_image_url_list_enabled #}
 
 {% if virtual_hard_drive_url_enabled %}
 ## virtual-hard-drive-url support BEGIN
@@ -412,16 +398,8 @@ virtual-hard-drive-url-json-config = ${virtual-hard-drive-url-json-config:output
 {% else %}
 virtual-hard-drive-url-json-config =
 {% endif %}
-{% if boot_image_url_list_enabled %}
 boot-image-url-list-json-config = ${boot-image-url-list-json-config:output}
-{% else %}
-boot-image-url-list-json-config =
-{% endif %}
-{% if boot_image_url_select_enabled %}
 boot-image-url-select-json-config = ${boot-image-url-select-json-config:output}
-{% else %}
-boot-image-url-select-json-config =
-{% endif %}
 nbd-host = ${slap-parameter:nbd-host}
 nbd-port = ${slap-parameter:nbd-port}
 nbd2-host = ${slap-parameter:nbd2-host}
@@ -556,12 +534,8 @@ update-command = ${:command}
 command = [ ! -f {{ '${' + key + '}' }} ] && touch {{ '${' +  key + '}' }}
 {%- endmacro %}
 {#- Create depending sections, as state files appear late, so it's better to have empty file which will impact the hash anyway #}
-{%- if boot_image_url_list_enabled %}
 {{    generate_depend_section('boot-image-url-list-depend', 'boot-image-url-list-download-wrapper:config') }}
-{%- endif %}
-{%- if boot_image_url_select_enabled %}
 {{    generate_depend_section('boot-image-url-select-depend', 'boot-image-url-select-download-wrapper:config') }}
-{%- endif %}
 {%- if virtual_hard_drive_url_enabled %}
 {{    generate_depend_section('virtual-hard-drive-url-depend', 'virtual-hard-drive-url-download-wrapper:config') }}
 {%- endif %}
@@ -1292,20 +1266,16 @@ parts =
   virtual-hard-drive-url-download-state-promise
   virtual-hard-drive-url-processed-config-promise
 {% endif %}
-{% if boot_image_url_list_enabled %}
   boot-image-url-list-download-wrapper
   boot-image-url-list-config-state-promise
   boot-image-url-list-download-md5sum-promise
   boot-image-url-list-download-state-promise
   boot-image-url-list-processed-config-promise
-{% endif %}
-{% if boot_image_url_select_enabled %}
   boot-image-url-select-download-wrapper
   boot-image-url-select-config-state-promise
   boot-image-url-select-download-md5sum-promise
   boot-image-url-select-download-state-promise
   boot-image-url-select-processed-config-promise
-{% endif %}
 {% if additional_frontend %}
   frontend-additional-promise
 {% endif %}

software/kvm/template/boot-image-select-source-config.json.in

@@ -8,6 +8,15 @@
  "Fedora Server 32 netinst x86_64" : "https://shacache.nxdcdn.com/c5a511f349a1146b615e6fab9c24f9be4362046adcf24f0ff82c470d361fac5f6628895e2110ebf8ff87db49d4c413a0a332699da6b1bec64275e0c17a15b999#ca7a1e555c04b4d9a549065fa2ddf713",
  "FreeBSD 12.1 RELEASE bootonly x86_64" : "https://shacache.nxdcdn.com/6c355def68b3c0427f21598cb054ffc893568902f205601ac60f192854769b31bc9cff8eeb6ce99ef975a8fb887d8d3e56fc6cd5ea5cb4b3bba1175c520047cb#57088b77f795ca44b00971e44782ee23",
 } -%}
-["{{ IMAGE_URL_MAPPING.get(boot_image_url_select, '') }}"]
-{#- Note-XXX: Above is explicitely set to fail in case if wrong key is present, as this will force users to select correct image name, especially after upgrade -#}
-{#-           Idea dropped, as promise shall be used, and when buildout fails, no promise runs -#}
+{%- if boot_image_url_select %}
+{#-   Fail in the promise if bad boot-image-url-select is set -#}
+{%-   set boot_image = IMAGE_URL_MAPPING.get(boot_image_url_select, boot_image_url_select) %}
+{%- else %}
+{#-   Use default ONLY if no boot-image-url-select is set -#}
+{%-   set boot_image = IMAGE_URL_MAPPING.get(boot_image_url_select_default) %}
+{%- endif %}
+{%- if boot_image -%}
+["{{ boot_image }}"]
+{%- else -%}
+[]
+{%- endif -%}

software/kvm/test/test.py

@@ -156,6 +156,24 @@ class KVMTestCase(InstanceTestCase):
 
 
 class KvmMixin:
+  def assertPromiseFails(self, promise):
+    partition_directory = os.path.join(
+      self.slap.instance_directory,
+      self.kvm_instance_partition_reference)
+    monitor_run_promise = os.path.join(
+      partition_directory, 'software_release', 'bin',
+      'monitor.runpromise'
+    )
+    monitor_configuration = os.path.join(
+      partition_directory, 'etc', 'monitor.conf')
+
+    self.assertNotEqual(
+      0,
+      subprocess.call([
+        monitor_run_promise, '-c', monitor_configuration, '-a', '-f',
+        '--run-only', promise])
+    )
+
   def getRunningImageList(
       self, kvm_instance_partition,
       _match_cdrom=re.compile('file=(.+),media=cdrom$').match,
@@ -219,7 +237,9 @@ class KvmMixin:
     with self.assertRaises(SlapOSNodeCommandError):
       self.slap.waitForInstance(max_retry=max_retry)
 
-  def rerequestInstance(self, parameter_dict, state='started'):
+  def rerequestInstance(self, parameter_dict=None, state='started'):
+    if parameter_dict is None:
+      parameter_dict = {}
     software_url = self.getSoftwareURL()
     software_type = self.getInstanceSoftwareType()
     return self.slap.request(
@@ -236,7 +256,9 @@ class KvmMixinJson:
     return {
       '_': json.dumps(super().getInstanceParameterDict())}
 
-  def rerequestInstance(self, parameter_dict, *args, **kwargs):
+  def rerequestInstance(self, parameter_dict=None, *args, **kwargs):
+    if parameter_dict is None:
+      parameter_dict = {}
     return super().rerequestInstance(
       parameter_dict={'_': json.dumps(parameter_dict)},
       *args, **kwargs
@@ -348,7 +370,7 @@ class TestMemoryManagement(KVMTestCase, KvmMixin):
     self.assertNotEqual(kvm_pid_1, kvm_pid_2, "Unexpected: KVM not restarted")
 
   def tearDown(self):
-    self.rerequestInstance({})
+    self.rerequestInstance()
     self.slap.waitForInstance(max_retry=10)
 
   def test_enable_device_hotplug(self):
@@ -989,14 +1011,13 @@ class FakeImageServerMixin(KvmMixin):
 
 
 @skipUnlessKvm
-class TestBootImageUrlList(KVMTestCase, FakeImageServerMixin):
+class TestBootImageUrlList(FakeImageServerMixin, KVMTestCase):
   __partition_reference__ = 'biul'
   kvm_instance_partition_reference = 'biul0'
 
   # variations
   key = 'boot-image-url-list'
   test_input = "%s#%s\n%s#%s"
-  empty_input = ""
   image_directory = 'boot-image-url-list-repository'
   config_state_promise = 'boot-image-url-list-config-state-promise.py'
   download_md5sum_promise = 'boot-image-url-list-download-md5sum-promise.py'
@@ -1031,7 +1052,7 @@ class TestBootImageUrlList(KVMTestCase, FakeImageServerMixin):
   def tearDown(self):
     # clean up the instance for other tests
     # move instance to "default" state
-    self.rerequestInstance({})
+    self.rerequestInstance()
     self.slap.waitForInstance(max_retry=10)
     super().tearDown()
 
@@ -1057,7 +1078,6 @@ class TestBootImageUrlList(KVMTestCase, FakeImageServerMixin):
       [
         f'${{inst}}/srv/{self.image_directory}/{self.fake_image_md5sum}',
         f'${{inst}}/srv/{self.image_directory}/{self.fake_image2_md5sum}',
-        '${shared}/debian-${ver}-amd64-netinst.iso',
       ],
       self.getRunningImageList(kvm_instance_partition)
     )
@@ -1078,17 +1098,13 @@ class TestBootImageUrlList(KVMTestCase, FakeImageServerMixin):
       [
         f'${{inst}}/srv/{self.image_directory}/{self.fake_image3_md5sum}',
         f'${{inst}}/srv/{self.image_directory}/{self.fake_image2_md5sum}',
-        '${shared}/debian-${ver}-amd64-netinst.iso',
       ],
       self.getRunningImageList(kvm_instance_partition)
     )
 
     # cleanup of images works, also asserts that configuration changes are
     # reflected
-    # Note: key is left and empty_input is provided, as otherwise the part
-    #       which generate images is simply removed, which can lead to
-    #       leftover
-    self.rerequestInstance({self.key: self.empty_input})
+    self.rerequestInstance()
     self.slap.waitForInstance(max_retry=10)
     self.assertEqual(
       os.listdir(image_repository),
@@ -1097,28 +1113,13 @@ class TestBootImageUrlList(KVMTestCase, FakeImageServerMixin):
 
     # again only default image is available in the running process
     self.assertEqual(
-      ['${shared}/debian-${ver}-amd64-netinst.iso'],
+      [
+        '${inst}/srv/boot-image-url-select-repository/'
+        '326b7737c4262e8eb09cd26773f3356a'
+      ],
       self.getRunningImageList(kvm_instance_partition)
     )
 
-  def assertPromiseFails(self, promise):
-    partition_directory = os.path.join(
-      self.slap.instance_directory,
-      self.kvm_instance_partition_reference)
-    monitor_run_promise = os.path.join(
-      partition_directory, 'software_release', 'bin',
-      'monitor.runpromise'
-    )
-    monitor_configuration = os.path.join(
-      partition_directory, 'etc', 'monitor.conf')
-
-    self.assertNotEqual(
-      0,
-      subprocess.call([
-        monitor_run_promise, '-c', monitor_configuration, '-a', '-f',
-        '--run-only', promise])
-    )
-
   def test_bad_parameter(self):
     self.rerequestInstance({
       self.key: self.bad_value
@@ -1189,23 +1190,10 @@ class TestBootImageUrlSelect(FakeImageServerMixin, KVMTestCase):
   __partition_reference__ = 'bius'
   kvm_instance_partition_reference = 'bius0'
 
-  # variations
-  key = 'boot-image-url-select'
-  test_input = '"%s#%s"'
-  empty_input = ''
-  image_directory = 'boot-image-url-select-repository'
   config_state_promise = 'boot-image-url-select-config-state-promise.py'
-  download_md5sum_promise = 'boot-image-url-select-download-md5sum-promise.py'
-  download_state_promise = 'boot-image-url-select-download-state-promise.py'
-
-  bad_value = 'jsutbad'
-  incorrect_md5sum_value_image = '%s#'
-  incorrect_md5sum_value = '"url#asdasd"'
-  single_image_value = '%s#%s'
-  unreachable_host_value = 'evennotahost#%s'
 
   def test(self):
-    # check default image
+    # check the default image
     image_repository = os.path.join(
       self.slap.instance_directory, self.kvm_instance_partition_reference,
       'srv', 'boot-image-url-select-repository')
@@ -1249,9 +1237,9 @@ class TestBootImageUrlSelect(FakeImageServerMixin, KVMTestCase):
       self.getRunningImageList(self.computer_partition_root_path)
     )
 
-  def test_not_json(self):
+  def test_bad_image(self):
     self.rerequestInstance({
-      self.key: 'notjson#notjson'
+      'boot-image-url-select': 'DOESNOTEXISTS'
     })
     self.raising_waitForInstance(3)
     self.assertPromiseFails(self.config_state_promise)
@@ -1326,12 +1314,11 @@ class TestBootImageUrlSelect(FakeImageServerMixin, KVMTestCase):
       image_md5sum = hashlib.md5(fh.read()).hexdigest()
     self.assertEqual(image_md5sum, self.fake_image_md5sum)
 
-    image_repository = os.path.join(
-      self.slap.instance_directory, self.kvm_instance_partition_reference,
-      'srv', 'boot-image-url-select-repository')
     self.assertEqual(
-      ['b710c178eb434d79ce40ce703d30a5f0'],  # XXX: No cleanup after deselecting it
-      os.listdir(image_repository)
+      [],
+      os.listdir(os.path.join(
+        self.slap.instance_directory, self.kvm_instance_partition_reference,
+        'srv', 'boot-image-url-select-repository'))
     )
 
     kvm_instance_partition = os.path.join(
@@ -1347,7 +1334,7 @@ class TestBootImageUrlSelect(FakeImageServerMixin, KVMTestCase):
 
     # cleanup of images works, also asserts that configuration changes are
     # reflected
-    self.rerequestInstance({})
+    self.rerequestInstance()
     self.slap.waitForInstance(max_retry=10)
 
     self.assertEqual(
@@ -1369,7 +1356,6 @@ class TestBootImageUrlSelect(FakeImageServerMixin, KVMTestCase):
       ],
       self.getRunningImageList(kvm_instance_partition)
     )
-    self.fail('XXX: No cleanup after deselecting it')
 
 
 @skipUnlessKvm
@@ -2143,9 +2129,9 @@ class TestExternalDisk(KVMTestCase, ExternalDiskMixin):
     restarted_drive_list = self.getRunningDriveList(kvm_instance_partition)
     self.assertEqual(drive_list, restarted_drive_list)
     # prove that even on resetting parameters, drives are still there
-    self.rerequestInstance({}, state='stopped')
+    self.rerequestInstance(state='stopped')
     self.waitForInstance()
-    self.rerequestInstance({})
+    self.rerequestInstance()
     self.waitForInstance()
     dropped_drive_list = self.getRunningDriveList(kvm_instance_partition)
     self.assertEqual(drive_list, dropped_drive_list)