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']))