From 727967d9543d6fc0d6f9284421461a0e131a2f1b Mon Sep 17 00:00:00 2001 From: Eloy Maillo <14107036+herodes1991@users.noreply.github.com> Date: Mon, 7 Nov 2022 08:26:16 +0100 Subject: [PATCH] Fix versions of ingresses in ready_check method (#191) --- fiaas_deploy_daemon/deployer/deploy.py | 5 +- .../kubernetes/ingress_networkingv1.py | 3 + .../deployer/kubernetes/ingress_v1beta1.py | 3 + .../deployer/kubernetes/ready_check.py | 6 +- .../deployer/kubernetes/test_ready_check.py | 185 ++++++------------ .../deployer/test_deploy.py | 14 +- 6 files changed, 87 insertions(+), 129 deletions(-) diff --git a/fiaas_deploy_daemon/deployer/deploy.py b/fiaas_deploy_daemon/deployer/deploy.py index ed8d66f3..e7bd63a7 100644 --- a/fiaas_deploy_daemon/deployer/deploy.py +++ b/fiaas_deploy_daemon/deployer/deploy.py @@ -32,13 +32,14 @@ class Deployer(DaemonThread): Mainly focused on bookkeeping, and leaving the hard work to the framework-adapter. """ - def __init__(self, deploy_queue, bookkeeper, adapter, scheduler, lifecycle, config): + def __init__(self, deploy_queue, bookkeeper, adapter, scheduler, lifecycle, ingress_adapter, config): super(Deployer, self).__init__() self._queue = _make_gen(deploy_queue.get) self._bookkeeper = bookkeeper self._adapter = adapter self._scheduler = scheduler self._lifecycle = lifecycle + self._ingress_adapter = ingress_adapter self._config = config def __call__(self): @@ -59,7 +60,7 @@ def _update(self, app_spec, lifecycle_subject): self._adapter.deploy(app_spec) if app_spec.name != "fiaas-deploy-daemon": self._scheduler.add(ReadyCheck(app_spec, self._bookkeeper, self._lifecycle, lifecycle_subject, - self._config)) + self._ingress_adapter, self._config)) else: self._lifecycle.success(lifecycle_subject) self._bookkeeper.success(app_spec) diff --git a/fiaas_deploy_daemon/deployer/kubernetes/ingress_networkingv1.py b/fiaas_deploy_daemon/deployer/kubernetes/ingress_networkingv1.py index b358b983..82299bed 100644 --- a/fiaas_deploy_daemon/deployer/kubernetes/ingress_networkingv1.py +++ b/fiaas_deploy_daemon/deployer/kubernetes/ingress_networkingv1.py @@ -78,6 +78,9 @@ def delete_list(self, app_spec): except NotFound: pass + def find(self, name, namespace): + return Ingress.find(name, namespace) + def _make_http_ingress_rule_value(self, app_spec, pathmappings): http_ingress_paths = [ HTTPIngressPath( diff --git a/fiaas_deploy_daemon/deployer/kubernetes/ingress_v1beta1.py b/fiaas_deploy_daemon/deployer/kubernetes/ingress_v1beta1.py index a397a1db..e1d7b44e 100644 --- a/fiaas_deploy_daemon/deployer/kubernetes/ingress_v1beta1.py +++ b/fiaas_deploy_daemon/deployer/kubernetes/ingress_v1beta1.py @@ -77,6 +77,9 @@ def delete_list(self, app_spec): except NotFound: pass + def find(self, name, namespace): + return Ingress.find(name, namespace) + def _make_http_ingress_rule_value(self, app_spec, pathmappings): http_ingress_paths = [ HTTPIngressPath(path=pm.path, backend=IngressBackend(serviceName=app_spec.name, servicePort=pm.port)) diff --git a/fiaas_deploy_daemon/deployer/kubernetes/ready_check.py b/fiaas_deploy_daemon/deployer/kubernetes/ready_check.py index 09d23e5d..5d3dc831 100644 --- a/fiaas_deploy_daemon/deployer/kubernetes/ready_check.py +++ b/fiaas_deploy_daemon/deployer/kubernetes/ready_check.py @@ -22,14 +22,13 @@ from k8s.client import NotFound from k8s.models.certificate import Certificate from k8s.models.deployment import Deployment -from k8s.models.ingress import Ingress from monotonic import monotonic as time_monotonic LOG = logging.getLogger(__name__) class ReadyCheck(object): - def __init__(self, app_spec, bookkeeper, lifecycle, lifecycle_subject, config): + def __init__(self, app_spec, bookkeeper, lifecycle, lifecycle_subject, ingress_adapter, config): self._app_spec = app_spec self._bookkeeper = bookkeeper self._lifecycle = lifecycle @@ -41,6 +40,7 @@ def __init__(self, app_spec, bookkeeper, lifecycle, lifecycle_subject, config): ) self._fail_after = time_monotonic() + self._fail_after_seconds self._should_check_ingress = config.tls_certificate_ready + self._ingress_adapter = ingress_adapter def __call__(self): if self._ready(): @@ -77,7 +77,7 @@ def _is_certificate_ready(self, cert): def _ingress_ready(self): try: - ing_list = Ingress.find(self._app_spec.name, self._app_spec.namespace) + ing_list = self._ingress_adapter.find(self._app_spec.name, self._app_spec.namespace) except NotFound: return False diff --git a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_ready_check.py b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_ready_check.py index 329a9fad..46604947 100644 --- a/tests/fiaas_deploy_daemon/deployer/kubernetes/test_ready_check.py +++ b/tests/fiaas_deploy_daemon/deployer/kubernetes/test_ready_check.py @@ -21,12 +21,14 @@ from monotonic import monotonic as time_monotonic from fiaas_deploy_daemon.deployer.bookkeeper import Bookkeeper +from fiaas_deploy_daemon.deployer.kubernetes.ingress_v1beta1 import V1Beta1IngressAdapter from fiaas_deploy_daemon.config import Configuration from fiaas_deploy_daemon.deployer.kubernetes.ready_check import ReadyCheck from fiaas_deploy_daemon.lifecycle import Lifecycle, Subject from fiaas_deploy_daemon.specs.models import LabelAndAnnotationSpec, IngressTLSSpec from k8s.models.certificate import Certificate, CertificateCondition from k8s.models.ingress import Ingress, IngressTLS +from k8s.models.networking_v1_ingress import Ingress as V1Ingress, IngressTLS as V1IngressTLS REPLICAS = 2 @@ -49,14 +51,23 @@ def lifecycle_subject(self, app_spec): def config(self): return Configuration([]) + @pytest.fixture + def ingress_adapter(self): + return mock.create_autospec(V1Beta1IngressAdapter) + + @pytest.fixture + def get_cert(self): + with mock.patch("k8s.models.certificate.Certificate.get") as get_cert: + yield get_cert + @pytest.mark.parametrize("generation,observed_generation", ( (0, 0), (0, 1) )) def test_deployment_complete(self, get, app_spec, bookkeeper, generation, observed_generation, lifecycle, - lifecycle_subject, config): + lifecycle_subject, ingress_adapter, config): self._create_response(get, generation=generation, observed_generation=observed_generation) - ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, config) + ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, ingress_adapter, config) assert ready() is False bookkeeper.success.assert_called_with(app_spec) @@ -73,9 +84,10 @@ def test_deployment_complete(self, get, app_spec, bookkeeper, generation, observ (2, 2, 2, 2, 1, 0), )) def test_deployment_incomplete(self, get, app_spec, bookkeeper, requested, replicas, available, updated, - generation, observed_generation, lifecycle, lifecycle_subject, config): + generation, observed_generation, lifecycle, lifecycle_subject, ingress_adapter, + config): self._create_response(get, requested, replicas, available, updated, generation, observed_generation) - ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, config) + ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, ingress_adapter, config) assert ready() is True bookkeeper.success.assert_not_called() @@ -91,13 +103,13 @@ def test_deployment_incomplete(self, get, app_spec, bookkeeper, requested, repli (2, 1, 1, 1, {"fiaas/source-repository": "xyz"}, "xyz"), )) def test_deployment_failed(self, get, app_spec, bookkeeper, requested, replicas, available, updated, - lifecycle, lifecycle_subject, annotations, repository, config): + lifecycle, lifecycle_subject, annotations, repository, ingress_adapter, config): if annotations: app_spec = app_spec._replace(annotations=LabelAndAnnotationSpec(*[annotations] * 7)) self._create_response(get, requested, replicas, available, updated) - ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, config) + ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, ingress_adapter, config) ready._fail_after = time_monotonic() assert ready() is False @@ -106,10 +118,11 @@ def test_deployment_failed(self, get, app_spec, bookkeeper, requested, replicas, lifecycle.success.assert_not_called() lifecycle.failed.assert_called_with(lifecycle_subject) - def test_deployment_complete_deactivated(self, get, app_spec, bookkeeper, lifecycle, lifecycle_subject, config): + def test_deployment_complete_deactivated(self, get, app_spec, bookkeeper, lifecycle, lifecycle_subject, + ingress_adapter, config): self._create_response_zero_replicas(get) - ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, config) + ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, ingress_adapter, config) assert ready() is False bookkeeper.success.assert_called_with(app_spec) @@ -117,125 +130,57 @@ def test_deployment_complete_deactivated(self, get, app_spec, bookkeeper, lifecy lifecycle.success.assert_called_with(lifecycle_subject) lifecycle.failed.assert_not_called() - def test_tls_ingress_ready(self, get, app_spec, bookkeeper, lifecycle, lifecycle_subject, config): - config.tls_certificate_ready = True - app_spec = app_spec._replace(ingress_tls=IngressTLSSpec(enabled=True, certificate_issuer=None)) - replicas = 2 - - with mock.patch("k8s.models.ingress.Ingress.find") as find: - ingress = mock.create_autospec(Ingress, spec_set=True) - ingress.spec.tls = [IngressTLS(hosts=["extra1.example.com"], secretName="secret1")] - find.return_value = [ingress] - - with mock.patch("k8s.models.certificate.Certificate.get") as get_crd: - expiration_date = datetime.now() + timedelta(days=5) - get_crd.return_value = self._mock_certificate(True, expiration_date) - - self._create_response(get, replicas, replicas, replicas, replicas) - ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, config) - - assert ready() is False - bookkeeper.success.assert_called_with(app_spec) - bookkeeper.failed.assert_not_called() - lifecycle.success.assert_called_with(lifecycle_subject) - lifecycle.failed.assert_not_called() - - def test_tls_ingress_ready_null_notafter(self, get, app_spec, bookkeeper, lifecycle, lifecycle_subject, config): - config.tls_certificate_ready = True - app_spec = app_spec._replace(ingress_tls=IngressTLSSpec(enabled=True, certificate_issuer=None)) - replicas = 2 - - with mock.patch("k8s.models.ingress.Ingress.find") as find: - ingress = mock.create_autospec(Ingress, spec_set=True) - ingress.spec.tls = [IngressTLS(hosts=["extra1.example.com"], secretName="secret1")] - find.return_value = [ingress] - - with mock.patch("k8s.models.certificate.Certificate.get") as get_crd: - get_crd.return_value = self._mock_certificate(True) - - self._create_response(get, replicas, replicas, replicas, replicas) - ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, config) - - assert ready() is False - bookkeeper.success.assert_called_with(app_spec) - bookkeeper.failed.assert_not_called() - lifecycle.success.assert_called_with(lifecycle_subject) - lifecycle.failed.assert_not_called() - - def test_tls_ingress_ready_expired_certificate(self, get, app_spec, bookkeeper, lifecycle, lifecycle_subject, config): - config.tls_certificate_ready = True - app_spec = app_spec._replace(ingress_tls=IngressTLSSpec(enabled=True, certificate_issuer=None)) - replicas = 2 - - with mock.patch("k8s.models.ingress.Ingress.find") as find: - ingress = mock.create_autospec(Ingress, spec_set=True) - ingress.spec.tls = [IngressTLS(hosts=["extra1.example.com"], secretName="secret1")] - find.return_value = [ingress] - - with mock.patch("k8s.models.certificate.Certificate.get") as get_crd: - expiration_date = datetime.now() - timedelta(days=5) - get_crd.return_value = self._mock_certificate(True, expiration_date) - - self._create_response(get, replicas, replicas, replicas, replicas) - ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, config) - ready._fail_after = time_monotonic() - - assert ready() is False - bookkeeper.failed.assert_called_with(app_spec) - bookkeeper.success.assert_not_called() - lifecycle.failed.assert_called_with(lifecycle_subject) - lifecycle.success.assert_not_called() - - def test_tls_ingress_not_ready_timeout(self, get, app_spec, bookkeeper, lifecycle, lifecycle_subject, config): - config.tls_certificate_ready = True - app_spec = app_spec._replace(ingress_tls=IngressTLSSpec(enabled=True, certificate_issuer=None)) - replicas = 2 - - with mock.patch("k8s.models.ingress.Ingress.find") as find: - ingress = mock.create_autospec(Ingress) - ingress.spec.tls = [IngressTLS(hosts=["extra1.example.com"], secretName="secret1")] - find.return_value = [ingress] - - with mock.patch("k8s.models.certificate.Certificate.get") as get_crd: - get_crd.return_value = self._mock_certificate(False) - - self._create_response(get, replicas, replicas, replicas, replicas) - ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, config) - ready._fail_after = time_monotonic() - - assert ready() is False - bookkeeper.failed.assert_called_with(app_spec) - bookkeeper.success.assert_not_called() - lifecycle.failed.assert_called_with(lifecycle_subject) - lifecycle.success.assert_not_called() - - def test_tls_ingress_not_ready(self, get, app_spec, bookkeeper, lifecycle, lifecycle_subject, config): + @pytest.mark.parametrize("ingress_class,ingress_tls,cert_valid,expiration_date,result,success", ( + (Ingress, IngressTLS, True, (datetime.now() + timedelta(days=5)), False, True), + (V1Ingress, V1IngressTLS, True, (datetime.now() + timedelta(days=5)), False, True), + (Ingress, IngressTLS, True, None, False, True), + (V1Ingress, V1IngressTLS, True, None, False, True), + (Ingress, IngressTLS, True, (datetime.now() - timedelta(days=5)), False, False), + (V1Ingress, V1IngressTLS, True, (datetime.now() - timedelta(days=5)), False, False), + (Ingress, IngressTLS, False, None, False, False), + (V1Ingress, V1IngressTLS, False, None, False, False), + (Ingress, IngressTLS, False, None, True, True), + (V1Ingress, V1IngressTLS, False, None, True, True) + )) + def test_tls_ingress(self, get, app_spec, bookkeeper, lifecycle, lifecycle_subject, get_cert, ingress_adapter, + ingress_class, ingress_tls, cert_valid, expiration_date, result, success, config): config.tls_certificate_ready = True app_spec = app_spec._replace(ingress_tls=IngressTLSSpec(enabled=True, certificate_issuer=None)) replicas = 2 - with mock.patch("k8s.models.ingress.Ingress.find") as find: - ingress = mock.create_autospec(Ingress) - ingress.spec.tls = [IngressTLS(hosts=["extra1.example.com"], secretName="secret1")] - find.return_value = [ingress] - - with mock.patch("k8s.models.certificate.Certificate.get") as get_crd: - get_crd.return_value = self._mock_certificate(False) - - self._create_response(get, replicas, replicas, replicas, replicas) - ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, config) - - assert ready() is True - bookkeeper.failed.assert_not_called() - bookkeeper.success.assert_not_called() - lifecycle.failed.assert_not_called() - lifecycle.success.assert_not_called() + ingress = mock.create_autospec(ingress_class, spec_set=True) + ingress.spec.tls = [ingress_tls(hosts=["extra1.example.com"], secretName="secret1")] + ingress_adapter.find.return_value = [ingress] + + get_cert.return_value = self._mock_certificate(cert_valid, expiration_date) + self._create_response(get, replicas, replicas, replicas, replicas) + + ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, ingress_adapter, config) + if not success: + ready._fail_after = time_monotonic() + + assert ready() is result + if result: + bookkeeper.success.assert_not_called() + lifecycle.success.assert_not_called() + bookkeeper.failed.assert_not_called() + lifecycle.failed.assert_not_called() + elif success: + bookkeeper.success.assert_called_with(app_spec) + lifecycle.success.assert_called_with(lifecycle_subject) + bookkeeper.failed.assert_not_called() + lifecycle.failed.assert_not_called() + else: + bookkeeper.success.assert_not_called() + lifecycle.success.assert_not_called() + bookkeeper.failed.assert_called_with(app_spec) + lifecycle.failed.assert_called_with(lifecycle_subject) - def test_deployment_tls_config_no_tls_extension(self, get, app_spec, bookkeeper, lifecycle, - lifecycle_subject, config): + def test_deployment_tls_config_no_tls_extension(self, get, app_spec, bookkeeper, lifecycle, lifecycle_subject, + ingress_adapter, config): config.tls_certificate_ready = True self._create_response(get) - ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, config) + ready = ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, ingress_adapter, config) assert ready() is False bookkeeper.success.assert_called_with(app_spec) diff --git a/tests/fiaas_deploy_daemon/deployer/test_deploy.py b/tests/fiaas_deploy_daemon/deployer/test_deploy.py index 9771c9ae..ca2f171d 100644 --- a/tests/fiaas_deploy_daemon/deployer/test_deploy.py +++ b/tests/fiaas_deploy_daemon/deployer/test_deploy.py @@ -24,6 +24,7 @@ from fiaas_deploy_daemon.deployer.bookkeeper import Bookkeeper from fiaas_deploy_daemon.deployer.deploy import Deployer from fiaas_deploy_daemon.deployer.kubernetes.adapter import K8s +from fiaas_deploy_daemon.deployer.kubernetes.ingress_v1beta1 import V1Beta1IngressAdapter from fiaas_deploy_daemon.deployer.kubernetes.ready_check import ReadyCheck from fiaas_deploy_daemon.deployer.scheduler import Scheduler from fiaas_deploy_daemon.lifecycle import Lifecycle, Subject, STATUS_STARTED, STATUS_FAILED @@ -54,13 +55,17 @@ def lifecycle_subject(self, app_spec): return Subject(app_spec.uid, app_spec.name, app_spec.namespace, app_spec.deployment_id, None, app_spec.labels.status, app_spec.annotations.status) + @pytest.fixture + def ingress_adapter(self): + return mock.create_autospec(V1Beta1IngressAdapter, spec_set=True) + @pytest.fixture def config(self): return Configuration([]) @pytest.fixture - def deployer(self, app_spec, bookkeeper, adapter, scheduler, lifecycle, lifecycle_subject, config): - deployer = Deployer(Queue(), bookkeeper, adapter, scheduler, lifecycle, config) + def deployer(self, app_spec, bookkeeper, adapter, scheduler, lifecycle, lifecycle_subject, ingress_adapter, config): + deployer = Deployer(Queue(), bookkeeper, adapter, scheduler, lifecycle, ingress_adapter, config) deployer._queue = [DeployerEvent("UPDATE", app_spec, lifecycle_subject)] return deployer @@ -96,8 +101,9 @@ def test_signals_failure_on_exception(self, app_spec, lifecycle, lifecycle_subje lifecycle.state_change_signal.send.assert_called_with(status=STATUS_FAILED, subject=lifecycle_subject) def test_schedules_ready_check(self, app_spec, scheduler, bookkeeper, deployer, lifecycle, lifecycle_subject, - config): + ingress_adapter, config): deployer() lifecycle.state_change_signal.send.assert_called_once_with(status=STATUS_STARTED, subject=lifecycle_subject) - scheduler.add.assert_called_with(ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, config)) + scheduler.add.assert_called_with(ReadyCheck(app_spec, bookkeeper, lifecycle, lifecycle_subject, ingress_adapter, + config))