From 3a21929adbf91bdfaa5da4c2d9b309c875e821fb Mon Sep 17 00:00:00 2001 From: Ashley Gau Date: Wed, 16 Jan 2019 14:50:40 -0800 Subject: [PATCH] add finalizers --- pkg/controller/controller.go | 42 +++++++++++++++++++++++++++++-- pkg/controller/controller_test.go | 6 +++++ pkg/controller/types.go | 5 ++-- pkg/utils/finalizer.go | 42 +++++++++++++++++++++++++++++++ 4 files changed, 91 insertions(+), 4 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index c6f4c44838..2057bac501 100644 --- a/pkg/controller/controller.go +++ b/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 diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index ef37bb6b1f..790d5d64e5 100644 --- a/pkg/controller/controller_test.go +++ b/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 diff --git a/pkg/controller/types.go b/pkg/controller/types.go index 52bc12cc3d..c24bbf1bbf 100644 --- a/pkg/controller/types.go +++ b/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. diff --git a/pkg/utils/finalizer.go b/pkg/utils/finalizer.go index b228e50ec2..495b690342 100644 --- a/pkg/utils/finalizer.go +++ b/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 +}