From 78040d2017c1edcf9f173af3e408499b23913529 Mon Sep 17 00:00:00 2001 From: Thales Elero Cervi Date: Wed, 28 Sep 2022 14:49:07 -0300 Subject: [PATCH] Add type checks to AppImageParser Recent changes [1] to AppImageParser _find_images_in_dict and generate_download_images_list methods made this code to break with both AttributeError and TypeError when stx-openstack application is being uploaded. This change includes extra protection against these types of errors and restablish the flow for generating stx-openstack image list based on its overrides. It also adds a new image resource to TestKubeAppImageParser unit tests, using an Openstack resource extracted from when debugging the original error. It should prevent this issue to happen again for future changes at AppImageParser logic. The original change to generate_download_images_list, for example, would fail the test: * TestKubeAppImageParser.test_generate_download_images_list [1] https://review.opendev.org/c/starlingx/config/+/858762 Test Plan: PASS - Locally execute unit tests: TestKubeAppImageParser PASS - Build the sysinv package with this change PASS - Upload stx-openstack app PASS - Apply stx-openstack app Closes-Bug: 1991115 Signed-off-by: Thales Elero Cervi Change-Id: I8a1384bfefd12f8a893249853cbeae3a9d3661e0 --- .../sysinv/sysinv/conductor/kube_app.py | 21 +++- .../conductor/data/chart_values_sample.yaml | 25 ++++ .../conductor/test_kube_app_image_parser.py | 111 +++++++++++++++++- 3 files changed, 149 insertions(+), 8 deletions(-) diff --git a/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py b/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py index d952d51091..8b26ba19e9 100644 --- a/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py +++ b/sysinv/sysinv/sysinv/sysinv/conductor/kube_app.py @@ -4116,7 +4116,11 @@ class AppImageParser(object): """ if isinstance(var_dict, dict): for k, v in six.iteritems(var_dict): - if k.lower() == 'images': + dict_key = k + if isinstance(dict_key, str): + dict_key = dict_key.lower() + + if dict_key == 'images': try: yield {k: {'tags': v['tags']}} except (KeyError, TypeError): @@ -4124,7 +4128,7 @@ class AppImageParser(object): yield {k: v} pass - elif k.lower() == 'image': + elif dict_key == 'image': try: image = {} keys = v.keys() @@ -4138,7 +4142,7 @@ class AppImageParser(object): if isinstance(v, str) or v is None: yield {k: v} - elif k in self.TAG_LIST: + elif dict_key in self.TAG_LIST: if isinstance(v, str) or v is None: yield {k: v} @@ -4209,17 +4213,22 @@ class AppImageParser(object): "is not a dict." % download_imgs_dict)) for k, v in six.iteritems(download_imgs_dict): - if k.lower() == 'images': + dict_key = k + if isinstance(dict_key, str): + dict_key = dict_key.lower() + + if dict_key == 'images': try: imgs = [_f for _f in v['tags'].values() if _f] download_imgs_list.extend(imgs) except (KeyError, TypeError): if v and isinstance(v, dict): - imgs = [_f for _f in v.values() if _f] + imgs = [_f for _f in v.values() + if _f and not isinstance(_f, dict)] download_imgs_list.extend(imgs) pass - elif k.lower() == 'image': + elif dict_key == 'image': try: img = v['repository'] + ':' + v['tag'] except (KeyError, TypeError): diff --git a/sysinv/sysinv/sysinv/sysinv/tests/conductor/data/chart_values_sample.yaml b/sysinv/sysinv/sysinv/sysinv/tests/conductor/data/chart_values_sample.yaml index e71c382a04..627012475b 100644 --- a/sysinv/sysinv/sysinv/sysinv/tests/conductor/data/chart_values_sample.yaml +++ b/sysinv/sysinv/sysinv/sysinv/tests/conductor/data/chart_values_sample.yaml @@ -47,6 +47,31 @@ exporter: image: docker.elastic.co/logstash/logstash-oss imagetag: "7.2.0" +openstack: + images: + ks_service: docker.io/starlingx/stx-heat:master-centos-stable-latest + db_drop: docker.io/starlingx/stx-heat:master-centos-stable-latest + image_repo_sync': null + bootstrap: docker.io/starlingx/stx-heat:master-centos-stable-latest + bootstrap: + structured: + images: + cirros: + properties: + os_distro: docker.io/cirros + name: docker.io/Cirros 0.3.5 64-bit + image_type: docker.io/qcow2 + container_format: docker.io/bare + private: true + source_url: http://download.cirros-cloud.net/0.3.5/ + min_disk: 1 + image_file: cirros-0.3.5-x86_64-disk.img + id: null + conf: + api_audit_map: + service_endpoints: + image: docker.io/service/storage/image + testFramework: tag: 0.4.0 diff --git a/sysinv/sysinv/sysinv/sysinv/tests/conductor/test_kube_app_image_parser.py b/sysinv/sysinv/sysinv/sysinv/tests/conductor/test_kube_app_image_parser.py index 7e1be51f4d..6ca92b69fd 100644 --- a/sysinv/sysinv/sysinv/sysinv/tests/conductor/test_kube_app_image_parser.py +++ b/sysinv/sysinv/sysinv/sysinv/tests/conductor/test_kube_app_image_parser.py @@ -53,6 +53,40 @@ IMAGES_RESOURCE = { 'image': 'docker.elastic.co/beats/filebeat-oss:7.4.0' } }, + 'openstack': { + 'images': { + 'ks_service': 'docker.io/starlingx/stx-heat:master-centos-stable-latest', + 'db_drop': 'docker.io/starlingx/stx-heat:master-centos-stable-latest', + "image_repo_sync'": None, + 'bootstrap': 'docker.io/starlingx/stx-heat:master-centos-stable-latest', + }, + 'bootstrap': { + 'structured': { + 'images': { + 'cirros': { + 'properties': { + 'os_distro': 'docker.io/cirros' + }, + 'name': 'docker.io/Cirros 0.3.5 64-bit', + 'image_type': 'docker.io/qcow2', + 'container_format': 'docker.io/bare', + 'private': True, + 'source_url': 'http://download.cirros-cloud.net/0.3.5/', + 'min_disk': 1, + 'image_file': 'cirros-0.3.5-x86_64-disk.img', + 'id': None + } + } + } + }, + 'conf': { + 'api_audit_map': { + 'service_endpoints': { + 'image': 'docker.io/service/storage/image' + } + } + } + }, 'image': { 'tag': '7.4.0', 'repository': 'docker.elastic.co/elasticsearch/elasticsearch-oss' @@ -118,6 +152,40 @@ class TestKubeAppImageParser(base.TestCase): 'image': 'registry.local:9001/docker.elastic.co/beats/filebeat-oss:7.4.0' } }, + 'openstack': { + 'images': { + 'ks_service': 'registry.local:9001/docker.io/starlingx/stx-heat:master-centos-stable-latest', + 'db_drop': 'registry.local:9001/docker.io/starlingx/stx-heat:master-centos-stable-latest', + "image_repo_sync'": None, + 'bootstrap': 'registry.local:9001/docker.io/starlingx/stx-heat:master-centos-stable-latest', + }, + 'bootstrap': { + 'structured': { + 'images': { + 'cirros': { + 'properties': { + 'os_distro': 'registry.local:9001/docker.io/cirros' + }, + 'name': 'registry.local:9001/docker.io/Cirros 0.3.5 64-bit', + 'image_type': 'registry.local:9001/docker.io/qcow2', + 'container_format': 'registry.local:9001/docker.io/bare', + 'private': True, + 'source_url': 'http://download.cirros-cloud.net/0.3.5/', + 'min_disk': 1, + 'image_file': 'registry.local:9001/cirros-0.3.5-x86_64-disk.img', + 'id': None + } + } + } + }, + 'conf': { + 'api_audit_map': { + 'service_endpoints': { + 'image': 'registry.local:9001/docker.io/service/storage/image' + } + } + } + }, 'image': { 'tag': '7.4.0', 'repository': 'registry.local:9001/docker.elastic.co/elasticsearch/elasticsearch-oss' @@ -153,6 +221,11 @@ class TestKubeAppImageParser(base.TestCase): 'image': 'docker.io/dduportal/bats', 'imageTag': '7.2.0' }, + 'openstack': { + 'images': { + 'bootstrap': 'docker.io/starlingx/stx-heat:master-centos-dev-latest' + } + }, 'image': { 'tag': '7.5.2' } @@ -199,7 +272,40 @@ class TestKubeAppImageParser(base.TestCase): 'image': 'docker.io/dduportal/bats', 'imageTag': '7.2.0' }, - + 'openstack': { + 'images': { + 'ks_service': 'docker.io/starlingx/stx-heat:master-centos-stable-latest', + 'db_drop': 'docker.io/starlingx/stx-heat:master-centos-stable-latest', + "image_repo_sync'": None, + 'bootstrap': 'docker.io/starlingx/stx-heat:master-centos-dev-latest', + }, + 'bootstrap': { + 'structured': { + 'images': { + 'cirros': { + 'properties': { + 'os_distro': 'docker.io/cirros' + }, + 'name': 'docker.io/Cirros 0.3.5 64-bit', + 'image_type': 'docker.io/qcow2', + 'container_format': 'docker.io/bare', + 'private': True, + 'source_url': 'http://download.cirros-cloud.net/0.3.5/', + 'min_disk': 1, + 'image_file': 'cirros-0.3.5-x86_64-disk.img', + 'id': None + } + } + } + }, + 'conf': { + 'api_audit_map': { + 'service_endpoints': { + 'image': 'docker.io/service/storage/image' + } + } + } + }, 'image': { 'tag': '7.5.2', 'repository': 'docker.elastic.co/elasticsearch/elasticsearch-oss' @@ -224,7 +330,8 @@ class TestKubeAppImageParser(base.TestCase): 'quay.io/silicom/tsync_extts:1.0.0', 'quay.io/silicom/phc2sys:3.1.1', 'quay.io/silicom/grpc-tsyncd:2.1.2.8', - 'quay.io/silicom/gpsd:3.23.1' + 'quay.io/silicom/gpsd:3.23.1', + 'docker.io/starlingx/stx-heat:master-centos-stable-latest' ] download_imgs_list = self.image_parser.generate_download_images_list(