From 04b77f02ee5a2a37eeb959da772dab5f306d1fba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Tyczy=C5=84ski?= Date: Mon, 2 May 2022 20:28:09 +0200 Subject: [PATCH] Minor cleanup to use t.Run() in test/integration --- test/integration/auth/bootstraptoken_test.go | 105 ++++----- test/integration/auth/rbac_test.go | 214 +++++++++--------- .../integration/replicaset/replicaset_test.go | 4 +- .../replicationcontroller_test.go | 4 +- .../statefulset/statefulset_test.go | 90 ++++---- 5 files changed, 211 insertions(+), 206 deletions(-) diff --git a/test/integration/auth/bootstraptoken_test.go b/test/integration/auth/bootstraptoken_test.go index 7aa19a56aa0..fba32c618e5 100644 --- a/test/integration/auth/bootstraptoken_test.go +++ b/test/integration/auth/bootstraptoken_test.go @@ -115,67 +115,68 @@ func TestBootstrapTokenAuth(t *testing.T) { }, } for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + authenticator := group.NewAuthenticatedGroupAdder(bearertoken.New(bootstrap.NewTokenAuthenticator(bootstrapSecrets{test.secret}))) + // Set up an API server + controlPlaneConfig := framework.NewIntegrationTestControlPlaneConfig() + controlPlaneConfig.GenericConfig.Authentication.Authenticator = authenticator + _, s, closeFn := framework.RunAnAPIServer(controlPlaneConfig) + defer closeFn() - authenticator := group.NewAuthenticatedGroupAdder(bearertoken.New(bootstrap.NewTokenAuthenticator(bootstrapSecrets{test.secret}))) - // Set up an API server - controlPlaneConfig := framework.NewIntegrationTestControlPlaneConfig() - controlPlaneConfig.GenericConfig.Authentication.Authenticator = authenticator - _, s, closeFn := framework.RunAnAPIServer(controlPlaneConfig) - defer closeFn() + ns := framework.CreateTestingNamespace("auth-bootstrap-token", s, t) + defer framework.DeleteTestingNamespace(ns, s, t) - ns := framework.CreateTestingNamespace("auth-bootstrap-token", s, t) - defer framework.DeleteTestingNamespace(ns, s, t) + previousResourceVersion := make(map[string]float64) + transport := http.DefaultTransport - previousResourceVersion := make(map[string]float64) - transport := http.DefaultTransport - - token := validTokenID + "." + validSecret - var bodyStr string - if test.request.body != "" { - sub := "" - if test.request.verb == "PUT" { - // For update operations, insert previous resource version - if resVersion := previousResourceVersion[getPreviousResourceVersionKey(test.request.URL, "")]; resVersion != 0 { - sub += fmt.Sprintf(",\r\n\"resourceVersion\": \"%v\"", resVersion) + token := validTokenID + "." + validSecret + var bodyStr string + if test.request.body != "" { + sub := "" + if test.request.verb == "PUT" { + // For update operations, insert previous resource version + if resVersion := previousResourceVersion[getPreviousResourceVersionKey(test.request.URL, "")]; resVersion != 0 { + sub += fmt.Sprintf(",\r\n\"resourceVersion\": \"%v\"", resVersion) + } + sub += fmt.Sprintf(",\r\n\"namespace\": %q", ns.Name) } - sub += fmt.Sprintf(",\r\n\"namespace\": %q", ns.Name) + bodyStr = fmt.Sprintf(test.request.body, sub) } - bodyStr = fmt.Sprintf(test.request.body, sub) - } - test.request.body = bodyStr - bodyBytes := bytes.NewReader([]byte(bodyStr)) - req, err := http.NewRequest(test.request.verb, s.URL+test.request.URL, bodyBytes) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) - if test.request.verb == "PATCH" { - req.Header.Set("Content-Type", "application/merge-patch+json") - } - - func() { - resp, err := transport.RoundTrip(req) + test.request.body = bodyStr + bodyBytes := bytes.NewReader([]byte(bodyStr)) + req, err := http.NewRequest(test.request.verb, s.URL+test.request.URL, bodyBytes) if err != nil { - t.Logf("case %v", test.name) t.Fatalf("unexpected error: %v", err) } - defer resp.Body.Close() - b, _ := io.ReadAll(resp.Body) - if _, ok := test.request.statusCodes[resp.StatusCode]; !ok { - t.Logf("case %v", test.name) - t.Errorf("Expected status one of %v, but got %v", test.request.statusCodes, resp.StatusCode) - t.Errorf("Body: %v", string(b)) - } else { - if test.request.verb == "POST" { - // For successful create operations, extract resourceVersion - id, currentResourceVersion, err := parseResourceVersion(b) - if err == nil { - key := getPreviousResourceVersionKey(test.request.URL, id) - previousResourceVersion[key] = currentResourceVersion - } - } + req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) + if test.request.verb == "PATCH" { + req.Header.Set("Content-Type", "application/merge-patch+json") } - }() + func() { + resp, err := transport.RoundTrip(req) + if err != nil { + t.Logf("case %v", test.name) + t.Fatalf("unexpected error: %v", err) + } + defer resp.Body.Close() + b, _ := io.ReadAll(resp.Body) + if _, ok := test.request.statusCodes[resp.StatusCode]; !ok { + t.Logf("case %v", test.name) + t.Errorf("Expected status one of %v, but got %v", test.request.statusCodes, resp.StatusCode) + t.Errorf("Body: %v", string(b)) + } else { + if test.request.verb == "POST" { + // For successful create operations, extract resourceVersion + id, currentResourceVersion, err := parseResourceVersion(b) + if err == nil { + key := getPreviousResourceVersionKey(test.request.URL, id) + previousResourceVersion[key] = currentResourceVersion + } + } + } + + }() + }) } } diff --git a/test/integration/auth/rbac_test.go b/test/integration/auth/rbac_test.go index 781324fa610..0cda9b5e149 100644 --- a/test/integration/auth/rbac_test.go +++ b/test/integration/auth/rbac_test.go @@ -518,124 +518,126 @@ func TestRBAC(t *testing.T) { } for i, tc := range tests { - // Create an API Server. - controlPlaneConfig := framework.NewIntegrationTestControlPlaneConfig() - controlPlaneConfig.GenericConfig.Authorization.Authorizer = newRBACAuthorizer(t, controlPlaneConfig) - controlPlaneConfig.GenericConfig.Authentication.Authenticator = group.NewAuthenticatedGroupAdder(bearertoken.New(tokenfile.New(map[string]*user.DefaultInfo{ - superUser: {Name: "admin", Groups: []string{"system:masters"}}, - "any-rolebinding-writer": {Name: "any-rolebinding-writer"}, - "any-rolebinding-writer-namespace": {Name: "any-rolebinding-writer-namespace"}, - "bob": {Name: "bob"}, - "job-writer": {Name: "job-writer"}, - "job-writer-namespace": {Name: "job-writer-namespace"}, - "nonescalating-rolebinding-writer": {Name: "nonescalating-rolebinding-writer"}, - "pod-reader": {Name: "pod-reader"}, - "limitrange-updater": {Name: "limitrange-updater"}, - "limitrange-patcher": {Name: "limitrange-patcher"}, - "user-with-no-permissions": {Name: "user-with-no-permissions"}, - }))) - controlPlaneConfig.GenericConfig.OpenAPIConfig = framework.DefaultOpenAPIConfig() - _, s, closeFn := framework.RunAnAPIServer(controlPlaneConfig) - defer closeFn() + t.Run(fmt.Sprintf("case-%d", i), func(t *testing.T) { + // Create an API Server. + controlPlaneConfig := framework.NewIntegrationTestControlPlaneConfig() + controlPlaneConfig.GenericConfig.Authorization.Authorizer = newRBACAuthorizer(t, controlPlaneConfig) + controlPlaneConfig.GenericConfig.Authentication.Authenticator = group.NewAuthenticatedGroupAdder(bearertoken.New(tokenfile.New(map[string]*user.DefaultInfo{ + superUser: {Name: "admin", Groups: []string{"system:masters"}}, + "any-rolebinding-writer": {Name: "any-rolebinding-writer"}, + "any-rolebinding-writer-namespace": {Name: "any-rolebinding-writer-namespace"}, + "bob": {Name: "bob"}, + "job-writer": {Name: "job-writer"}, + "job-writer-namespace": {Name: "job-writer-namespace"}, + "nonescalating-rolebinding-writer": {Name: "nonescalating-rolebinding-writer"}, + "pod-reader": {Name: "pod-reader"}, + "limitrange-updater": {Name: "limitrange-updater"}, + "limitrange-patcher": {Name: "limitrange-patcher"}, + "user-with-no-permissions": {Name: "user-with-no-permissions"}, + }))) + controlPlaneConfig.GenericConfig.OpenAPIConfig = framework.DefaultOpenAPIConfig() + _, s, closeFn := framework.RunAnAPIServer(controlPlaneConfig) + defer closeFn() - clientConfig := &restclient.Config{Host: s.URL, ContentConfig: restclient.ContentConfig{NegotiatedSerializer: legacyscheme.Codecs}} + clientConfig := &restclient.Config{Host: s.URL, ContentConfig: restclient.ContentConfig{NegotiatedSerializer: legacyscheme.Codecs}} - // Bootstrap the API Server with the test case's initial roles. - superuserClient, _ := clientsetForToken(superUser, clientConfig) - if err := tc.bootstrapRoles.bootstrap(superuserClient); err != nil { - t.Errorf("case %d: failed to apply initial roles: %v", i, err) - continue - } - previousResourceVersion := make(map[string]float64) + // Bootstrap the API Server with the test case's initial roles. + superuserClient, _ := clientsetForToken(superUser, clientConfig) + if err := tc.bootstrapRoles.bootstrap(superuserClient); err != nil { + t.Errorf("case %d: failed to apply initial roles: %v", i, err) + return + } + previousResourceVersion := make(map[string]float64) - for j, r := range tc.requests { - path := "/" - if r.apiGroup == "" { - path = gopath.Join(path, "api/v1") - } else { - path = gopath.Join(path, "apis", r.apiGroup, "v1") - } - if r.namespace != "" { - path = gopath.Join(path, "namespaces", r.namespace) - } - if r.resource != "" { - path = gopath.Join(path, r.resource) - } - if r.name != "" { - path = gopath.Join(path, r.name) - } + for j, r := range tc.requests { + path := "/" + if r.apiGroup == "" { + path = gopath.Join(path, "api/v1") + } else { + path = gopath.Join(path, "apis", r.apiGroup, "v1") + } + if r.namespace != "" { + path = gopath.Join(path, "namespaces", r.namespace) + } + if r.resource != "" { + path = gopath.Join(path, r.resource) + } + if r.name != "" { + path = gopath.Join(path, r.name) + } - var body io.Reader - if r.body != "" { - sub := "" - if r.verb == "PUT" { - // For update operations, insert previous resource version - if resVersion := previousResourceVersion[getPreviousResourceVersionKey(path, "")]; resVersion != 0 { - sub += fmt.Sprintf(",\"resourceVersion\": \"%v\"", resVersion) + var body io.Reader + if r.body != "" { + sub := "" + if r.verb == "PUT" { + // For update operations, insert previous resource version + if resVersion := previousResourceVersion[getPreviousResourceVersionKey(path, "")]; resVersion != 0 { + sub += fmt.Sprintf(",\"resourceVersion\": \"%v\"", resVersion) + } } + body = strings.NewReader(fmt.Sprintf(r.body, sub)) } - body = strings.NewReader(fmt.Sprintf(r.body, sub)) - } - req, err := http.NewRequest(r.verb, s.URL+path, body) - if r.verb == "PATCH" { - // For patch operations, use the apply content type - req.Header.Add("Content-Type", string(types.ApplyPatchType)) - q := req.URL.Query() - q.Add("fieldManager", "rbac_test") - req.URL.RawQuery = q.Encode() - } + req, err := http.NewRequest(r.verb, s.URL+path, body) + if r.verb == "PATCH" { + // For patch operations, use the apply content type + req.Header.Add("Content-Type", string(types.ApplyPatchType)) + q := req.URL.Query() + q.Add("fieldManager", "rbac_test") + req.URL.RawQuery = q.Encode() + } - if err != nil { - t.Fatalf("failed to create request: %v", err) - } - - func() { - reqDump, err := httputil.DumpRequest(req, true) if err != nil { - t.Fatalf("failed to dump request: %v", err) - return + t.Fatalf("failed to create request: %v", err) } - resp, err := clientForToken(r.token).Do(req) - if err != nil { - t.Errorf("case %d, req %d: failed to make request: %v", i, j, err) - return - } - defer resp.Body.Close() - - respDump, err := httputil.DumpResponse(resp, true) - if err != nil { - t.Fatalf("failed to dump response: %v", err) - return - } - - if resp.StatusCode != r.expectedStatus { - // When debugging is on, dump the entire request and response. Very helpful for - // debugging malformed test cases. - // - // To turn on debugging, use the '-args' flag. - // - // go test -v -tags integration -run RBAC -args -v 10 - // - klog.V(8).Infof("case %d, req %d: %s\n%s\n", i, j, reqDump, respDump) - t.Errorf("case %d, req %d: %s expected %q got %q", i, j, r, statusCode(r.expectedStatus), statusCode(resp.StatusCode)) - } - - b, _ := io.ReadAll(resp.Body) - - if r.verb == "POST" && (resp.StatusCode/100) == 2 { - // For successful create operations, extract resourceVersion - id, currentResourceVersion, err := parseResourceVersion(b) - if err == nil { - key := getPreviousResourceVersionKey(path, id) - previousResourceVersion[key] = currentResourceVersion - } else { - t.Logf("error in trying to extract resource version: %s", err) + func() { + reqDump, err := httputil.DumpRequest(req, true) + if err != nil { + t.Fatalf("failed to dump request: %v", err) + return } - } - }() - } + + resp, err := clientForToken(r.token).Do(req) + if err != nil { + t.Errorf("case %d, req %d: failed to make request: %v", i, j, err) + return + } + defer resp.Body.Close() + + respDump, err := httputil.DumpResponse(resp, true) + if err != nil { + t.Fatalf("failed to dump response: %v", err) + return + } + + if resp.StatusCode != r.expectedStatus { + // When debugging is on, dump the entire request and response. Very helpful for + // debugging malformed test cases. + // + // To turn on debugging, use the '-args' flag. + // + // go test -v -tags integration -run RBAC -args -v 10 + // + klog.V(8).Infof("case %d, req %d: %s\n%s\n", i, j, reqDump, respDump) + t.Errorf("case %d, req %d: %s expected %q got %q", i, j, r, statusCode(r.expectedStatus), statusCode(resp.StatusCode)) + } + + b, _ := io.ReadAll(resp.Body) + + if r.verb == "POST" && (resp.StatusCode/100) == 2 { + // For successful create operations, extract resourceVersion + id, currentResourceVersion, err := parseResourceVersion(b) + if err == nil { + key := getPreviousResourceVersionKey(path, id) + previousResourceVersion[key] = currentResourceVersion + } else { + t.Logf("error in trying to extract resource version: %s", err) + } + } + }() + } + }) } } diff --git a/test/integration/replicaset/replicaset_test.go b/test/integration/replicaset/replicaset_test.go index 82cf9ec2510..910fa9a8f10 100644 --- a/test/integration/replicaset/replicaset_test.go +++ b/test/integration/replicaset/replicaset_test.go @@ -422,7 +422,7 @@ func TestAdoption(t *testing.T) { }, } for i, tc := range testCases { - func() { + t.Run(tc.name, func(t *testing.T) { s, closeFn, rm, informers, clientSet := rmSetup(t) defer closeFn() ns := framework.CreateTestingNamespace(fmt.Sprintf("rs-adoption-%d", i), s, t) @@ -461,7 +461,7 @@ func TestAdoption(t *testing.T) { }); err != nil { t.Fatalf("test %q failed: %v", tc.name, err) } - }() + }) } } diff --git a/test/integration/replicationcontroller/replicationcontroller_test.go b/test/integration/replicationcontroller/replicationcontroller_test.go index b25e64421b5..edd5139a6a4 100644 --- a/test/integration/replicationcontroller/replicationcontroller_test.go +++ b/test/integration/replicationcontroller/replicationcontroller_test.go @@ -410,7 +410,7 @@ func TestAdoption(t *testing.T) { }, } for i, tc := range testCases { - func() { + t.Run(tc.name, func(t *testing.T) { s, closeFn, rm, informers, clientSet := rmSetup(t) defer closeFn() ns := framework.CreateTestingNamespace(fmt.Sprintf("rc-adoption-%d", i), s, t) @@ -449,7 +449,7 @@ func TestAdoption(t *testing.T) { }); err != nil { t.Fatalf("test %q failed: %v", tc.name, err) } - }() + }) } } diff --git a/test/integration/statefulset/statefulset_test.go b/test/integration/statefulset/statefulset_test.go index 678e07eae91..c99727c8f8e 100644 --- a/test/integration/statefulset/statefulset_test.go +++ b/test/integration/statefulset/statefulset_test.go @@ -262,53 +262,55 @@ func TestStatefulSetAvailable(t *testing.T) { }, } for _, test := range tests { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, test.enabled)() - s, closeFn, rm, informers, c := scSetup(t) - defer closeFn() - ns := framework.CreateTestingNamespace("test-available-pods", s, t) - defer framework.DeleteTestingNamespace(ns, s, t) - stopCh := runControllerAndInformers(rm, informers) - defer close(stopCh) + t.Run(test.name, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StatefulSetMinReadySeconds, test.enabled)() + s, closeFn, rm, informers, c := scSetup(t) + defer closeFn() + ns := framework.CreateTestingNamespace("test-available-pods", s, t) + defer framework.DeleteTestingNamespace(ns, s, t) + stopCh := runControllerAndInformers(rm, informers) + defer close(stopCh) - labelMap := labelMap() - sts := newSTS("sts", ns.Name, 4) - sts.Spec.MinReadySeconds = int32(3600) - stss, _ := createSTSsPods(t, c, []*appsv1.StatefulSet{sts}, []*v1.Pod{}) - sts = stss[0] - waitSTSStable(t, c, sts) + labelMap := labelMap() + sts := newSTS("sts", ns.Name, 4) + sts.Spec.MinReadySeconds = int32(3600) + stss, _ := createSTSsPods(t, c, []*appsv1.StatefulSet{sts}, []*v1.Pod{}) + sts = stss[0] + waitSTSStable(t, c, sts) - // Verify STS creates 4 pods - podClient := c.CoreV1().Pods(ns.Name) - pods := getPods(t, podClient, labelMap) - if len(pods.Items) != 4 { - t.Fatalf("len(pods) = %d, want 4", len(pods.Items)) - } - - // Separate 3 pods into their own list - firstPodList := &v1.PodList{Items: pods.Items[:1]} - secondPodList := &v1.PodList{Items: pods.Items[1:2]} - thirdPodList := &v1.PodList{Items: pods.Items[2:]} - // First pod: Running, but not Ready - // by setting the Ready condition to false with LastTransitionTime to be now - setPodsReadyCondition(t, c, firstPodList, v1.ConditionFalse, time.Now()) - // Second pod: Running and Ready, but not Available - // by setting LastTransitionTime to now - setPodsReadyCondition(t, c, secondPodList, v1.ConditionTrue, time.Now()) - // Third pod: Running, Ready, and Available - // by setting LastTransitionTime to more than 3600 seconds ago - setPodsReadyCondition(t, c, thirdPodList, v1.ConditionTrue, time.Now().Add(-120*time.Minute)) - - stsClient := c.AppsV1().StatefulSets(ns.Name) - if err := wait.PollImmediate(interval, timeout, func() (bool, error) { - newSts, err := stsClient.Get(context.TODO(), sts.Name, metav1.GetOptions{}) - if err != nil { - return false, err + // Verify STS creates 4 pods + podClient := c.CoreV1().Pods(ns.Name) + pods := getPods(t, podClient, labelMap) + if len(pods.Items) != 4 { + t.Fatalf("len(pods) = %d, want 4", len(pods.Items)) } - // Verify 4 pods exist, 3 pods are Ready, and 2 pods are Available - return newSts.Status.Replicas == test.totalReplicas && newSts.Status.ReadyReplicas == test.readyReplicas && newSts.Status.AvailableReplicas == test.activeReplicas, nil - }); err != nil { - t.Fatalf("Failed to verify number of Replicas, ReadyReplicas and AvailableReplicas of rs %s to be as expected: %v", sts.Name, err) - } + + // Separate 3 pods into their own list + firstPodList := &v1.PodList{Items: pods.Items[:1]} + secondPodList := &v1.PodList{Items: pods.Items[1:2]} + thirdPodList := &v1.PodList{Items: pods.Items[2:]} + // First pod: Running, but not Ready + // by setting the Ready condition to false with LastTransitionTime to be now + setPodsReadyCondition(t, c, firstPodList, v1.ConditionFalse, time.Now()) + // Second pod: Running and Ready, but not Available + // by setting LastTransitionTime to now + setPodsReadyCondition(t, c, secondPodList, v1.ConditionTrue, time.Now()) + // Third pod: Running, Ready, and Available + // by setting LastTransitionTime to more than 3600 seconds ago + setPodsReadyCondition(t, c, thirdPodList, v1.ConditionTrue, time.Now().Add(-120*time.Minute)) + + stsClient := c.AppsV1().StatefulSets(ns.Name) + if err := wait.PollImmediate(interval, timeout, func() (bool, error) { + newSts, err := stsClient.Get(context.TODO(), sts.Name, metav1.GetOptions{}) + if err != nil { + return false, err + } + // Verify 4 pods exist, 3 pods are Ready, and 2 pods are Available + return newSts.Status.Replicas == test.totalReplicas && newSts.Status.ReadyReplicas == test.readyReplicas && newSts.Status.AvailableReplicas == test.activeReplicas, nil + }); err != nil { + t.Fatalf("Failed to verify number of Replicas, ReadyReplicas and AvailableReplicas of rs %s to be as expected: %v", sts.Name, err) + } + }) } }