Skip to content

Commit

Permalink
Merge pull request #1821 from panslava/firewall-namer-l4netlb
Browse files Browse the repository at this point in the history 
Use namer.L4Firewall and namer.L4Backend instead of servicePort
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Oct 3, 2022
2 parents 0a04a59 + 221d825 commit 42eca78
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 27 deletions.
16 changes: 13 additions & 3 deletions pkg/l4lb/l4netlbcontroller.go
View file
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
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)
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
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

0 comments on commit 42eca78

Please sign in to comment.