Add supplementary e2e tests for Finalizer

cmd/e2e-test/finalizer_test.go

@@ -20,141 +20,269 @@ 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"
 )
 
+// TestFinalizer asserts that finalizer is added/ removed appropriately during the life cycle
+// of an ingress resource.
 func TestFinalizer(t *testing.T) {
 	t.Parallel()
 	ctx := context.Background()
 	port80 := intstr.FromInt(80)
+	numForwardingRules := 1
+	numBackendServices := 2
+	svcName := "service-1"
 	ing := fuzz.NewIngressBuilder("", "ingress-1", "").
-		AddPath("foo.com", "/", "service-1", port80).
+		AddPath("foo.com", "/", svcName, port80).
 		SetIngressClass("gce").
 		Build()
 
 	Framework.RunWithSandbox("finalizer", t, func(t *testing.T, s *e2e.Sandbox) {
-		_, err := e2e.CreateEchoService(s, "service-1", nil)
+		_, err := e2e.CreateEchoService(s, svcName, nil)
 		if err != nil {
-			t.Fatalf("error creating echo service: %v", err)
+			t.Fatalf("CreateEchoService(_, %q, nil): %v, want nil", svcName, err)
 		}
-		t.Logf("Echo service created (%s/%s)", s.Namespace, "service-1")
+		t.Logf("Echo service created (%s/%s)", s.Namespace, svcName)
 
 		crud := e2e.IngressCRUD{C: Framework.Clientset}
 		ing.Namespace = s.Namespace
 		if _, err := crud.Create(ing); err != nil {
-			t.Fatalf("error creating Ingress spec: %v", err)
+			t.Fatalf("create(%s/%s) = %v, want nil; Ingress: %v", ing.Namespace, ing.Name, err, ing)
 		}
 		t.Logf("Ingress created (%s/%s)", s.Namespace, ing.Name)
+		ing = waitForStableIngress(true, ing, s, t)
 
-		ing, err := e2e.WaitForIngress(s, ing, nil)
+		ingFinalizers := ing.GetFinalizers()
+		if len(ingFinalizers) != 1 || ingFinalizers[0] != utils.FinalizerKey {
+			t.Fatalf("GetFinalizers() = %+v, want [%q]", ingFinalizers, utils.FinalizerKey)
+		}
+
+		gclb := checkGCLB(t, s, ing, numForwardingRules, numBackendServices)
+
+		deleteOptions := &fuzz.GCLBDeleteOptions{
+			SkipDefaultBackend: true,
+		}
+		if err := e2e.WaitForIngressDeletion(ctx, gclb, s, ing, deleteOptions); err != nil {
+			t.Errorf("e2e.WaitForIngressDeletion(..., %q, nil) = %v, want nil", ing.Name, err)
+		}
+	})
+}
+
+// TestFinalizerIngressClassChange asserts that finalizer is removed from ingress after its
+// associated resources are cleaned up when ingress class is updated to a non glbc type.
+func TestFinalizerIngressClassChange(t *testing.T) {
+	t.Parallel()
+	ctx := context.Background()
+	port80 := intstr.FromInt(80)
+	numForwardingRules := 1
+	numBackendServices := 2
+	svcName := "service-1"
+	ing := fuzz.NewIngressBuilder("", "ingress-1", "").
+		AddPath("foo.com", "/", svcName, port80).
+		SetIngressClass("gce").
+		Build()
+
+	Framework.RunWithSandbox("finalizer-ingress-class-change", t, func(t *testing.T, s *e2e.Sandbox) {
+		_, err := e2e.CreateEchoService(s, svcName, nil)
 		if err != nil {
-			t.Fatalf("error waiting for Ingress to stabilize: %v", err)
+			t.Fatalf("CreateEchoService(_, %q, nil): %v, want nil", svcName, err)
 		}
-		t.Logf("GCLB resources created (%s/%s)", s.Namespace, ing.Name)
+		t.Logf("Echo service created (%s/%s)", s.Namespace, svcName)
 
-		// Perform whitebox testing.
-		if len(ing.Status.LoadBalancer.Ingress) < 1 {
-			t.Fatalf("Ingress does not have an IP: %+v", ing.Status)
+		crud := e2e.IngressCRUD{C: Framework.Clientset}
+		ing.Namespace = s.Namespace
+		if _, err := crud.Create(ing); err != nil {
+			t.Fatalf("create(%s/%s) = %v, want nil; Ingress: %v", ing.Namespace, ing.Name, err, ing)
 		}
+		t.Logf("Ingress created (%s/%s)", s.Namespace, ing.Name)
+		ing = waitForStableIngress(true, ing, s, t)
 
 		ingFinalizers := ing.GetFinalizers()
 		if len(ingFinalizers) != 1 || ingFinalizers[0] != utils.FinalizerKey {
-			t.Fatalf("GetFinalizers() = %+v, want [%s]", ingFinalizers, utils.FinalizerKey)
+			t.Fatalf("GetFinalizers() = %+v, want [%q]", ingFinalizers, utils.FinalizerKey)
 		}
 
-		vip := ing.Status.LoadBalancer.Ingress[0].IP
-		t.Logf("Ingress %s/%s VIP = %s", s.Namespace, ing.Name, vip)
-		gclb, err := fuzz.GCLBForVIP(context.Background(), Framework.Cloud, vip, fuzz.FeatureValidators(features.All))
-		if err != nil {
-			t.Fatalf("Error getting GCP resources for LB with IP = %q: %v", vip, err)
-		}
+		gclb := checkGCLB(t, s, ing, numForwardingRules, numBackendServices)
+
+		// Change Ingress class
+		newIngClass := "nginx"
+		ing = fuzz.NewIngressBuilderFromExisting(ing).SetIngressClass(newIngClass).Build()
 
-		if err = e2e.CheckGCLB(gclb, 1, 2); err != nil {
-			t.Error(err)
+		if _, err := crud.Update(ing); err != nil {
+			t.Fatalf("update(%s/%s) = %v, want nil; ingress: %v", ing.Namespace, ing.Name, err, ing)
 		}
+		t.Logf("Ingress (%s/%s) class changed to %s", s.Namespace, ing.Name, newIngClass)
 
 		deleteOptions := &fuzz.GCLBDeleteOptions{
 			SkipDefaultBackend: true,
 		}
-		if err := e2e.WaitForIngressDeletion(ctx, gclb, s, ing, deleteOptions); err != nil {
-			t.Errorf("e2e.WaitForIngressDeletion(..., %q, nil) = %v, want nil", ing.Name, err)
+		if err := e2e.WaitForFinalizerDeletion(ctx, gclb, s, ing.Name, deleteOptions); err != nil {
+			t.Errorf("e2e.WaitForFinalizerDeletion(..., %q, _) = %v, want nil", ing.Name, err)
 		}
+		t.Logf("Finalizer for Ingress (%s/%s) deleted", s.Namespace, ing.Name)
 	})
 }
 
-func TestFinalizerIngressClassChange(t *testing.T) {
+// TestFinalizerIngressesWithSharedResources asserts that sync does not return error with finalizer
+// when multiple ingresses with shared resources are created or deleted.
+func TestFinalizerIngressesWithSharedResources(t *testing.T) {
 	t.Parallel()
 	ctx := context.Background()
 	port80 := intstr.FromInt(80)
+	numForwardingRules := 1
+	numBackendServices := 2
+	svcName := "service-1"
 	ing := fuzz.NewIngressBuilder("", "ingress-1", "").
-		AddPath("foo.com", "/", "service-1", port80).
+		AddPath("foo.com", "/", svcName, port80).
+		SetIngressClass("gce").
+		Build()
+	otherIng := fuzz.NewIngressBuilder("", "ingress-2", "").
+		AddPath("foo.com", "/", svcName, port80).
 		SetIngressClass("gce").
 		Build()
 
 	Framework.RunWithSandbox("finalizer-ingress-class-change", t, func(t *testing.T, s *e2e.Sandbox) {
-		_, err := e2e.CreateEchoService(s, "service-1", nil)
+		_, err := e2e.CreateEchoService(s, svcName, nil)
 		if err != nil {
-			t.Fatalf("error creating echo service: %v", err)
+			t.Fatalf("CreateEchoService(_, %q, nil): %v, want nil", svcName, err)
 		}
-		t.Logf("Echo service created (%s/%s)", s.Namespace, "service-1")
+		t.Logf("Echo service created (%s/%s)", s.Namespace, svcName)
 
 		crud := e2e.IngressCRUD{C: Framework.Clientset}
 		ing.Namespace = s.Namespace
 		if _, err := crud.Create(ing); err != nil {
-			t.Fatalf("error creating Ingress spec: %v", err)
+			t.Fatalf("create(%s/%s) = %v, want nil; Ingress: %v", ing.Namespace, ing.Name, err, ing)
 		}
 		t.Logf("Ingress created (%s/%s)", s.Namespace, ing.Name)
 
-		ing, err := e2e.WaitForIngress(s, ing, nil)
-		if err != nil {
-			t.Fatalf("error waiting for Ingress to stabilize: %v", err)
+		otherIng.Namespace = s.Namespace
+		if _, err := crud.Create(otherIng); err != nil {
+			t.Fatalf("create(%s/%s) = %v, want nil; Ingress: %v", otherIng.Namespace, otherIng.Name, err, otherIng)
 		}
-		t.Logf("GCLB resources created (%s/%s)", s.Namespace, ing.Name)
+		t.Logf("Ingress created (%s/%s)", s.Namespace, otherIng.Name)
+
+		ing = waitForStableIngress(true, ing, s, t)
+		otherIng = waitForStableIngress(true, otherIng, s, t)
 
 		ingFinalizers := ing.GetFinalizers()
 		if len(ingFinalizers) != 1 || ingFinalizers[0] != utils.FinalizerKey {
 			t.Fatalf("GetFinalizers() = %+v, want [%q]", ingFinalizers, utils.FinalizerKey)
 		}
+		otherIngFinalizers := otherIng.GetFinalizers()
+		if len(otherIngFinalizers) != 1 || otherIngFinalizers[0] != utils.FinalizerKey {
+			t.Fatalf("GetFinalizers() = %+v, want [%q]", otherIngFinalizers, utils.FinalizerKey)
+		}
 
-		// Perform whitebox testing.
-		if len(ing.Status.LoadBalancer.Ingress) < 1 {
-			t.Fatalf("Ingress does not have an IP: %+v", ing.Status)
+		gclb := checkGCLB(t, s, ing, numForwardingRules, numBackendServices)
+		otherGclb := checkGCLB(t, s, otherIng, numForwardingRules, numBackendServices)
+
+		// SkipBackends ensure that we dont wait on deletion of shared backends.
+		deleteOptions := &fuzz.GCLBDeleteOptions{
+			SkipDefaultBackend: true,
+			SkipBackends:       true,
+		}
+		if err := e2e.WaitForIngressDeletion(ctx, gclb, s, ing, deleteOptions); err != nil {
+			t.Errorf("e2e.WaitForIngressDeletion(..., %q, nil) = %v, want nil", ing.Name, err)
+		}
+		deleteOptions = &fuzz.GCLBDeleteOptions{
+			SkipDefaultBackend: true,
 		}
+		if err := e2e.WaitForIngressDeletion(ctx, otherGclb, s, otherIng, deleteOptions); err != nil {
+			t.Errorf("e2e.WaitForIngressDeletion(..., %q, nil) = %v, want nil", otherIng.Name, err)
+		}
+	})
+}
 
-		vip := ing.Status.LoadBalancer.Ingress[0].IP
-		t.Logf("Ingress %s/%s VIP = %s", s.Namespace, ing.Name, vip)
-		gclb, err := fuzz.GCLBForVIP(context.Background(), Framework.Cloud, vip, fuzz.FeatureValidators(features.All))
+// 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)
+	numForwardingRules := 1
+	numBackendServices := 2
+	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("Error getting GCP resources for LB with IP = %q: %v", vip, err)
+			t.Fatalf("CreateEchoService(_, %q, nil): %v, want nil", svcName, err)
 		}
+		t.Logf("Echo service created (%s/%s)", s.Namespace, svcName)
 
-		if err = e2e.CheckGCLB(gclb, 1, 2); err != nil {
-			t.Error(err)
+		crud := e2e.IngressCRUD{C: Framework.Clientset}
+		ing.Namespace = s.Namespace
+		if _, err := crud.Create(ing); err != nil {
+			t.Fatalf("create(%s/%s) = %v, want nil; Ingress: %v", ing.Namespace, ing.Name, err, ing)
 		}
+		t.Logf("Ingress created (%s/%s)", s.Namespace, ing.Name)
+		waitForStableIngress(true, ing, s, t)
 
-		// Change Ingress class
-		newIngClass := "nginx"
-		ing = fuzz.NewIngressBuilderFromExisting(ing).SetIngressClass(newIngClass).Build()
+		// 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)
+		}
 
-		if _, err := crud.Update(ing); err != nil {
-			t.Fatalf("error updating Ingress spec: %v", err)
+		checkGCLB(t, s, ing, numForwardingRules, numBackendServices)
+
+		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
+			}
 		}
-		t.Logf("Ingress (%s/%s) class changed to %s", s.Namespace, ing.Name, newIngClass)
 
+		// 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", ing.Name, err)
+		}
+
+		gclb := checkGCLB(t, s, ing, numForwardingRules, numBackendServices)
+
+		// 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.WaitForFinalizerDeletion(ctx, gclb, s, ing.Name, deleteOptions); err != nil {
-			t.Errorf("e2e.WaitForFinalizerDeletion(...) = %v, want nil", err)
-		}
-		t.Logf("Finalizer for Ingress (%s/%s) deleted", s.Namespace, ing.Name)
-
-		if err := e2e.WaitForIngressDeletion(ctx, gclb, s, ing, deleteOptions); err != nil {
+		if err := e2e.WaitForIngressDeletion(context.Background(), gclb, s, ing, deleteOptions); err != nil {
 			t.Errorf("e2e.WaitForIngressDeletion(..., %q, nil) = %v, want nil", ing.Name, 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)
+	gclb, err := fuzz.GCLBForVIP(context.Background(), Framework.Cloud, vip, fuzz.FeatureValidators(features.All))
+	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
+}

pkg/e2e/helpers.go

@@ -37,6 +37,7 @@ import (
 	"k8s.io/ingress-gce/pkg/annotations"
 	"k8s.io/ingress-gce/pkg/fuzz"
 	"k8s.io/ingress-gce/pkg/fuzz/features"
+	"k8s.io/ingress-gce/pkg/utils"
 	"k8s.io/klog"
 	"net/http"
 	"strings"
@@ -114,6 +115,27 @@ func WaitForIngress(s *Sandbox, ing *v1beta1.Ingress, options *WaitForIngressOpt
 	return ing, err
 }
 
+// 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)
+			return false, nil
+		}
+		ingFinalizers := ing.GetFinalizers()
+		if len(ingFinalizers) != 1 || ingFinalizers[0] != utils.FinalizerKey {
+			klog.Infof("WaitForFinalizer(%s/%s) = %v, finalizer not added for Ingress %v", s.Namespace, ingName, ingFinalizers, ing)
+			return false, nil
+		}
+		return true, nil
+	})
+}
+
 // WaitForIngressDeletion deletes the given ingress and waits for the
 // resources associated with it to be deleted.
 func WaitForIngressDeletion(ctx context.Context, g *fuzz.GCLB, s *Sandbox, ing *v1beta1.Ingress, options *fuzz.GCLBDeleteOptions) error {
@@ -143,7 +165,7 @@ func WaitForFinalizerDeletion(ctx context.Context, g *fuzz.GCLB, s *Sandbox, ing
 	return wait.Poll(k8sApiPoolInterval, k8sApiPollTimeout, func() (bool, error) {
 		ing, err := crud.Get(s.Namespace, ingName)
 		if err != nil {
-			klog.Infof("WaitForFinalizerDeletion(%s/%s) = Error retrieving Ingress: %v", s.Namespace, ing.Name, err)
+			klog.Infof("WaitForFinalizerDeletion(%s/%s) = Error retrieving Ingress: %v", s.Namespace, ingName, err)
 			return false, nil
 		}
 		if len(ing.GetFinalizers()) != 0 {