Skip to content

Commit

Permalink
fix: retain non-nginx canary annotations. Fixes: #1070 (#3806)
Browse files Browse the repository at this point in the history
fix: retain nginx canary annotations

Signed-off-by: Jahvon Dockery <jdockery@cargurus.com>
  • Loading branch information
jahvon committed Aug 27, 2024
1 parent 8a9ef9f commit f9b62a8
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 0 deletions.
14 changes: 14 additions & 0 deletions rollout/trafficrouting/nginx/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ func (r *Reconciler) SetWeightPerIngress(desiredWeight int32, ingresses []string
}

// Make patches
desiredCanaryIngress.SetAnnotations(getDesiredAnnotations(canaryIngress, desiredCanaryIngress))
patch, modified, err := ingressutil.BuildIngressPatch(canaryIngress.Mode(), canaryIngress,
desiredCanaryIngress, ingressutil.WithAnnotations(), ingressutil.WithLabels(), ingressutil.WithSpec())

Expand Down Expand Up @@ -371,3 +372,16 @@ func (r *Reconciler) SetMirrorRoute(setMirrorRoute *v1alpha1.SetMirrorRoute) err
func (r *Reconciler) RemoveManagedRoutes() error {
return nil
}

func getDesiredAnnotations(current, desired *ingressutil.Ingress) map[string]string {
// Merge existing annotations into the desired Ingress (giving precedence to the desired values)
// This is necessary because the desired Ingress may not have all annotations previously added
// by other controllers (e.g. Rancher)
desiredAnnotations := desired.GetAnnotations()
for k, v := range current.GetAnnotations() {
if _, ok := desiredAnnotations[k]; !ok {
desiredAnnotations[k] = v
}
}
return desiredAnnotations
}
31 changes: 31 additions & 0 deletions rollout/trafficrouting/nginx/nginx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,37 @@ func TestCanaryIngressAdditionalAnnotationsNewIngress(t *testing.T) {
}
}

func TestCanaryIngressRetainCurrentAnnotations(t *testing.T) {
tests := generateMultiIngressTestData()
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
for _, ing := range test.ingresses {
r := Reconciler{
cfg: ReconcilerConfig{
Rollout: fakeRollout(stableService, canaryService, test.singleIngress, test.multiIngress),
},
}
stable := networkingIngress(StableIngress, 80, stableService)
stableIngress := ingressutil.NewIngress(stable)
canary := networkingIngress(ingressutil.GetCanaryIngressName(r.cfg.Rollout.GetName(), ing), 80, canaryService)
canary.SetAnnotations(map[string]string{
"nginx.ingress.kubernetes.io/canary": "false",
"annotation-prefix.some.group/some-key": "some-value",
})
canaryIngress := ingressutil.NewIngress(canary)

desiredCanaryIngress, err := r.canaryIngress(stableIngress, ingressutil.GetCanaryIngressName(r.cfg.Rollout.GetName(), ing), 15)
assert.Nil(t, err, "No error returned when calling canaryIngress")

annotations := getDesiredAnnotations(canaryIngress, desiredCanaryIngress)
assert.Equal(t, "true", annotations["nginx.ingress.kubernetes.io/canary"], "canary annotation set to true")
assert.Equal(t, "15", annotations["nginx.ingress.kubernetes.io/canary-weight"], "canary-weight annotation set to expected value")
assert.Equal(t, "some-value", annotations["annotation-prefix.some.group/some-key"], "existing canary annotation is retained")
}
})
}
}

func TestCanaryIngressMaxWeightInTrafficRouting(t *testing.T) {
maxWeights := []*int32{nil, pointer.Int32(1000)}
for _, maxWeight := range maxWeights {
Expand Down

0 comments on commit f9b62a8

Please sign in to comment.