From be6d22b40986f0d0030c02cce43057aa8fa2a281 Mon Sep 17 00:00:00 2001 From: Ayumu Ueha Date: Mon, 5 Dec 2022 14:12:44 +0000 Subject: [PATCH] Fix waiting for pod removal when container update This patch fixes the problem of aborting the wait process even if there are pods left to be removed by replace when container update feature. Closes-Bug: #1998756 Change-Id: I6bc5c10fe4c7f646b84798d3b4a296721ccea0f8 --- .../container_update/container_update_mgmt.py | 61 ++++++++++++++- .../TOSCA-Metadata/TOSCA.meta | 2 +- .../TOSCA-Metadata/TOSCA.meta | 2 +- tacker/tests/unit/vnfm/mgmt_drivers/fakes.py | 30 ++++++-- .../test_container_update_mgmt.py | 75 +++++++++++++++---- 5 files changed, 144 insertions(+), 26 deletions(-) diff --git a/samples/mgmt_driver/kubernetes/container_update/container_update_mgmt.py b/samples/mgmt_driver/kubernetes/container_update/container_update_mgmt.py index ca0f15680..50b23f8ad 100644 --- a/samples/mgmt_driver/kubernetes/container_update/container_update_mgmt.py +++ b/samples/mgmt_driver/kubernetes/container_update/container_update_mgmt.py @@ -156,15 +156,50 @@ class ContainerUpdateMgmtDriver(vnflcm_abstract_driver. return response + def _read_scale_api(self, k8s_clients, namespace, k8s_obj): + # get api + name = k8s_obj['object'].metadata.name + kind = k8s_obj['object'].kind + api_version = k8s_obj['object'].api_version + + def convert(name): + name_with_underscores = re.sub( + '(.)([A-Z][a-z]+)', r'\1_\2', name) + return re.sub('([a-z0-9])([A-Z])', r'\1_\2', + name_with_underscores).lower() + + snake_case_kind = convert(kind) + try: + read_scale_api = eval(f'k8s_clients["{api_version}"].' + f'read_namespaced_{snake_case_kind}_scale') + response = read_scale_api(name=name, namespace=namespace) + except Exception as exp: + raise exceptions.MgmtDriverOtherError( + error_message=encodeutils.exception_to_unicode(exp)) + + return response + def _replace_wait_k8s(self, kube_driver, k8s_pod_objs, - core_v1_api_client, vnf_instance): + k8s_clients, vnf_instance): try: time.sleep(kube_driver.STACK_RETRY_WAIT) status = 'Pending' stack_retries = kube_driver.STACK_RETRIES + core_v1_api_client = k8s_clients['v1'] + + # get replicas of scalable resources for checking number of pod + scalable_kinds = ("Deployment", "ReplicaSet") + for k8s_pod_obj in k8s_pod_objs: + if k8s_pod_obj['object'].kind in scalable_kinds: + scale_info = self._read_scale_api( + k8s_clients=k8s_clients, + namespace=k8s_pod_obj.get('namespace'), + k8s_obj=k8s_pod_obj) + k8s_pod_obj['replicas'] = scale_info.spec.replicas while status == 'Pending' and stack_retries > 0: pods_information = [] + unmatch_pods_num_flag = False for k8s_pod_obj in k8s_pod_objs: kind = k8s_pod_obj['object'].kind namespace = k8s_pod_obj.get('namespace') @@ -175,11 +210,29 @@ class ContainerUpdateMgmtDriver(vnflcm_abstract_driver. response = core_v1_api_client.list_namespaced_pod( namespace=namespace) + tmp_pods_info = [] for pod in response.items: match_result = kube_driver.is_match_pod_naming_rule( kind, name, pod.metadata.name) if match_result: - pods_information.append(pod) + tmp_pods_info.append(pod) + # NOTE(ueha): The status of pod being replaced is retrieved + # as "Running", which cause incorrect information to be + # stored in vnfcResouceInfo. Therefore, for the scalable + # kinds, by comparing the actual number of pods with the + # replicas, it can wait until the pod deletion is complete + # and store correct information to vnfcResourceInfo. + if (k8s_pod_obj['object'].kind in scalable_kinds and + k8s_pod_obj.get('replicas') != len(tmp_pods_info)): + LOG.warning("Unmatch number of pod. (kind: %(kind)s," + " name: %(name)s, replicas: %(replicas)s," + " actual_pod_num: %(actual_pod_num)s)", { + 'kind': k8s_pod_obj['object'].kind, + 'name': k8s_pod_obj['object'].metadata.name, + 'replicas': str(k8s_pod_obj.get('replicas')), + 'actual_pod_num': str(len(tmp_pods_info))}) + unmatch_pods_num_flag = True + pods_information.extend(tmp_pods_info) status = kube_driver.get_pod_status(pods_information) if status == 'Unknown': wait = (kube_driver.STACK_RETRIES * @@ -190,7 +243,7 @@ class ContainerUpdateMgmtDriver(vnflcm_abstract_driver. f"{vnf_instance.id} is not completed") raise exceptions.MgmtDriverOtherError( error_message=error_reason) - if status == 'Pending': + if status == 'Pending' or unmatch_pods_num_flag: stack_retries = stack_retries - 1 time.sleep(kube_driver.STACK_RETRY_WAIT) if stack_retries == 0 and status != 'Running': @@ -371,7 +424,7 @@ class ContainerUpdateMgmtDriver(vnflcm_abstract_driver. body=pod) # _replace_wait_k8s - self._replace_wait_k8s(kube_driver, k8s_pod_objs, core_v1_api_client, + self._replace_wait_k8s(kube_driver, k8s_pod_objs, k8s_clients, vnf_instance) # Get all pod information under the specified namespace pods = core_v1_api_client.list_namespaced_pod(namespace=namespace) diff --git a/tacker/tests/etc/samples/etsi/nfv/test_cnf_container_update_after/TOSCA-Metadata/TOSCA.meta b/tacker/tests/etc/samples/etsi/nfv/test_cnf_container_update_after/TOSCA-Metadata/TOSCA.meta index 658be7a44..fbe772a49 100644 --- a/tacker/tests/etc/samples/etsi/nfv/test_cnf_container_update_after/TOSCA-Metadata/TOSCA.meta +++ b/tacker/tests/etc/samples/etsi/nfv/test_cnf_container_update_after/TOSCA-Metadata/TOSCA.meta @@ -66,4 +66,4 @@ Hash: 2a1b16ab7e9ee34a23a67a3aa0535311c927f4194b639bcb0a70905578940695 Name: Scripts/container_update_mgmt.py Content-Type: text/x-python Algorithm: SHA-256 -Hash: 334316e69326a0aaad58dc339bc3541786f96bbf1345e9770759d9cab60382f9 +Hash: fdfdd05769a3b95e2ff5b8261cb51ebe19fa6d7d816d28d94f0c5af231e5366f diff --git a/tacker/tests/etc/samples/etsi/nfv/test_cnf_container_update_before/TOSCA-Metadata/TOSCA.meta b/tacker/tests/etc/samples/etsi/nfv/test_cnf_container_update_before/TOSCA-Metadata/TOSCA.meta index f6512fd47..8850b33f4 100644 --- a/tacker/tests/etc/samples/etsi/nfv/test_cnf_container_update_before/TOSCA-Metadata/TOSCA.meta +++ b/tacker/tests/etc/samples/etsi/nfv/test_cnf_container_update_before/TOSCA-Metadata/TOSCA.meta @@ -66,4 +66,4 @@ Hash: 2a1b16ab7e9ee34a23a67a3aa0535311c927f4194b639bcb0a70905578940695 Name: Scripts/container_update_mgmt.py Content-Type: text/x-python Algorithm: SHA-256 -Hash: 334316e69326a0aaad58dc339bc3541786f96bbf1345e9770759d9cab60382f9 +Hash: fdfdd05769a3b95e2ff5b8261cb51ebe19fa6d7d816d28d94f0c5af231e5366f diff --git a/tacker/tests/unit/vnfm/mgmt_drivers/fakes.py b/tacker/tests/unit/vnfm/mgmt_drivers/fakes.py index c11afd50a..ac3ef9425 100644 --- a/tacker/tests/unit/vnfm/mgmt_drivers/fakes.py +++ b/tacker/tests/unit/vnfm/mgmt_drivers/fakes.py @@ -274,8 +274,8 @@ def fake_replicaset_container_config_changed(): ) ) ), - status=client.V1PodStatus( - phase='Running', + status=client.V1ReplicaSetStatus( + replicas=1, ) ) @@ -345,8 +345,8 @@ def fake_replicaset_volume_config_changed(): ) ) ), - status=client.V1PodStatus( - phase='Running', + status=client.V1ReplicaSetStatus( + replicas=1, ) ) @@ -393,8 +393,8 @@ def fake_replicaset_image_changed(): ) ) ), - status=client.V1PodStatus( - phase='Running', + status=client.V1ReplicaSetStatus( + replicas=1, ) ) @@ -538,3 +538,21 @@ def return_cnf_additional_params(): "namespace": "default" } return additional_params + + +def fake_k8s_clients(): + k8s_clients = { + 'v1': client.CoreV1Api(), + 'apiregistration.k8s.io/v1': client.ApiregistrationV1Api(), + 'apps/v1': client.AppsV1Api(), + 'authentication.k8s.io/v1': client.AuthenticationV1Api(), + 'authorization.k8s.io/v1': client.AuthorizationV1Api(), + 'autoscaling/v1': client.AutoscalingV1Api(), + 'batch/v1': client.BatchV1Api(), + 'coordination.k8s.io/v1': client.CoordinationV1Api(), + 'networking.k8s.io/v1': client.NetworkingV1Api(), + 'rbac.authorization.k8s.io/v1': client.RbacAuthorizationV1Api(), + 'scheduling.k8s.io/v1': client.SchedulingV1Api(), + 'storage.k8s.io/v1': client.StorageV1Api() + } + return k8s_clients diff --git a/tacker/tests/unit/vnfm/mgmt_drivers/test_container_update_mgmt.py b/tacker/tests/unit/vnfm/mgmt_drivers/test_container_update_mgmt.py index 24e19eb46..81faea4ca 100644 --- a/tacker/tests/unit/vnfm/mgmt_drivers/test_container_update_mgmt.py +++ b/tacker/tests/unit/vnfm/mgmt_drivers/test_container_update_mgmt.py @@ -115,7 +115,7 @@ class TestContainerUpdate(base.TestCase): self.assertRaises( exceptions.MgmtDriverOtherError, self.cntr_update_mgmt._replace_wait_k8s, kube_driver, - k8s_pod_objs, client.CoreV1Api, self.vnf_instance) + k8s_pod_objs, mgmt_fakes.fake_k8s_clients(), self.vnf_instance) @mock.patch.object(client.CoreV1Api, 'list_namespaced_pod') def test_container_update_replace_wait_k8s_unknown(self, mock_list_pod): @@ -129,7 +129,25 @@ class TestContainerUpdate(base.TestCase): self.assertRaises( exceptions.MgmtDriverOtherError, self.cntr_update_mgmt._replace_wait_k8s, kube_driver, - k8s_pod_objs, client.CoreV1Api, self.vnf_instance) + k8s_pod_objs, mgmt_fakes.fake_k8s_clients(), self.vnf_instance) + + @mock.patch.object(client.AppsV1Api, 'read_namespaced_replica_set') + @mock.patch.object(client.CoreV1Api, 'list_namespaced_pod') + def test_container_update_replace_wait_unmatch_podnum( + self, mock_list_pod, mock_read_namespaced_replicaset_scale): + k8s_pod_objs = [ + {'namespace': 'default', + 'object': mgmt_fakes.fake_replicaset_container_config_changed()} + ] + kube_driver = Kubernetes() + mock_list_pod.return_value = mgmt_fakes.fake_list_pod() + mock_read_namespaced_replicaset_scale.return_value = client.V1Scale( + spec=client.V1ScaleSpec(replicas=1), + status=client.V1ScaleStatus(replicas=1)) + self.assertRaises( + exceptions.MgmtDriverOtherError, + self.cntr_update_mgmt._replace_wait_k8s, kube_driver, + k8s_pod_objs, mgmt_fakes.fake_k8s_clients(), self.vnf_instance) @mock.patch('tacker.objects.vnf_instance.VnfInstance.save') @mock.patch('tacker.vnflcm.utils._get_vnfd_dict') @@ -160,6 +178,7 @@ class TestContainerUpdate(base.TestCase): @mock.patch('tacker.objects.vnf_instance.VnfInstance.save') @mock.patch('tacker.vnflcm.utils._get_vnfd_dict') + @mock.patch.object(client.AppsV1Api, 'read_namespaced_replica_set_scale') @mock.patch.object(client.CoreV1Api, 'list_namespaced_pod') @mock.patch.object(client.CoreV1Api, 'replace_namespaced_secret') @mock.patch.object(client.CoreV1Api, 'replace_namespaced_config_map') @@ -167,17 +186,24 @@ class TestContainerUpdate(base.TestCase): @mock.patch.object(client.CoreV1Api, 'read_namespaced_pod') @mock.patch.object(client.AppsV1Api, 'read_namespaced_replica_set') @mock.patch.object(client.AppsV1Api, 'replace_namespaced_replica_set') + @mock.patch.object(client.CoreV1Api, 'delete_namespaced_pod') def test_container_update_modify_information_end_container_config_changed( - self, mock_replace_replica_set, mock_read_replicaset, + self, mock_delete_namespaced_pod, + mock_replace_replica_set, mock_read_replicaset, mock_read_pod, mock_replace_pod, mock_replace_config_map, - mock_replace_secret, mock_list_pod, mock_vnfd_dict, mock_save, + mock_replace_secret, mock_list_pod, + mock_read_namespaced_replicaset_scale, mock_vnfd_dict, mock_save, ): mock_read_replicaset.return_value = (mgmt_fakes. fake_replicaset_container_config_changed()) mock_read_pod.return_value = (mgmt_fakes. fake_pod_container_config_changed()) mock_list_pod.return_value = client.V1PodList(items=[ - mgmt_fakes.get_fake_pod_info(kind='Pod', name='vdu1')]) + mgmt_fakes.get_fake_pod_info(kind='Pod', name='vdu1'), + mgmt_fakes.get_fake_pod_info(kind='ReplicaSet', name='vdu2')]) + mock_read_namespaced_replicaset_scale.return_value = client.V1Scale( + spec=client.V1ScaleSpec(replicas=1), + status=client.V1ScaleStatus(replicas=1)) mock_vnfd_dict.return_value = fakes.vnfd_dict_cnf() kwargs = { 'old_vnf_package_path': self.yaml_path_before, @@ -192,6 +218,7 @@ class TestContainerUpdate(base.TestCase): @mock.patch('tacker.objects.vnf_instance.VnfInstance.save') @mock.patch('tacker.vnflcm.utils._get_vnfd_dict') + @mock.patch.object(client.AppsV1Api, 'read_namespaced_replica_set_scale') @mock.patch.object(client.CoreV1Api, 'list_namespaced_pod') @mock.patch.object(client.CoreV1Api, 'replace_namespaced_secret') @mock.patch.object(client.CoreV1Api, 'replace_namespaced_config_map') @@ -199,17 +226,24 @@ class TestContainerUpdate(base.TestCase): @mock.patch.object(client.CoreV1Api, 'read_namespaced_pod') @mock.patch.object(client.AppsV1Api, 'read_namespaced_replica_set') @mock.patch.object(client.AppsV1Api, 'replace_namespaced_replica_set') + @mock.patch.object(client.CoreV1Api, 'delete_namespaced_pod') def test_container_update_modify_information_end_volume_config_changed( - self, mock_replace_replica_set, mock_read_replicaset, + self, mock_delete_namespaced_pod, + mock_replace_replica_set, mock_read_replicaset, mock_read_pod, mock_replace_pod, mock_replace_config_map, - mock_replace_secret, mock_list_pod, mock_vnfd_dict, mock_save, + mock_replace_secret, mock_list_pod, + mock_read_namespaced_replicaset_scale, mock_vnfd_dict, mock_save, ): mock_read_replicaset.return_value = (mgmt_fakes. fake_replicaset_volume_config_changed()) mock_read_pod.return_value = (mgmt_fakes. fake_pod_volume_config_changed()) mock_list_pod.return_value = client.V1PodList(items=[ - mgmt_fakes.get_fake_pod_info(kind='Pod', name='vdu1')]) + mgmt_fakes.get_fake_pod_info(kind='Pod', name='vdu1'), + mgmt_fakes.get_fake_pod_info(kind='ReplicaSet', name='vdu2')]) + mock_read_namespaced_replicaset_scale.return_value = client.V1Scale( + spec=client.V1ScaleSpec(replicas=1), + status=client.V1ScaleStatus(replicas=1)) mock_vnfd_dict.return_value = fakes.vnfd_dict_cnf() kwargs = { 'old_vnf_package_path': self.yaml_path_before, @@ -224,6 +258,7 @@ class TestContainerUpdate(base.TestCase): @mock.patch('tacker.objects.vnf_instance.VnfInstance.save') @mock.patch('tacker.vnflcm.utils._get_vnfd_dict') + @mock.patch.object(client.AppsV1Api, 'read_namespaced_replica_set_scale') @mock.patch.object(client.CoreV1Api, 'list_namespaced_pod') @mock.patch.object(client.CoreV1Api, 'replace_namespaced_secret') @mock.patch.object(client.CoreV1Api, 'replace_namespaced_config_map') @@ -231,16 +266,23 @@ class TestContainerUpdate(base.TestCase): @mock.patch.object(client.CoreV1Api, 'read_namespaced_pod') @mock.patch.object(client.AppsV1Api, 'read_namespaced_replica_set') @mock.patch.object(client.AppsV1Api, 'replace_namespaced_replica_set') + @mock.patch.object(client.CoreV1Api, 'delete_namespaced_pod') def test_container_update_modify_information_end_image_changed( - self, mock_replace_replica_set, mock_read_replicaset, + self, mock_delete_namespaced_pod, + mock_replace_replica_set, mock_read_replicaset, mock_read_pod, mock_replace_pod, mock_replace_config_map, - mock_replace_secret, mock_list_pod, mock_vnfd_dict, mock_save, + mock_replace_secret, mock_list_pod, + mock_read_namespaced_replicaset_scale, mock_vnfd_dict, mock_save, ): mock_read_replicaset.return_value = (mgmt_fakes. fake_replicaset_image_changed()) mock_read_pod.return_value = mgmt_fakes.fake_pod_image_changed() mock_list_pod.return_value = client.V1PodList(items=[ - mgmt_fakes.get_fake_pod_info(kind='Pod', name='vdu1')]) + mgmt_fakes.get_fake_pod_info(kind='Pod', name='vdu1'), + mgmt_fakes.get_fake_pod_info(kind='ReplicaSet', name='vdu2')]) + mock_read_namespaced_replicaset_scale.return_value = client.V1Scale( + spec=client.V1ScaleSpec(replicas=1), + status=client.V1ScaleStatus(replicas=1)) mock_vnfd_dict.return_value = fakes.vnfd_dict_cnf() kwargs = { 'old_vnf_package_path': self.yaml_path_before, @@ -255,6 +297,7 @@ class TestContainerUpdate(base.TestCase): @mock.patch('tacker.objects.vnf_instance.VnfInstance.save') @mock.patch('tacker.vnflcm.utils._get_vnfd_dict') + @mock.patch.object(client.AppsV1Api, 'read_namespaced_replica_set_scale') @mock.patch.object(client.CoreV1Api, 'list_namespaced_pod') @mock.patch.object(client.CoreV1Api, 'replace_namespaced_secret') @mock.patch.object(client.CoreV1Api, 'replace_namespaced_config_map') @@ -267,14 +310,18 @@ class TestContainerUpdate(base.TestCase): self, mock_delete_namespaced_pod, mock_replace_replica_set, mock_read_replicaset, mock_read_pod, mock_replace_pod, mock_replace_config_map, - mock_replace_secret, mock_list_pod, mock_vnfd_dict, mock_save, + mock_replace_secret, mock_list_pod, + mock_read_namespaced_replicaset_scale, mock_vnfd_dict, mock_save, ): mock_read_replicaset.return_value = (mgmt_fakes. fake_replicaset_image_changed()) mock_read_pod.return_value = mgmt_fakes.fake_pod() mock_list_pod.return_value = client.V1PodList(items=[ - mgmt_fakes.get_fake_pod_info(kind='Pod', - name='vdu2-rldmg')]) + mgmt_fakes.get_fake_pod_info(kind='Pod', name='vdu2-rldmg'), + mgmt_fakes.get_fake_pod_info(kind='ReplicaSet', name='vdu2')]) + mock_read_namespaced_replicaset_scale.return_value = client.V1Scale( + spec=client.V1ScaleSpec(replicas=1), + status=client.V1ScaleStatus(replicas=1)) mock_vnfd_dict.return_value = fakes.vnfd_dict_cnf() kwargs = { 'old_vnf_package_path': self.yaml_path_before,