From 628bddd07d7f222505f10b3cbe783db06144186f Mon Sep 17 00:00:00 2001 From: Satish Matti Date: Tue, 19 Nov 2019 15:52:24 -0800 Subject: [PATCH] Migrate finalizer upgrade test to use the upgrade framework --- cmd/e2e-test/finalizer_test.go | 100 ----------------------- cmd/e2e-test/upgrade/basic_http.go | 5 +- cmd/e2e-test/upgrade/finalizer.go | 127 +++++++++++++++++++++++++++++ cmd/e2e-test/upgrade_test.go | 74 +++++++++-------- pkg/e2e/helpers.go | 36 +++++--- 5 files changed, 197 insertions(+), 145 deletions(-) create mode 100644 cmd/e2e-test/upgrade/finalizer.go diff --git a/cmd/e2e-test/finalizer_test.go b/cmd/e2e-test/finalizer_test.go index 0d29a03929..f91ca52687 100644 --- a/cmd/e2e-test/finalizer_test.go +++ b/cmd/e2e-test/finalizer_test.go @@ -20,11 +20,9 @@ import ( "context" "testing" - "k8s.io/api/networking/v1beta1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/ingress-gce/pkg/e2e" "k8s.io/ingress-gce/pkg/fuzz" - "k8s.io/ingress-gce/pkg/fuzz/features" "k8s.io/ingress-gce/pkg/utils/common" ) @@ -221,101 +219,3 @@ func TestFinalizerIngressesWithSharedResources(t *testing.T) { } }) } - -// TestUpdateTo1dot7 asserts that finalizer is added to an ingress when upgraded from a version -// without finalizer to version 1.7. Also, verifies that ingress is deleted with finalizer enabled. -// Note: The test is named in such a way that it does run as a normal test or an upgrade test for -// other versions. -func TestUpdateTo1dot7(t *testing.T) { - port80 := intstr.FromInt(80) - svcName := "service-1" - ing := fuzz.NewIngressBuilder("", "ingress-1", ""). - AddPath("foo.com", "/", svcName, port80). - SetIngressClass("gce"). - Build() - Framework.RunWithSandbox("finalizer-master-upgrade", t, func(t *testing.T, s *e2e.Sandbox) { - t.Parallel() - - _, err := e2e.CreateEchoService(s, svcName, nil) - if err != nil { - t.Fatalf("CreateEchoService(_, %q, nil): %v, want nil", svcName, err) - } - t.Logf("Echo service created (%s/%s)", s.Namespace, svcName) - - crud := e2e.IngressCRUD{C: Framework.Clientset} - ing.Namespace = s.Namespace - ingKey := common.NamespacedName(ing) - if _, err := crud.Create(ing); err != nil { - t.Fatalf("create(%s) = %v, want nil; Ingress: %v", ingKey, err, ing) - } - t.Logf("Ingress created (%s)", ingKey) - - if ing, err = e2e.WaitForIngress(s, ing, &e2e.WaitForIngressOptions{ExpectUnreachable: true}); err != nil { - t.Fatalf("error waiting for Ingress %s to stabilize: %v", ingKey, err) - } - - // Check that finalizer is not added in old version in which finalizer add is not enabled. - ingFinalizers := ing.GetFinalizers() - if l := len(ingFinalizers); l != 0 { - t.Fatalf("GetFinalizers() = %d, want 0", l) - } - // Perform whitebox testing. - gclb, err := e2e.WhiteboxTest(ing, s, Framework.Cloud, "") - if err != nil { - t.Fatalf("e2e.WhiteboxTest(%s, ...) = %v, want nil", ingKey, err) - } - - for { - // While k8s master is upgrading, it will return a connection refused - // error for any k8s resource we try to hit. We loop until the - // master upgrade has finished. - if s.MasterUpgrading() { - continue - } - - if s.MasterUpgraded() { - t.Logf("Detected master upgrade") - break - } - } - - // Wait for finalizer to be added and verify that correct finalizer is added to the ingress after the upgrade. - if err := e2e.WaitForFinalizer(s, ing.Name); err != nil { - t.Errorf("e2e.WaitForFinalizer(_, %q) = %v, want nil", ingKey, err) - } - - // Perform whitebox testing. - if gclb, err = e2e.WhiteboxTest(ing, s, Framework.Cloud, ""); err != nil { - t.Fatalf("e2e.WhiteboxTest(%s, ...) = %v, want nil", ingKey, err) - } - - // If the Master has upgraded and the Ingress is stable, - // we delete the Ingress and exit out of the loop to indicate that - // the test is done. - deleteOptions := &fuzz.GCLBDeleteOptions{ - SkipDefaultBackend: true, - } - if err := e2e.WaitForIngressDeletion(context.Background(), gclb, s, ing, deleteOptions); err != nil { - t.Errorf("e2e.WaitForIngressDeletion(..., %q, nil) = %v, want nil", ingKey, err) - } - }) -} - -func checkGCLB(t *testing.T, s *e2e.Sandbox, ing *v1beta1.Ingress, numForwardingRules, numBackendServices int) *fuzz.GCLB { - // Perform whitebox testing. - if len(ing.Status.LoadBalancer.Ingress) < 1 { - t.Fatalf("Ingress does not have an IP: %+v", ing.Status) - } - vip := ing.Status.LoadBalancer.Ingress[0].IP - t.Logf("Ingress %s/%s VIP = %s", s.Namespace, ing.Name, vip) - params := &fuzz.GCLBForVIPParams{VIP: vip, Validators: fuzz.FeatureValidators(features.All)} - gclb, err := fuzz.GCLBForVIP(context.Background(), Framework.Cloud, params) - if err != nil { - t.Fatalf("GCLBForVIP(..., %q, _) = %v, want nil; error getting GCP resources for LB with IP", vip, err) - } - - if err = e2e.CheckGCLB(gclb, numForwardingRules, numBackendServices); err != nil { - t.Error(err) - } - return gclb -} diff --git a/cmd/e2e-test/upgrade/basic_http.go b/cmd/e2e-test/upgrade/basic_http.go index 6cacea60a1..2286f1f850 100644 --- a/cmd/e2e-test/upgrade/basic_http.go +++ b/cmd/e2e-test/upgrade/basic_http.go @@ -78,13 +78,12 @@ func (bh *BasicHTTP) PreUpgrade() error { } bh.t.Logf("Ingress created (%s)", ingKey) - if ing, err = e2e.WaitForIngress(bh.s, ing, &e2e.WaitForIngressOptions{ExpectUnreachable: true}); err != nil { + if bh.ing, err = e2e.UpgradeTestWaitForIngress(bh.s, ing, &e2e.WaitForIngressOptions{ExpectUnreachable: true}); err != nil { bh.t.Fatalf("error waiting for Ingress %s to stabilize: %v", ingKey, err) } - bh.s.PutStatus(e2e.Stable) bh.t.Logf("GCLB resources created (%s)", ingKey) - if _, err := e2e.WhiteboxTest(ing, bh.s, bh.framework.Cloud, ""); err != nil { + if _, err := e2e.WhiteboxTest(bh.ing, bh.s, bh.framework.Cloud, ""); err != nil { bh.t.Fatalf("e2e.WhiteboxTest(%s, ...) = %v, want nil", ingKey, err) } return nil diff --git a/cmd/e2e-test/upgrade/finalizer.go b/cmd/e2e-test/upgrade/finalizer.go new file mode 100644 index 0000000000..549536b6ed --- /dev/null +++ b/cmd/e2e-test/upgrade/finalizer.go @@ -0,0 +1,127 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package upgrade + +import ( + "context" + "testing" + + "k8s.io/api/networking/v1beta1" + "k8s.io/ingress-gce/pkg/e2e" + "k8s.io/ingress-gce/pkg/fuzz" + "k8s.io/ingress-gce/pkg/utils/common" +) + +// Finalizer implements e2e.UpgradeTest interface. +type Finalizer struct { + t *testing.T + s *e2e.Sandbox + framework *e2e.Framework + crud e2e.IngressCRUD + ing *v1beta1.Ingress +} + +// NewFinalizerUpgradeTest returns an upgrade test that asserts that finalizer +// is added to an ingress when upgraded from a version without finalizer to v1.7.0. +// Also, verifies that ingress is deleted with finalizer enabled. +func NewFinalizerUpgradeTest() e2e.UpgradeTest { + return &Finalizer{} +} + +// Name implements e2e.UpgradeTest.Init. +func (fr *Finalizer) Name() string { + return "FinalizerUpgrade" +} + +// Init implements e2e.UpgradeTest.Init. +func (fr *Finalizer) Init(t *testing.T, s *e2e.Sandbox, framework *e2e.Framework) error { + fr.t = t + fr.s = s + fr.framework = framework + return nil +} + +// PreUpgrade implements e2e.UpgradeTest.PreUpgrade. +func (fr *Finalizer) PreUpgrade() error { + _, err := e2e.CreateEchoService(fr.s, svcName, nil) + if err != nil { + fr.t.Fatalf("error creating echo service: %v", err) + } + fr.t.Logf("Echo service created (%s/%s)", fr.s.Namespace, svcName) + + ing := fuzz.NewIngressBuilder(fr.s.Namespace, ingName, ""). + AddPath("foo.com", "/", svcName, port80). + Build() + ingKey := common.NamespacedName(ing) + fr.crud = e2e.IngressCRUD{C: fr.framework.Clientset} + if _, err := fr.crud.Create(ing); err != nil { + fr.t.Fatalf("error creating Ingress %s: %v", ingKey, err) + } + fr.t.Logf("Ingress created (%s)", ingKey) + + if fr.ing, err = e2e.UpgradeTestWaitForIngress(fr.s, ing, &e2e.WaitForIngressOptions{ExpectUnreachable: true}); err != nil { + fr.t.Fatalf("error waiting for Ingress %s to stabilize: %v", ingKey, err) + } + + fr.t.Logf("GCLB resources created (%s)", ingKey) + + // Check that finalizer is not added in old version in which finalizer add is not enabled. + ingFinalizers := fr.ing.GetFinalizers() + if l := len(ingFinalizers); l != 0 { + fr.t.Fatalf("len(GetFinalizers()) = %d, want 0", l) + } + + if _, err := e2e.WhiteboxTest(fr.ing, fr.s, fr.framework.Cloud, ""); err != nil { + fr.t.Fatalf("e2e.WhiteboxTest(%s, ...) = %v, want nil", ingKey, err) + } + return nil +} + +// DuringUpgrade implements e2e.UpgradeTest.DuringUpgrade. +func (fr *Finalizer) DuringUpgrade() error { + return nil +} + +// PostUpgrade implements e2e.UpgradeTest.PostUpgrade +func (fr *Finalizer) PostUpgrade() error { + ingKey := common.NamespacedName(fr.ing) + // A finalizer is expected to be added on the ingress after master upgrade. + // Ingress status is updated to unstable, which would be set back to stable + // after WaitForFinalizer below finishes successfully. + fr.s.PutStatus(e2e.Unstable) + // Wait for finalizer to be added and verify that correct finalizer is added to the ingress after the upgrade. + ing, err := e2e.WaitForFinalizer(fr.s, fr.ing) + if err != nil { + fr.t.Fatalf("e2e.WaitForFinalizer(_, %q) = _, %v, want nil", ingKey, err) + } + gclb, err := e2e.WhiteboxTest(ing, fr.s, fr.framework.Cloud, "") + if err != nil { + fr.t.Fatalf("e2e.WhiteboxTest(%s, ...) = %v, want nil", ingKey, err) + } + + // If the Master has upgraded and the Ingress is stable, + // we delete the Ingress and exit out of the loop to indicate that + // the test is done. + deleteOptions := &fuzz.GCLBDeleteOptions{ + SkipDefaultBackend: true, + } + + if err := e2e.WaitForIngressDeletion(context.Background(), gclb, fr.s, fr.ing, deleteOptions); err != nil { + fr.t.Errorf("e2e.WaitForIngressDeletion(..., %q, nil) = %v, want nil", ingKey, err) + } + return nil +} diff --git a/cmd/e2e-test/upgrade_test.go b/cmd/e2e-test/upgrade_test.go index 1f5c3129ff..028c601611 100644 --- a/cmd/e2e-test/upgrade_test.go +++ b/cmd/e2e-test/upgrade_test.go @@ -31,43 +31,53 @@ func TestGenericUpgrade(t *testing.T) { upgrade.NewStandaloneNegUpgradeTest(), } { test := test // Capture test as we are running this in parallel. - desc := test.Name() - Framework.RunWithSandbox(desc, t, func(t *testing.T, s *e2e.Sandbox) { - t.Parallel() + runUpgradeTest(t, test) + } +} - t.Logf("Running upgrade test %v", desc) - if err := test.Init(t, s, Framework); err != nil { - t.Fatalf("For upgrade test %v, step Init failed due to %v", desc, err) - } +// TestUpgradeToV1dot7 runs upgrade tests for features that are introduced in v1.7.0. +// Note that this test runs only when an upgrade results in enabling these features. +func TestUpgradeToV1dot7(t *testing.T) { + runUpgradeTest(t, upgrade.NewFinalizerUpgradeTest()) +} - s.PutStatus(e2e.Unstable) - func() { - // always mark the test as stable in order to unblock other upgrade tests. - defer s.PutStatus(e2e.Stable) - if err := test.PreUpgrade(); err != nil { - t.Fatalf("For upgrade test %v, step PreUpgrade failed due to %v", desc, err) - } - }() +func runUpgradeTest(t *testing.T, test e2e.UpgradeTest) { + desc := test.Name() + Framework.RunWithSandbox(desc, t, func(t *testing.T, s *e2e.Sandbox) { + t.Parallel() - for { - // While k8s master is upgrading, it will return a connection refused - // error for any k8s resource we try to hit. We loop until the - // master upgrade has finished. - if s.MasterUpgrading() { - if err := test.DuringUpgrade(); err != nil { - t.Fatalf("For upgrade test %v, step DuringUpgrade failed due to %v", desc, err) - } - continue - } + t.Logf("Running upgrade test %v", desc) + if err := test.Init(t, s, Framework); err != nil { + t.Fatalf("For upgrade test %v, step Init failed due to %v", desc, err) + } + + s.PutStatus(e2e.Unstable) + func() { + // always mark the test as stable in order to unblock other upgrade tests. + defer s.PutStatus(e2e.Stable) + if err := test.PreUpgrade(); err != nil { + t.Fatalf("For upgrade test %v, step PreUpgrade failed due to %v", desc, err) + } + }() - if s.MasterUpgraded() { - t.Logf("Detected master upgrade, continuing upgrade test %v", desc) - break + for { + // While k8s master is upgrading, it will return a connection refused + // error for any k8s resource we try to hit. We loop until the + // master upgrade has finished. + if s.MasterUpgrading() { + if err := test.DuringUpgrade(); err != nil { + t.Fatalf("For upgrade test %v, step DuringUpgrade failed due to %v", desc, err) } + continue } - if err := test.PostUpgrade(); err != nil { - t.Fatalf("For upgrade test %v, step PostUpgrade failed due to %v", desc, err) + + if s.MasterUpgraded() { + t.Logf("Detected master upgrade, continuing upgrade test %v", desc) + break } - }) - } + } + if err := test.PostUpgrade(); err != nil { + t.Fatalf("For upgrade test %v, step PostUpgrade failed due to %v", desc, err) + } + }) } diff --git a/pkg/e2e/helpers.go b/pkg/e2e/helpers.go index 393c0ba422..2fd247c84b 100644 --- a/pkg/e2e/helpers.go +++ b/pkg/e2e/helpers.go @@ -90,6 +90,17 @@ func IsRfc1918Addr(addr string) bool { return false } +// UpgradeTestWaitForIngress waits for ingress to stabilize and set sandbox status to stable. +// Note that this is used only for upgrade tests. +func UpgradeTestWaitForIngress(s *Sandbox, ing *v1beta1.Ingress, options *WaitForIngressOptions) (*v1beta1.Ingress, error) { + ing, err := WaitForIngress(s, ing, options) + if err != nil { + return nil, err + } + s.PutStatus(Stable) + return ing, nil +} + // WaitForIngress to stabilize. // We expect the ingress to be unreachable at first as LB is // still programming itself (i.e 404's / 502's) @@ -120,24 +131,29 @@ func WaitForIngress(s *Sandbox, ing *v1beta1.Ingress, options *WaitForIngressOpt } // WaitForFinalizer waits for Finalizer to be added. -// This is used when master is upgraded from version without finalizer. -// We wait for new lb controller to add finalizer. -func WaitForFinalizer(s *Sandbox, ingName string) error { - crud := IngressCRUD{s.f.Clientset} - klog.Infof("Waiting for Finalizer to be added for Ingress %s/%s", s.Namespace, ingName) - return wait.Poll(k8sApiPoolInterval, k8sApiPollTimeout, func() (bool, error) { - ing, err := crud.Get(s.Namespace, ingName) - if err != nil { - klog.Infof("WaitForFinalizer(%s/%s) = %v, error retrieving Ingress", s.Namespace, ingName, err) +// Note that this is used only for upgrade tests. +func WaitForFinalizer(s *Sandbox, ing *v1beta1.Ingress) (*v1beta1.Ingress, error) { + ingKey := fmt.Sprintf("%s/%s", s.Namespace, ing.Name) + klog.Infof("Waiting for Finalizer to be added for Ingress %s", ingKey) + err := wait.Poll(k8sApiPoolInterval, k8sApiPollTimeout, func() (bool, error) { + var err error + crud := IngressCRUD{s.f.Clientset} + if ing, err = crud.Get(s.Namespace, ing.Name); err != nil { + klog.Infof("WaitForFinalizer(%s) = %v, error retrieving Ingress", ingKey, err) return false, nil } ingFinalizers := ing.GetFinalizers() if len(ingFinalizers) != 1 || ingFinalizers[0] != common.FinalizerKey { - klog.Infof("WaitForFinalizer(%s/%s) = %v, finalizer not added for Ingress %v", s.Namespace, ingName, ingFinalizers, ing) + klog.Infof("WaitForFinalizer(%s) = %v, finalizer not added for Ingress %v", ingKey, ingFinalizers, ing) return false, nil } return true, nil }) + if err == nil { + // Set status back to stable. + s.PutStatus(Stable) + } + return ing, err } // WhiteboxTest retrieves GCP load-balancer for Ingress VIP and runs the whitebox tests.