do not leak LB when ingress class is changed

pkg/controller/controller.go

@@ -48,6 +48,7 @@ import (
 	ingsync "k8s.io/ingress-gce/pkg/sync"
 	"k8s.io/ingress-gce/pkg/tls"
 	"k8s.io/ingress-gce/pkg/utils"
+	"k8s.io/kubernetes/pkg/util/slice"
 )
 
 // LoadBalancerController watches the kubernetes api and adds/removes services
@@ -153,6 +154,14 @@ func NewLoadBalancerController(
 		UpdateFunc: func(old, cur interface{}) {
 			curIng := cur.(*extensions.Ingress)
 			if !utils.IsGLBCIngress(curIng) {
+				oldIng := old.(*extensions.Ingress)
+				// If ingress was GLBC Ingress, we need to track ingress class change
+				// and run GC to delete LB resources.
+				if utils.IsGLBCIngress(oldIng) {
+					glog.V(4).Infof("Ingress %v class was changed, enqueuing", utils.IngressKeyFunc(curIng))
+					lbc.ingQueue.Enqueue(cur)
+					return
+				}
 				return
 			}
 			if reflect.DeepEqual(old, cur) {
@@ -456,6 +465,14 @@ func (lbc *LoadBalancerController) sync(key string) error {
 	}
 	ing = ing.DeepCopy()
 
+	// Check if ingress class was changed to non-GLBC to remove ingress LB from state and trigger GC
+	if !utils.IsGLBCIngress(ing) {
+		glog.V(2).Infof("Ingress %q class was changed, triggering GC", key)
+		// Remove lb from state for GC
+		gcState.lbNames = slice.RemoveString(gcState.lbNames, key, nil)
+		return lbc.ingSyncer.GC(gcState)
+	}
+
 	// Bootstrap state for GCP sync.
 	urlMap, errs := lbc.Translator.TranslateIngress(ing, lbc.ctx.DefaultBackendSvcPortID)
 	syncState := &syncState{urlMap, ing}

pkg/controller/controller_test.go

@@ -104,6 +104,11 @@ func addIngress(lbc *LoadBalancerController, ing *extensions.Ingress) {
 	lbc.ctx.IngressInformer.GetIndexer().Add(ing)
 }
 
+func updateIngress(lbc *LoadBalancerController, ing *extensions.Ingress) {
+	lbc.ctx.KubeClient.Extensions().Ingresses(ing.Namespace).Update(ing)
+	lbc.ctx.IngressInformer.GetIndexer().Update(ing)
+}
+
 func deleteIngress(lbc *LoadBalancerController, ing *extensions.Ingress) {
 	lbc.ctx.KubeClient.Extensions().Ingresses(ing.Namespace).Delete(ing.Name, &meta_v1.DeleteOptions{})
 	lbc.ctx.IngressInformer.GetIndexer().Delete(ing)
@@ -183,6 +188,47 @@ func TestIngressCreateDelete(t *testing.T) {
 	}
 }
 
+// TestIngressClassChange asserts that `sync` will not return an error for a good ingress config
+// status is updated and LB is deleted after class change.
+func TestIngressClassChange(t *testing.T) {
+	lbc := newLoadBalancerController()
+	svc := test.NewService(types.NamespacedName{Name: "my-service", Namespace: "default"}, api_v1.ServiceSpec{
+		Type:  api_v1.ServiceTypeNodePort,
+		Ports: []api_v1.ServicePort{{Port: 80}},
+	})
+	addService(lbc, svc)
+	defaultBackend := backend("my-service", intstr.FromInt(80))
+	ing := test.NewIngress(types.NamespacedName{Name: "my-ingress", Namespace: "default"},
+		extensions.IngressSpec{
+			Backend: &defaultBackend,
+		})
+	ing.ObjectMeta.Annotations = map[string]string{"kubernetes.io/ingress.class": "gce"}
+	addIngress(lbc, ing)
+
+	ingStoreKey := getKey(ing, t)
+	if err := lbc.sync(ingStoreKey); err != nil {
+		t.Fatalf("lbc.sync(%v) = err %v", ingStoreKey, err)
+	}
+
+	ing.ObjectMeta.Annotations = map[string]string{"kubernetes.io/ingress.class": "new-class"}
+	updateIngress(lbc, ing)
+
+	if err := lbc.sync(ingStoreKey); err != nil {
+		t.Fatalf("lbc.sync(%v) = err %v", ingStoreKey, err)
+	}
+
+	// Check status of LoadBalancer is updated after class changes
+	updatedIng, _ := lbc.ctx.KubeClient.ExtensionsV1beta1().Ingresses(ing.Namespace).Get(ing.Name, meta_v1.GetOptions{})
+	if len(updatedIng.Status.LoadBalancer.Ingress) != 0 {
+		t.Error("Ingress status wasn't updated after class changed")
+	}
+
+	// Check LB for ingress is deleted after class changed
+	if pool, _ := lbc.l7Pool.Get(ingStoreKey); pool != nil {
+		t.Errorf("LB(%v) wasn't deleted after class changed", ingStoreKey)
+	}
+}
+
 // TestEnsureMCIngress asserts a multi-cluster ingress will result with correct status annotations.
 func TestEnsureMCIngress(t *testing.T) {
 	lbc := newLoadBalancerController()