Merge pull request #948 from skmatti/v2namer-fix

Fix naming scheme on creation

cmd/glbc/main.go

@@ -25,7 +25,6 @@ import (
 
 	flag "github.com/spf13/pflag"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/ingress-gce/pkg/frontendconfig"
 	"k8s.io/klog"
 
@@ -125,15 +124,12 @@ func main() {
 		klog.V(0).Infof("Cluster name: %+v", namer.UID())
 	}
 
-	var kubeSystemUID types.UID
-	if flags.F.EnableV2FrontendNamer {
-		// Get kube-system UID that will be used for v2 frontend naming scheme.
-		kubeSystemNS, err := kubeClient.CoreV1().Namespaces().Get("kube-system", metav1.GetOptions{})
-		if err != nil {
-			klog.Fatalf("Error getting kube-system namespace: %v", err)
-		}
-		kubeSystemUID = kubeSystemNS.GetUID()
+	// Get kube-system UID that will be used for v2 frontend naming scheme.
+	kubeSystemNS, err := kubeClient.CoreV1().Namespaces().Get("kube-system", metav1.GetOptions{})
+	if err != nil {
+		klog.Fatalf("Error getting kube-system namespace: %v", err)
 	}
+	kubeSystemUID := kubeSystemNS.GetUID()
 
 	cloud := app.NewGCEClient()
 	defaultBackendServicePort := app.DefaultBackendServicePort(kubeClient)

pkg/controller/controller.go

@@ -525,7 +525,6 @@ func (lbc *LoadBalancerController) sync(key string) error {
 	// Determine if the ingress needs to be GCed.
 	if !ingExists || utils.NeedsCleanup(ing) {
 		frontendGCAlgorithm := frontendGCAlgorithm(ingExists, ing)
-		klog.V(3).Infof("Using algorithm %v to GC ingress %v", frontendGCAlgorithm, ing)
 		// GC will find GCE resources that were used for this ingress and delete them.
 		err := lbc.ingSyncer.GC(allIngresses, ing, frontendGCAlgorithm)
 		if err != nil {
@@ -536,7 +535,7 @@ func (lbc *LoadBalancerController) sync(key string) error {
 
 	// Ensure that a finalizer is attached.
 	if flags.F.FinalizerAdd {
-		if err = lbc.ensureFinalizer(ing); err != nil {
+		if ing, err = lbc.ensureFinalizer(ing); err != nil {
 			return err
 		}
 	}
@@ -561,7 +560,6 @@ func (lbc *LoadBalancerController) sync(key string) error {
 	// it could have been caused by quota issues; therefore, garbage collecting now may
 	// free up enough quota for the next sync to pass.
 	frontendGCAlgorithm := frontendGCAlgorithm(ingExists, ing)
-	klog.V(3).Infof("Using algorithm %v to GC ingress %v", frontendGCAlgorithm, ing)
 	if gcErr := lbc.ingSyncer.GC(allIngresses, ing, frontendGCAlgorithm); gcErr != nil {
 		lbc.ctx.Recorder(ing.Namespace).Eventf(ing, apiv1.EventTypeWarning, "GC", fmt.Sprintf("Error during GC: %v", gcErr))
 		return fmt.Errorf("error during sync %v, error during GC %v", syncErr, gcErr)
@@ -702,28 +700,30 @@ func (lbc *LoadBalancerController) defaultFrontendNamingScheme(ing *v1beta1.Ingr
 }
 
 // ensureFinalizer ensures that a finalizer is attached.
-func (lbc *LoadBalancerController) ensureFinalizer(ing *v1beta1.Ingress) error {
+func (lbc *LoadBalancerController) ensureFinalizer(ing *v1beta1.Ingress) (*v1beta1.Ingress, error) {
 	ingKey := common.NamespacedName(ing)
 	if common.HasFinalizer(ing.ObjectMeta) {
 		klog.V(4).Infof("Finalizer exists for ingress %s", ingKey)
-		return nil
+		return ing, nil
 	}
-	// Get ingress and DeepCopy for assurance that we don't pollute other goroutines with changes.
-	ing = ing.DeepCopy()
-	ingClient := lbc.ctx.KubeClient.NetworkingV1beta1().Ingresses(ing.Namespace)
 	namingScheme, err := lbc.defaultFrontendNamingScheme(ing)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	finalizerKey, err := namer.FinalizerForNamingScheme(namingScheme)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	if err := common.EnsureFinalizer(ing, ingClient, finalizerKey); err != nil {
+	ingClient := lbc.ctx.KubeClient.NetworkingV1beta1().Ingresses(ing.Namespace)
+	// Update ingress with finalizer so that load-balancer pool uses correct naming scheme
+	// while ensuring frontend resources. Note that this updates only the finalizer annotation
+	// which may be inconsistent with ingress store for a short period.
+	updatedIng, err := common.EnsureFinalizer(ing, ingClient, finalizerKey)
+	if err != nil {
 		klog.Errorf("Failed to ensure finalizer %s for ingress %s: %v", finalizerKey, ingKey, err)
-		return err
+		return nil, err
 	}
-	return nil
+	return updatedIng, nil
 }
 
 // frontendGCAlgorithm returns the naming scheme using which frontend resources needs to be cleanedup.

pkg/sync/sync.go

@@ -23,7 +23,9 @@ import (
 	"k8s.io/api/networking/v1beta1"
 	"k8s.io/ingress-gce/pkg/common/operator"
 	"k8s.io/ingress-gce/pkg/utils"
+	"k8s.io/ingress-gce/pkg/utils/common"
 	"k8s.io/ingress-gce/pkg/utils/namer"
+	"k8s.io/klog"
 )
 
 // ErrSkipBackendsSync is an error that can be returned by a Controller to
@@ -67,6 +69,7 @@ func (s *IngressSyncer) GC(ings []*v1beta1.Ingress, currIng *v1beta1.Ingress, fr
 	var errs []error
 	switch frontendGCAlgorithm {
 	case utils.CleanupV2FrontendResources:
+		klog.V(3).Infof("Using algorithm CleanupV2FrontendResources to GC frontend of ingress %s", common.NamespacedName(currIng))
 		lbErr = s.controller.GCv2LoadBalancer(currIng)
 
 		defer func() {
@@ -76,6 +79,7 @@ func (s *IngressSyncer) GC(ings []*v1beta1.Ingress, currIng *v1beta1.Ingress, fr
 			err = s.controller.EnsureDeleteV2Finalizer(currIng)
 		}()
 	case utils.CleanupV1FrontendResources:
+		klog.V(3).Infof("Using algorithm CleanupV1FrontendResources to GC frontend of ingress %s", common.NamespacedName(currIng))
 		// Filter GCE ingresses that use v1 naming scheme.
 		v1Ingresses := operator.Ingresses(ings).Filter(func(ing *v1beta1.Ingress) bool {
 			return namer.FrontendNamingScheme(ing) == namer.V1NamingScheme
@@ -93,6 +97,7 @@ func (s *IngressSyncer) GC(ings []*v1beta1.Ingress, currIng *v1beta1.Ingress, fr
 			err = s.controller.EnsureDeleteV1Finalizers(toCleanupV1.AsList())
 		}()
 	case utils.NoCleanUpNeeded:
+		klog.V(3).Infof("Using algorithm NoCleanUpNeeded to GC frontend of ingress %s", common.NamespacedName(currIng))
 	default:
 		lbErr = fmt.Errorf("unexpected frontend GC algorithm %v", frontendGCAlgorithm)
 	}

pkg/utils/common/finalizer.go

@@ -52,18 +52,18 @@ func HasGivenFinalizer(m meta_v1.ObjectMeta, key string) bool {
 }
 
 // EnsureFinalizer ensures that the specified finalizer exists on given Ingress.
-func EnsureFinalizer(ing *v1beta1.Ingress, ingClient client.IngressInterface, finalizerKey string) error {
+func EnsureFinalizer(ing *v1beta1.Ingress, ingClient client.IngressInterface, finalizerKey string) (*v1beta1.Ingress, error) {
+	updated := ing.DeepCopy()
 	if needToAddFinalizer(ing.ObjectMeta, finalizerKey) {
-		updated := ing.DeepCopy()
 		updated.ObjectMeta.Finalizers = append(updated.ObjectMeta.Finalizers, finalizerKey)
 		// TODO(smatti): Make this optimistic concurrency control compliant on write.
 		// Refer: https://github.com/eBay/Kubernetes/blob/master/docs/devel/api-conventions.md#concurrency-control-and-consistency
 		if _, err := ingClient.Update(updated); err != nil {
-			return fmt.Errorf("error updating Ingress %s/%s: %v", ing.Namespace, ing.Name, err)
+			return nil, fmt.Errorf("error updating Ingress %s/%s: %v", ing.Namespace, ing.Name, err)
 		}
 		klog.V(2).Infof("Added finalizer %q for Ingress %s/%s", finalizerKey, ing.Namespace, ing.Name)
 	}
-	return nil
+	return updated, nil
 }
 
 // needToAddFinalizer is true if the passed in meta does not contain the specified finalizer.