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

Extract NewL4 arguments into params struct #1787

Merged
merged 1 commit into from
Aug 24, 2022
Merged

Extract NewL4 arguments into params struct #1787

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: 14 additions & 2 deletions pkg/l4lb/l4controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,13 @@ func (l4c *L4Controller) processServiceCreateOrUpdate(key string, service *v1.Se
}
// Use the same function for both create and updates. If controller crashes and restarts,
// all existing services will show up as Service Adds.
l4 := loadbalancers.NewL4Handler(service, l4c.ctx.Cloud, meta.Regional, l4c.namer, l4c.ctx.Recorder(service.Namespace))
l4ilbParams := &loadbalancers.L4ILBParams{
Service: service,
Cloud: l4c.ctx.Cloud,
Namer: l4c.namer,
Recorder: l4c.ctx.Recorder(service.Namespace),
}
l4 := loadbalancers.NewL4Handler(l4ilbParams)
syncResult := l4.EnsureInternalLoadBalancer(nodeNames, service)
// syncResult will not be nil
if syncResult.Error != nil {
Expand Down Expand Up @@ -248,7 +254,13 @@ func (l4c *L4Controller) processServiceCreateOrUpdate(key string, service *v1.Se
}

func (l4c *L4Controller) processServiceDeletion(key string, svc *v1.Service) *loadbalancers.L4ILBSyncResult {
l4 := loadbalancers.NewL4Handler(svc, l4c.ctx.Cloud, meta.Regional, l4c.namer, l4c.ctx.Recorder(svc.Namespace))
l4ilbParams := &loadbalancers.L4ILBParams{
Service: svc,
Cloud: l4c.ctx.Cloud,
Namer: l4c.namer,
Recorder: l4c.ctx.Recorder(svc.Namespace),
}
l4 := loadbalancers.NewL4Handler(l4ilbParams)
l4c.ctx.Recorder(svc.Namespace).Eventf(svc, v1.EventTypeNormal, "DeletingLoadBalancer", "Deleting load balancer for %s", key)
result := l4.EnsureInternalLoadBalancerDeleted(svc)
if result.Error != nil {
Expand Down
8 changes: 7 additions & 1 deletion pkg/l4lb/l4netlbcontroller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,13 @@ func TestIsRBSBasedService(t *testing.T) {
func TestIsRBSBasedServiceWithILBServices(t *testing.T) {
controller := newL4NetLBServiceController()
ilbSvc := test.NewL4ILBService(false, 8080)
ilbFrName := loadbalancers.NewL4Handler(ilbSvc, controller.ctx.Cloud, meta.Regional, controller.namer, record.NewFakeRecorder(100)).GetFRName()
l4ilbParams := &loadbalancers.L4ILBParams{
Service: ilbSvc,
Cloud: controller.ctx.Cloud,
Namer: controller.namer,
Recorder: record.NewFakeRecorder(100),
}
ilbFrName := loadbalancers.NewL4Handler(l4ilbParams).GetFRName()
ilbSvc.Annotations = map[string]string{
annotations.TCPForwardingRuleKey: ilbFrName,
annotations.UDPForwardingRuleKey: ilbFrName,
Expand Down
22 changes: 15 additions & 7 deletions pkg/loadbalancers/l4.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,18 +68,26 @@ type L4ILBSyncResult struct {
StartTime time.Time
}

type L4ILBParams struct {
Service *corev1.Service
Cloud *gce.Cloud
Namer namer.L4ResourcesNamer
Recorder record.EventRecorder
}

// NewL4Handler creates a new L4Handler for the given L4 service.
func NewL4Handler(service *corev1.Service, cloud *gce.Cloud, scope meta.KeyType, namer namer.L4ResourcesNamer, recorder record.EventRecorder) *L4 {
func NewL4Handler(params *L4ILBParams) *L4 {
var scope meta.KeyType = meta.Regional
l := &L4{
cloud: cloud,
cloud: params.Cloud,
scope: scope,
namer: namer,
recorder: recorder,
Service: service,
namer: params.Namer,
recorder: params.Recorder,
Service: params.Service,
l4HealthChecks: healthchecks.L4(),
forwardingRules: forwardingrules.New(cloud, meta.VersionGA, scope),
forwardingRules: forwardingrules.New(params.Cloud, meta.VersionGA, scope),
}
l.NamespacedName = types.NamespacedName{Name: service.Name, Namespace: service.Namespace}
l.NamespacedName = types.NamespacedName{Name: params.Service.Name, Namespace: params.Service.Namespace}
l.backendPool = backends.NewPool(l.cloud, l.namer)
l.ServicePort = utils.ServicePort{ID: utils.ServicePortID{Service: l.NamespacedName}, BackendNamer: l.namer,
VMIPNEGEnabled: true}
Expand Down
137 changes: 119 additions & 18 deletions pkg/loadbalancers/l4_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,13 @@ func TestEnsureInternalBackendServiceUpdates(t *testing.T) {

svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

bsName, _ := l.namer.L4Backend(l.Service.Namespace, l.Service.Name)
Expand Down Expand Up @@ -119,7 +125,13 @@ func TestEnsureInternalLoadBalancer(t *testing.T) {

svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil {
Expand Down Expand Up @@ -176,7 +188,13 @@ func TestEnsureInternalLoadBalancerTypeChange(t *testing.T) {

svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil {
Expand Down Expand Up @@ -210,7 +228,13 @@ func TestEnsureInternalLoadBalancerWithExistingResources(t *testing.T) {
svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)

l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil {
Expand Down Expand Up @@ -253,7 +277,13 @@ func TestEnsureInternalLoadBalancerClearPreviousResources(t *testing.T) {

svc := test.NewL4ILBService(true, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

_, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName)
Expand Down Expand Up @@ -373,7 +403,13 @@ func TestUpdateResourceLinks(t *testing.T) {

svc := test.NewL4ILBService(true, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

_, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName)
Expand Down Expand Up @@ -451,7 +487,13 @@ func TestEnsureInternalLoadBalancerHealthCheckConfigurable(t *testing.T) {

svc := test.NewL4ILBService(true, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

_, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName)
Expand Down Expand Up @@ -494,7 +536,13 @@ func TestEnsureInternalLoadBalancerDeleted(t *testing.T) {
nodeNames := []string{"test-node-1"}
svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil {
Expand Down Expand Up @@ -526,7 +574,13 @@ func TestEnsureInternalLoadBalancerDeletedTwiceDoesNotError(t *testing.T) {
nodeNames := []string{"test-node-1"}
svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil {
Expand Down Expand Up @@ -643,7 +697,13 @@ func TestHealthCheckFirewallDeletionWithNetLB(t *testing.T) {

func ensureService(fakeGCE *gce.Cloud, namer *namer_util.L4Namer, nodeNames []string, zoneName string, port int, t *testing.T) (*v1.Service, *L4, *L4ILBSyncResult) {
svc := test.NewL4ILBService(false, 8080)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, zoneName); err != nil {
Expand All @@ -668,7 +728,13 @@ func TestEnsureInternalLoadBalancerWithSpecialHealthCheck(t *testing.T) {
nodeNames := []string{"test-node-1"}
svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil {
Expand Down Expand Up @@ -774,8 +840,13 @@ func TestEnsureInternalLoadBalancerErrors(t *testing.T) {
}
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
fakeGCE := getFakeGCECloud(gce.DefaultTestClusterValues())

l := NewL4Handler(params.service, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: params.service,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

//lbName := l.namer.L4Backend(params.service.Namespace, params.service.Name)
Expand Down Expand Up @@ -858,7 +929,13 @@ func TestEnsureInternalLoadBalancerEnableGlobalAccess(t *testing.T) {
nodeNames := []string{"test-node-1"}
svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil {
Expand Down Expand Up @@ -940,7 +1017,13 @@ func TestEnsureInternalLoadBalancerCustomSubnet(t *testing.T) {

svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil {
Expand Down Expand Up @@ -1038,7 +1121,13 @@ func TestEnsureInternalFirewallPortRanges(t *testing.T) {

svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

fwName, _ := l.namer.L4Backend(l.Service.Namespace, l.Service.Name)
Expand Down Expand Up @@ -1093,7 +1182,13 @@ func TestEnsureInternalLoadBalancerModifyProtocol(t *testing.T) {
nodeNames := []string{"test-node-1"}
svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

_, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName)
Expand Down Expand Up @@ -1185,7 +1280,13 @@ func TestEnsureInternalLoadBalancerAllPorts(t *testing.T) {
nodeNames := []string{"test-node-1"}
svc := test.NewL4ILBService(false, 8080)
namer := namer_util.NewL4Namer(kubeSystemUID, nil)
l := NewL4Handler(svc, fakeGCE, meta.Regional, namer, record.NewFakeRecorder(100))
l4ilbParams := &L4ILBParams{
Service: svc,
Cloud: fakeGCE,
Namer: namer,
Recorder: record.NewFakeRecorder(100),
}
l := NewL4Handler(l4ilbParams)
l.l4HealthChecks = healthchecks.FakeL4(fakeGCE, &test.FakeRecorderSource{})

if _, err := test.CreateAndInsertNodes(l.cloud, nodeNames, vals.ZoneName); err != nil {
Expand Down