From a2784c533e83de1d623b3547b4ec5254cde17415 Mon Sep 17 00:00:00 2001 From: Stefan Prodan Date: Wed, 26 May 2021 09:59:26 +0300 Subject: [PATCH] Upgrade Ingress to networking/v1 - breaking change: drop support for Ingress `k8s.io/api/networking/v1beta1` - routing: use Ingress `k8s.io/api/networking/v1` for NGINX and Skipper routers - e2e: update ingress-nginx v0.46.0 and skipper to v0.13.61 Signed-off-by: Stefan Prodan --- pkg/router/ingress.go | 22 +++++++++--------- pkg/router/ingress_test.go | 8 +++---- pkg/router/router_test.go | 30 ++++++++++++++----------- pkg/router/skipper.go | 22 +++++++++--------- pkg/router/skipper_test.go | 20 ++++++++--------- test/nginx/install.sh | 2 +- test/nginx/test-canary.sh | 40 ++++++++++++++++++++------------- test/skipper/kustomization.yaml | 8 +++---- test/skipper/test-canary.sh | 22 ++++++++++-------- 9 files changed, 95 insertions(+), 79 deletions(-) diff --git a/pkg/router/ingress.go b/pkg/router/ingress.go index c6f4047a..01fd58a3 100644 --- a/pkg/router/ingress.go +++ b/pkg/router/ingress.go @@ -24,7 +24,7 @@ import ( "github.com/google/go-cmp/cmp" "go.uber.org/zap" - "k8s.io/api/networking/v1beta1" + netv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" @@ -48,7 +48,7 @@ func (i *IngressRouter) Reconcile(canary *flaggerv1.Canary) error { canaryName := fmt.Sprintf("%s-canary", apexName) canaryIngressName := fmt.Sprintf("%s-canary", canary.Spec.IngressRef.Name) - ingress, err := i.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Get(context.TODO(), canary.Spec.IngressRef.Name, metav1.GetOptions{}) + ingress, err := i.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Get(context.TODO(), canary.Spec.IngressRef.Name, metav1.GetOptions{}) if err != nil { return fmt.Errorf("ingress %s.%s get query error: %w", canary.Spec.IngressRef.Name, canary.Namespace, err) } @@ -59,8 +59,8 @@ func (i *IngressRouter) Reconcile(canary *flaggerv1.Canary) error { backendExists := false for k, v := range ingressClone.Spec.Rules { for x, y := range v.HTTP.Paths { - if y.Backend.ServiceName == apexName { - ingressClone.Spec.Rules[k].HTTP.Paths[x].Backend.ServiceName = canaryName + if y.Backend.Service != nil && y.Backend.Service.Name == apexName { + ingressClone.Spec.Rules[k].HTTP.Paths[x].Backend.Service.Name = canaryName backendExists = true } } @@ -70,10 +70,10 @@ func (i *IngressRouter) Reconcile(canary *flaggerv1.Canary) error { return fmt.Errorf("backend %s not found in ingress %s", apexName, canary.Spec.IngressRef.Name) } - canaryIngress, err := i.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Get(context.TODO(), canaryIngressName, metav1.GetOptions{}) + canaryIngress, err := i.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Get(context.TODO(), canaryIngressName, metav1.GetOptions{}) if errors.IsNotFound(err) { - ing := &v1beta1.Ingress{ + ing := &netv1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: canaryIngressName, Namespace: canary.Namespace, @@ -90,7 +90,7 @@ func (i *IngressRouter) Reconcile(canary *flaggerv1.Canary) error { Spec: ingressClone.Spec, } - _, err := i.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Create(context.TODO(), ing, metav1.CreateOptions{}) + _, err := i.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Create(context.TODO(), ing, metav1.CreateOptions{}) if err != nil { return fmt.Errorf("ingress %s.%s create error: %w", ing.Name, ing.Namespace, err) } @@ -106,7 +106,7 @@ func (i *IngressRouter) Reconcile(canary *flaggerv1.Canary) error { iClone := canaryIngress.DeepCopy() iClone.Spec = ingressClone.Spec - _, err := i.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Update(context.TODO(), iClone, metav1.UpdateOptions{}) + _, err := i.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Update(context.TODO(), iClone, metav1.UpdateOptions{}) if err != nil { return fmt.Errorf("ingress %s.%s update error: %w", canaryIngressName, iClone.Namespace, err) } @@ -125,7 +125,7 @@ func (i *IngressRouter) GetRoutes(canary *flaggerv1.Canary) ( err error, ) { canaryIngressName := fmt.Sprintf("%s-canary", canary.Spec.IngressRef.Name) - canaryIngress, err := i.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Get(context.TODO(), canaryIngressName, metav1.GetOptions{}) + canaryIngress, err := i.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Get(context.TODO(), canaryIngressName, metav1.GetOptions{}) if err != nil { err = fmt.Errorf("ingress %s.%s get query error: %w", canaryIngressName, canary.Namespace, err) return @@ -166,7 +166,7 @@ func (i *IngressRouter) SetRoutes( _ bool, ) error { canaryIngressName := fmt.Sprintf("%s-canary", canary.Spec.IngressRef.Name) - canaryIngress, err := i.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Get(context.TODO(), canaryIngressName, metav1.GetOptions{}) + canaryIngress, err := i.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Get(context.TODO(), canaryIngressName, metav1.GetOptions{}) if err != nil { return fmt.Errorf("ingress %s.%s get query error: %w", canaryIngressName, canary.Namespace, err) } @@ -201,7 +201,7 @@ func (i *IngressRouter) SetRoutes( iClone.Annotations = i.makeAnnotations(iClone.Annotations) } - _, err = i.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Update(context.TODO(), iClone, metav1.UpdateOptions{}) + _, err = i.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Update(context.TODO(), iClone, metav1.UpdateOptions{}) if err != nil { return fmt.Errorf("ingress %s.%s update error %v", iClone.Name, iClone.Namespace, err) } diff --git a/pkg/router/ingress_test.go b/pkg/router/ingress_test.go index f327406a..53c2df50 100644 --- a/pkg/router/ingress_test.go +++ b/pkg/router/ingress_test.go @@ -45,7 +45,7 @@ func TestIngressRouter_Reconcile(t *testing.T) { canaryWeightAn := "custom.ingress.kubernetes.io/canary-weight" canaryName := fmt.Sprintf("%s-canary", mocks.ingressCanary.Spec.IngressRef.Name) - inCanary, err := router.kubeClient.NetworkingV1beta1().Ingresses("default").Get(context.TODO(), canaryName, metav1.GetOptions{}) + inCanary, err := router.kubeClient.NetworkingV1().Ingresses("default").Get(context.TODO(), canaryName, metav1.GetOptions{}) require.NoError(t, err) // test initialisation @@ -78,7 +78,7 @@ func TestIngressRouter_GetSetRoutes(t *testing.T) { canaryWeightAn := "prefix1.nginx.ingress.kubernetes.io/canary-weight" canaryName := fmt.Sprintf("%s-canary", mocks.ingressCanary.Spec.IngressRef.Name) - inCanary, err := router.kubeClient.NetworkingV1beta1().Ingresses("default").Get(context.TODO(), canaryName, metav1.GetOptions{}) + inCanary, err := router.kubeClient.NetworkingV1().Ingresses("default").Get(context.TODO(), canaryName, metav1.GetOptions{}) require.NoError(t, err) // test rollout @@ -92,7 +92,7 @@ func TestIngressRouter_GetSetRoutes(t *testing.T) { err = router.SetRoutes(mocks.ingressCanary, p, c, m) require.NoError(t, err) - inCanary, err = router.kubeClient.NetworkingV1beta1().Ingresses("default").Get(context.TODO(), canaryName, metav1.GetOptions{}) + inCanary, err = router.kubeClient.NetworkingV1().Ingresses("default").Get(context.TODO(), canaryName, metav1.GetOptions{}) require.NoError(t, err) // test promotion @@ -175,7 +175,7 @@ func TestIngressRouter_ABTest(t *testing.T) { canaryAn := router.GetAnnotationWithPrefix("canary") canaryName := fmt.Sprintf("%s-canary", table.makeCanary().Spec.IngressRef.Name) - inCanary, err := router.kubeClient.NetworkingV1beta1().Ingresses("default").Get(context.TODO(), canaryName, metav1.GetOptions{}) + inCanary, err := router.kubeClient.NetworkingV1().Ingresses("default").Get(context.TODO(), canaryName, metav1.GetOptions{}) require.NoError(t, err) // test initialisation diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go index 9ecbd93d..ee3cb24f 100644 --- a/pkg/router/router_test.go +++ b/pkg/router/router_test.go @@ -20,7 +20,7 @@ import ( "go.uber.org/zap" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - "k8s.io/api/networking/v1beta1" + netv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes" @@ -415,7 +415,7 @@ func newTestCanaryIngress() *flaggerv1.Canary { }, IngressRef: &flaggerv1.CrossNamespaceObjectReference{ Name: "podinfo", - APIVersion: "extensions/v1beta1", + APIVersion: "networking.k8s.io/v1", Kind: "Ingress", }, Service: flaggerv1.CanaryService{ @@ -437,9 +437,9 @@ func newTestCanaryIngress() *flaggerv1.Canary { return cd } -func newTestIngress() *v1beta1.Ingress { - return &v1beta1.Ingress{ - TypeMeta: metav1.TypeMeta{APIVersion: v1beta1.SchemeGroupVersion.String()}, +func newTestIngress() *netv1.Ingress { + return &netv1.Ingress{ + TypeMeta: metav1.TypeMeta{APIVersion: netv1.SchemeGroupVersion.String()}, ObjectMeta: metav1.ObjectMeta{ Namespace: "default", Name: "podinfo", @@ -447,18 +447,22 @@ func newTestIngress() *v1beta1.Ingress { "kubernetes.io/ingress.class": "nginx", }, }, - Spec: v1beta1.IngressSpec{ - Rules: []v1beta1.IngressRule{ + Spec: netv1.IngressSpec{ + Rules: []netv1.IngressRule{ { Host: "app.example.com", - IngressRuleValue: v1beta1.IngressRuleValue{ - HTTP: &v1beta1.HTTPIngressRuleValue{ - Paths: []v1beta1.HTTPIngressPath{ + IngressRuleValue: netv1.IngressRuleValue{ + HTTP: &netv1.HTTPIngressRuleValue{ + Paths: []netv1.HTTPIngressPath{ { Path: "/", - Backend: v1beta1.IngressBackend{ - ServiceName: "podinfo", - ServicePort: intstr.FromInt(9898), + Backend: netv1.IngressBackend{ + Service: &netv1.IngressServiceBackend{ + Name: "podinfo", + Port: netv1.ServiceBackendPort{ + Number: 9898, + }, + }, }, }, }, diff --git a/pkg/router/skipper.go b/pkg/router/skipper.go index a33208ab..76285dbe 100644 --- a/pkg/router/skipper.go +++ b/pkg/router/skipper.go @@ -69,7 +69,7 @@ func (skp *SkipperRouter) Reconcile(canary *flaggerv1.Canary) error { apexIngressName, canaryIngressName := skp.getIngressNames(canary.Spec.IngressRef.Name) // retrieving apex ingress - apexIngress, err := skp.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Get( + apexIngress, err := skp.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Get( context.TODO(), apexIngressName, metav1.GetOptions{}) if err != nil { return fmt.Errorf("apexIngress %s.%s get query error: %w", apexIngressName, canary.Namespace, err) @@ -81,12 +81,12 @@ func (skp *SkipperRouter) Reconcile(canary *flaggerv1.Canary) error { rule := &iClone.Spec.Rules[x] // ref not value for y := range rule.HTTP.Paths { path := &rule.HTTP.Paths[y] // ref not value - if path.Backend.ServiceName == apexSvcName { + if path.Backend.Service != nil && path.Backend.Service.Name == apexSvcName { // flipping to primary service - path.Backend.ServiceName = primarySvcName + path.Backend.Service.Name = primarySvcName // adding second canary service canaryBackend := path.DeepCopy() - canaryBackend.Backend.ServiceName = canarySvcName + canaryBackend.Backend.Service.Name = canarySvcName rule.HTTP.Paths = append(rule.HTTP.Paths, *canaryBackend) } } @@ -107,14 +107,14 @@ func (skp *SkipperRouter) Reconcile(canary *flaggerv1.Canary) error { } // search for existence - canaryIngress, err := skp.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Get( + canaryIngress, err := skp.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Get( context.TODO(), canaryIngressName, metav1.GetOptions{}) // new ingress if errors.IsNotFound(err) { // Let K8s set this. Otherwise K8s API complains with "resourceVersion should not be set on objects to be created" iClone.ObjectMeta.ResourceVersion = "" - _, err := skp.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Create(context.TODO(), iClone, metav1.CreateOptions{}) + _, err := skp.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Create(context.TODO(), iClone, metav1.CreateOptions{}) if err != nil { return fmt.Errorf("ingress %s.%s create error: %w", iClone.Name, iClone.Namespace, err) } @@ -131,7 +131,7 @@ func (skp *SkipperRouter) Reconcile(canary *flaggerv1.Canary) error { ingressClone.Spec = iClone.Spec ingressClone.Annotations = iClone.Annotations - _, err := skp.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Update(context.TODO(), ingressClone, metav1.UpdateOptions{}) + _, err := skp.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Update(context.TODO(), ingressClone, metav1.UpdateOptions{}) if err != nil { return fmt.Errorf("ingress %s.%s update error: %w", canaryIngressName, ingressClone.Namespace, err) } @@ -145,7 +145,7 @@ func (skp *SkipperRouter) GetRoutes(canary *flaggerv1.Canary) (primaryWeight, ca _, primarySvcName, canarySvcName := canary.GetServiceNames() _, canaryIngressName := skp.getIngressNames(canary.Spec.IngressRef.Name) - canaryIngress, err := skp.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Get(context.TODO(), canaryIngressName, metav1.GetOptions{}) + canaryIngress, err := skp.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Get(context.TODO(), canaryIngressName, metav1.GetOptions{}) if err != nil { err = fmt.Errorf("ingress %s.%s get query error: %w", canaryIngressName, canary.Namespace, err) return @@ -176,7 +176,7 @@ func (skp *SkipperRouter) GetRoutes(canary *flaggerv1.Canary) (primaryWeight, ca func (skp *SkipperRouter) SetRoutes(canary *flaggerv1.Canary, primaryWeight, canaryWeight int, _ bool) (err error) { _, primarySvcName, canarySvcName := canary.GetServiceNames() _, canaryIngressName := skp.getIngressNames(canary.Spec.IngressRef.Name) - canaryIngress, err := skp.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Get(context.TODO(), canaryIngressName, metav1.GetOptions{}) + canaryIngress, err := skp.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Get(context.TODO(), canaryIngressName, metav1.GetOptions{}) if err != nil { return fmt.Errorf("ingress %s.%s get query error: %w", canaryIngressName, canary.Namespace, err) } @@ -197,7 +197,7 @@ func (skp *SkipperRouter) SetRoutes(canary *flaggerv1.Canary, primaryWeight, can iClone.Annotations[skipperpredicateAnnotationKey] = insertPredicate(iClone.Annotations[skipperpredicateAnnotationKey], canaryRouteDisable) } - _, err = skp.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Update( + _, err = skp.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Update( context.TODO(), iClone, metav1.UpdateOptions{}) if err != nil { return fmt.Errorf("ingress %s.%s update error %w", iClone.Name, iClone.Namespace, err) @@ -214,7 +214,7 @@ func (skp *SkipperRouter) Finalize(canary *flaggerv1.Canary) error { skp.logger.With("deleteCanaryIngress", fmt.Sprintf("%s.%s", canary.Name, canary.Namespace)). Debugf("Deleting Canary Ingress: %s", canaryIngressName) - err := skp.kubeClient.NetworkingV1beta1().Ingresses(canary.Namespace).Delete( + err := skp.kubeClient.NetworkingV1().Ingresses(canary.Namespace).Delete( context.TODO(), canaryIngressName, metav1.DeleteOptions{GracePeriodSeconds: &gracePeriodSeconds}) if err != nil { return fmt.Errorf("ingress %s.%s unable to remove canary ingress: %w", canaryIngressName, canary.Namespace, err) diff --git a/pkg/router/skipper_test.go b/pkg/router/skipper_test.go index 3b96f0bc..2d95a77d 100644 --- a/pkg/router/skipper_test.go +++ b/pkg/router/skipper_test.go @@ -43,7 +43,7 @@ func TestSkipperRouter_Reconcile(t *testing.T) { func() fixture { ti := newTestIngress() ti.Annotations["something"] = "changed" - _, err := mocks.kubeClient.NetworkingV1beta1().Ingresses("default").Update( + _, err := mocks.kubeClient.NetworkingV1().Ingresses("default").Update( context.TODO(), ti, metav1.UpdateOptions{}) assert.NoError(err) return mocks @@ -60,21 +60,21 @@ func TestSkipperRouter_Reconcile(t *testing.T) { } assert.NoError(router.Reconcile(mocks.ingressCanary)) canaryName := fmt.Sprintf("%s-canary", mocks.ingressCanary.Spec.IngressRef.Name) - inCanary, err := router.kubeClient.NetworkingV1beta1().Ingresses("default").Get( + inCanary, err := router.kubeClient.NetworkingV1().Ingresses("default").Get( context.TODO(), canaryName, metav1.GetOptions{}) assert.NoError(err) // test initialisation assert.JSONEq(`{ "podinfo-primary": 100, "podinfo-canary": 0 }`, inCanary.Annotations["zalando.org/backend-weights"]) - assert.Equal("podinfo-primary", inCanary.Spec.Rules[0].HTTP.Paths[0].Backend.ServiceName, "backend flipped over") - assert.Equal("podinfo-canary", inCanary.Spec.Rules[0].HTTP.Paths[1].Backend.ServiceName, "backend flipped over") + assert.Equal("podinfo-primary", inCanary.Spec.Rules[0].HTTP.Paths[0].Backend.Service.Name, "backend flipped over") + assert.Equal("podinfo-canary", inCanary.Spec.Rules[0].HTTP.Paths[1].Backend.Service.Name, "backend flipped over") assert.Len(inCanary.Spec.Rules[0].HTTP.Paths, 2) - inApex, err := router.kubeClient.NetworkingV1beta1().Ingresses("default").Get( + inApex, err := router.kubeClient.NetworkingV1().Ingresses("default").Get( context.TODO(), mocks.ingressCanary.Spec.IngressRef.Name, metav1.GetOptions{}) assert.NoError(err) - assert.Equal(inCanary.Spec.Rules[0].HTTP.Paths[0].Backend.ServicePort, - inApex.Spec.Rules[0].HTTP.Paths[0].Backend.ServicePort, "canary backend not cloned") - assert.Equal(inCanary.Spec.Rules[0].HTTP.Paths[0].Backend.ServicePort, - inCanary.Spec.Rules[0].HTTP.Paths[1].Backend.ServicePort, "canary backend not cloned") + assert.Equal(inCanary.Spec.Rules[0].HTTP.Paths[0].Backend.Service.Port.Number, + inApex.Spec.Rules[0].HTTP.Paths[0].Backend.Service.Port.Number, "canary backend not cloned") + assert.Equal(inCanary.Spec.Rules[0].HTTP.Paths[0].Backend.Service.Port.Number, + inCanary.Spec.Rules[0].HTTP.Paths[1].Backend.Service.Port.Number, "canary backend not cloned") }) } } @@ -107,7 +107,7 @@ func TestSkipperRouter_GetSetRoutes(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { assert.NoError(router.SetRoutes(mocks.ingressCanary, tt.primary, tt.canary, false)) - inCanary, err := router.kubeClient.NetworkingV1beta1().Ingresses("default").Get( + inCanary, err := router.kubeClient.NetworkingV1().Ingresses("default").Get( context.TODO(), fmt.Sprintf("%s-canary", mocks.ingressCanary.Spec.IngressRef.Name), metav1.GetOptions{}) assert.NoError(err) assert.JSONEq(fmt.Sprintf(`{"podinfo-primary": %d,"podinfo-canary": %d}`, tt.primary, tt.canary), diff --git a/test/nginx/install.sh b/test/nginx/install.sh index 672b11af..63da0c6b 100755 --- a/test/nginx/install.sh +++ b/test/nginx/install.sh @@ -2,7 +2,7 @@ set -o errexit -NGINX_HELM_VERSION=3.15.2 # ingress v0.41.2 +NGINX_HELM_VERSION=3.31.0 # ingress v0.46.0 REPO_ROOT=$(git rev-parse --show-toplevel) mkdir -p ${REPO_ROOT}/bin diff --git a/test/nginx/test-canary.sh b/test/nginx/test-canary.sh index b2a166ed..04a2129f 100755 --- a/test/nginx/test-canary.sh +++ b/test/nginx/test-canary.sh @@ -8,7 +8,7 @@ set -o errexit REPO_ROOT=$(git rev-parse --show-toplevel) cat <>> Create metric templates' @@ -98,7 +102,7 @@ spec: kind: Deployment name: podinfo ingressRef: - apiVersion: networking.k8s.io/v1beta1 + apiVersion: networking.k8s.io/v1 kind: Ingress name: podinfo progressDeadlineSeconds: 60 @@ -198,7 +202,7 @@ echo '✔ Canary promotion test passed' echo 'Testing original ingress update after canary promotion to pass validation webhook' cat <>> Creating ingress' cat <>> Creating canary' @@ -41,7 +45,7 @@ spec: kind: Deployment name: podinfo ingressRef: - apiVersion: networking.k8s.io/v1beta1 + apiVersion: networking.k8s.io/v1 kind: Ingress name: podinfo-ingress service: