Skip to content

Commit 06e2c30

Browse files
leoluzdanielm-codefresh
authored andcommitted
fix: nginx traffic router patching wrong ingress resource (argoproj#1655)
Signed-off-by: Leonardo Luz Almeida <[email protected]>
1 parent 197f9db commit 06e2c30

File tree

2 files changed

+42
-2
lines changed

2 files changed

+42
-2
lines changed

rollout/trafficrouting/nginx/nginx.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ func (r *Reconciler) SetWeight(desiredWeight int32, additionalDestinations ...v1
298298
r.log.WithField(logutil.IngressKey, canaryIngressName).WithField("desiredWeight", desiredWeight).Info("updating canary Ingress")
299299
r.cfg.Recorder.Eventf(r.cfg.Rollout, record.EventOptions{EventReason: "PatchingCanaryIngress"}, "Updating Ingress `%s` to desiredWeight '%d'", canaryIngressName, desiredWeight)
300300

301-
_, err = r.cfg.Client.ExtensionsV1beta1().Ingresses(r.cfg.Rollout.Namespace).Patch(ctx, canaryIngressName, types.MergePatchType, patch, metav1.PatchOptions{})
301+
_, err = r.cfg.IngressWrapper.Patch(ctx, r.cfg.Rollout.Namespace, canaryIngressName, types.MergePatchType, patch, metav1.PatchOptions{})
302302
if err != nil {
303303
r.log.WithField(logutil.IngressKey, canaryIngressName).WithField("err", err.Error()).Error("error patching canary ingress")
304304
return fmt.Errorf("error patching canary ingress `%s`: %v", canaryIngressName, err)

rollout/trafficrouting/nginx/nginx_test.go

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func networkingIngress(name string, port int, serviceName string) *networkingv1.
3030
return &networkingv1.Ingress{
3131
ObjectMeta: metav1.ObjectMeta{
3232
Name: name,
33-
Namespace: "some-namespace",
33+
Namespace: metav1.NamespaceDefault,
3434
Annotations: map[string]string{
3535
"annotation-key1": "annotation-value1",
3636
},
@@ -511,6 +511,46 @@ func TestReconcileStableAndCanaryIngressFoundPatch(t *testing.T) {
511511
}
512512
}
513513

514+
func TestReconcileWillInvokeNetworkingIngress(t *testing.T) {
515+
// given
516+
rollout := fakeRollout("stable-service", "canary-service", "stable-ingress")
517+
stableIngress := networkingIngress("stable-ingress", 80, "stable-service")
518+
canaryIngress := networkingIngress("rollout-stable-ingress-canary", 80, "canary-service")
519+
canaryIngress.SetAnnotations(map[string]string{
520+
"nginx.ingress.kubernetes.io/canary": "true",
521+
"nginx.ingress.kubernetes.io/canary-weight": "15",
522+
})
523+
canaryIngress.SetOwnerReferences([]metav1.OwnerReference{*metav1.NewControllerRef(rollout, schema.GroupVersionKind{Group: "argoproj.io", Version: "v1alpha1", Kind: "Rollout"})})
524+
client := fake.NewSimpleClientset(stableIngress, canaryIngress)
525+
k8sI := kubeinformers.NewSharedInformerFactory(client, 0)
526+
k8sI.Networking().V1().Ingresses().Informer().GetIndexer().Add(stableIngress)
527+
k8sI.Networking().V1().Ingresses().Informer().GetIndexer().Add(canaryIngress)
528+
ingressWrapper, err := ingressutil.NewIngressWrapper(ingressutil.IngressModeNetworking, client, k8sI)
529+
if err != nil {
530+
t.Fatal(err)
531+
}
532+
r := NewReconciler(ReconcilerConfig{
533+
Rollout: rollout,
534+
Client: client,
535+
Recorder: record.NewFakeEventRecorder(),
536+
ControllerKind: schema.GroupVersionKind{Group: "foo", Version: "v1", Kind: "Bar"},
537+
IngressWrapper: ingressWrapper,
538+
})
539+
540+
// when
541+
err = r.SetWeight(10)
542+
543+
// then
544+
assert.Nil(t, err, "Reconcile returns no error")
545+
actions := client.Actions()
546+
assert.Len(t, actions, 1)
547+
if !t.Failed() {
548+
// Avoid "index out of range" errors
549+
assert.Equal(t, "patch", actions[0].GetVerb(), "action: patch canary ingress")
550+
assert.Equal(t, schema.GroupVersionResource{Group: "networking.k8s.io", Version: "v1", Resource: "ingresses"}, actions[0].GetResource(), "action: patch canary ingress")
551+
}
552+
}
553+
514554
func TestReconcileStableAndCanaryIngressFoundNoChange(t *testing.T) {
515555
rollout := fakeRollout("stable-service", "canary-service", "stable-ingress")
516556
stableIngress := extensionsIngress("stable-ingress", 80, "stable-service")

0 commit comments

Comments
 (0)