Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert "Skip ILB creation on GCE if neg annotation is present" #79356

Merged
merged 1 commit into from
Jun 30, 2019
Merged

Revert "Skip ILB creation on GCE if neg annotation is present" #79356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 0 additions & 3 deletions staging/src/k8s.io/legacy-cloud-providers/gce/gce_annotations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,6 @@ const (

// NetworkTierAnnotationPremium is an annotation to indicate the Service is on the Premium network tier
NetworkTierAnnotationPremium = cloud.NetworkTierPremium

// NEGAnnotation is an annotation to indicate that the loadbalancer service is using NEGs instead of InstanceGroups
NEGAnnotation = "cloud.google.com/neg"
)

// GetLoadBalancerAnnotationType returns the type of GCP load balancer which should be assembled.
Expand Down
13 changes: 0 additions & 13 deletions staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,7 @@ const (
allInstances = "ALL"
)

func usesNEG(service *v1.Service) bool {
_, ok := service.GetAnnotations()[NEGAnnotation]
return ok
}

func (g *Cloud) ensureInternalLoadBalancer(clusterName, clusterID string, svc *v1.Service, existingFwdRule *compute.ForwardingRule, nodes []*v1.Node) (*v1.LoadBalancerStatus, error) {
if usesNEG(svc) {
// Do not manage loadBalancer for services using NEGs
return &svc.Status.LoadBalancer, nil
}
nm := types.NamespacedName{Name: svc.Name, Namespace: svc.Namespace}
ports, protocol := getPortsAndProtocol(svc.Spec.Ports)
if protocol != v1.ProtocolTCP && protocol != v1.ProtocolUDP {
Expand Down Expand Up @@ -210,10 +201,6 @@ func (g *Cloud) clearPreviousInternalResources(svc *v1.Service, loadBalancerName
// updateInternalLoadBalancer is called when the list of nodes has changed. Therefore, only the instance groups
// and possibly the backend service need to be updated.
func (g *Cloud) updateInternalLoadBalancer(clusterName, clusterID string, svc *v1.Service, nodes []*v1.Node) error {
if usesNEG(svc) {
// Do not manage loadBalancer for services using NEGs
return nil
}
g.sharedResourceLock.Lock()
defer g.sharedResourceLock.Unlock()

Expand Down
59 changes: 0 additions & 59 deletions staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_internal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -847,62 +847,3 @@ func TestCompareHealthChecks(t *testing.T) {
})
}
}

func TestEnsureInternalLoadBalancerNEG(t *testing.T) {
t.Parallel()

vals := DefaultTestClusterValues()
gce, err := fakeGCECloud(vals)
require.NoError(t, err)

nodeNames := []string{"test-node-1"}
nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName)
require.NoError(t, err)

apiService := fakeLoadbalancerServiceWithNEGs(string(LBTypeInternal))
status, err := gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, apiService, nodes)
assert.NoError(t, err)
// No loadbalancer resources will be created due to the NEG annotation
assert.Empty(t, status.Ingress)
assertInternalLbResourcesDeleted(t, gce, apiService, vals, true)
// Invoking delete should be a no-op
err = gce.EnsureLoadBalancerDeleted(context.Background(), vals.ClusterName, apiService)
assert.NoError(t, err)
// Now remove the annotation so that lb resources are created
delete(apiService.Annotations, NEGAnnotation)
status, err = gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, apiService, nodes)
assert.NoError(t, err)
assert.NotEmpty(t, status.Ingress)
assertInternalLbResources(t, gce, apiService, vals, nodeNames)
}

func TestEnsureInternalLoadBalancerDeletedNEGs(t *testing.T) {
t.Parallel()

vals := DefaultTestClusterValues()
gce, err := fakeGCECloud(vals)
require.NoError(t, err)

nodeNames := []string{"test-node-1"}
nodes, err := createAndInsertNodes(gce, nodeNames, vals.ZoneName)
require.NoError(t, err)

apiService := fakeLoadbalancerService(string(LBTypeInternal))

status, err := gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, apiService, nodes)
assert.NoError(t, err)
assert.NotEmpty(t, status.Ingress)
// Annotation gets added to service
apiService.Annotations[NEGAnnotation] = "{\"ilb\": true}"
newLBStatus := v1.LoadBalancerStatus{Ingress: []v1.LoadBalancerIngress{{IP: "1.2.3.4"}}}
// mock scenario where a different controller modifies status.
apiService.Status.LoadBalancer = newLBStatus
status, err = gce.EnsureLoadBalancer(context.Background(), vals.ClusterName, apiService, nodes)
assert.NoError(t, err)
// ensure that the status info is intact
assert.Equal(t, status, &newLBStatus)
// Invoked when service is deleted.
err = gce.EnsureLoadBalancerDeleted(context.Background(), vals.ClusterName, apiService)
assert.NoError(t, err)
assertInternalLbResourcesDeleted(t, gce, apiService, vals, true)
}
6 changes: 0 additions & 6 deletions staging/src/k8s.io/legacy-cloud-providers/gce/gce_loadbalancer_utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,6 @@ func fakeLoadbalancerService(lbType string) *v1.Service {
}
}

func fakeLoadbalancerServiceWithNEGs(lbType string) *v1.Service {
svc := fakeLoadbalancerService(lbType)
svc.Annotations[NEGAnnotation] = "{\"ilb\": true}"
return svc
}

var (
FilewallChangeMsg = fmt.Sprintf("%s %s %s", v1.EventTypeNormal, eventReasonManualChange, eventMsgFirewallChange)
)
Expand Down