Skip to content

Commit

Permalink
Merge pull request #1090 from prameshj/vmip-neg
Browse files Browse the repository at this point in the history
Replace GCE_PRIMARY_VM_IP to GCE_VM_IP NEG.
  • Loading branch information
k8s-ci-robot authored May 1, 2020
2 parents 49ca081 + b469fb4 commit 0425dad
Show file tree
Hide file tree
Showing 24 changed files with 147 additions and 146 deletions.
10 changes: 5 additions & 5 deletions pkg/backends/features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ const (
// FeatureL7ILB defines the feature name of L7 Internal Load Balancer
// L7-ILB Resources are currently alpha and regional
FeatureL7ILB = "L7ILB"
//FeaturePrimaryVMIPNEG defines the feature name of GCE_PRIMARY_VM_IP NEGs which are used for L4 ILB.
FeaturePrimaryVMIPNEG = "PrimaryVMIPNEG"
//FeatureVMIPNEG defines the feature name of GCE_VM_IP NEGs which are used for L4 ILB.
FeatureVMIPNEG = "VMIPNEG"
)

var (
Expand All @@ -48,7 +48,7 @@ var (
}
// TODO: (shance) refactor all scope to be above the serviceport level
scopeToFeatures = map[meta.KeyType][]string{
meta.Regional: []string{FeatureL7ILB, FeaturePrimaryVMIPNEG},
meta.Regional: []string{FeatureL7ILB, FeatureVMIPNEG},
}
)

Expand All @@ -70,8 +70,8 @@ func featuresFromServicePort(sp *utils.ServicePort) []string {
if sp.NEGEnabled {
features = append(features, FeatureNEG)
}
if sp.PrimaryIPNEGEnabled {
features = append(features, FeaturePrimaryVMIPNEG)
if sp.VMIPNEGEnabled {
features = append(features, FeatureVMIPNEG)
}
if sp.L7ILBEnabled {
features = append(features, FeatureL7ILB)
Expand Down
4 changes: 2 additions & 2 deletions pkg/backends/syncer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (p *portset) check(fakeGCE *gce.Cloud) error {
} else {
bs, err := composite.GetBackendService(fakeGCE, key, features.VersionFromServicePort(&sp))
if err == nil || !utils.IsHTTPErrorCode(err, http.StatusNotFound) {
if sp.PrimaryIPNEGEnabled {
if sp.VMIPNEGEnabled {
// It is expected that these Backends should not get cleaned up in the GC loop.
continue
}
Expand Down Expand Up @@ -350,7 +350,7 @@ func TestGCMixed(t *testing.T) {
{NodePort: 84, Protocol: annotations.ProtocolHTTP, NEGEnabled: true, L7ILBEnabled: true, BackendNamer: defaultNamer},
{NodePort: 85, Protocol: annotations.ProtocolHTTPS, NEGEnabled: true, L7ILBEnabled: true, BackendNamer: defaultNamer},
{NodePort: 86, Protocol: annotations.ProtocolHTTP, NEGEnabled: true, L7ILBEnabled: true, BackendNamer: defaultNamer},
{ID: utils.ServicePortID{Service: types.NamespacedName{Name: "testsvc"}}, PrimaryIPNEGEnabled: true, BackendNamer: defaultNamer},
{ID: utils.ServicePortID{Service: types.NamespacedName{Name: "testsvc"}}, VMIPNEGEnabled: true, BackendNamer: defaultNamer},
}
ps := newPortset(svcNodePorts)
if err := ps.add(svcNodePorts); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/l4/l4controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func deleteILBService(l4c *L4Controller, svc *api_v1.Service) {

func addNEG(l4c *L4Controller, svc *api_v1.Service) {
// Also create a fake NEG for this service since the sync code will try to link the backend service to NEG
negName := l4c.ctx.ClusterNamer.PrimaryIPNEG(svc.Namespace, svc.Name)
negName := l4c.ctx.ClusterNamer.VMIPNEG(svc.Namespace, svc.Name)
neg := &composite.NetworkEndpointGroup{Name: negName}
key := meta.ZonalKey(negName, types.TestZone1)
composite.CreateNetworkEndpointGroup(l4c.ctx.Cloud, key, neg)
Expand Down
8 changes: 4 additions & 4 deletions pkg/loadbalancers/l4.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func NewL4Handler(service *corev1.Service, cloud *gce.Cloud, scope meta.KeyType,
l.NamespacedName = types.NamespacedName{Name: service.Name, Namespace: service.Namespace}
l.backendPool = backends.NewPool(l.cloud, l.namer)
l.ServicePort = utils.ServicePort{ID: utils.ServicePortID{Service: l.NamespacedName}, BackendNamer: l.namer,
PrimaryIPNEGEnabled: true}
VMIPNEGEnabled: true}
return l
}

Expand All @@ -81,7 +81,7 @@ func (l *L4) EnsureInternalLoadBalancerDeleted(svc *corev1.Service) error {
klog.V(2).Infof("EnsureInternalLoadBalancerDeleted(%s): attempting delete of load balancer resources", l.NamespacedName.String())
sharedHC := !helpers.RequestsOnlyLocalTraffic(svc)
// All resources use the NEG Name, except forwarding rule.
name := l.namer.PrimaryIPNEG(svc.Namespace, svc.Name)
name := l.namer.VMIPNEG(svc.Namespace, svc.Name)
frName := l.GetFRName()
key, err := l.CreateKey(frName)
if err != nil {
Expand Down Expand Up @@ -158,7 +158,7 @@ func (l *L4) EnsureInternalLoadBalancerDeleted(svc *corev1.Service) error {
// service.
func (l *L4) GetFRName() string {
_, _, protocol := utils.GetPortsAndProtocol(l.Service.Spec.Ports)
lbName := l.namer.PrimaryIPNEG(l.Service.Namespace, l.Service.Name)
lbName := l.namer.VMIPNEG(l.Service.Namespace, l.Service.Name)
return lbName + "-" + strings.ToLower(string(protocol))
}

Expand All @@ -167,7 +167,7 @@ func (l *L4) GetFRName() string {
func (l *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service) (*corev1.LoadBalancerStatus, error) {
// Use the same resource name for NEG, BackendService as well as FR, FWRule.
l.Service = svc
name := l.namer.PrimaryIPNEG(l.Service.Namespace, l.Service.Name)
name := l.namer.VMIPNEG(l.Service.Namespace, l.Service.Name)
options := getILBOptions(l.Service)

// create healthcheck
Expand Down
22 changes: 11 additions & 11 deletions pkg/loadbalancers/l4_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestEnsureInternalBackendServiceUpdates(t *testing.T) {
svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewNamer(clusterUID, "")
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100), &sync.Mutex{}, fakeMetricsCollector)
bsName := l.namer.PrimaryIPNEG(l.Service.Namespace, l.Service.Name)
bsName := l.namer.VMIPNEG(l.Service.Namespace, l.Service.Name)
_, err := l.backendPool.EnsureL4BackendService(bsName, "", "TCP", string(svc.Spec.SessionAffinity), string(cloud.SchemeInternal), l.NamespacedName, meta.VersionGA)
if err != nil {
t.Errorf("Failed to ensure backend service %s - err %v", bsName, err)
Expand Down Expand Up @@ -181,7 +181,7 @@ func TestEnsureInternalLoadBalancerWithExistingResources(t *testing.T) {
t.Errorf("Unexpected error when adding nodes %v", err)
}

lbName := l.namer.PrimaryIPNEG(svc.Namespace, svc.Name)
lbName := l.namer.VMIPNEG(svc.Namespace, svc.Name)

// Create the expected resources necessary for an Internal Load Balancer
sharedHC := !servicehelper.RequestsOnlyLocalTraffic(svc)
Expand Down Expand Up @@ -223,7 +223,7 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) {
t.Errorf("Unexpected error when adding nodes %v", err)
}

lbName := l.namer.PrimaryIPNEG(svc.Namespace, svc.Name)
lbName := l.namer.VMIPNEG(svc.Namespace, svc.Name)
frName := l.GetFRName()
key, err := composite.CreateKey(l.cloud, frName, meta.Regional)
if err != nil {
Expand Down Expand Up @@ -333,7 +333,7 @@ func TestUpdateResourceLinks(t *testing.T) {
t.Errorf("Unexpected error when adding nodes %v", err)
}

lbName := l.namer.PrimaryIPNEG(svc.Namespace, svc.Name)
lbName := l.namer.VMIPNEG(svc.Namespace, svc.Name)
key, err := composite.CreateKey(l.cloud, lbName, meta.Regional)
if err != nil {
t.Errorf("Unexpected error when creating key - %v", err)
Expand Down Expand Up @@ -409,7 +409,7 @@ func TestEnsureInternalLoadBalancerHealthCheckConfigurable(t *testing.T) {
if err != nil {
t.Errorf("Unexpected error when adding nodes %v", err)
}
lbName := l.namer.PrimaryIPNEG(svc.Namespace, svc.Name)
lbName := l.namer.VMIPNEG(svc.Namespace, svc.Name)
key, err := composite.CreateKey(l.cloud, lbName, meta.Regional)
if err != nil {
t.Errorf("Unexpected error when creating key - %v", err)
Expand Down Expand Up @@ -529,7 +529,7 @@ func TestEnsureInternalLoadBalancerWithSpecialHealthCheck(t *testing.T) {
}
assertInternalLbResources(t, svc, l, nodeNames)

lbName := l.namer.PrimaryIPNEG(svc.Namespace, svc.Name)
lbName := l.namer.VMIPNEG(svc.Namespace, svc.Name)
key, err := composite.CreateKey(l.cloud, lbName, meta.Global)
if err != nil {
t.Errorf("Unexpected error when creating key - %v", err)
Expand Down Expand Up @@ -615,7 +615,7 @@ func TestEnsureInternalLoadBalancerErrors(t *testing.T) {
}
namer := namer_util.NewNamer(clusterUID, "")
l := NewL4Handler(params.service, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100), &sync.Mutex{}, fakeMetricsCollector)
//lbName := l.namer.PrimaryIPNEG(params.service.Namespace, params.service.Name)
//lbName := l.namer.VMIPNEG(params.service.Namespace, params.service.Name)
frName := l.GetFRName()
key, err := composite.CreateKey(l.cloud, frName, meta.Regional)
if err != nil {
Expand Down Expand Up @@ -661,7 +661,7 @@ func TestEnsureLoadBalancerDeletedSucceedsOnXPN(t *testing.T) {
if err != nil {
t.Errorf("Unexpected error when adding nodes %v", err)
}
fwName := l.namer.PrimaryIPNEG(svc.Namespace, svc.Name)
fwName := l.namer.VMIPNEG(svc.Namespace, svc.Name)
status, err := l.EnsureInternalLoadBalancer(nodeNames, svc)
if err != nil {
t.Errorf("Failed to ensure loadBalancer, err %v", err)
Expand Down Expand Up @@ -936,7 +936,7 @@ func TestEnsureInternalFirewallPortRanges(t *testing.T) {
svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewNamer(clusterUID, "")
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100), &sync.Mutex{}, fakeMetricsCollector)
fwName := l.namer.PrimaryIPNEG(l.Service.Namespace, l.Service.Name)
fwName := l.namer.VMIPNEG(l.Service.Namespace, l.Service.Name)
tc := struct {
Input []int
Result []string
Expand Down Expand Up @@ -981,7 +981,7 @@ func TestEnsureInternalFirewallPortRanges(t *testing.T) {
func assertInternalLbResources(t *testing.T, apiService *v1.Service, l *L4, nodeNames []string) {
// Check that Firewalls are created for the LoadBalancer and the HealthCheck
sharedHC := !servicehelper.RequestsOnlyLocalTraffic(apiService)
resourceName := l.namer.PrimaryIPNEG(l.Service.Namespace, l.Service.Name)
resourceName := l.namer.VMIPNEG(l.Service.Namespace, l.Service.Name)
resourceDesc, err := utils.MakeL4ILBServiceDescription(utils.ServiceKeyFunc(apiService.Namespace, apiService.Name), "", meta.VersionGA)
if err != nil {
t.Errorf("Failed to create description for resources, err %v", err)
Expand Down Expand Up @@ -1067,7 +1067,7 @@ func assertInternalLbResources(t *testing.T, apiService *v1.Service, l *L4, node
func assertInternalLbResourcesDeleted(t *testing.T, apiService *v1.Service, firewallsDeleted bool, l *L4) {
frName := l.GetFRName()
sharedHC := !servicehelper.RequestsOnlyLocalTraffic(apiService)
resourceName := l.namer.PrimaryIPNEG(l.Service.Namespace, l.Service.Name)
resourceName := l.namer.VMIPNEG(l.Service.Namespace, l.Service.Name)
hcName, hcFwName := healthchecks.HealthCheckName(sharedHC, l.namer.UID(), resourceName)

if firewallsDeleted {
Expand Down
12 changes: 6 additions & 6 deletions pkg/metrics/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,12 @@ const (
cookieAffinity = feature("CookieAffinity")
customRequestHeaders = feature("CustomRequestHeaders")

standaloneNeg = feature("StandaloneNEG")
ingressNeg = feature("IngressNEG")
asmNeg = feature("AsmNEG")
vmPrimaryIpNeg = feature("VmPrimaryIpNEG")
vmPrimaryIpNegLocal = feature("VmPrimaryIpNegLocal")
vmPrimaryIpNegCluster = feature("VmPrimaryIpNegCluster")
standaloneNeg = feature("StandaloneNEG")
ingressNeg = feature("IngressNEG")
asmNeg = feature("AsmNEG")
vmIpNeg = feature("VmIpNEG")
vmIpNegLocal = feature("VmIpNegLocal")
vmIpNegCluster = feature("VmIpNegCluster")

l4ILBService = feature("L4ILBService")
l4IlbGlobalAccess = feature("L4ILBGlobalAccess")
Expand Down
26 changes: 13 additions & 13 deletions pkg/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,29 +279,29 @@ func (im *ControllerMetrics) computeNegMetrics() map[feature]int {
klog.V(4).Infof("Computing NEG usage metrics from neg state map: %#v", im.negMap)

counts := map[feature]int{
standaloneNeg: 0,
ingressNeg: 0,
asmNeg: 0,
neg: 0,
vmPrimaryIpNeg: 0,
vmPrimaryIpNegLocal: 0,
vmPrimaryIpNegCluster: 0,
standaloneNeg: 0,
ingressNeg: 0,
asmNeg: 0,
neg: 0,
vmIpNeg: 0,
vmIpNegLocal: 0,
vmIpNegCluster: 0,
}

for key, negState := range im.negMap {
klog.V(6).Infof("For service %s, it has standaloneNegs:%d, ingressNegs:%d, asmNeg:%d and vmPrimaryNeg:%v",
key, negState.StandaloneNeg, negState.IngressNeg, negState.AsmNeg, negState.VmPrimaryIpNeg)
key, negState.StandaloneNeg, negState.IngressNeg, negState.AsmNeg, negState.VmIpNeg)
counts[standaloneNeg] += negState.StandaloneNeg
counts[ingressNeg] += negState.IngressNeg
counts[asmNeg] += negState.AsmNeg
counts[neg] += negState.AsmNeg + negState.StandaloneNeg + negState.IngressNeg
if negState.VmPrimaryIpNeg != nil {
if negState.VmIpNeg != nil {
counts[neg] += 1
counts[vmPrimaryIpNeg] += 1
if negState.VmPrimaryIpNeg.trafficPolicyLocal {
counts[vmPrimaryIpNegLocal] += 1
counts[vmIpNeg] += 1
if negState.VmIpNeg.trafficPolicyLocal {
counts[vmIpNegLocal] += 1
} else {
counts[vmPrimaryIpNegCluster] += 1
counts[vmIpNegCluster] += 1
}
}
}
Expand Down
72 changes: 36 additions & 36 deletions pkg/metrics/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -782,13 +782,13 @@ func TestComputeNegMetrics(t *testing.T) {
"empty input",
[]NegServiceState{},
map[feature]int{
standaloneNeg: 0,
ingressNeg: 0,
asmNeg: 0,
neg: 0,
vmPrimaryIpNeg: 0,
vmPrimaryIpNegLocal: 0,
vmPrimaryIpNegCluster: 0,
standaloneNeg: 0,
ingressNeg: 0,
asmNeg: 0,
neg: 0,
vmIpNeg: 0,
vmIpNegLocal: 0,
vmIpNegCluster: 0,
},
},
{
Expand All @@ -797,46 +797,46 @@ func TestComputeNegMetrics(t *testing.T) {
newNegState(0, 0, 1, nil),
},
map[feature]int{
standaloneNeg: 0,
ingressNeg: 0,
asmNeg: 1,
neg: 1,
vmPrimaryIpNeg: 0,
vmPrimaryIpNegLocal: 0,
vmPrimaryIpNegCluster: 0,
standaloneNeg: 0,
ingressNeg: 0,
asmNeg: 1,
neg: 1,
vmIpNeg: 0,
vmIpNegLocal: 0,
vmIpNegCluster: 0,
},
},
{
"vm primary ip neg in traffic policy cluster mode",
[]NegServiceState{
newNegState(0, 0, 1, &VmPrimaryIpNegType{trafficPolicyLocal: false}),
newNegState(0, 0, 1, &VmIpNegType{trafficPolicyLocal: false}),
},
map[feature]int{
standaloneNeg: 0,
ingressNeg: 0,
asmNeg: 1,
neg: 2,
vmPrimaryIpNeg: 1,
vmPrimaryIpNegLocal: 0,
vmPrimaryIpNegCluster: 1,
standaloneNeg: 0,
ingressNeg: 0,
asmNeg: 1,
neg: 2,
vmIpNeg: 1,
vmIpNegLocal: 0,
vmIpNegCluster: 1,
},
},
{
"many neg services",
[]NegServiceState{
newNegState(0, 0, 1, nil),
newNegState(0, 1, 0, &VmPrimaryIpNegType{trafficPolicyLocal: false}),
newNegState(5, 0, 0, &VmPrimaryIpNegType{trafficPolicyLocal: true}),
newNegState(0, 1, 0, &VmIpNegType{trafficPolicyLocal: false}),
newNegState(5, 0, 0, &VmIpNegType{trafficPolicyLocal: true}),
newNegState(5, 3, 2, nil),
},
map[feature]int{
standaloneNeg: 10,
ingressNeg: 4,
asmNeg: 3,
neg: 19,
vmPrimaryIpNeg: 2,
vmPrimaryIpNegLocal: 1,
vmPrimaryIpNegCluster: 1,
standaloneNeg: 10,
ingressNeg: 4,
asmNeg: 3,
neg: 19,
vmIpNeg: 2,
vmIpNegLocal: 1,
vmIpNegCluster: 1,
},
},
} {
Expand All @@ -856,12 +856,12 @@ func TestComputeNegMetrics(t *testing.T) {
}
}

func newNegState(standalone, ingress, asm int, negType *VmPrimaryIpNegType) NegServiceState {
func newNegState(standalone, ingress, asm int, negType *VmIpNegType) NegServiceState {
return NegServiceState{
IngressNeg: ingress,
StandaloneNeg: standalone,
AsmNeg: asm,
VmPrimaryIpNeg: negType,
IngressNeg: ingress,
StandaloneNeg: standalone,
AsmNeg: asm,
VmIpNeg: negType,
}
}

Expand Down
14 changes: 7 additions & 7 deletions pkg/metrics/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,19 @@ type NegServiceState struct {
IngressNeg int
// asmNeg is the count of NEGs created for ASM
AsmNeg int
// VmPrimaryIpNeg specifies if a service uses GCE_VM_PRIMARY_IP NEG.
VmPrimaryIpNeg *VmPrimaryIpNegType
// VmIpNeg specifies if a service uses GCE_VM_IP NEG.
VmIpNeg *VmIpNegType
}

// VmPrimaryIpNegType contains whether a GCE_VM_PRIMARY_IP NEG is requesting for
// VmIpNegType contains whether a GCE_VM_IP NEG is requesting for
// local traffic (or service external policy set to local).
type VmPrimaryIpNegType struct {
type VmIpNegType struct {
trafficPolicyLocal bool
}

// NewVmPrimaryIpNegType returns a new VmPrimaryIpNegType.
func NewVmPrimaryIpNegType(trafficPolicyLocal bool) *VmPrimaryIpNegType {
return &VmPrimaryIpNegType{trafficPolicyLocal: trafficPolicyLocal}
// NewVmIpNegType returns a new VmIpNegType.
func NewVmIpNegType(trafficPolicyLocal bool) *VmIpNegType {
return &VmIpNegType{trafficPolicyLocal: trafficPolicyLocal}
}

// L4ILBServiceState defines if global access and subnet features are enabled
Expand Down
Loading

0 comments on commit 0425dad

Please sign in to comment.