Merge pull request #636 from kubernetes/revert-627-finalizer-cherrypi…

…ck-1

Revert "Cherry-pick #613 onto 1.4, part 1"

pkg/controller/controller.go

@@ -40,7 +40,6 @@ 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"
@@ -49,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
@@ -370,17 +370,6 @@ 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
 }
 
@@ -460,17 +449,9 @@ 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.
@@ -482,25 +463,6 @@ 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,12 +186,6 @@ 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,9 +24,8 @@ import (
 
 // gcState is used by the controller to maintain state for garbage collection routines.
 type gcState struct {
-	ingresses []extensions.Ingress
-	lbNames   []string
-	svcPorts  []utils.ServicePort
+	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,12 +14,7 @@ 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"
 )
 
@@ -41,40 +36,3 @@ 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
-}