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

Small refactor of l4controller_test assertions #1817

Merged
merged 1 commit into from
Sep 16, 2022
Merged
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
86 changes: 52 additions & 34 deletions pkg/l4lb/l4controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,16 @@ const (
testGCEZone = "us-central1-b"
)

var (
ilbAnnotationKeys = []string{
annotations.FirewallRuleKey,
annotations.BackendServiceKey,
annotations.HealthcheckKey,
annotations.TCPForwardingRuleKey,
annotations.FirewallRuleForHealthcheckKey,
}
)

func newServiceController(t *testing.T, fakeGCE *gce.Cloud) *L4Controller {
kubeClient := fake.NewSimpleClientset()

Expand Down Expand Up @@ -119,38 +129,46 @@ func getKeyForSvc(svc *api_v1.Service, t *testing.T) string {
return key
}

func validateSvcStatus(svc *api_v1.Service, expectStatus bool, t *testing.T) {
if common.HasGivenFinalizer(svc.ObjectMeta, common.ILBFinalizerV2) != expectStatus {
t.Fatalf("Expected L4 finalizer present to be %v, but it was %v", expectStatus, !expectStatus)
func verifyILBServiceProvisioned(t *testing.T, svc *api_v1.Service) {
t.Helper()

if !common.HasGivenFinalizer(svc.ObjectMeta, common.ILBFinalizerV2) {
t.Errorf("ILB v2 finalizer is not found, expected to exist, svc %+v", svc)
}
if len(svc.Status.LoadBalancer.Ingress) == 0 || svc.Status.LoadBalancer.Ingress[0].IP == "" {
if expectStatus {
t.Fatalf("Invalid LoadBalancer status field in service - %+v", svc.Status.LoadBalancer)
t.Errorf("Invalid LoadBalancer status field in service - %+v", svc.Status.LoadBalancer)
}

var missingKeys []string
for _, key := range ilbAnnotationKeys {
if _, ok := svc.Annotations[key]; !ok {
missingKeys = append(missingKeys, key)
}
}
if len(svc.Status.LoadBalancer.Ingress) > 0 && !expectStatus {
t.Fatalf("Expected LoadBalancer status to be empty, Got %v", svc.Status.LoadBalancer)
if len(missingKeys) > 0 {
t.Errorf("Cannot find annotations %v in ILB service, Got %v", missingKeys, svc.Annotations)
}
}

expectedAnnotationKeys := []string{annotations.FirewallRuleKey, annotations.BackendServiceKey, annotations.HealthcheckKey,
annotations.TCPForwardingRuleKey, annotations.FirewallRuleForHealthcheckKey}
func verifyILBServiceNotProvisioned(t *testing.T, svc *api_v1.Service) {
t.Helper()

missingKeys := []string{}
for _, key := range expectedAnnotationKeys {
if common.HasGivenFinalizer(svc.ObjectMeta, common.ILBFinalizerV2) {
t.Errorf("ILB v2 finalizer should not exist on service %+v", svc)
}

if len(svc.Status.LoadBalancer.Ingress) > 0 {
t.Errorf("Expected LoadBalancer status to be empty, Got %v", svc.Status.LoadBalancer)
}

var missingKeys []string
for _, key := range ilbAnnotationKeys {
if _, ok := svc.Annotations[key]; !ok {
missingKeys = append(missingKeys, key)
}
}
if expectStatus {
// All annotations are expected to be present in this case
if len(missingKeys) > 0 {
t.Fatalf("Cannot find annotations %v in ILB service, Got %v", missingKeys, svc.Annotations)
}
} else {
//None of the ILB keys should be present since the ILB has been deleted.
if len(missingKeys) != len(expectedAnnotationKeys) {
t.Fatalf("Unexpected ILB annotations still present, Got %v", svc.Annotations)
}
if len(missingKeys) != len(ilbAnnotationKeys) {
t.Errorf("Unexpected ILB annotations present, Got %v", svc.Annotations)
}
}

Expand Down Expand Up @@ -198,7 +216,7 @@ func TestProcessCreateOrUpdate(t *testing.T) {
if err != nil {
t.Errorf("Failed to lookup service %s, err: %v", newSvc.Name, err)
}
validateSvcStatus(newSvc, true, t)
verifyILBServiceProvisioned(t, newSvc)
currMetrics, metricErr := test.GetL4ILBLatencyMetric()
if metricErr != nil {
t.Errorf("Error getting L4 ILB latency metrics err: %v", metricErr)
Expand All @@ -217,7 +235,7 @@ func TestProcessCreateOrUpdate(t *testing.T) {
if err != nil {
t.Errorf("Failed to lookup service %s, err: %v", newSvc.Name, err)
}
validateSvcStatus(newSvc, true, t)
verifyILBServiceProvisioned(t, newSvc)
currMetrics, metricErr = test.GetL4ILBLatencyMetric()
if metricErr != nil {
t.Errorf("Error getting L4 ILB latency metrics err: %v", metricErr)
Expand All @@ -236,7 +254,7 @@ func TestProcessCreateOrUpdate(t *testing.T) {
if err != nil {
t.Errorf("Failed to lookup service %s, err: %v", newSvc.Name, err)
}
validateSvcStatus(newSvc, false, t)
verifyILBServiceNotProvisioned(t, newSvc)
currMetrics, metricErr = test.GetL4ILBLatencyMetric()
if metricErr != nil {
t.Errorf("Error getting L4 ILB latency metrics err: %v", metricErr)
Expand Down Expand Up @@ -275,7 +293,7 @@ func TestProcessUpdateExternalTrafficPolicy(t *testing.T) {
if err != nil {
t.Errorf("Failed to lookup service %s, err: %v", svc.Name, err)
}
validateSvcStatus(svc, true, t)
verifyILBServiceProvisioned(t, svc)

// Set ExternalTrafficPolicy to Cluster.
svc.Spec.ExternalTrafficPolicy = api_v1.ServiceExternalTrafficPolicyTypeCluster
Expand All @@ -289,7 +307,7 @@ func TestProcessUpdateExternalTrafficPolicy(t *testing.T) {
if err != nil {
t.Errorf("Failed to lookup service %s, err: %v", svc.Name, err)
}
validateSvcStatus(svc, true, t)
verifyILBServiceProvisioned(t, svc)
// Verify that both health checks were created.
for _, isShared := range []bool{true, false} {
hcName := l4c.namer.L4HealthCheck(svc.Namespace, svc.Name, isShared)
Expand Down Expand Up @@ -331,7 +349,7 @@ func TestProcessDeletion(t *testing.T) {
if err != nil {
t.Errorf("Failed to lookup service %s, err: %v", newSvc.Name, err)
}
validateSvcStatus(newSvc, true, t)
verifyILBServiceProvisioned(t, newSvc)
currMetrics, metricErr := test.GetL4ILBLatencyMetric()
if metricErr != nil {
t.Errorf("Error getting L4 ILB latency metrics err: %v", metricErr)
Expand All @@ -354,7 +372,7 @@ func TestProcessDeletion(t *testing.T) {
if err != nil {
t.Errorf("Failed to lookup service %s, err: %v", newSvc.Name, err)
}
validateSvcStatus(newSvc, false, t)
verifyILBServiceNotProvisioned(t, newSvc)
currMetrics, metricErr = test.GetL4ILBLatencyMetric()
if metricErr != nil {
t.Errorf("Error getting L4 ILB latency metrics err: %v", metricErr)
Expand Down Expand Up @@ -386,7 +404,7 @@ func TestProcessCreateLegacyService(t *testing.T) {
if err != nil {
t.Errorf("Failed to lookup service %s, err: %v", newSvc.Name, err)
}
validateSvcStatus(svc, false, t)
verifyILBServiceNotProvisioned(t, svc)
currMetrics, metricErr := test.GetL4ILBLatencyMetric()
if metricErr != nil {
t.Errorf("Error getting L4 ILB latency metrics err: %v", metricErr)
Expand Down Expand Up @@ -418,7 +436,7 @@ func TestProcessCreateServiceWithLegacyInternalForwardingRule(t *testing.T) {
if err != nil {
t.Errorf("Failed to lookup service %s, err: %v", newSvc.Name, err)
}
validateSvcStatus(svc, false, t)
verifyILBServiceNotProvisioned(t, svc)
currMetrics, metricErr := test.GetL4ILBLatencyMetric()
if metricErr != nil {
t.Errorf("Error getting L4 ILB latency metrics err: %v", metricErr)
Expand Down Expand Up @@ -449,7 +467,7 @@ func TestProcessCreateServiceWithLegacyExternalForwardingRule(t *testing.T) {
if err != nil {
t.Errorf("Failed to lookup service %s, err: %v", newSvc.Name, err)
}
validateSvcStatus(svc, true, t)
verifyILBServiceProvisioned(t, svc)
currMetrics, metricErr := test.GetL4ILBLatencyMetric()
if metricErr != nil {
t.Errorf("Error getting L4 ILB latency metrics %v", metricErr)
Expand Down Expand Up @@ -495,7 +513,7 @@ func TestProcessUpdateClusterIPToILBService(t *testing.T) {
if err != nil {
t.Errorf("Failed to lookup service %s, err: %v", newSvc.Name, err)
}
validateSvcStatus(newSvc, true, t)
verifyILBServiceProvisioned(t, newSvc)
// this will be a create metric since an ILB IP is being assigned for the first time.
currMetrics, metricErr := test.GetL4ILBLatencyMetric()
if metricErr != nil {
Expand Down Expand Up @@ -545,7 +563,7 @@ func TestProcessMultipleServices(t *testing.T) {
// Perform a full validation of the service once it is ready.
for _, name := range svcNames {
newSvc, _ := l4c.client.CoreV1().Services(testNs).Get(context2.TODO(), name, v1.GetOptions{})
validateSvcStatus(newSvc, true, t)
verifyILBServiceProvisioned(t, newSvc)
}
// this will be a create metric since an ILB IP is being assigned for the first time.
currMetrics, metricErr := test.GetL4ILBLatencyMetric()
Expand Down Expand Up @@ -596,7 +614,7 @@ func TestProcessServiceWithDelayedNEGAdd(t *testing.T) {
}); err != nil {
t.Error(err)
}
validateSvcStatus(newSvc, true, t)
verifyILBServiceProvisioned(t, newSvc)
}

func TestProcessServiceOnError(t *testing.T) {
Expand Down