Merge pull request #627 from agau4779/finalizer-cherrypick-1

Cherry-pick #613 onto 1.4, part 1

pkg/controller/controller.go

@@ -40,6 +40,7 @@ import (
 	"k8s.io/ingress-gce/pkg/healthchecks"
 	"k8s.io/ingress-gce/pkg/instances"
 	"k8s.io/kubernetes/pkg/cloudprovider/providers/gce/cloud/meta"
+	"k8s.io/kubernetes/pkg/util/slice"
 
 	"k8s.io/ingress-gce/pkg/annotations"
 	"k8s.io/ingress-gce/pkg/context"
@@ -48,7 +49,6 @@ 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
@@ -370,6 +370,17 @@ func (lbc *LoadBalancerController) GCBackends(state interface{}) error {
 			return err
 		}
 	}
+
+	for _, ing := range gcState.ingresses {
+		if utils.IsDeletionCandidate(ing.ObjectMeta, utils.FinalizerKey()) {
+			ingClient := lbc.ctx.KubeClient.Extensions().Ingresses(ing.Namespace)
+			if err := utils.RemoveFinalizer(&ing, ingClient); err != nil {
+				glog.Errorf("Failed to remove Finalizer from Ingress %v/%v: %v", ing.Namespace, ing.Name, err)
+				return err
+			}
+		}
+	}
+
 	return nil
 }
 
@@ -449,9 +460,17 @@ func (lbc *LoadBalancerController) sync(key string) error {
 	// gceSvcPorts contains the ServicePorts used by only single-cluster ingress.
 	gceSvcPorts := lbc.ToSvcPorts(&gceIngresses)
 	lbNames := lbc.ingLister.Store.ListKeys()
-	gcState := &gcState{lbNames, gceSvcPorts}
 
 	obj, ingExists, err := lbc.ingLister.Store.GetByKey(key)
+	if err != nil {
+		return fmt.Errorf("error getting Ingress for key %s: %v", key, err)
+	}
+	ingList, err := lbc.ingLister.ListGCEIngresses()
+	if err != nil {
+		return fmt.Errorf("error getting list of Ingresses: %v", err)
+
+	}
+	gcState := &gcState{ingList.Items, lbNames, gceSvcPorts}
 	if !ingExists {
 		glog.V(2).Infof("Ingress %q no longer exists, triggering GC", key)
 		// GC will find GCE resources that were used for this ingress and delete them.
@@ -463,6 +482,25 @@ func (lbc *LoadBalancerController) sync(key string) error {
 	if !ok {
 		return fmt.Errorf("invalid object (not of type Ingress), type was %T", obj)
 	}
+
+	ing = ing.DeepCopy()
+	ingClient := lbc.ctx.KubeClient.Extensions().Ingresses(ing.Namespace)
+	if err := utils.AddFinalizer(ing, ingClient); err != nil {
+		glog.Errorf("Failed to add Finalizer to Ingress %q: %v", key, err)
+		return err
+	}
+
+	// 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)
+		if gcErr := lbc.ingSyncer.GC(gcState); gcErr != nil {
+			return gcErr
+		}
+
+		return nil
+	}
 	ing = ing.DeepCopy()
 
 	// Check if ingress class was changed to non-GLBC to remove ingress LB from state and trigger GC

pkg/controller/controller_test.go

@@ -186,6 +186,12 @@ func TestIngressCreateDelete(t *testing.T) {
 	if err := lbc.sync(ingStoreKey); err != nil {
 		t.Fatalf("lbc.sync(%v) = err %v", ingStoreKey, err)
 	}
+
+	// Check Ingress has been deleted
+	updatedIng, _ = lbc.ctx.KubeClient.Extensions().Ingresses(ing.Namespace).Get(ing.Name, meta_v1.GetOptions{})
+	if updatedIng != nil {
+		t.Fatalf("Ingress was not deleted, got: %+v", updatedIng)
+	}
 }
 
 // TestIngressClassChange asserts that `sync` will not return an error for a good ingress config

pkg/controller/types.go

@@ -24,8 +24,9 @@ import (
 
 // gcState is used by the controller to maintain state for garbage collection routines.
 type gcState struct {
-	lbNames  []string
-	svcPorts []utils.ServicePort
+	ingresses []extensions.Ingress
+	lbNames   []string
+	svcPorts  []utils.ServicePort
 }
 
 // syncState is used by the controller to maintain state for routines that sync GCP resources of an Ingress.

pkg/utils/finalizer.go

@@ -14,7 +14,12 @@ limitations under the License.
 package utils
 
 import (
+	"fmt"
+
+	"github.com/golang/glog"
+	extensions "k8s.io/api/extensions/v1beta1"
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	client "k8s.io/client-go/kubernetes/typed/extensions/v1beta1"
 	"k8s.io/kubernetes/pkg/util/slice"
 )
 
@@ -36,3 +41,40 @@ func NeedToAddFinalizer(m meta_v1.ObjectMeta, key string) bool {
 func HasFinalizer(m meta_v1.ObjectMeta, key string) bool {
 	return slice.ContainsString(m.Finalizers, key, nil)
 }
+
+// FinalizerKey generates the finalizer string for Ingress
+func FinalizerKey() string {
+	return fmt.Sprintf("%s.%s", "ingress", FinalizerKeySuffix)
+}
+
+// AddFinalizer tries to add a finalizer to an Ingress. If a finalizer
+// already exists, it does nothing.
+func AddFinalizer(ing *extensions.Ingress, ingClient client.IngressInterface) error {
+	ingKey := FinalizerKey()
+	if NeedToAddFinalizer(ing.ObjectMeta, ingKey) {
+		updated := ing.DeepCopy()
+		updated.ObjectMeta.Finalizers = append(updated.ObjectMeta.Finalizers, ingKey)
+		if _, err := ingClient.Update(updated); err != nil {
+			return fmt.Errorf("error updating Ingress %s/%s: %v", ing.Namespace, ing.Name, err)
+		}
+		glog.V(3).Infof("Added finalizer %q for Ingress %s/%s", ingKey, ing.Namespace, ing.Name)
+	}
+
+	return nil
+}
+
+// RemoveFinalizer tries to remove a Finalizer from an Ingress. If a
+// finalizer is not on the Ingress, it does nothing.
+func RemoveFinalizer(ing *extensions.Ingress, ingClient client.IngressInterface) error {
+	ingKey := FinalizerKey()
+	if HasFinalizer(ing.ObjectMeta, ingKey) {
+		updated := ing.DeepCopy()
+		updated.ObjectMeta.Finalizers = slice.RemoveString(updated.ObjectMeta.Finalizers, ingKey, nil)
+		if _, err := ingClient.Update(updated); err != nil {
+			return fmt.Errorf("error updating Ingress %s/%s: %v", ing.Namespace, ing.Name, err)
+		}
+		glog.V(3).Infof("Removed finalizer %q for Ingress %s/%s", ingKey, ing.Namespace, ing.Name)
+	}
+
+	return nil
+}