From feaa276469a0bd79b5e253602f105fb89266da57 Mon Sep 17 00:00:00 2001 From: Isac Souza Date: Tue, 11 May 2021 09:48:33 -0300 Subject: [PATCH] Migrate nginx overrides from old to new chart Adds a lifecycle hook to migrate the user overrides from the old chart name (nginx-ingress) into the new one (ingress-nginx). The chart name was changed upstream as part of a major refactor. The lifecycle hook will listen to the pre apply event and query the database for the overrides of the newest inactive app (i.e. the previous version). If it finds overrides and the version being installed do not have overrides, the old ones will be migrated to the new chart name. Tested by performing an application-update from a version before the chart name change to a version built with this hook. Also tested a fresh install and upgrading from r4 to master for SX and DX. Closes-Bug: 1927003 Signed-off-by: Isac Souza Change-Id: Id328dd3bff79290d249d25e0875214c463cf76c1 --- .../common/constants.py | 1 + .../lifecycle_nginx_ingress_controller.py | 84 +++++++- .../tests/test_plugins.py | 180 +++++++++++++++++- 3 files changed, 258 insertions(+), 7 deletions(-) diff --git a/python-k8sapp-nginx-ingress-controller/k8sapp_nginx_ingress_controller/k8sapp_nginx_ingress_controller/common/constants.py b/python-k8sapp-nginx-ingress-controller/k8sapp_nginx_ingress_controller/k8sapp_nginx_ingress_controller/common/constants.py index 3e963f1..9d27277 100644 --- a/python-k8sapp-nginx-ingress-controller/k8sapp_nginx_ingress_controller/k8sapp_nginx_ingress_controller/common/constants.py +++ b/python-k8sapp-nginx-ingress-controller/k8sapp_nginx_ingress_controller/k8sapp_nginx_ingress_controller/common/constants.py @@ -8,3 +8,4 @@ from sysinv.helm import common HELM_NS_NGINX_INGRESS_CONTROLLER = common.HELM_NS_KUBE_SYSTEM HELM_CHART_INGRESS_NGINX = 'ingress-nginx' +HELM_CHART_LEGACY_INGRESS_NGINX = 'nginx-ingress' diff --git a/python-k8sapp-nginx-ingress-controller/k8sapp_nginx_ingress_controller/k8sapp_nginx_ingress_controller/lifecycle/lifecycle_nginx_ingress_controller.py b/python-k8sapp-nginx-ingress-controller/k8sapp_nginx_ingress_controller/k8sapp_nginx_ingress_controller/lifecycle/lifecycle_nginx_ingress_controller.py index 847afda..628dfbb 100644 --- a/python-k8sapp-nginx-ingress-controller/k8sapp_nginx_ingress_controller/k8sapp_nginx_ingress_controller/lifecycle/lifecycle_nginx_ingress_controller.py +++ b/python-k8sapp-nginx-ingress-controller/k8sapp_nginx_ingress_controller/k8sapp_nginx_ingress_controller/lifecycle/lifecycle_nginx_ingress_controller.py @@ -11,7 +11,10 @@ from k8sapp_nginx_ingress_controller.common import constants as app_constants from oslo_log import log as logging from sysinv.common import constants +from sysinv.common import exception +from sysinv.db import api as dbapi from sysinv.helm import lifecycle_base as base +from sysinv.helm import lifecycle_utils LOG = logging.getLogger(__name__) @@ -20,23 +23,82 @@ class NginxIngressControllerAppLifecycleOperator(base.AppLifecycleOperator): def app_lifecycle_actions(self, context, conductor_obj, app_op, app, hook_info): """Perform lifecycle actions for an operation - :param context: request context - :param conductor_obj: conductor object + :param context: request context, can be None + :param conductor_obj: conductor object, can be None :param app_op: AppOperator object :param app: AppOperator.Application object :param hook_info: LifecycleHookInfo object """ - # Operation if hook_info.lifecycle_type == constants.APP_LIFECYCLE_TYPE_OPERATION: if hook_info.operation == constants.APP_ETCD_BACKUP: if hook_info.relative_timing == constants.APP_LIFECYCLE_TIMING_PRE: return self.pre_etcd_backup(app_op) - # Use the default behaviour for other hooks + if hook_info.lifecycle_type == constants.APP_LIFECYCLE_TYPE_RESOURCE: + if hook_info.operation == constants.APP_APPLY_OP: + if hook_info.relative_timing == constants.APP_LIFECYCLE_TIMING_PRE: + return self.pre_apply(app_op, app, hook_info) + super(NginxIngressControllerAppLifecycleOperator, self).app_lifecycle_actions( context, conductor_obj, app_op, app, hook_info) + def pre_apply(self, app_op, app, hook_info): + """Pre Apply actions + + Creates the local registry secret and migrates helm user overrides + from one chart name to another + + :param app_op: AppOperator object + :param app: AppOperator.Application object + :param hook_info: LifecycleHookInfo object + """ + LOG.debug("Executing pre_apply for {} app".format(constants.HELM_APP_NGINX_IC)) + + # As this overrides the functionality provided in the base class, make + # sure we perform the operations defined there + lifecycle_utils.create_local_registry_secrets(app_op, app, hook_info) + + dbapi_instance = dbapi.get_instance() + + # get most recently created inactive app + inactive_db_apps = dbapi_instance.kube_app_get_inactive( + app.name, limit=1, sort_key='created_at', sort_dir='desc') + + if not inactive_db_apps: + # nothing to migrate from + return + + from_db_app = inactive_db_apps[0] + to_db_app = dbapi_instance.kube_app_get(app.name) + + old_chart_overrides = self._get_helm_overrides( + dbapi_instance, + from_db_app, + app_constants.HELM_CHART_LEGACY_INGRESS_NGINX, + app_constants.HELM_NS_NGINX_INGRESS_CONTROLLER) + + new_chart_overrides = self._get_helm_overrides( + dbapi_instance, + to_db_app, + app_constants.HELM_CHART_INGRESS_NGINX, + app_constants.HELM_NS_NGINX_INGRESS_CONTROLLER) + + if old_chart_overrides and not new_chart_overrides: + # if there are user overrides on the old chart name and there are no user overrides + # on the new chart name, migrate them from the old to the new name + # we do not want to do the migration if there are overrides for the new name because + # that means the user has already done it. + LOG.info("Migrating user_overrides from {} to {}".format( + app_constants.HELM_CHART_LEGACY_INGRESS_NGINX, app_constants.HELM_CHART_INGRESS_NGINX)) + + self._update_helm_user_overrides( + dbapi_instance, + to_db_app, + app_constants.HELM_CHART_INGRESS_NGINX, + app_constants.HELM_NS_NGINX_INGRESS_CONTROLLER, + old_chart_overrides) + def pre_etcd_backup(self, app_op): """Pre Etcd backup actions @@ -53,3 +115,17 @@ class NginxIngressControllerAppLifecycleOperator(base.AppLifecycleOperator): admission_webhook = webhooks[0].metadata.name # pylint: disable=protected-access app_op._kube.kube_delete_validating_webhook_configuration(admission_webhook) + + def _get_helm_overrides(self, dbapi_instance, app, chart, namespace): + try: + override = dbapi_instance.helm_override_get(app_id=app.id, name=chart, namespace=namespace) + except exception.HelmOverrideNotFound: + # Override for this chart not be found, nothing to be done + return None + + return override['user_overrides'] + + def _update_helm_user_overrides(self, dbapi_instance, app, chart, namespace, overrides): + values = {'user_overrides': overrides} + dbapi_instance.helm_override_update( + app_id=app.id, name=chart, namespace=namespace, values=values) diff --git a/python-k8sapp-nginx-ingress-controller/k8sapp_nginx_ingress_controller/k8sapp_nginx_ingress_controller/tests/test_plugins.py b/python-k8sapp-nginx-ingress-controller/k8sapp_nginx_ingress_controller/k8sapp_nginx_ingress_controller/tests/test_plugins.py index bee4e34..40850f1 100644 --- a/python-k8sapp-nginx-ingress-controller/k8sapp_nginx_ingress_controller/k8sapp_nginx_ingress_controller/tests/test_plugins.py +++ b/python-k8sapp-nginx-ingress-controller/k8sapp_nginx_ingress_controller/k8sapp_nginx_ingress_controller/tests/test_plugins.py @@ -4,16 +4,190 @@ # SPDX-License-Identifier: Apache-2.0 # +import json +import mock + +from k8sapp_nginx_ingress_controller.common import constants as app_constants +from k8sapp_nginx_ingress_controller.lifecycle.lifecycle_nginx_ingress_controller \ + import NginxIngressControllerAppLifecycleOperator as NginxICOperator +from sysinv.common import constants +from sysinv.db import api as db_api +from sysinv.helm import lifecycle_hook from sysinv.tests.db import base +from sysinv.tests.db import utils as dbutils class NginxICTestCase(base.DbTestCase): def setUp(self): super(NginxICTestCase, self).setUp() + self.dbapi = db_api.get_instance() + self.app_op = mock.MagicMock() + self.new_app = mock.MagicMock() + + self.new_app.id = 2 + self.new_app.name = constants.HELM_APP_NGINX_IC + + self.old_db_app = dbutils.create_test_app( + id=1, + name=constants.HELM_APP_NGINX_IC, + app_version='1.0-0') + # creating an inactive app does not work, need to update the status later + self.dbapi.kube_app_update(self.old_db_app.id, {'status': constants.APP_INACTIVE_STATE}) + + self.new_db_app = dbutils.create_test_app( + id=self.new_app.id, + status=constants.APP_APPLY_IN_PROGRESS, + name=constants.HELM_APP_NGINX_IC, + app_version='1.1-1') + + self.hook_info = lifecycle_hook.LifecycleHookInfo() + self.hook_info.init( + constants.APP_LIFECYCLE_MODE_AUTO, + constants.APP_LIFECYCLE_TYPE_RESOURCE, + constants.APP_LIFECYCLE_TIMING_PRE, + constants.APP_APPLY_OP) + + self.old_overrides = { + 'udp': { + '161': 'kube-system/snmpd-service:161' + }, + 'tcp': { + '162': 'kube-system/snmpd-service:162' + } + } + + self.new_overrides = { + 'host': 'dummy-service.host.wrs.com' + } + def tearDown(self): super(NginxICTestCase, self).tearDown() - def test_plugins(self): - # placeholder for future unit tests - pass + @mock.patch(NginxICOperator.__module__ + '.lifecycle_utils.create_local_registry_secrets') + def test_local_registry_secret_is_created(self, mock_create_secrets): + """Test that the super class is called. + + Test that if a pre apply hook is received, the super class + method is still called. It must be called to execute bootstrapping code + like installing the default registry secret. + """ + lifecycleOperator = NginxICOperator() + lifecycleOperator.app_lifecycle_actions(None, None, self.app_op, self.new_app, self.hook_info) + mock_create_secrets.assert_called_once() + + def test_has_old_override_no_new_override(self): + """Test that overrides are properly migrated. + + Test that if the old app version has an override and the + new app version does not, the old override is migrated to the + new version. + """ + dbutils.create_test_helm_overrides( + app_id=self.old_db_app.id, + name=app_constants.HELM_CHART_LEGACY_INGRESS_NGINX, + namespace=app_constants.HELM_NS_NGINX_INGRESS_CONTROLLER, + user_overrides=json.dumps(self.old_overrides)) + + dbutils.create_test_helm_overrides( + app_id=self.new_db_app.id, + name=app_constants.HELM_CHART_INGRESS_NGINX, + namespace=app_constants.HELM_NS_NGINX_INGRESS_CONTROLLER, + user_overrides=None) + + lifecycleOperator = NginxICOperator() + lifecycleOperator.app_lifecycle_actions(None, None, self.app_op, self.new_app, self.hook_info) + + overrides = self.dbapi.helm_override_get( + app_id=self.new_app.id, + name=app_constants.HELM_CHART_INGRESS_NGINX, + namespace=app_constants.HELM_NS_NGINX_INGRESS_CONTROLLER) + + self.assertEqual(self.old_overrides, json.loads(overrides['user_overrides'])) + + def test_has_no_old_override_no_new_override(self): + """Test that no overrides are migrated. + + Test that if the old app version does not trigger migration + when no override is present + """ + old_overrides = None + + dbutils.create_test_helm_overrides( + app_id=self.old_db_app.id, + name=app_constants.HELM_CHART_LEGACY_INGRESS_NGINX, + namespace=app_constants.HELM_NS_NGINX_INGRESS_CONTROLLER) + + dbutils.create_test_helm_overrides( + app_id=self.new_db_app.id, + name=app_constants.HELM_CHART_INGRESS_NGINX, + namespace=app_constants.HELM_NS_NGINX_INGRESS_CONTROLLER, + user_overrides=None) + + lifecycleOperator = NginxICOperator() + lifecycleOperator.app_lifecycle_actions(None, None, self.app_op, self.new_app, self.hook_info) + + overrides = self.dbapi.helm_override_get( + app_id=self.new_app.id, + name=app_constants.HELM_CHART_INGRESS_NGINX, + namespace=app_constants.HELM_NS_NGINX_INGRESS_CONTROLLER) + + self.assertEqual(old_overrides, overrides['user_overrides']) + + def test_has_old_override_new_override(self): + """Test that no overrides are migrated. + + Test that the migration does not occur when a override + is present in the new version + """ + + dbutils.create_test_helm_overrides( + app_id=self.old_db_app.id, + name=app_constants.HELM_CHART_LEGACY_INGRESS_NGINX, + namespace=app_constants.HELM_NS_NGINX_INGRESS_CONTROLLER, + user_overrides=json.dumps(self.old_overrides)) + + dbutils.create_test_helm_overrides( + app_id=self.new_db_app.id, + name=app_constants.HELM_CHART_INGRESS_NGINX, + namespace=app_constants.HELM_NS_NGINX_INGRESS_CONTROLLER, + user_overrides=json.dumps(self.new_overrides)) + + lifecycleOperator = NginxICOperator() + lifecycleOperator.app_lifecycle_actions(None, None, self.app_op, self.new_app, self.hook_info) + + overrides = self.dbapi.helm_override_get( + app_id=self.new_app.id, + name=app_constants.HELM_CHART_INGRESS_NGINX, + namespace=app_constants.HELM_NS_NGINX_INGRESS_CONTROLLER) + + self.assertEqual(self.new_overrides, json.loads(overrides['user_overrides'])) + + def test_has_no_old_app(self): + """Test that no overrides are migrated. + + Test if new application is successfully applied with + overrides when no previous version is present + """ + + self.dbapi.kube_app_destroy( + name=self.old_db_app.name, + version=self.old_db_app.app_version, + inactive=True, + ) + + dbutils.create_test_helm_overrides( + app_id=self.new_db_app.id, + name=app_constants.HELM_CHART_INGRESS_NGINX, + namespace=app_constants.HELM_NS_NGINX_INGRESS_CONTROLLER, + user_overrides=json.dumps(self.new_overrides)) + + lifecycleOperator = NginxICOperator() + lifecycleOperator.app_lifecycle_actions(None, None, self.app_op, self.new_app, self.hook_info) + + overrides = self.dbapi.helm_override_get( + app_id=self.new_app.id, + name=app_constants.HELM_CHART_INGRESS_NGINX, + namespace=app_constants.HELM_NS_NGINX_INGRESS_CONTROLLER) + + self.assertEqual(self.new_overrides, json.loads(overrides['user_overrides']))