Merge pull request #260 from fiaas/pdb-unhealthy-pod-eviction-policy

Add support in PDBs to be able to set unhealthyPodEvictionPolicy.

README.md

@@ -21,7 +21,7 @@ limitations under the License.
 
 [![Build status](https://fiaas-svc.semaphoreci.com/badges/fiaas-deploy-daemon.svg?style=shields)](https://fiaas-svc.semaphoreci.com/projects/fiaas-deploy-daemon)
 
-You need Python 3.9+ and `pip`(7.x.x or higher)  on your `PATH` to work with fiaas-deploy-daemon.
+You need Python 3.12 and `pip`(7.x.x or higher)  on your `PATH` to work with fiaas-deploy-daemon.
 
 Supported use-cases
 -------------------

docs/operator_guide.md

@@ -211,6 +211,13 @@ This feature is disabled by default.
 
 By default, PDB will be set with maxUnavailable = 1 in case the deployment has more than 1 replica. This parameter allows to change that value either with another int value or with a string percentage (i.e. "20%"). Recommendation is some low value like 10% to avoid issues. Also in case of a number, something below min_replicas should be used to allow evictions.
 
+### pdb-unhealthy-pod-eviction-policy
+This option is useful for avoiding operational problems, with node rotations, when there exists deployments with only unhealthy pods. This can be useful in dev clusters which have deployment with only unhealthy pods or in production clusters where the applications tolerate unhealthy pods being disrupted, while new pods are being started.
+
+This option Controls when unhealthy running pods can be evicted. The default 'IfHealthyBudget' allows eviction only if enough healthy pods are available to maintain the desired count of healthy pods. 'AlwaysAllow' permits eviction of unhealthy running pods even if doing so would leave fewer pods than desired temporarily.
+
+[Kubernetes documentation is available here.](https://kubernetes.io/docs/tasks/run-application/configure-pdb/#unhealthy-pod-eviction-policy)
+
 ### disable-service-links
 
 By default, [enableServiceLinks](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#podspec-v1-core) is set to true, which means that environment variables are automatically created for each service in the same namespace. This parameter allows you to disable this feature by setting disable-service-links to true. This can be useful in scenarios where you have a large number of services and you want to reduce the number of environment variables that are automatically created. For new installations, it's recommended to disable this feature to streamline the environment setup. However, for existing installations, proceed with caution when disabling this feature. This is due to potential compatibility issues, as some services might rely on these automatically created environment variables for communication.

fiaas_deploy_daemon/config.py

@@ -328,6 +328,16 @@ def _parse_args(self, args):
             default="1",
             type=_int_or_unicode
         )
+        parser.add_argument(
+            "--pdb-unhealthy-pod-eviction-policy",
+            help=(
+                "The policy for unhealthy pods. IfHealthyBudget will only evict pods if the there "
+                "are enough healthy pods to maintain the budget. AlwaysAllow will allow "
+                "disruptions of unhealthy pods, even when the budget is not met."
+            ),
+            default="IfHealthyBudget",
+            choices=("IfHealthyBudget", "AlwaysAllow"),
+        )
         # Logic is inverted due to ConfigArgParse not supporting boolean flags with negation
         parser.add_argument(
             "--disable-service-links",

fiaas_deploy_daemon/deployer/kubernetes/pod_disruption_budget.py

@@ -34,6 +34,7 @@ def __init__(self, owner_references, extension_hook, config):
         self._owner_references = owner_references
         self._extension_hook = extension_hook
         self.max_unavailable = config.pdb_max_unavailable
+        self.unhealthy_pod_eviction_policy = config.pdb_unhealthy_pod_eviction_policy
 
     @retry_on_upsert_conflict
     def deploy(self, app_spec: AppSpec, selector: dict[str, any], labels: dict[str, any]):
@@ -58,7 +59,8 @@ def deploy(self, app_spec: AppSpec, selector: dict[str, any], labels: dict[str,
 
         spec = PodDisruptionBudgetSpec(
             selector=LabelSelector(matchLabels=selector),
-            maxUnavailable=max_unavailable
+            maxUnavailable=max_unavailable,
+            unhealthyPodEvictionPolicy=self.unhealthy_pod_eviction_policy
         )
 
         pdb = PodDisruptionBudget.get_or_create(metadata=metadata, spec=spec)

tests/fiaas_deploy_daemon/deployer/kubernetes/test_pod_disruption_budget_deploy.py

@@ -43,8 +43,7 @@ def deployer(self, owner_references, extension_hook, config):
 
     @pytest.fixture
     def config(self):
-        config = create_autospec(Configuration([]), spec_set=True)
-        config.pdb_max_unavailable = 1
+        config = Configuration([])
         return config
 
     @pytest.mark.usefixtures("get")
@@ -54,6 +53,7 @@ def test_new_pdb(self, deployer, post, app_spec, owner_references, extension_hoo
             "spec": {
                 "maxUnavailable": 1,
                 "selector": {"matchExpressions": [], "matchLabels": {"app": "testapp"}},
+                "unhealthyPodEvictionPolicy": "IfHealthyBudget",
             },
         }
         mock_response = create_autospec(Response)
@@ -101,6 +101,7 @@ def test_setting_max_int(self, deployer, post, app_spec, owner_references, exten
             "spec": {
                 "maxUnavailable": 2,
                 "selector": {"matchExpressions": [], "matchLabels": {"app": "testapp"}},
+                "unhealthyPodEvictionPolicy": "IfHealthyBudget",
             },
         }
         mock_response = create_autospec(Response)
@@ -132,6 +133,31 @@ def test_setting_max_str(self, deployer, post, app_spec, owner_references, exten
             "spec": {
                 "maxUnavailable": "20%",
                 "selector": {"matchExpressions": [], "matchLabels": {"app": "testapp"}},
+                "unhealthyPodEvictionPolicy": "IfHealthyBudget",
+            },
+        }
+        mock_response = create_autospec(Response)
+        mock_response.json.return_value = expected_pdb
+        post.return_value = mock_response
+
+        deployer.deploy(app_spec, SELECTOR, LABELS)
+
+        pytest.helpers.assert_any_call(post, PDB_API, expected_pdb)
+        owner_references.apply.assert_called_once_with(TypeMatcher(PodDisruptionBudget), app_spec)
+        extension_hook.apply.assert_called_once_with(TypeMatcher(PodDisruptionBudget), app_spec)
+
+    @pytest.mark.usefixtures("get")
+    def test_setting_unhealthy_policy(self, deployer, post, app_spec, owner_references, extension_hook):
+        app_spec = app_spec._replace(
+            autoscaler=AutoscalerSpec(enabled=True, min_replicas=4, max_replicas=6, cpu_threshold_percentage=50)
+        )
+        deployer.unhealthy_pod_eviction_policy = "AlwaysAllow"
+        expected_pdb = {
+            "metadata": pytest.helpers.create_metadata("testapp", labels=LABELS),
+            "spec": {
+                "maxUnavailable": 1,
+                "selector": {"matchExpressions": [], "matchLabels": {"app": "testapp"}},
+                "unhealthyPodEvictionPolicy": "AlwaysAllow",
             },
         }
         mock_response = create_autospec(Response)