Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(backport) feat: add configurable replicas and antiaffinity to gateway deployment #556

Merged
merged 1 commit into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading