Fix service-mirror template when running in HA mode#11609
Merged
mateiidavid merged 3 commits intomainfrom Nov 22, 2023
Merged
Conversation
Two clusters can be linked in HA mode. When HA values are used, the service-mirror deployment receives some pod affinity rules to ensure fair scheduling of pods across a cluster's nodes. The service-mirror Deployment's template seems to be broken at the moment when using HA values. Affinity rules are incorrectly grouped under a top-level `podAntiAffinity` field. The Kubernetes API requires the rules to be grouped under a top-level `affinity` field. This change rectifies that by introducing the missing parent. Fixes #11603 Signed-off-by: Matei David <[email protected]>
adleong
reviewed
Nov 13, 2023
Member
adleong
left a comment
adleong
left a comment
There was a problem hiding this comment.
Would a test in multicluster/cmd/link_test.go for HA mode have caught this? can we add one?
Signed-off-by: Matei David <[email protected]>
Signed-off-by: Matei David <[email protected]>
Member
Author
|
@adleong added a test, good suggestion! I was actually unaware we already had some fixture-based tests for link output. It might've saved us when we introduced the change, it maybe would've forced us to re-read the manifest and notice the missing key. Ah well, moving forward we will have a golden file to compare against. I noticed we had an awkward newline when not using HA: 157- spec:
158-
159: containers:
160- - args:
161- - service-mirror
162- - -log-level=infoGo somehow still renders the newline even if the subtemplate eats the whitespace correctly... using a guard is the only way I managed to fix this. It makes it a bit more complicated than it has to be but at least it looks good. Last few things I did:
|
adleong
approved these changes
Nov 17, 2023
Merged
hawkw
added a commit
that referenced
this pull request
Nov 22, 2023
## edge-23.11.4 This edge release introduces support for the native sidecar containers entering beta support in Kubernetes 1.29. This improves the startup and shutdown ordering for the proxy relative to other containers, fixing the long-standing shutdown issue with injected `Job`s. Furthermore, traffic from other `initContainer`s can now be proxied by Linkerd. In addition, this edge release includes Helm chart improvements, and improvements to the multicluster extension. * Added a new `config.alpha.linkerd.io/proxy-enable-native-sidecar` annotation and `Proxy.NativeSidecar` Helm option that causes the proxy container to run as an init-container (thanks @teejaded!) (#11465; fixes #11461) * Fixed broken affinity rules for the multicluster `service-mirror` when running in HA mode (#11609; fixes #11603) * Added a new check to `linkerd check` that ensures all extension namespaces are configured properly (#11629; fixes #11509) * Updated the Prometheus Docker image used by the `linkerd-viz` extension to v2.48.0, resolving a number of CVEs in older Prometheus versions (#11633) * Added `nodeAffinity` to `deployment` templates in the `linkerd-viz` and `linkerd-jaeger` Helm charts (thanks @naing2victor!) (#11464; fixes #10680)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two clusters can be linked in HA mode. When HA values are used, the service-mirror deployment receives some pod affinity rules to ensure fair scheduling of pods across a cluster's nodes.
The service-mirror Deployment's template seems to be broken at the moment when using HA values. Affinity rules are incorrectly grouped under a top-level
podAntiAffinityfield. The Kubernetes API requires the rules to be grouped under a top-levelaffinityfield. This change rectifies that by introducing the missing parent.Fixes #11603
Tests
linkerd.affinitysubchart (i.e. partial). Instead of bringing in parts of the subchart, we are now copying the install values and including the entire partial template. Based on the values, it should populate the affinity rules.Before:
After:
We're not breaking non-ha:
Render diff: