diff --git a/pkg/loadbalancers/l4_test.go b/pkg/loadbalancers/l4_test.go index 026989b110..65fffb88e7 100644 --- a/pkg/loadbalancers/l4_test.go +++ b/pkg/loadbalancers/l4_test.go @@ -1706,10 +1706,6 @@ func TestEnsureInternalDualStackLoadBalancer(t *testing.T) { // for dual-stack service. In total 401 combinations func TestDualStackLoadBalancerTransitions(t *testing.T) { t.Parallel() - nodeNames := []string{"test-node-1"} - vals := gce.DefaultTestClusterValues() - - namer := namer_util.NewL4Namer(kubeSystemUID, nil) trafficPolicyStates := []v1.ServiceExternalTrafficPolicyType{v1.ServiceExternalTrafficPolicyTypeLocal, v1.ServiceExternalTrafficPolicyTypeCluster} protocols := []v1.Protocol{v1.ProtocolTCP, v1.ProtocolUDP} @@ -1727,73 +1723,15 @@ func TestDualStackLoadBalancerTransitions(t *testing.T) { for _, finalTrafficPolicy := range trafficPolicyStates { for _, initialProtocol := range protocols { for _, finalProtocol := range protocols { - initialIPFamily := initialIPFamily - finalIPFamily := finalIPFamily - initialTrafficPolicy := initialTrafficPolicy - finalTrafficPolicy := finalTrafficPolicy - initialProtocol := initialProtocol - finalProtocol := finalProtocol - - var stringInitialIPFamily []string - for _, f := range initialIPFamily { - stringInitialIPFamily = append(stringInitialIPFamily, string(f)) - } - - var stringFinalIPFamily []string - for _, f := range finalIPFamily { - stringFinalIPFamily = append(stringFinalIPFamily, string(f)) - } - desc := struct { - fromIPFamily string - toIPFamily string - fromTrafficPolicy string - toTrafficPolicy string - fromProtocol string - toProtocol string - }{ - strings.Join(stringInitialIPFamily, ","), - strings.Join(stringFinalIPFamily, ","), - string(initialTrafficPolicy), - string(finalTrafficPolicy), - string(initialProtocol), - string(finalProtocol), - } - - t.Run(fmt.Sprintf("+%v", desc), func(t *testing.T) { - t.Parallel() - - fakeGCE := getFakeGCECloud(vals) - - svc := test.NewL4ILBDualStackService(8080, initialProtocol, initialIPFamily, initialTrafficPolicy) - l4ilbParams := &L4ILBParams{ - Service: svc, - Cloud: fakeGCE, - Namer: namer, - Recorder: record.NewFakeRecorder(100), - DualStackEnabled: true, - } - l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecksl4.Fake(fakeGCE, l4ilbParams.Recorder) - - if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { - t.Errorf("Unexpected error when adding nodes %v", err) - } - - result := l4.EnsureInternalLoadBalancer(nodeNames, svc) - svc.Annotations = result.Annotations - assertDualStackILBResources(t, l4, nodeNames) - - finalSvc := test.NewL4ILBDualStackService(8080, finalProtocol, finalIPFamily, finalTrafficPolicy) - finalSvc.Annotations = svc.Annotations - l4.Service = finalSvc - - result = l4.EnsureInternalLoadBalancer(nodeNames, svc) - finalSvc.Annotations = result.Annotations - assertDualStackILBResources(t, l4, nodeNames) - - l4.EnsureInternalLoadBalancerDeleted(l4.Service) - assertDualStackILBResourcesDeleted(t, l4) - }) + runL4ILBDualStackTransitionTest( + t, + initialIPFamily, + finalIPFamily, + initialTrafficPolicy, + finalTrafficPolicy, + initialProtocol, + finalProtocol, + ) } } } @@ -1802,109 +1740,192 @@ func TestDualStackLoadBalancerTransitions(t *testing.T) { } } -func TestDualStackILBCleansOnlyAnnotationResources(t *testing.T) { +func runL4ILBDualStackTransitionTest(t *testing.T, initialIPFamily []v1.IPFamily, finalIPFamily []v1.IPFamily, initialTrafficPolicy v1.ServiceExternalTrafficPolicyType, finalTrafficPolicy v1.ServiceExternalTrafficPolicyType, initialProtocol v1.Protocol, finalProtocol v1.Protocol) { + desc := dualStackILBTransitionTestDesc( + initialIPFamily, finalIPFamily, initialTrafficPolicy, finalTrafficPolicy, initialProtocol, finalProtocol, + ) + t.Run(desc, func(t *testing.T) { + t.Parallel() + + nodeNames := []string{"test-node-1"} + vals := gce.DefaultTestClusterValues() + + namer := namer_util.NewL4Namer(kubeSystemUID, nil) + fakeGCE := getFakeGCECloud(vals) + + svc := test.NewL4ILBDualStackService(8080, initialProtocol, initialIPFamily, initialTrafficPolicy) + l4ilbParams := &L4ILBParams{ + Service: svc, + Cloud: fakeGCE, + Namer: namer, + Recorder: record.NewFakeRecorder(100), + DualStackEnabled: true, + } + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, l4ilbParams.Recorder) + + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { + t.Errorf("Unexpected error when adding nodes %v", err) + } + + result := l4.EnsureInternalLoadBalancer(nodeNames, svc) + svc.Annotations = result.Annotations + assertDualStackILBResources(t, l4, nodeNames) + + finalSvc := test.NewL4ILBDualStackService(8080, finalProtocol, finalIPFamily, finalTrafficPolicy) + finalSvc.Annotations = svc.Annotations + l4.Service = finalSvc + + result = l4.EnsureInternalLoadBalancer(nodeNames, svc) + finalSvc.Annotations = result.Annotations + assertDualStackILBResources(t, l4, nodeNames) + + l4.EnsureInternalLoadBalancerDeleted(l4.Service) + assertDualStackILBResourcesDeleted(t, l4) + }) +} + +func dualStackILBTransitionTestDesc(initialIPFamily []v1.IPFamily, finalIPFamily []v1.IPFamily, initialTrafficPolicy v1.ServiceExternalTrafficPolicyType, finalTrafficPolicy v1.ServiceExternalTrafficPolicyType, initialProtocol v1.Protocol, finalProtocol v1.Protocol) string { + var stringInitialIPFamily []string + for _, f := range initialIPFamily { + stringInitialIPFamily = append(stringInitialIPFamily, string(f)) + } + + var stringFinalIPFamily []string + for _, f := range finalIPFamily { + stringFinalIPFamily = append(stringFinalIPFamily, string(f)) + } + desc := struct { + fromIPFamily string + toIPFamily string + fromTrafficPolicy string + toTrafficPolicy string + fromProtocol string + toProtocol string + }{ + strings.Join(stringInitialIPFamily, ","), + strings.Join(stringFinalIPFamily, ","), + string(initialTrafficPolicy), + string(finalTrafficPolicy), + string(initialProtocol), + string(finalProtocol), + } + return fmt.Sprintf("%v", desc) +} + +func TestDualStackILBSyncIgnoresNoAnnotationIPv6Resources(t *testing.T) { t.Parallel() nodeNames := []string{"test-node-1"} vals := gce.DefaultTestClusterValues() - testCases := []struct { - desc string - ipFamiliesStates [2][]v1.IPFamily - annotationsToDelete []string - verifyResourcesNotDeleted func(l4 *L4) error - }{ - { - desc: "Should not delete IPv6 resources if they not exist in annotation", - ipFamiliesStates: [2][]v1.IPFamily{{v1.IPv4Protocol, v1.IPv6Protocol}, {v1.IPv4Protocol}}, - annotationsToDelete: []string{annotations.TCPForwardingRuleIPv6Key, annotations.FirewallRuleIPv6Key, annotations.FirewallRuleForHealthcheckIPv6Key}, - verifyResourcesNotDeleted: func(l4 *L4) error { - // Verify IPv6 Firewall was not deleted - ipv6FWName := l4.namer.L4IPv6Firewall(l4.Service.Namespace, l4.Service.Name) - err := verifyFirewallNotExists(l4.cloud, ipv6FWName) - if err == nil { - return fmt.Errorf("firewall rule %s was deleted, expected not", ipv6FWName) - } + namer := namer_util.NewL4Namer(kubeSystemUID, nil) + fakeGCE := getFakeGCECloud(vals) - // Verify IPv6 Forwarding Rule was not deleted - ipv6FRName := l4.getIPv6FRName() - err = verifyForwardingRuleNotExists(l4.cloud, ipv6FRName) - if err == nil { - return fmt.Errorf("forwarding rule %s was deleted, expected not", ipv6FRName) - } - return nil - }, - }, - { - desc: "Should not delete IPv4 resources if they not exist in annotation", - ipFamiliesStates: [2][]v1.IPFamily{{v1.IPv6Protocol, v1.IPv4Protocol}, {v1.IPv6Protocol}}, - annotationsToDelete: []string{annotations.TCPForwardingRuleKey, annotations.FirewallRuleKey, annotations.FirewallRuleForHealthcheckKey}, - verifyResourcesNotDeleted: func(l4 *L4) error { - // Verify IPv6 Firewall was not deleted - backendServiceName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) - err := verifyFirewallNotExists(l4.cloud, backendServiceName) - if err == nil { - return fmt.Errorf("firewall rule %s was deleted, expected not", backendServiceName) - } + svc := test.NewL4ILBService(false, 8080) + l4ilbParams := &L4ILBParams{ + Service: svc, + Cloud: fakeGCE, + Namer: namer, + Recorder: record.NewFakeRecorder(100), + DualStackEnabled: true, + } + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, l4ilbParams.Recorder) - // Verify IPv6 Forwarding Rule was not deleted - ipv4FRName := l4.GetFRName() - err = verifyForwardingRuleNotExists(l4.cloud, ipv4FRName) - if err == nil { - return fmt.Errorf("forwarding rule %s was deleted, expected not", ipv4FRName) - } - return nil - }, - }, + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { + t.Errorf("Unexpected error when adding nodes %v", err) } - for _, tc := range testCases { - tc := tc - t.Run(tc.desc, func(t *testing.T) { - t.Parallel() + svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol} + result := l4.EnsureInternalLoadBalancer(nodeNames, svc) + svc.Annotations = result.Annotations + assertDualStackILBResources(t, l4, nodeNames) - namer := namer_util.NewL4Namer(kubeSystemUID, nil) - fakeGCE := getFakeGCECloud(vals) + // Delete resources annotation + annotationsToDelete := []string{annotations.TCPForwardingRuleIPv6Key, annotations.FirewallRuleIPv6Key, annotations.FirewallRuleForHealthcheckIPv6Key} + for _, annotationToDelete := range annotationsToDelete { + delete(svc.Annotations, annotationToDelete) + } + svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv4Protocol} - svc := test.NewL4ILBService(false, 8080) - l4ilbParams := &L4ILBParams{ - Service: svc, - Cloud: fakeGCE, - Namer: namer, - Recorder: record.NewFakeRecorder(100), - DualStackEnabled: true, - } - l4 := NewL4Handler(l4ilbParams) - l4.healthChecks = healthchecksl4.Fake(fakeGCE, l4ilbParams.Recorder) + // Run new sync. Controller should not delete resources, if they don't exist in annotation + result = l4.EnsureInternalLoadBalancer(nodeNames, svc) + svc.Annotations = result.Annotations - if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { - t.Errorf("Unexpected error when adding nodes %v", err) - } + ipv6FWName := l4.namer.L4IPv6Firewall(l4.Service.Namespace, l4.Service.Name) + err := verifyFirewallNotExists(l4.cloud, ipv6FWName) + if err == nil { + t.Errorf("firewall rule %s was deleted, expected not", ipv6FWName) + } - svc.Spec.IPFamilies = tc.ipFamiliesStates[0] - result := l4.EnsureInternalLoadBalancer(nodeNames, svc) - svc.Annotations = result.Annotations - assertDualStackILBResources(t, l4, nodeNames) + // Verify IPv6 Forwarding Rule was not deleted + ipv6FRName := l4.getIPv6FRName() + err = verifyForwardingRuleNotExists(l4.cloud, ipv6FRName) + if err == nil { + t.Errorf("forwarding rule %s was deleted, expected not", ipv6FRName) + } - // Delete resources annotation - for _, annotationToDelete := range tc.annotationsToDelete { - delete(svc.Annotations, annotationToDelete) - } - svc.Spec.IPFamilies = tc.ipFamiliesStates[1] + l4.EnsureInternalLoadBalancerDeleted(l4.Service) + // After complete deletion, IPv6 and IPv4 resources should be cleaned up, even if the were leaked + assertDualStackILBResourcesDeleted(t, l4) +} - // Run new sync. Controller should not delete resources, if they don't exist in annotation - result = l4.EnsureInternalLoadBalancer(nodeNames, svc) - svc.Annotations = result.Annotations +func TestDualStackILBSyncIgnoresNoAnnotationIPv4Resources(t *testing.T) { + t.Parallel() + nodeNames := []string{"test-node-1"} + vals := gce.DefaultTestClusterValues() - err := tc.verifyResourcesNotDeleted(l4) - if err != nil { - t.Errorf("tc.verifyResourcesNotDeleted(_) returned error %v, want nil", err) - } + namer := namer_util.NewL4Namer(kubeSystemUID, nil) + fakeGCE := getFakeGCECloud(vals) - l4.EnsureInternalLoadBalancerDeleted(l4.Service) - // After complete deletion, IPv6 and IPv4 resources should be cleaned up, even if the were leaked - assertDualStackILBResourcesDeleted(t, l4) - }) + svc := test.NewL4ILBService(false, 8080) + l4ilbParams := &L4ILBParams{ + Service: svc, + Cloud: fakeGCE, + Namer: namer, + Recorder: record.NewFakeRecorder(100), + DualStackEnabled: true, + } + l4 := NewL4Handler(l4ilbParams) + l4.healthChecks = healthchecksl4.Fake(fakeGCE, l4ilbParams.Recorder) + + if _, err := test.CreateAndInsertNodes(l4.cloud, nodeNames, vals.ZoneName); err != nil { + t.Errorf("Unexpected error when adding nodes %v", err) } + + svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol} + result := l4.EnsureInternalLoadBalancer(nodeNames, svc) + svc.Annotations = result.Annotations + assertDualStackILBResources(t, l4, nodeNames) + + // Delete resources annotation + annotationsToDelete := []string{annotations.TCPForwardingRuleKey, annotations.FirewallRuleKey, annotations.FirewallRuleForHealthcheckKey} + for _, annotationToDelete := range annotationsToDelete { + delete(svc.Annotations, annotationToDelete) + } + svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv6Protocol} + + // Run new sync. Controller should not delete resources, if they don't exist in annotation + result = l4.EnsureInternalLoadBalancer(nodeNames, svc) + svc.Annotations = result.Annotations + + // Verify IPv6 Firewall was not deleted + backendServiceName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name) + err := verifyFirewallNotExists(l4.cloud, backendServiceName) + if err == nil { + t.Errorf("firewall rule %s was deleted, expected not", backendServiceName) + } + + // Verify IPv6 Forwarding Rule was not deleted + ipv4FRName := l4.GetFRName() + err = verifyForwardingRuleNotExists(l4.cloud, ipv4FRName) + if err == nil { + t.Errorf("forwarding rule %s was deleted, expected not", ipv4FRName) + } + + l4.EnsureInternalLoadBalancerDeleted(l4.Service) + // After complete deletion, IPv6 and IPv4 resources should be cleaned up, even if the were leaked + assertDualStackILBResourcesDeleted(t, l4) } func assertILBResources(t *testing.T, l4 *L4, nodeNames []string, resourceAnnotations map[string]string) { diff --git a/pkg/loadbalancers/l4netlb_test.go b/pkg/loadbalancers/l4netlb_test.go index 7d306cfe19..5a6b95f610 100644 --- a/pkg/loadbalancers/l4netlb_test.go +++ b/pkg/loadbalancers/l4netlb_test.go @@ -402,10 +402,6 @@ func TestEnsureExternalDualStackLoadBalancer(t *testing.T) { // for dual-stack service. In total 400 combinations func TestDualStackNetLBLoadBalancerTransitions(t *testing.T) { t.Parallel() - nodeNames := []string{"test-node-1"} - vals := gce.DefaultTestClusterValues() - - namer := namer_util.NewL4Namer(kubeSystemUID, nil) trafficPolicyStates := []v1.ServiceExternalTrafficPolicyType{v1.ServiceExternalTrafficPolicyTypeLocal, v1.ServiceExternalTrafficPolicyTypeCluster} protocols := []v1.Protocol{v1.ProtocolTCP, v1.ProtocolUDP} @@ -423,71 +419,15 @@ func TestDualStackNetLBLoadBalancerTransitions(t *testing.T) { for _, finalTrafficPolicy := range trafficPolicyStates { for _, initialProtocol := range protocols { for _, finalProtocol := range protocols { - initialIPFamily := initialIPFamily - finalIPFamily := finalIPFamily - initialTrafficPolicy := initialTrafficPolicy - finalTrafficPolicy := finalTrafficPolicy - initialProtocol := initialProtocol - finalProtocol := finalProtocol - - var stringInitialIPFamily []string - for _, f := range initialIPFamily { - stringInitialIPFamily = append(stringInitialIPFamily, string(f)) - } - - var stringFinalIPFamily []string - for _, f := range finalIPFamily { - stringFinalIPFamily = append(stringFinalIPFamily, string(f)) - } - desc := struct { - fromIPFamily string - toIPFamily string - fromTrafficPolicy string - toTrafficPolicy string - fromProtocol string - toProtocol string - }{ - strings.Join(stringInitialIPFamily, ","), - strings.Join(stringFinalIPFamily, ","), - string(initialTrafficPolicy), - string(finalTrafficPolicy), - string(initialProtocol), - string(finalProtocol), - } - - t.Run(fmt.Sprintf("+%v", desc), func(t *testing.T) { - t.Parallel() - - fakeGCE := getFakeGCECloud(vals) - - svc := test.NewL4NetLBRBSDualStackService(8080, initialProtocol, initialIPFamily, initialTrafficPolicy) - l4NetLBParams := &L4NetLBParams{ - Service: svc, - Cloud: fakeGCE, - Namer: namer, - Recorder: record.NewFakeRecorder(100), - DualStackEnabled: true, - } - l4NetLB := NewL4NetLB(l4NetLBParams) - l4NetLB.healthChecks = healthchecksl4.Fake(fakeGCE, l4NetLBParams.Recorder) - - if _, err := test.CreateAndInsertNodes(l4NetLB.cloud, nodeNames, vals.ZoneName); err != nil { - t.Errorf("Unexpected error when adding nodes %v", err) - } - - result := l4NetLB.EnsureFrontend(nodeNames, svc) - if result.Error != nil { - t.Errorf("Failed to ensure loadBalancer, err %v", result.Error) - } - if len(result.Status.Ingress) == 0 { - t.Errorf("Got empty loadBalancer status using handler %v", l4NetLB) - } - l4NetLB.Service.Annotations = result.Annotations - assertDualStackNetLBResources(t, l4NetLB, nodeNames) - - l4NetLB.EnsureLoadBalancerDeleted(l4NetLB.Service) - assertDualStackNetLBResourcesDeleted(t, l4NetLB) - }) + runL4NetLBDualStackTransitionTest( + t, + initialIPFamily, + finalIPFamily, + initialTrafficPolicy, + finalTrafficPolicy, + initialProtocol, + finalProtocol, + ) } } } @@ -496,109 +436,199 @@ func TestDualStackNetLBLoadBalancerTransitions(t *testing.T) { } } -func TestDualStackNetLBCleansOnlyAnnotationResources(t *testing.T) { +func runL4NetLBDualStackTransitionTest( + t *testing.T, + initialIPFamily []v1.IPFamily, + finalIPFamily []v1.IPFamily, + initialTrafficPolicy v1.ServiceExternalTrafficPolicyType, + finalTrafficPolicy v1.ServiceExternalTrafficPolicyType, + initialProtocol v1.Protocol, + finalProtocol v1.Protocol, +) { + desc := dualStackNetLBTransitionTestDesc( + initialIPFamily, finalIPFamily, initialTrafficPolicy, finalTrafficPolicy, initialProtocol, finalProtocol, + ) + + t.Run(desc, func(t *testing.T) { + t.Parallel() + + nodeNames := []string{"test-node-1"} + vals := gce.DefaultTestClusterValues() + namer := namer_util.NewL4Namer(kubeSystemUID, nil) + fakeGCE := getFakeGCECloud(vals) + + svc := test.NewL4NetLBRBSDualStackService(8080, initialProtocol, initialIPFamily, initialTrafficPolicy) + l4NetLBParams := &L4NetLBParams{ + Service: svc, + Cloud: fakeGCE, + Namer: namer, + Recorder: record.NewFakeRecorder(100), + DualStackEnabled: true, + } + l4NetLB := NewL4NetLB(l4NetLBParams) + l4NetLB.healthChecks = healthchecksl4.Fake(fakeGCE, l4NetLBParams.Recorder) + + if _, err := test.CreateAndInsertNodes(l4NetLB.cloud, nodeNames, vals.ZoneName); err != nil { + t.Errorf("Unexpected error when adding nodes %v", err) + } + + result := l4NetLB.EnsureFrontend(nodeNames, svc) + if result.Error != nil { + t.Errorf("Failed to ensure loadBalancer, err %v", result.Error) + } + if len(result.Status.Ingress) == 0 { + t.Errorf("Got empty loadBalancer status using handler %v", l4NetLB) + } + l4NetLB.Service.Annotations = result.Annotations + assertDualStackNetLBResources(t, l4NetLB, nodeNames) + + l4NetLB.EnsureLoadBalancerDeleted(l4NetLB.Service) + assertDualStackNetLBResourcesDeleted(t, l4NetLB) + }) +} + +func dualStackNetLBTransitionTestDesc(initialIPFamily []v1.IPFamily, finalIPFamily []v1.IPFamily, initialTrafficPolicy v1.ServiceExternalTrafficPolicyType, finalTrafficPolicy v1.ServiceExternalTrafficPolicyType, initialProtocol v1.Protocol, finalProtocol v1.Protocol) string { + var stringInitialIPFamily []string + for _, f := range initialIPFamily { + stringInitialIPFamily = append(stringInitialIPFamily, string(f)) + } + + var stringFinalIPFamily []string + for _, f := range finalIPFamily { + stringFinalIPFamily = append(stringFinalIPFamily, string(f)) + } + desc := struct { + fromIPFamily string + toIPFamily string + fromTrafficPolicy string + toTrafficPolicy string + fromProtocol string + toProtocol string + }{ + strings.Join(stringInitialIPFamily, ","), + strings.Join(stringFinalIPFamily, ","), + string(initialTrafficPolicy), + string(finalTrafficPolicy), + string(initialProtocol), + string(finalProtocol), + } + return fmt.Sprintf("%v", desc) +} + +func TestDualStackNetLBSyncIgnoresNoAnnotationIPv6Resources(t *testing.T) { t.Parallel() nodeNames := []string{"test-node-1"} vals := gce.DefaultTestClusterValues() - testCases := []struct { - desc string - ipFamiliesStates [2][]v1.IPFamily - annotationsToDelete []string - verifyResourcesNotDeleted func(l4netlb *L4NetLB) error - }{ - { - desc: "Should not delete IPv6 resources if they not exist in annotation", - ipFamiliesStates: [2][]v1.IPFamily{{v1.IPv4Protocol, v1.IPv6Protocol}, {v1.IPv4Protocol}}, - annotationsToDelete: []string{annotations.TCPForwardingRuleIPv6Key, annotations.FirewallRuleIPv6Key, annotations.FirewallRuleForHealthcheckIPv6Key}, - verifyResourcesNotDeleted: func(l4netlb *L4NetLB) error { - // Verify IPv6 Firewall was not deleted - ipv6FWName := l4netlb.namer.L4IPv6Firewall(l4netlb.Service.Namespace, l4netlb.Service.Name) - err := verifyFirewallNotExists(l4netlb.cloud, ipv6FWName) - if err == nil { - return fmt.Errorf("firewall rule %s was deleted, expected not", ipv6FWName) - } + namer := namer_util.NewL4Namer(kubeSystemUID, nil) + fakeGCE := getFakeGCECloud(vals) - // Verify IPv6 Forwarding Rule was not deleted - ipv6FRName := l4netlb.ipv6FRName() - err = verifyForwardingRuleNotExists(l4netlb.cloud, ipv6FRName) - if err == nil { - return fmt.Errorf("forwarding rule %s was deleted, expected not", ipv6FRName) - } - return nil - }, - }, - { - desc: "Should not delete IPv4 resources if they not exist in annotation", - ipFamiliesStates: [2][]v1.IPFamily{{v1.IPv6Protocol, v1.IPv4Protocol}, {v1.IPv6Protocol}}, - annotationsToDelete: []string{annotations.TCPForwardingRuleKey, annotations.FirewallRuleKey, annotations.FirewallRuleForHealthcheckKey}, - verifyResourcesNotDeleted: func(l4netlb *L4NetLB) error { - // Verify IPv6 Firewall was not deleted - backendServiceName := l4netlb.namer.L4Backend(l4netlb.Service.Namespace, l4netlb.Service.Name) - err := verifyFirewallNotExists(l4netlb.cloud, backendServiceName) - if err == nil { - return fmt.Errorf("firewall rule %s was deleted, expected not", backendServiceName) - } + svc := test.NewL4NetLBRBSService(8080) + l4NetLBParams := &L4NetLBParams{ + Service: svc, + Cloud: fakeGCE, + Namer: namer, + Recorder: record.NewFakeRecorder(100), + DualStackEnabled: true, + } + l4NetLB := NewL4NetLB(l4NetLBParams) + l4NetLB.healthChecks = healthchecksl4.Fake(fakeGCE, l4NetLBParams.Recorder) - // Verify IPv6 Forwarding Rule was not deleted - ipv4FRName := l4netlb.frName() - err = verifyForwardingRuleNotExists(l4netlb.cloud, ipv4FRName) - if err == nil { - return fmt.Errorf("forwarding rule %s was deleted, expected not", ipv4FRName) - } - return nil - }, - }, + if _, err := test.CreateAndInsertNodes(l4NetLB.cloud, nodeNames, vals.ZoneName); err != nil { + t.Errorf("Unexpected error when adding nodes %v", err) } - for _, tc := range testCases { - tc := tc - t.Run(tc.desc, func(t *testing.T) { - t.Parallel() + svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv4Protocol, v1.IPv6Protocol} + result := l4NetLB.EnsureFrontend(nodeNames, svc) + svc.Annotations = result.Annotations + assertDualStackNetLBResources(t, l4NetLB, nodeNames) - namer := namer_util.NewL4Namer(kubeSystemUID, nil) - fakeGCE := getFakeGCECloud(vals) + // Delete IPv4 resources annotation + annotationsToDelete := []string{annotations.TCPForwardingRuleIPv6Key, annotations.FirewallRuleIPv6Key, annotations.FirewallRuleForHealthcheckIPv6Key} + for _, annotationToDelete := range annotationsToDelete { + delete(svc.Annotations, annotationToDelete) + } + svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv6Protocol} - svc := test.NewL4NetLBRBSService(8080) - l4NetLBParams := &L4NetLBParams{ - Service: svc, - Cloud: fakeGCE, - Namer: namer, - Recorder: record.NewFakeRecorder(100), - DualStackEnabled: true, - } - l4NetLB := NewL4NetLB(l4NetLBParams) - l4NetLB.healthChecks = healthchecksl4.Fake(fakeGCE, l4NetLBParams.Recorder) + // Run new sync. Controller should not delete resources, if they don't exist in annotation + result = l4NetLB.EnsureFrontend(nodeNames, svc) + svc.Annotations = result.Annotations - if _, err := test.CreateAndInsertNodes(l4NetLB.cloud, nodeNames, vals.ZoneName); err != nil { - t.Errorf("Unexpected error when adding nodes %v", err) - } + // Verify IPv4 Firewall was not deleted + ipv6FWName := l4NetLB.namer.L4IPv6Firewall(l4NetLB.Service.Namespace, l4NetLB.Service.Name) + err := verifyFirewallNotExists(l4NetLB.cloud, ipv6FWName) + if err == nil { + t.Errorf("firewall rule %s was deleted, expected not", ipv6FWName) + } - svc.Spec.IPFamilies = tc.ipFamiliesStates[0] - result := l4NetLB.EnsureFrontend(nodeNames, svc) - svc.Annotations = result.Annotations - assertDualStackNetLBResources(t, l4NetLB, nodeNames) + // Verify IPv6 Forwarding Rule was not deleted + ipv6FRName := l4NetLB.ipv6FRName() + err = verifyForwardingRuleNotExists(l4NetLB.cloud, ipv6FRName) + if err == nil { + t.Errorf("forwarding rule %s was deleted, expected not", ipv6FRName) + } - // Delete resources annotation - for _, annotationToDelete := range tc.annotationsToDelete { - delete(svc.Annotations, annotationToDelete) - } - svc.Spec.IPFamilies = tc.ipFamiliesStates[1] + l4NetLB.EnsureLoadBalancerDeleted(l4NetLB.Service) + // After complete deletion, IPv6 and IPv4 resources should be cleaned up, even if the were leaked + assertDualStackNetLBResourcesDeleted(t, l4NetLB) +} - // Run new sync. Controller should not delete resources, if they don't exist in annotation - result = l4NetLB.EnsureFrontend(nodeNames, svc) - svc.Annotations = result.Annotations +func TestDualStackNetLBSyncIgnoresNoAnnotationIPv4Resources(t *testing.T) { + t.Parallel() + nodeNames := []string{"test-node-1"} + vals := gce.DefaultTestClusterValues() - err := tc.verifyResourcesNotDeleted(l4NetLB) - if err != nil { - t.Errorf("tc.verifyResourcesNotDeleted(_) returned error %v, want nil", err) - } + namer := namer_util.NewL4Namer(kubeSystemUID, nil) + fakeGCE := getFakeGCECloud(vals) - l4NetLB.EnsureLoadBalancerDeleted(l4NetLB.Service) - // After complete deletion, IPv6 and IPv4 resources should be cleaned up, even if the were leaked - assertDualStackNetLBResourcesDeleted(t, l4NetLB) - }) + svc := test.NewL4NetLBRBSService(8080) + l4NetLBParams := &L4NetLBParams{ + Service: svc, + Cloud: fakeGCE, + Namer: namer, + Recorder: record.NewFakeRecorder(100), + DualStackEnabled: true, + } + l4NetLB := NewL4NetLB(l4NetLBParams) + l4NetLB.healthChecks = healthchecksl4.Fake(fakeGCE, l4NetLBParams.Recorder) + + if _, err := test.CreateAndInsertNodes(l4NetLB.cloud, nodeNames, vals.ZoneName); err != nil { + t.Errorf("Unexpected error when adding nodes %v", err) + } + + svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv6Protocol, v1.IPv4Protocol} + result := l4NetLB.EnsureFrontend(nodeNames, svc) + svc.Annotations = result.Annotations + assertDualStackNetLBResources(t, l4NetLB, nodeNames) + + // Delete IPv4 resources annotation + annotationsToDelete := []string{annotations.TCPForwardingRuleKey, annotations.FirewallRuleKey, annotations.FirewallRuleForHealthcheckKey} + for _, annotationToDelete := range annotationsToDelete { + delete(svc.Annotations, annotationToDelete) } + svc.Spec.IPFamilies = []v1.IPFamily{v1.IPv6Protocol} + + // Run new sync. Controller should not delete resources, if they don't exist in annotation + result = l4NetLB.EnsureFrontend(nodeNames, svc) + svc.Annotations = result.Annotations + + // Verify IPv4 Firewall was not deleted + fwName := l4NetLB.namer.L4Backend(l4NetLB.Service.Namespace, l4NetLB.Service.Name) + err := verifyFirewallNotExists(l4NetLB.cloud, fwName) + if err == nil { + t.Errorf("firewall rule %s was deleted, expected not", fwName) + } + + // Verify IPv4 Forwarding Rule was not deleted + ipv4FRName := l4NetLB.frName() + err = verifyForwardingRuleNotExists(l4NetLB.cloud, ipv4FRName) + if err == nil { + t.Errorf("forwarding rule %s was deleted, expected not", ipv4FRName) + } + + l4NetLB.EnsureLoadBalancerDeleted(l4NetLB.Service) + // After complete deletion, IPv6 and IPv4 resources should be cleaned up, even if the were leaked + assertDualStackNetLBResourcesDeleted(t, l4NetLB) } func ensureLoadBalancer(port int, vals gce.TestClusterValues, fakeGCE *gce.Cloud, t *testing.T) (*v1.Service, *L4NetLB) {