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.

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

Replace GCE_PRIMARY_VM_IP to GCE_VM_IP NEG. #1090

Merged
merged 1 commit into from
May 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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