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

Use namer.L4Firewall and namer.L4Backend instead of servicePort #1821

Merged
merged 1 commit into from
Oct 3, 2022
Merged

Use namer.L4Firewall and namer.L4Backend instead of servicePort #1821

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions pkg/l4lb/l4netlbcontroller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ func (lc *L4NetLBController) syncInternal(service *v1.Service) *loadbalancers.L4
return syncResult
}

if err = lc.ensureBackendLinking(l4netlb.ServicePort); err != nil {
if err = lc.ensureBackendLinking(service); err != nil {
lc.ctx.Recorder(service.Namespace).Eventf(service, v1.EventTypeWarning, "SyncExternalLoadBalancerFailed",
"Error linking instance groups to backend service, err: %v", err)
syncResult.Error = err
Expand All @@ -475,12 +475,22 @@ func (lc *L4NetLBController) syncInternal(service *v1.Service) *loadbalancers.L4
return syncResult
}

func (lc *L4NetLBController) ensureBackendLinking(port utils.ServicePort) error {
func (lc *L4NetLBController) ensureBackendLinking(service *v1.Service) error {
zones, err := lc.translator.ListZones(utils.CandidateNodesPredicate)
if err != nil {
return err
}
return lc.igLinker.Link(port, lc.ctx.Cloud.ProjectID(), zones)

namespacedName := types.NamespacedName{Name: service.Name, Namespace: service.Namespace}
portId := utils.ServicePortID{Service: namespacedName}
servicePort := utils.ServicePort{
ID: portId,
BackendNamer: lc.namer,
NodePort: utils.GetServiceNodePort(service),
L4RBSEnabled: true,
}

return lc.igLinker.Link(servicePort, lc.ctx.Cloud.ProjectID(), zones)
}

func (lc *L4NetLBController) ensureInstanceGroups(service *v1.Service, nodeNames []string) error {
Expand Down
40 changes: 17 additions & 23 deletions pkg/loadbalancers/l4netlb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ type L4NetLB struct {
// recorder is used to generate k8s Events.
recorder record.EventRecorder
Service *corev1.Service
ServicePort utils.ServicePort
NamespacedName types.NamespacedName
healthChecks healthchecksl4.L4HealthChecks
forwardingRules ForwardingRulesProvider
Expand Down Expand Up @@ -102,13 +101,6 @@ func NewL4NetLB(params *L4NetLBParams) *L4NetLB {
healthChecks: healthchecksl4.GetInstance(),
forwardingRules: forwardingrules.New(params.Cloud, meta.VersionGA, meta.Regional),
}
portId := utils.ServicePortID{Service: l4netlb.NamespacedName}
l4netlb.ServicePort = utils.ServicePort{
ID: portId,
BackendNamer: l4netlb.namer,
NodePort: utils.GetServiceNodePort(params.Service),
L4RBSEnabled: true,
}
return l4netlb
}

Expand All @@ -133,26 +125,25 @@ func (l4netlb *L4NetLB) EnsureFrontend(nodeNames []string, svc *corev1.Service)

sharedHC := !helpers.RequestsOnlyLocalTraffic(svc)
hcResult := l4netlb.healthChecks.EnsureHealthCheckWithFirewall(l4netlb.Service, l4netlb.namer, sharedHC, l4netlb.scope, utils.XLB, nodeNames)

if hcResult.Err != nil {
result.GCEResourceInError = hcResult.GceResourceInError
result.Error = fmt.Errorf("Failed to ensure health check %s - %w", hcResult.HCName, hcResult.Err)
return result
}
result.Annotations[annotations.HealthcheckKey] = hcResult.HCName
result.Annotations[annotations.FirewallRuleForHealthcheckKey] = hcResult.HCFirewallRuleName

name := l4netlb.ServicePort.BackendName()
bsName := l4netlb.namer.L4Backend(l4netlb.Service.Namespace, l4netlb.Service.Name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is ServicePort used after this refactoring? If not maybe you should delete it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, good catch, done,
but it needed to move a bit of logic upper to l4netlbcontroller, so now it's also changed

servicePorts := l4netlb.Service.Spec.Ports
portRanges := utils.GetServicePortRanges(servicePorts)
protocol := utils.GetProtocol(servicePorts)

bs, err := l4netlb.backendPool.EnsureL4BackendService(name, hcResult.HCLink, string(protocol), string(l4netlb.Service.Spec.SessionAffinity), string(cloud.SchemeExternal), l4netlb.NamespacedName, meta.VersionGA)
bs, err := l4netlb.backendPool.EnsureL4BackendService(bsName, hcResult.HCLink, string(protocol), string(l4netlb.Service.Spec.SessionAffinity), string(cloud.SchemeExternal), l4netlb.NamespacedName, meta.VersionGA)
if err != nil {
result.GCEResourceInError = annotations.BackendServiceResource
result.Error = fmt.Errorf("Failed to ensure backend service %s - %w", name, err)
result.Error = fmt.Errorf("Failed to ensure backend service %s - %w", bsName, err)
return result
}
result.Annotations[annotations.BackendServiceKey] = name
result.Annotations[annotations.BackendServiceKey] = bsName

fr, ipAddrType, err := l4netlb.ensureExternalForwardingRule(bs.SelfLink)
if err != nil {
// User can misconfigure the forwarding rule if Network Tier will not match service level Network Tier.
Expand All @@ -170,12 +161,13 @@ func (l4netlb *L4NetLB) EnsureFrontend(nodeNames []string, svc *corev1.Service)
result.MetricsState.IsPremiumTier = fr.NetworkTier == cloud.NetworkTierPremium.ToGCEValue()
result.MetricsState.IsManagedIP = ipAddrType == IPAddrManaged

res := l4netlb.createFirewalls(name, nodeNames, fr.IPAddress, portRanges, string(protocol))
firewallName := l4netlb.namer.L4Firewall(l4netlb.Service.Namespace, l4netlb.Service.Name)
portRanges := utils.GetServicePortRanges(servicePorts)
res := l4netlb.createFirewalls(firewallName, nodeNames, fr.IPAddress, portRanges, string(protocol))
if res.Error != nil {
return res
}
result.Annotations[annotations.FirewallRuleKey] = name
result.Annotations[annotations.FirewallRuleForHealthcheckKey] = hcResult.HCFirewallRuleName
result.Annotations[annotations.FirewallRuleKey] = firewallName

return result
}
Expand All @@ -185,8 +177,8 @@ func (l4netlb *L4NetLB) EnsureFrontend(nodeNames []string, svc *corev1.Service)
func (l4netlb *L4NetLB) EnsureLoadBalancerDeleted(svc *corev1.Service) *L4NetLBSyncResult {
result := NewL4SyncResult(SyncTypeDelete)

frName := l4netlb.GetFRName()
// If any resource deletion fails, log the error and continue cleanup.
frName := l4netlb.GetFRName()
err := l4netlb.forwardingRules.Delete(frName)
if err != nil {
klog.Errorf("Failed to delete forwarding rule %s for service %s - %v", frName, l4netlb.NamespacedName.String(), err)
Expand All @@ -199,15 +191,17 @@ func (l4netlb *L4NetLB) EnsureLoadBalancerDeleted(svc *corev1.Service) *L4NetLBS
result.GCEResourceInError = annotations.AddressResource
}

name := l4netlb.ServicePort.BackendName()
err = l4netlb.deleteFirewall(name)
firewallName := l4netlb.namer.L4Firewall(l4netlb.Service.Namespace, l4netlb.Service.Name)
err = l4netlb.deleteFirewall(firewallName)
if err != nil {
klog.Errorf("Failed to delete firewall rule %s for service %s - %v", name, l4netlb.NamespacedName.String(), err)
klog.Errorf("Failed to delete firewall rule %s for service %s - %v", firewallName, l4netlb.NamespacedName.String(), err)
result.GCEResourceInError = annotations.FirewallRuleResource
result.Error = err
}

// delete backend service
err = utils.IgnoreHTTPNotFound(l4netlb.backendPool.Delete(name, meta.VersionGA, meta.Regional))
bsName := l4netlb.namer.L4Backend(l4netlb.Service.Namespace, l4netlb.Service.Name)
err = utils.IgnoreHTTPNotFound(l4netlb.backendPool.Delete(bsName, meta.VersionGA, meta.Regional))
if err != nil {
klog.Errorf("Failed to delete backends for L4 External LoadBalancer service %s - %v", l4netlb.NamespacedName.String(), err)
result.GCEResourceInError = annotations.BackendServiceResource
Expand Down
2 changes: 1 addition & 1 deletion pkg/loadbalancers/l4netlb_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func TestEnsureL4NetLoadBalancer(t *testing.T) {
}

func checkAnnotations(result *L4NetLBSyncResult, l4netlb *L4NetLB) error {
expBackendName := l4netlb.ServicePort.BackendName()
expBackendName := l4netlb.namer.L4Backend(l4netlb.Service.Namespace, l4netlb.Service.Name)
if result.Annotations[annotations.BackendServiceKey] != expBackendName {
return fmt.Errorf("BackendServiceKey mismatch %v != %v", expBackendName, result.Annotations[annotations.BackendServiceKey])
}
Expand Down