From aed4f2002f6699d86e13cdfebe22aba2cf8b9461 Mon Sep 17 00:00:00 2001 From: Noha Ihab <49988746+NohaIhab@users.noreply.github.com> Date: Tue, 1 Oct 2024 10:03:39 +0300 Subject: [PATCH] feat: add configurable replicas and antiaffinity to gateway deployment for HA (#553) * add configurable replicas to gateway deployment * add integration test for replicas * add unit tests --- charms/istio-gateway/config.yaml | 4 + charms/istio-gateway/src/charm.py | 2 + charms/istio-gateway/src/manifest.yaml | 10 +++ .../tests/unit/data/egress-example.yaml | 10 +++ .../tests/unit/data/ingress-example.yaml | 10 +++ charms/istio-gateway/tests/unit/test_charm.py | 78 +++++++++++++++++++ tests/test_bundle.py | 35 +++++++++ 7 files changed, 149 insertions(+) diff --git a/charms/istio-gateway/config.yaml b/charms/istio-gateway/config.yaml index 2dadadeb..54fb1aa1 100644 --- a/charms/istio-gateway/config.yaml +++ b/charms/istio-gateway/config.yaml @@ -12,3 +12,7 @@ options: default: 'docker.io/istio/proxyv2:1.22.0' description: Istio Proxy image type: string + replicas: + default: 1 + description: Number of replicas for the istio-ingressgateway pods + type: int diff --git a/charms/istio-gateway/src/charm.py b/charms/istio-gateway/src/charm.py index 3c258b01..b1ec9199 100755 --- a/charms/istio-gateway/src/charm.py +++ b/charms/istio-gateway/src/charm.py @@ -94,6 +94,7 @@ def start(self, event): pilot_host=pilot["service-name"], pilot_port=pilot["service-port"], gateway_service_type=self.model.config["gateway_service_type"], + replicas=self.model.config["replicas"], ) for obj in codecs.load_all_yaml(rendered): @@ -113,6 +114,7 @@ def remove(self, event): proxy_image=self.model.config["proxy-image"], pilot_host="foo", pilot_port="foo", + replicas=self.model.config["replicas"], ) try: diff --git a/charms/istio-gateway/src/manifest.yaml b/charms/istio-gateway/src/manifest.yaml index 5fb11c6c..af8b2581 100644 --- a/charms/istio-gateway/src/manifest.yaml +++ b/charms/istio-gateway/src/manifest.yaml @@ -25,6 +25,7 @@ metadata: name: istio-{{ kind }}gateway-workload namespace: {{ namespace }} spec: + replicas: {{ replicas }} selector: matchLabels: app: istio-{{ kind }}gateway @@ -54,6 +55,15 @@ spec: sidecar.istio.io/inject: "false" spec: affinity: + podAntiAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + - labelSelector: + matchExpressions: + - key: app + operator: In + values: + - istio-{{ kind }}gateway + topologyKey: kubernetes.io/hostname nodeAffinity: preferredDuringSchedulingIgnoredDuringExecution: null requiredDuringSchedulingIgnoredDuringExecution: null diff --git a/charms/istio-gateway/tests/unit/data/egress-example.yaml b/charms/istio-gateway/tests/unit/data/egress-example.yaml index a60b4005..4c6a602c 100644 --- a/charms/istio-gateway/tests/unit/data/egress-example.yaml +++ b/charms/istio-gateway/tests/unit/data/egress-example.yaml @@ -24,6 +24,7 @@ metadata: name: istio-egressgateway-workload namespace: None spec: + replicas: 1 selector: matchLabels: app: istio-egressgateway @@ -53,6 +54,15 @@ spec: sidecar.istio.io/inject: "false" spec: affinity: + podAntiAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + - labelSelector: + matchExpressions: + - key: app + operator: In + values: + - istio-egressgateway + topologyKey: kubernetes.io/hostname nodeAffinity: preferredDuringSchedulingIgnoredDuringExecution: null requiredDuringSchedulingIgnoredDuringExecution: null diff --git a/charms/istio-gateway/tests/unit/data/ingress-example.yaml b/charms/istio-gateway/tests/unit/data/ingress-example.yaml index 56b30b0f..2e83fb29 100644 --- a/charms/istio-gateway/tests/unit/data/ingress-example.yaml +++ b/charms/istio-gateway/tests/unit/data/ingress-example.yaml @@ -24,6 +24,7 @@ metadata: name: istio-ingressgateway-workload namespace: None spec: + replicas: 1 selector: matchLabels: app: istio-ingressgateway @@ -53,6 +54,15 @@ spec: sidecar.istio.io/inject: "false" spec: affinity: + podAntiAffinity: + requiredDuringSchedulingIgnoredDuringExecution: + - labelSelector: + matchExpressions: + - key: app + operator: In + values: + - istio-ingressgateway + topologyKey: kubernetes.io/hostname nodeAffinity: preferredDuringSchedulingIgnoredDuringExecution: null requiredDuringSchedulingIgnoredDuringExecution: null diff --git a/charms/istio-gateway/tests/unit/test_charm.py b/charms/istio-gateway/tests/unit/test_charm.py index b271f2c0..9d1cac8f 100644 --- a/charms/istio-gateway/tests/unit/test_charm.py +++ b/charms/istio-gateway/tests/unit/test_charm.py @@ -158,3 +158,81 @@ def test_metrics(harness): } ], ) + + +def test_manifests_applied_with_anti_affinity(configured_harness, kind, mocked_client): + """ + Asserts that the Deployment manifest called by `lightkube_client.apply` + contains the correct anti-affinity rule + """ + + # Arrange + # Reset the mock so that the calls list does not include any calls from other hooks + mocked_client.reset_mock() + + # Define the expected labelSelector based on the kind + expected_label_selector = {"key": "app", "operator": "In", "values": [f"istio-{kind}gateway"]} + + # Act + configured_harness.charm.on.start.emit() + + actual_objects = [] + + # Get all the objects called by lightkube client `.apply` + for call in mocked_client.return_value.apply.call_args_list: + # The first (and only) argument to the apply method is the obj + # Convert the object to a dictionary and add it to the list + actual_objects.append(call.args[0].to_dict()) + + # Filter out the objects with Deployment kind + deployments = filter(lambda obj: obj.get("kind") == "Deployment", actual_objects) + # The gateway deployment is the only Deployment object in the manifests + gateway_deployment = list(deployments)[0] + + # Assert the Deployment has the correct antiaffinity rule + assert ( + gateway_deployment["spec"] + .get("template") + .get("spec") + .get("affinity") + .get("podAntiAffinity") + .get("requiredDuringSchedulingIgnoredDuringExecution")[0] + .get("labelSelector") + .get("matchExpressions")[0] + == expected_label_selector + ) + + +def test_manifests_applied_with_replicas_config(configured_harness, mocked_client): + """ + Asserts that the Deployment manifest called by `lightkube_client.apply` + contains the replicas config value. + """ + + # Arrange + # Reset the mock so that the calls list does not include any calls from other hooks + mocked_client.reset_mock() + + # Update the replicas config in the harness + replicas_config_value = 2 + configured_harness.update_config({"replicas": replicas_config_value}) + + # Act + configured_harness.charm.on.install.emit() + + actual_objects = [] + + # Get all the objects called by lightkube client `.apply` + for call in mocked_client.return_value.apply.call_args_list: + # The first (and only) argument to the apply method is the obj + # Convert the object to a dictionary and add it to the list + actual_objects.append(call.args[0].to_dict()) + + # Filter out the objects with Deployment kind + deployments = filter(lambda obj: obj.get("kind") == "Deployment", actual_objects) + # The gateway deployment is the only Deployment object in the manifests + gateway_deployment = list(deployments)[0] + + # Assert + assert gateway_deployment["spec"].get("replicas") == replicas_config_value + assert configured_harness.charm.model.unit.status == ActiveStatus("") diff --git a/tests/test_bundle.py b/tests/test_bundle.py index dde7bf27..e26d550f 100644 --- a/tests/test_bundle.py +++ b/tests/test_bundle.py @@ -15,6 +15,7 @@ create_namespaced_resource, load_in_cluster_generic_resources, ) +from lightkube.resources.core_v1 import Pod from pytest_operator.plugin import OpsTest log = logging.getLogger(__name__) @@ -332,6 +333,40 @@ async def test_disable_ingress_auth(ops_test: OpsTest): ) +async def test_gateway_replicas_config_pod_anti_affinity(ops_test: OpsTest): + """Test changing the replicas config to 2, and Assert the new Pod was not scheduled + due to only 1 Node being available. + """ + + replicas_value = "2" + await ops_test.model.applications[ISTIO_GATEWAY_APP_NAME].set_config( + {"replicas": replicas_value} + ) + await ops_test.model.wait_for_idle(apps=[ISTIO_GATEWAY_APP_NAME], status="active", timeout=300) + + client = lightkube.Client() + + # List gateway pods that are in Pending status + pending_gateway_pods = list( + client.list( + Pod, + namespace=ops_test.model_name, + labels={"app": "istio-ingressgateway"}, + fields={"status.phase": "Pending"}, + ) + ) + + # Assert one Pod is in Pending status + assert len(pending_gateway_pods) == 1 + + # Get the status message for the Pending pod + pending_gateway_pod = pending_gateway_pods[0] + message = pending_gateway_pod.status.conditions[0].message + + # Assert the status message is about anti-affinity + assert "didn't match pod anti-affinity rules" in message + + async def test_charms_removal(ops_test: OpsTest): """Test the istio-operators can be removed without errors.""" # NOTE: the istio-gateway charm has to be removed before istio-pilot since