Merge pull request #221 from fiaas/add-pdb

Add PodDisruptionBudget support

docs/v3_spec.md

@@ -601,6 +601,7 @@ annotations:
   service: {}
   service_account: {}
   pod: {}
+  pod_disruption_budget: {}
 ```
 
 The annotations are organized under the Kubernetes resource they will be applied to. To specify custom anotations, set
@@ -635,6 +636,7 @@ labels:
   service: {}
   service_account: {}
   pod: {}
+  pod_disruption_budget: {}
 ```
 
 The labels are organized under the Kubernetes resource they will be applied to. To specify custom labels, set

docs/v3_whats_new.md

@@ -270,6 +270,7 @@ labels:
   ingress: {}
   service: {}
   pod: {}
+  pod_disruption_budget: {}
 ```
 
 The labels are organized under the Kubernetes resource they will be applied to. To specify custom labels, set
@@ -301,6 +302,7 @@ annotations:
   ingress: {}
   service: {}
   pod: {}
+  pod_disruption_budget: {}
 ```
 
 Like labels, annotations are organized under the Kubernetes resource they will be applied to. To specify custom

fiaas_deploy_daemon/crd/types.py

@@ -31,6 +31,7 @@ class AdditionalLabelsOrAnnotations(Model):
     service_account = Field(dict)
     pod = Field(dict)
     status = Field(dict)
+    pod_disruption_budget = Field(dict)
 
 
 class FiaasApplicationSpec(Model):

fiaas_deploy_daemon/deployer/kubernetes/__init__.py

@@ -27,6 +27,7 @@
 from .service import ServiceDeployer
 from .service_account import ServiceAccountDeployer
 from .owner_references import OwnerReferences
+from .pod_disruption_budget import PodDisruptionBudgetDeployer
 from k8s.models.ingress import IngressTLS as V1Beta1IngressTLS
 from k8s.models.networking_v1_ingress import IngressTLS as StableIngressTLS
 
@@ -43,6 +44,7 @@ def configure(self, bind):
         bind("autoscaler", to_class=AutoscalerDeployer)
         bind("ingress_tls_deployer", to_class=IngressTLSDeployer)
         bind("owner_references", to_class=OwnerReferences)
+        bind("pod_disruption_budget_deployer", to_class=PodDisruptionBudgetDeployer)
 
         if self.use_networkingv1_ingress:
             bind("ingress_adapter", to_class=NetworkingV1IngressAdapter)

fiaas_deploy_daemon/deployer/kubernetes/adapter.py

@@ -27,6 +27,7 @@
 from .ingress import IngressDeployer
 from .service import ServiceDeployer
 from .service_account import ServiceAccountDeployer
+from .pod_disruption_budget import PodDisruptionBudgetDeployer
 
 LOG = logging.getLogger(__name__)
 
@@ -35,7 +36,8 @@ class K8s(object):
     """Adapt from an AppSpec to the necessary definitions for a kubernetes cluster"""
 
     def __init__(
-        self, config, service_deployer, deployment_deployer, ingress_deployer, autoscaler, service_account_deployer
+        self, config, service_deployer, deployment_deployer, ingress_deployer,
+        autoscaler, service_account_deployer, pod_disruption_budget_deployer
     ):
         self._version = config.version
         self._enable_service_account_per_app = config.enable_service_account_per_app
@@ -44,6 +46,7 @@ def __init__(
         self._ingress_deployer: IngressDeployer = ingress_deployer
         self._autoscaler_deployer: AutoscalerDeployer = autoscaler
         self._service_account_deployer: ServiceAccountDeployer = service_account_deployer
+        self._pod_disruption_budget_deployer: PodDisruptionBudgetDeployer = pod_disruption_budget_deployer
 
     def deploy(self, app_spec: AppSpec):
         if _besteffort_qos_is_required(app_spec):
@@ -56,6 +59,7 @@ def deploy(self, app_spec: AppSpec):
         self._ingress_deployer.deploy(app_spec, labels)
         self._deployment_deployer.deploy(app_spec, selector, labels, _besteffort_qos_is_required(app_spec))
         self._autoscaler_deployer.deploy(app_spec, labels)
+        self._pod_disruption_budget_deployer.deploy(app_spec, selector, labels)
 
     def delete(self, app_spec: AppSpec):
         self._ingress_deployer.delete(app_spec)

fiaas_deploy_daemon/deployer/kubernetes/pod_disruption_budget.py

@@ -0,0 +1,72 @@
+#!/usr/bin/env python
+# -*- coding: utf-8
+
+# Copyright 2017-2019 The FIAAS Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+import logging
+
+from k8s.client import NotFound
+from k8s.models.common import ObjectMeta, LabelSelector
+from k8s.models.policy_v1_pod_disruption_budget import PodDisruptionBudget, PodDisruptionBudgetSpec
+
+from fiaas_deploy_daemon.retry import retry_on_upsert_conflict
+from fiaas_deploy_daemon.specs.models import AppSpec
+from fiaas_deploy_daemon.tools import merge_dicts
+
+LOG = logging.getLogger(__name__)
+
+
+class PodDisruptionBudgetDeployer(object):
+    def __init__(self, owner_references, extension_hook):
+        self._owner_references = owner_references
+        self._extension_hook = extension_hook
+
+    @retry_on_upsert_conflict
+    def deploy(self, app_spec: AppSpec, selector: dict[str, any], labels: dict[str, any]):
+        if app_spec.autoscaler.min_replicas == 1 or app_spec.autoscaler.max_replicas == 1:
+            # delete possible existing pdb
+            self.delete(app_spec)
+            return
+
+        custom_labels = labels
+        custom_labels = merge_dicts(app_spec.labels.pod_disruption_budget, custom_labels)
+        metadata = ObjectMeta(
+            name=app_spec.name,
+            namespace=app_spec.namespace,
+            labels=custom_labels,
+            annotations=app_spec.annotations.pod_disruption_budget
+        )
+
+        # Conservative default to ensure that we don't block evictions.
+        # See https://github.com/fiaas/fiaas-deploy-daemon/issues/220 for discussion.
+        max_unavailable = 1
+        spec = PodDisruptionBudgetSpec(
+            selector=LabelSelector(matchLabels=selector),
+            maxUnavailable=max_unavailable
+        )
+
+        pdb = PodDisruptionBudget.get_or_create(metadata=metadata, spec=spec)
+
+        self._owner_references.apply(pdb, app_spec)
+        self._extension_hook.apply(pdb, app_spec)
+        pdb.save()
+
+    def delete(self, app_spec):
+        LOG.info("Deleting podDisruptionBudget for %s", app_spec.name)
+        try:
+            PodDisruptionBudget.delete(app_spec.name, app_spec.namespace)
+        except NotFound:
+            pass

fiaas_deploy_daemon/specs/models.py

@@ -109,6 +109,7 @@ def version(self):
         "service_account",
         "pod",
         "status",
+        "pod_disruption_budget",
     ],
 )
 

fiaas_deploy_daemon/specs/v3/defaults.yml

@@ -84,13 +84,15 @@ annotations: # Annotations to be set on generated objects
   ingress: {}
   service: {}
   pod: {}
+  pod_disruption_budget: {}
 labels: # Labels to be set on generated objects
   deployment: {}
   horizontal_pod_autoscaler: {}
   service_account: {}
   ingress: {}
   service: {}
   pod: {}
+  pod_disruption_budget: {}
 secrets_in_environment: false # If a Secret of the same name exists, it is exposed to the pod as environment variables
 admin_access: false # What access the pod has to the k8s api server
 extensions:

fiaas_deploy_daemon/specs/v3/factory.py

@@ -220,6 +220,7 @@ def _labels_annotations_spec(labels_annotations_lookup, overrides):
             "service_account": dict(labels_annotations_lookup["service_account"]),
             "pod": dict(labels_annotations_lookup["pod"]),
             "status": {},
+            "pod_disruption_budget": dict(labels_annotations_lookup["pod_disruption_budget"]),
         }
         if overrides:
             globals = _get_value("_global", overrides)

tests/fiaas_deploy_daemon/conftest.py

@@ -91,8 +91,8 @@ def app_spec():
         teams=["foo"],
         tags=["bar"],
         deployment_id="test_app_deployment_id",
-        labels=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}),
-        annotations=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}),
+        labels=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}, {}),
+        annotations=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}, {}),
         ingresses=[
             IngressItemSpec(host=None, pathmappings=[IngressPathMappingSpec(path="/", port=80)], annotations={})
         ],

tests/fiaas_deploy_daemon/deployer/kubernetes/test_adapter.py

@@ -23,6 +23,7 @@
 from fiaas_deploy_daemon.deployer.kubernetes.ingress import IngressDeployer
 from fiaas_deploy_daemon.deployer.kubernetes.service import ServiceDeployer
 from fiaas_deploy_daemon.deployer.kubernetes.service_account import ServiceAccountDeployer
+from fiaas_deploy_daemon.deployer.kubernetes.pod_disruption_budget import PodDisruptionBudgetDeployer
 from fiaas_deploy_daemon.specs.models import ResourcesSpec, ResourceRequirementSpec
 
 FIAAS_VERSION = "1"
@@ -51,6 +52,10 @@ def ingress_deployer(self):
     def autoscaler_deployer(self):
         return mock.create_autospec(AutoscalerDeployer)
 
+    @pytest.fixture(autouse=True)
+    def pod_disruption_budget_deployer(self):
+        return mock.create_autospec(PodDisruptionBudgetDeployer)
+
     @pytest.fixture(autouse=True)
     def resource_quota_list(self):
         with mock.patch("k8s.models.resourcequota.ResourceQuota.list") as mockk:
@@ -59,7 +64,9 @@ def resource_quota_list(self):
 
     @pytest.fixture
     def k8s(
-        self, service_deployer, deployment_deployer, ingress_deployer, autoscaler_deployer, service_account_deployer
+        self, service_deployer, deployment_deployer, ingress_deployer,
+        autoscaler_deployer, service_account_deployer,
+        pod_disruption_budget_deployer
     ):
         config = mock.create_autospec(Configuration([]), spec_set=True)
         config.version = FIAAS_VERSION
@@ -70,6 +77,7 @@ def k8s(
             ingress_deployer,
             autoscaler_deployer,
             service_account_deployer,
+            pod_disruption_budget_deployer,
         )
 
     def test_make_labels(self, k8s, app_spec):
@@ -174,6 +182,14 @@ def test_pass_to_service(self, app_spec, k8s, service_deployer, resource_quota_l
 
         pytest.helpers.assert_any_call(service_deployer.deploy, app_spec, selector, labels)
 
+    def test_pass_to_pod_disruption_budget(self, app_spec, k8s, pod_disruption_budget_deployer, resource_quota_list):
+        selector = _make_selector(app_spec)
+        labels = k8s._make_labels(app_spec)
+
+        k8s.deploy(app_spec)
+
+        pytest.helpers.assert_any_call(pod_disruption_budget_deployer.deploy, app_spec, selector, labels)
+
     @pytest.mark.parametrize("service_account_per_app_enabled", (True, False))
     def test_pass_to_service_account(
         self,
@@ -186,6 +202,7 @@ def test_pass_to_service_account(
         autoscaler_deployer,
         service_account_deployer,
         service_account_per_app_enabled,
+        pod_disruption_budget_deployer,
     ):
 
         config = mock.create_autospec(Configuration([]), spec_set=True)
@@ -198,6 +215,7 @@ def test_pass_to_service_account(
             ingress_deployer,
             autoscaler_deployer,
             service_account_deployer,
+            pod_disruption_budget_deployer,
         )
 
         labels = k8s._make_labels(app_spec)

tests/fiaas_deploy_daemon/deployer/kubernetes/test_autoscaler.py

@@ -116,6 +116,7 @@ def test_new_autoscaler_with_custom_labels_and_annotations(
             pod={},
             status={},
             service_account={},
+            pod_disruption_budget={},
         )
         annotations = LabelAndAnnotationSpec(
             deployment={},
@@ -125,6 +126,7 @@ def test_new_autoscaler_with_custom_labels_and_annotations(
             pod={},
             status={},
             service_account={},
+            pod_disruption_budget={},
         )
         app_spec = app_spec._replace(labels=labels, annotations=annotations)
 

tests/fiaas_deploy_daemon/deployer/kubernetes/test_deployment_deploy.py

@@ -216,6 +216,7 @@ def app_spec(self, request, app_spec):
             service_account={},
             pod=pod_labels,
             status={},
+            pod_disruption_budget={},
         )
         annotations = LabelAndAnnotationSpec(
             deployment=deploy_annotations,
@@ -225,6 +226,7 @@ def app_spec(self, request, app_spec):
             service_account={},
             pod=pod_annotations,
             status={},
+            pod_disruption_budget={},
         )
 
         if generic_toggle:

tests/fiaas_deploy_daemon/deployer/kubernetes/test_ingress_deploy.py

@@ -100,8 +100,8 @@ def app_spec(**kwargs):
         teams=["foo"],
         tags=["bar"],
         deployment_id="test_app_deployment_id",
-        labels=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}),
-        annotations=LabelAndAnnotationSpec({}, {}, ANNOTATIONS.copy(), {}, {}, {}, {}),
+        labels=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}, {}),
+        annotations=LabelAndAnnotationSpec({}, {}, ANNOTATIONS.copy(), {}, {}, {}, {}, {}),
         ingresses=[
             IngressItemSpec(host=None, pathmappings=[IngressPathMappingSpec(path="/", port=80)], annotations={})
         ],
@@ -655,6 +655,7 @@ class IngressTestcase:
                 service_account={},
                 pod={},
                 status={},
+                pod_disruption_budget={},
             ),
             annotations=LabelAndAnnotationSpec(
                 deployment={},
@@ -664,6 +665,7 @@ class IngressTestcase:
                 service_account={},
                 pod={},
                 status={},
+                pod_disruption_budget={},
             ),
         ),
         ingress(

tests/fiaas_deploy_daemon/deployer/kubernetes/test_networking_v1_ingress.py

@@ -98,8 +98,8 @@ def app_spec(**kwargs):
         teams=["foo"],
         tags=["bar"],
         deployment_id="test_app_deployment_id",
-        labels=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}),
-        annotations=LabelAndAnnotationSpec({}, {}, ANNOTATIONS.copy(), {}, {}, {}, {}),
+        labels=LabelAndAnnotationSpec({}, {}, {}, {}, {}, {}, {}, {}),
+        annotations=LabelAndAnnotationSpec({}, {}, ANNOTATIONS.copy(), {}, {}, {}, {}, {}),
         ingresses=[
             IngressItemSpec(host=None, pathmappings=[IngressPathMappingSpec(path="/", port=80)], annotations={})
         ],
@@ -366,6 +366,7 @@ class IngressTestcase:
                 service_account={},
                 pod={},
                 status={},
+                pod_disruption_budget={},
             ),
             annotations=LabelAndAnnotationSpec(
                 deployment={},
@@ -375,6 +376,7 @@ class IngressTestcase:
                 service_account={},
                 pod={},
                 status={},
+                pod_disruption_budget={},
             ),
         ),
         ingress(