Skip to content

Commit

Permalink
Merge pull request #556 from canonical/KF-6086-backport-istio-resiliency
Browse files Browse the repository at this point in the history
(backport) feat: add configurable replicas and antiaffinity to gateway deployment
  • Loading branch information
NohaIhab authored Oct 1, 2024
2 parents e6c8b33 + d24ca01 commit b81ba7b
Show file tree
Hide file tree
Showing 7 changed files with 149 additions and 0 deletions.
4 changes: 4 additions & 0 deletions charms/istio-gateway/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions charms/istio-gateway/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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:
Expand Down
10 changes: 10 additions & 0 deletions charms/istio-gateway/src/manifest.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ metadata:
name: istio-{{ kind }}gateway-workload
namespace: {{ namespace }}
spec:
replicas: {{ replicas }}
selector:
matchLabels:
app: istio-{{ kind }}gateway
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions charms/istio-gateway/tests/unit/data/egress-example.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ metadata:
name: istio-egressgateway-workload
namespace: None
spec:
replicas: 1
selector:
matchLabels:
app: istio-egressgateway
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions charms/istio-gateway/tests/unit/data/ingress-example.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ metadata:
name: istio-ingressgateway-workload
namespace: None
spec:
replicas: 1
selector:
matchLabels:
app: istio-ingressgateway
Expand Down Expand Up @@ -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
Expand Down
78 changes: 78 additions & 0 deletions charms/istio-gateway/tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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("")
35 changes: 35 additions & 0 deletions tests/test_bundle.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -333,6 +334,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
Expand Down

0 comments on commit b81ba7b

Please sign in to comment.