From 347986677801ccff484be424a541ad928193c8a4 Mon Sep 17 00:00:00 2001 From: Elod Illes Date: Tue, 18 Jun 2024 15:10:13 +0200 Subject: [PATCH 1/2] [CI] Replace deprecated regex and remove Centos8 and pin xena-last Latest Zuul drops the following warnings: All regular expressions must conform to RE2 syntax, but an expression using the deprecated Perl-style syntax has been detected. Adjust the configuration to conform to RE2 syntax. The RE2 syntax error is: invalid perl operator: (?! This patch replaces the 'irrelevant-files' to 'files' with explicitly listing the pattern which files should be the tests run against. Conflicts: .zuul.yaml NOTE(elod.illes): parts of the Xena version of this patch were not needed as they were not introduced in Wallaby yet. Also, devstack-gate based base job is dropped as it is not used in zuul.yaml at all and devstack-gate is retired and removed completely. Change-Id: If287e800fb9ff428dbe6f9c4c046627f22afe3df (cherry picked from commit 9b77bae8a32ff41712b96bb6a67c7eacae45a4c9) (cherry picked from commit 8223e6a7c429441c28178316e455767a66d3e8f8) (cherry picked from commit 510a27ba36fc47309308228bc45b5b9ea6ba695b) (cherry picked from commit 197b14d7659252c62b7436bcdd2a9b8c8b470771) (cherry picked from commit ffc252eeae01f4829a92a0549b47fb9e4175c2da) (cherry picked from commit 23fc1b9206691a4b5861f49db46d48487acb8187) (cherry picked from commit 6fadd9980d53d7d93852d0622974bcf5c9a4a19f) --- .zuul.yaml | 77 ++++++++++++++++++------------------------------------ 1 file changed, 26 insertions(+), 51 deletions(-) diff --git a/.zuul.yaml b/.zuul.yaml index 401f6f9f895..04c960ac40b 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -1,34 +1,6 @@ # See https://docs.openstack.org/infra/manual/drivers.html#naming-with-zuul-v3 # for job naming conventions. -- job: - name: nova-dsvm-multinode-base - parent: legacy-dsvm-base-multinode - description: | - Base job for multinode nova devstack/tempest jobs. - Will setup firewall rules on all the nodes allowing them to talk to - each other. - timeout: 10800 - required-projects: - - openstack/devstack-gate - - openstack/nova - - openstack/tempest - irrelevant-files: &dsvm-irrelevant-files - - ^api-.*$ - - ^(test-|)requirements.txt$ - - ^.*\.rst$ - - ^.git.*$ - - ^doc/.*$ - - ^nova/hacking/.*$ - - ^nova/locale/.*$ - - ^nova/policies/.*$ - - ^nova/tests/.*$ - - ^nova/test.py$ - - ^releasenotes/.*$ - - ^setup.cfg$ - - ^tools/.*$ - - ^tox.ini$ - - job: name: nova-tox-functional-py38 parent: openstack-tox-functional-py38 @@ -95,7 +67,21 @@ description: | Run tempest live migration tests against local qcow2 ephemeral storage and shared LVM/iSCSI cinder volumes. - irrelevant-files: *dsvm-irrelevant-files + irrelevant-files: &dsvm-irrelevant-files + - ^api-.*$ + - ^(test-|)requirements.txt$ + - ^.*\.rst$ + - ^.git.*$ + - ^doc/.*$ + - ^nova/hacking/.*$ + - ^nova/locale/.*$ + - ^nova/policies/.*$ + - ^nova/tests/.*$ + - ^nova/test.py$ + - ^releasenotes/.*$ + - ^setup.cfg$ + - ^tools/.*$ + - ^tox.ini$ vars: tox_envlist: all tempest_test_regex: (^tempest\.api\.compute\.admin\.(test_live_migration|test_migration)) @@ -189,24 +175,11 @@ parent: devstack-tempest description: | Run tempest compute API tests using LVM image backend. This only runs - against nova/virt/libvirt/* changes. - # Copy irrelevant-files from nova-dsvm-multinode-base and then exclude - # anything that is not in nova/virt/libvirt/* or nova/privsep/*. - irrelevant-files: - - ^(?!.zuul.yaml)(?!nova/virt/libvirt/)(?!nova/privsep/).*$ - - ^api-.*$ - - ^(test-|)requirements.txt$ - - ^.*\.rst$ - - ^.git.*$ - - ^doc/.*$ - - ^nova/hacking/.*$ - - ^nova/locale/.*$ - - ^nova/tests/.*$ - - ^nova/test.py$ - - ^releasenotes/.*$ - - ^setup.cfg$ - - ^tools/.*$ - - ^tox.ini$ + against nova/virt/libvirt/*, nova/privsep/* and .zuul.yaml changes. + files: + - ^nova/virt/libvirt/.*$ + - ^nova/privsep/.*$ + - .zuul.yaml vars: # We use the "all" environment for tempest_test_regex and # tempest_black_regex. @@ -528,11 +501,12 @@ - nova-ceph-multistore: irrelevant-files: *dsvm-irrelevant-files - neutron-tempest-linuxbridge: - irrelevant-files: + files: # NOTE(mriedem): This job has its own irrelevant-files section # so that we only run it on changes to networking and libvirt/vif # code; we don't need to run this on all changes. - - ^(?!nova/network/.*)(?!nova/virt/libvirt/vif.py).*$ + - ^nova/network/.*$ + - nova/virt/libvirt/vif.py - nova-live-migration - nova-lvm - nova-multi-cell @@ -582,11 +556,12 @@ - nova-ceph-multistore: irrelevant-files: *dsvm-irrelevant-files - neutron-tempest-linuxbridge: - irrelevant-files: + files: # NOTE(mriedem): This job has its own irrelevant-files section # so that we only run it on changes to networking and libvirt/vif # code; we don't need to run this on all changes. - - ^(?!nova/network/.*)(?!nova/virt/libvirt/vif.py).*$ + - ^nova/network/.*$ + - nova/virt/libvirt/vif.py - tempest-integrated-compute: irrelevant-files: *policies-irrelevant-files - nova-grenade-multinode: From ae915a9abb0779a3128bce15d3939c951508e784 Mon Sep 17 00:00:00 2001 From: Bence Romsics Date: Wed, 2 Aug 2023 16:22:55 +0200 Subject: [PATCH 2/2] Do not untrack resources of a server being unshelved This patch concerns the time when a VM is being unshelved and the compute manager set the task_state to spawning, claimed resources of the VM and then called driver.spawn(). So the instance is in vm_state SHELVED_OFFLOADED, task_state spawning. If at this point a new update_available_resource periodic job is started that collects all the instances assigned to the node to calculate resource usage. However the calculation assumed that a VM in SHELVED_OFFLOADED state does not need resource allocation on the node (probably being removed from the node as it is offloaded) and deleted the resource claim. Given all this we ended up with the VM spawned successfully but having lost the resource claim on the node. This patch changes what we do in vm_state SHELVED_OFFLOADED, task_state spawning. We no longer delete the resource claim in this state and keep tracking the resource in stats. Change-Id: I8c9944810c09d501a6d3f60f095d9817b756872d Closes-Bug: #2025480 (cherry picked from commit f1dc4ec39bcfda1bd4b97e233a9da498b6378c4f) (cherry picked from commit 4239d1fec2814c074482b740a2fd38a5d5ce6942) (cherry picked from commit 683ecc060e3bca818b9fb514d297e323bc8cb220) (cherry picked from commit e41962f5fa59c47e63468945e82c2e7164c24c38) (cherry picked from commit 22dfd1fa42aaff6bd0a63b61d47b569422a5be14) (cherry picked from commit 26a9cae4a5212b2938ab1a9e87a0802e0e1112cb) --- nova/compute/manager.py | 6 +++--- nova/compute/resource_tracker.py | 7 +++++-- nova/compute/stats.py | 3 ++- nova/compute/vm_states.py | 11 +++++++++-- .../functional/regressions/test_bug_2025480.py | 5 ++--- nova/tests/unit/compute/test_stats.py | 16 ++++++++++++++++ 6 files changed, 37 insertions(+), 11 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 440dbcf354d..4b8e4f9568a 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -6488,9 +6488,9 @@ def _shelve_offload_instance(self, context, instance, clean_shutdown, instance.power_state = current_power_state # NOTE(mriedem): The vm_state has to be set before updating the - # resource tracker, see vm_states.ALLOW_RESOURCE_REMOVAL. The host/node - # values cannot be nulled out until after updating the resource tracker - # though. + # resource tracker, see vm_states.allow_resource_removal(). The + # host/node values cannot be nulled out until after updating the + # resource tracker though. instance.vm_state = vm_states.SHELVED_OFFLOADED instance.task_state = None instance.save(expected_task_state=[task_states.SHELVING, diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index 0febdd717f4..2f5148035e3 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -1465,7 +1465,8 @@ def _update_usage_from_instance(self, context, instance, nodename, # NOTE(sfinucan): Both brand new instances as well as instances that # are being unshelved will have is_new_instance == True is_removed_instance = not is_new_instance and (is_removed or - instance['vm_state'] in vm_states.ALLOW_RESOURCE_REMOVAL) + vm_states.allow_resource_removal( + vm_state=instance['vm_state'], task_state=instance.task_state)) if is_new_instance: self.tracked_instances.add(uuid) @@ -1524,7 +1525,9 @@ def _update_usage_from_instances(self, context, instances, nodename): instance_by_uuid = {} for instance in instances: - if instance.vm_state not in vm_states.ALLOW_RESOURCE_REMOVAL: + if not vm_states.allow_resource_removal( + vm_state=instance['vm_state'], + task_state=instance.task_state): self._update_usage_from_instance(context, instance, nodename) instance_by_uuid[instance.uuid] = instance return instance_by_uuid diff --git a/nova/compute/stats.py b/nova/compute/stats.py index cfbee2e6bc1..e9180ec6d6d 100644 --- a/nova/compute/stats.py +++ b/nova/compute/stats.py @@ -105,7 +105,8 @@ def update_stats_for_instance(self, instance, is_removed=False): (vm_state, task_state, os_type, project_id) = \ self._extract_state_from_instance(instance) - if is_removed or vm_state in vm_states.ALLOW_RESOURCE_REMOVAL: + if is_removed or vm_states.allow_resource_removal( + vm_state=vm_state, task_state=task_state): self._decrement("num_instances") self.states.pop(uuid) else: diff --git a/nova/compute/vm_states.py b/nova/compute/vm_states.py index 633894c1ea4..1c4da06d155 100644 --- a/nova/compute/vm_states.py +++ b/nova/compute/vm_states.py @@ -27,6 +27,7 @@ See http://wiki.openstack.org/VMState """ +from nova.compute import task_states from nova.objects import fields @@ -74,5 +75,11 @@ # states we allow to trigger crash dump ALLOW_TRIGGER_CRASH_DUMP = [ACTIVE, PAUSED, RESCUED, RESIZED, ERROR] -# states we allow resources to be freed in -ALLOW_RESOURCE_REMOVAL = [DELETED, SHELVED_OFFLOADED] + +def allow_resource_removal(vm_state, task_state=None): + """(vm_state, task_state) combinations we allow resources to be freed in""" + + return ( + vm_state == DELETED or + vm_state == SHELVED_OFFLOADED and task_state != task_states.SPAWNING + ) diff --git a/nova/tests/functional/regressions/test_bug_2025480.py b/nova/tests/functional/regressions/test_bug_2025480.py index 45b201ce45e..77699275c46 100644 --- a/nova/tests/functional/regressions/test_bug_2025480.py +++ b/nova/tests/functional/regressions/test_bug_2025480.py @@ -77,6 +77,5 @@ def fake_spawn(*args, **kwargs): node = compute_node.ComputeNode.get_by_nodename( context.get_admin_context(), 'compute1') - # This is the bug, the instance should have resources claimed - # self.assertEqual(1, node.vcpus_used) - self.assertEqual(0, node.vcpus_used) + # After the fix, the instance should have resources claimed + self.assertEqual(1, node.vcpus_used) diff --git a/nova/tests/unit/compute/test_stats.py b/nova/tests/unit/compute/test_stats.py index e713794a19a..b95475f09db 100644 --- a/nova/tests/unit/compute/test_stats.py +++ b/nova/tests/unit/compute/test_stats.py @@ -208,6 +208,22 @@ def test_update_stats_for_instance_offloaded(self): self.assertEqual(0, self.stats.num_os_type("Linux")) self.assertEqual(0, self.stats["num_vm_" + vm_states.BUILDING]) + def test_update_stats_for_instance_being_unshelved(self): + instance = self._create_instance() + self.stats.update_stats_for_instance(instance) + self.assertEqual(1, self.stats.num_instances_for_project("1234")) + + instance["vm_state"] = vm_states.SHELVED_OFFLOADED + instance["task_state"] = task_states.SPAWNING + self.stats.update_stats_for_instance(instance) + + self.assertEqual(1, self.stats.num_instances) + self.assertEqual(1, self.stats.num_instances_for_project(1234)) + self.assertEqual(1, self.stats["num_os_type_Linux"]) + self.assertEqual(1, self.stats["num_vm_%s" % + vm_states.SHELVED_OFFLOADED]) + self.assertEqual(1, self.stats["num_task_%s" % task_states.SPAWNING]) + def test_io_workload(self): vms = [vm_states.ACTIVE, vm_states.BUILDING, vm_states.PAUSED] tasks = [task_states.RESIZE_MIGRATING, task_states.REBUILDING,