Merge pull request #1162 from skmatti/neglink-fix

Avoid unnecessary backend service update in NEG linker
kubernetes · Jun 19, 2020 · e33650b · e33650b
2 parents 15f450f + 7f8b8f8
commit e33650b
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 16 deletions.
diff --git a/pkg/backends/features/features.go b/pkg/backends/features/features.go
@@ -44,7 +44,8 @@ var (
 	// versionToFeatures stores the mapping from the required API
 	// version to feature names.
 	versionToFeatures = map[meta.Version][]string{
-		meta.VersionBeta: []string{FeatureSecurityPolicy, FeatureHTTP2, FeatureL7ILB},
+		meta.VersionAlpha: {FeatureVMIPNEG},
+		meta.VersionBeta:  {FeatureSecurityPolicy, FeatureHTTP2, FeatureL7ILB},
 	}
 	// TODO: (shance) refactor all scope to be above the serviceport level
 	scopeToFeatures = map[meta.KeyType][]string{

diff --git a/pkg/backends/neg_linker.go b/pkg/backends/neg_linker.go
@@ -18,6 +18,7 @@ import (
 	befeatures "k8s.io/ingress-gce/pkg/backends/features"
 	"k8s.io/ingress-gce/pkg/composite"
 	"k8s.io/ingress-gce/pkg/utils"
+	"k8s.io/klog"
 	"k8s.io/legacy-cloud-providers/gce"
 )
 
@@ -44,6 +45,7 @@ func NewNEGLinker(
 
 // Link implements Link.
 func (l *negLinker) Link(sp utils.ServicePort, groups []GroupKey) error {
+	version := befeatures.VersionFromServicePort(&sp)
 	var negs []*composite.NetworkEndpointGroup
 	var err error
 	for _, group := range groups {
@@ -53,16 +55,14 @@ func (l *negLinker) Link(sp utils.ServicePort, groups []GroupKey) error {
 		if negName == "" {
 			negName = sp.BackendName()
 		}
-		neg, err := l.negGetter.GetNetworkEndpointGroup(negName, group.Zone, utils.GetAPIVersionFromServicePort(&sp))
+		neg, err := l.negGetter.GetNetworkEndpointGroup(negName, group.Zone, version)
 		if err != nil {
 			return err
 		}
 		negs = append(negs, neg)
 	}
 
 	beName := sp.BackendName()
-
-	version := befeatures.VersionFromServicePort(&sp)
 	scope := befeatures.ScopeFromServicePort(&sp)
 
 	key, err := composite.CreateKey(l.cloud, beName, scope)
@@ -88,6 +88,7 @@ func (l *negLinker) Link(sp utils.ServicePort, groups []GroupKey) error {
 	}
 
 	if !oldBackends.Equal(newBackends) {
+		klog.V(2).Infof("Backends changed for service port %s, removing: %s and adding: %s", sp.ID, oldBackends.Difference(newBackends), newBackends.Difference(oldBackends))
 		backendService.Backends = targetBackends
 		return composite.UpdateBackendService(l.cloud, key, backendService)
 	}

diff --git a/pkg/utils/serviceport.go b/pkg/utils/serviceport.go
@@ -19,8 +19,6 @@ package utils
 import (
 	"fmt"
 
-	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
-
 	"k8s.io/api/networking/v1beta1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -56,16 +54,6 @@ type ServicePort struct {
 	BackendNamer   namer.BackendNamer
 }
 
-// GetAPIVersionFromServicePort returns the compute API version to be used
-// for creating NEGs associated with the given ServicePort.
-func GetAPIVersionFromServicePort(sp *ServicePort) meta.Version {
-	if sp.VMIPNEGEnabled {
-		// this uses VM_IP NEGS which requires alpha API
-		return meta.VersionAlpha
-	}
-	return meta.VersionGA
-}
-
 // GetDescription returns a Description for this ServicePort.
 func (sp ServicePort) GetDescription() Description {
 	return Description{