Browse code

Merge pull request #9770 from smarterclayton/fix_build_controller_race

Merged by openshift-bot

OpenShift Bot authored on 2016/07/12 03:59:49
Showing 10 changed files
... ...
@@ -36,15 +36,23 @@ func TestAuthProxyOnAuthorize(t *testing.T) {
36 36
 
37 37
 	testutil.RequireEtcd(t)
38 38
 	masterConfig, err := testserver.DefaultMasterOptions()
39
-	checkErr(t, err)
39
+	if err != nil {
40
+		t.Fatal(err)
41
+	}
40 42
 	masterConfig.OAuthConfig.IdentityProviders = []configapi.IdentityProvider{idp}
41 43
 
42 44
 	clusterAdminKubeConfig, err := testserver.StartConfiguredMasterAPI(masterConfig)
43
-	checkErr(t, err)
45
+	if err != nil {
46
+		t.Fatal(err)
47
+	}
44 48
 	clusterAdminClientConfig, err := testutil.GetClusterAdminClientConfig(clusterAdminKubeConfig)
45
-	checkErr(t, err)
49
+	if err != nil {
50
+		t.Fatal(err)
51
+	}
46 52
 	clusterAdminClient, err := testutil.GetClusterAdminClient(clusterAdminKubeConfig)
47
-	checkErr(t, err)
53
+	if err != nil {
54
+		t.Fatal(err)
55
+	}
48 56
 
49 57
 	// set up a front proxy guarding the oauth server
50 58
 	proxyHTTPHandler := NewBasicAuthChallenger("TestRegistryAndServer", validUsers, NewXRemoteUserProxyingHandler(clusterAdminClientConfig.Host))
... ...
@@ -54,7 +62,9 @@ func TestAuthProxyOnAuthorize(t *testing.T) {
54 54
 
55 55
 	// need to prime clients so that we can get back a code.  the client must be valid
56 56
 	result := clusterAdminClient.RESTClient.Post().Resource("oAuthClients").Body(&oauthapi.OAuthClient{ObjectMeta: kapi.ObjectMeta{Name: "test"}, Secret: "secret", RedirectURIs: []string{clusterAdminClientConfig.Host}}).Do()
57
-	checkErr(t, result.Error())
57
+	if result.Error() != nil {
58
+		t.Fatal(result.Error())
59
+	}
58 60
 
59 61
 	// our simple URL to get back a code.  We want to go through the front proxy
60 62
 	rawAuthorizeRequest := proxyServer.URL + origin.OpenShiftOAuthAPIPrefix + "/authorize?response_type=code&client_id=test"
... ...
@@ -90,11 +90,15 @@ func TestConcurrentBuildControllers(t *testing.T) {
90 90
 	// Create a build
91 91
 	ns := testutil.Namespace()
92 92
 	b, err := osClient.Builds(ns).Create(mockBuild())
93
-	checkErr(t, err)
93
+	if err != nil {
94
+		t.Fatal(err)
95
+	}
94 96
 
95 97
 	// Start watching builds for New -> Pending transition
96 98
 	buildWatch, err := osClient.Builds(ns).Watch(kapi.ListOptions{FieldSelector: fields.OneTermEqualSelector("metadata.name", b.Name), ResourceVersion: b.ResourceVersion})
97
-	checkErr(t, err)
99
+	if err != nil {
100
+		t.Fatal(err)
101
+	}
98 102
 	defer buildWatch.Stop()
99 103
 	buildModifiedCount := int32(0)
100 104
 	go func() {
... ...
@@ -118,7 +122,9 @@ func TestConcurrentBuildControllers(t *testing.T) {
118 118
 
119 119
 	// Watch build pods as they are created
120 120
 	podWatch, err := kClient.Pods(ns).Watch(kapi.ListOptions{FieldSelector: fields.OneTermEqualSelector("metadata.name", buildapi.GetBuildPodName(b))})
121
-	checkErr(t, err)
121
+	if err != nil {
122
+		t.Fatal(err)
123
+	}
122 124
 	defer podWatch.Stop()
123 125
 	podAddedCount := int32(0)
124 126
 	go func() {
... ...
@@ -206,16 +212,20 @@ func TestConcurrentBuildPodControllers(t *testing.T) {
206 206
 
207 207
 		// Create a build
208 208
 		b, err := osClient.Builds(ns).Create(mockBuild())
209
-		checkErr(t, err)
209
+		if err != nil {
210
+			t.Fatal(err)
211
+		}
210 212
 
211 213
 		// Watch build pod for transition to pending
212 214
 		podWatch, err := kClient.Pods(ns).Watch(kapi.ListOptions{FieldSelector: fields.OneTermEqualSelector("metadata.name", buildapi.GetBuildPodName(b))})
213
-		checkErr(t, err)
215
+		if err != nil {
216
+			t.Fatal(err)
217
+		}
214 218
 		go func() {
215 219
 			for e := range podWatch.ResultChan() {
216 220
 				pod, ok := e.Object.(*kapi.Pod)
217 221
 				if !ok {
218
-					checkErr(t, fmt.Errorf("%s: unexpected object received: %#v\n", test.Name, e.Object))
222
+					t.Fatalf("%s: unexpected object received: %#v\n", test.Name, e.Object)
219 223
 				}
220 224
 				if pod.Status.Phase == kapi.PodPending {
221 225
 					podReadyChan <- pod
... ...
@@ -241,15 +251,26 @@ func TestConcurrentBuildPodControllers(t *testing.T) {
241 241
 		podWatch.Stop()
242 242
 
243 243
 		for _, state := range test.States {
244
-			// Update pod state and verify that corresponding build state happens accordingly
245
-			pod, err := kClient.Pods(ns).Get(pod.Name)
246
-			checkErr(t, err)
247
-			pod.Status.Phase = state.PodPhase
248
-			_, err = kClient.Pods(ns).UpdateStatus(pod)
249
-			checkErr(t, err)
244
+			if err := kclient.RetryOnConflict(kclient.DefaultRetry, func() error {
245
+				// Update pod state and verify that corresponding build state happens accordingly
246
+				pod, err := kClient.Pods(ns).Get(pod.Name)
247
+				if err != nil {
248
+					return err
249
+				}
250
+				if pod.Status.Phase == state.PodPhase {
251
+					return fmt.Errorf("another client altered the pod phase to %s: %#v", state.PodPhase, pod)
252
+				}
253
+				pod.Status.Phase = state.PodPhase
254
+				_, err = kClient.Pods(ns).UpdateStatus(pod)
255
+				return err
256
+			}); err != nil {
257
+				t.Fatal(err)
258
+			}
250 259
 
251 260
 			buildWatch, err := osClient.Builds(ns).Watch(kapi.ListOptions{FieldSelector: fields.OneTermEqualSelector("metadata.name", b.Name), ResourceVersion: b.ResourceVersion})
252
-			checkErr(t, err)
261
+			if err != nil {
262
+				t.Fatal(err)
263
+			}
253 264
 			defer buildWatch.Stop()
254 265
 			go func() {
255 266
 				done := false
... ...
@@ -320,33 +341,37 @@ func TestConcurrentBuildConfigControllers(t *testing.T) {
320 320
 	runBuildConfigChangeControllerTest(t, osClient, kClient)
321 321
 }
322 322
 
323
-func checkErr(t *testing.T, err error) {
324
-	if err != nil {
325
-		t.Fatalf("unexpected error: %v", err)
326
-	}
327
-}
328
-
329 323
 func setupBuildControllerTest(counts controllerCount, t *testing.T) (*client.Client, *kclient.Client) {
330 324
 	testutil.RequireEtcd(t)
331 325
 	master, clusterAdminKubeConfig, err := testserver.StartTestMaster()
332
-	checkErr(t, err)
326
+	if err != nil {
327
+		t.Fatal(err)
328
+	}
333 329
 
334 330
 	clusterAdminClient, err := testutil.GetClusterAdminClient(clusterAdminKubeConfig)
335
-	checkErr(t, err)
331
+	if err != nil {
332
+		t.Fatal(err)
333
+	}
336 334
 
337 335
 	clusterAdminKubeClient, err := testutil.GetClusterAdminKubeClient(clusterAdminKubeConfig)
338
-	checkErr(t, err)
336
+	if err != nil {
337
+		t.Fatal(err)
338
+	}
339 339
 	_, err = clusterAdminKubeClient.Namespaces().Create(&kapi.Namespace{
340 340
 		ObjectMeta: kapi.ObjectMeta{Name: testutil.Namespace()},
341 341
 	})
342
-	checkErr(t, err)
342
+	if err != nil {
343
+		t.Fatal(err)
344
+	}
343 345
 
344 346
 	if err := testserver.WaitForServiceAccounts(clusterAdminKubeClient, testutil.Namespace(), []string{bootstrappolicy.BuilderServiceAccountName, bootstrappolicy.DefaultServiceAccountName}); err != nil {
345
-		t.Errorf("unexpected error: %v", err)
347
+		t.Fatalf("unexpected error: %v", err)
346 348
 	}
347 349
 
348 350
 	openshiftConfig, err := origin.BuildMasterConfig(*master)
349
-	checkErr(t, err)
351
+	if err != nil {
352
+		t.Fatal(err)
353
+	}
350 354
 
351 355
 	// Get the build controller clients, since those rely on service account tokens
352 356
 	// We don't want to proceed with the rest of the test until those are available
... ...
@@ -16,9 +16,13 @@ import (
16 16
 func TestCLIGetToken(t *testing.T) {
17 17
 	testutil.RequireEtcd(t)
18 18
 	_, clusterAdminKubeConfig, err := testserver.StartTestMasterAPI()
19
-	checkErr(t, err)
19
+	if err != nil {
20
+		t.Fatal(err)
21
+	}
20 22
 	clusterAdminClientConfig, err := testutil.GetClusterAdminClientConfig(clusterAdminKubeConfig)
21
-	checkErr(t, err)
23
+	if err != nil {
24
+		t.Fatal(err)
25
+	}
22 26
 
23 27
 	anonymousConfig := clientcmd.AnonymousClientConfig(clusterAdminClientConfig)
24 28
 	reader := bytes.NewBufferString("user\npass")
... ...
@@ -33,10 +37,14 @@ func TestCLIGetToken(t *testing.T) {
33 33
 	clientConfig := clientcmd.AnonymousClientConfig(clusterAdminClientConfig)
34 34
 	clientConfig.BearerToken = accessToken
35 35
 	osClient, err := client.New(&clientConfig)
36
-	checkErr(t, err)
36
+	if err != nil {
37
+		t.Fatal(err)
38
+	}
37 39
 
38 40
 	user, err := osClient.Users().Get("~")
39
-	checkErr(t, err)
41
+	if err != nil {
42
+		t.Fatal(err)
43
+	}
40 44
 
41 45
 	if user.Name != "user" {
42 46
 		t.Errorf("expected %v, got %v", "user", user.Name)
... ...
@@ -30,7 +30,9 @@ func TestClusterResourceOverridePluginWithNoLimits(t *testing.T) {
30 30
 	// test with no limits object present
31 31
 
32 32
 	podCreated, err := podHandler.Create(testClusterResourceOverridePod("limitless", "2Gi", "1"))
33
-	checkErr(t, err)
33
+	if err != nil {
34
+		t.Fatal(err)
35
+	}
34 36
 	if memory := podCreated.Spec.Containers[0].Resources.Requests.Memory(); memory.Cmp(resource.MustParse("1Gi")) != 0 {
35 37
 		t.Errorf("limitlesspod: Memory did not match expected 1Gi: %#v", memory)
36 38
 	}
... ...
@@ -69,9 +71,13 @@ func TestClusterResourceOverridePluginWithLimits(t *testing.T) {
69 69
 		Spec:       kapi.LimitRangeSpec{Limits: []kapi.LimitRangeItem{limitItem}},
70 70
 	}
71 71
 	_, err := limitHandler.Create(limit)
72
-	checkErr(t, err)
72
+	if err != nil {
73
+		t.Fatal(err)
74
+	}
73 75
 	podCreated, err := podHandler.Create(testClusterResourceOverridePod("limit-with-default", "", "1"))
74
-	checkErr(t, err)
76
+	if err != nil {
77
+		t.Fatal(err)
78
+	}
75 79
 	if memory := podCreated.Spec.Containers[0].Resources.Limits.Memory(); memory.Cmp(resource.MustParse("512Mi")) != 0 {
76 80
 		t.Errorf("limit-with-default: Memory limit did not match default 512Mi: %v", memory)
77 81
 	}
... ...
@@ -96,7 +102,9 @@ func TestClusterResourceOverridePluginWithLimits(t *testing.T) {
96 96
 
97 97
 func setupClusterResourceOverrideTest(t *testing.T, pluginConfig *overrideapi.ClusterResourceOverrideConfig) kclient.Interface {
98 98
 	masterConfig, err := testserver.DefaultMasterOptions()
99
-	checkErr(t, err)
99
+	if err != nil {
100
+		t.Fatal(err)
101
+	}
100 102
 	// fill in possibly-empty config values
101 103
 	if masterConfig.KubernetesMasterConfig == nil {
102 104
 		masterConfig.KubernetesMasterConfig = &api.KubernetesMasterConfig{}
... ...
@@ -111,16 +119,26 @@ func setupClusterResourceOverrideTest(t *testing.T, pluginConfig *overrideapi.Cl
111 111
 
112 112
 	// start up a server and return useful clients to that server
113 113
 	clusterAdminKubeConfig, err := testserver.StartConfiguredMaster(masterConfig)
114
-	checkErr(t, err)
114
+	if err != nil {
115
+		t.Fatal(err)
116
+	}
115 117
 	clusterAdminKubeClient, err := testutil.GetClusterAdminKubeClient(clusterAdminKubeConfig)
116
-	checkErr(t, err)
118
+	if err != nil {
119
+		t.Fatal(err)
120
+	}
117 121
 	clusterAdminClient, err := testutil.GetClusterAdminClient(clusterAdminKubeConfig)
118
-	checkErr(t, err)
122
+	if err != nil {
123
+		t.Fatal(err)
124
+	}
119 125
 	// need to create a project and return client for project admin
120 126
 	clusterAdminClientConfig, err := testutil.GetClusterAdminClientConfig(clusterAdminKubeConfig)
121
-	checkErr(t, err)
127
+	if err != nil {
128
+		t.Fatal(err)
129
+	}
122 130
 	_, err = testserver.CreateNewProject(clusterAdminClient, *clusterAdminClientConfig, testutil.Namespace(), "peon")
123
-	checkErr(t, err)
131
+	if err != nil {
132
+		t.Fatal(err)
133
+	}
124 134
 	checkErr(t, testserver.WaitForServiceAccounts(clusterAdminKubeClient, testutil.Namespace(), []string{bootstrappolicy.DefaultServiceAccountName}))
125 135
 	return clusterAdminKubeClient
126 136
 }
... ...
@@ -20,15 +20,25 @@ func TestDeployScale(t *testing.T) {
20 20
 
21 21
 	testutil.RequireEtcd(t)
22 22
 	_, clusterAdminKubeConfig, err := testserver.StartTestMaster()
23
-	checkErr(t, err)
23
+	if err != nil {
24
+		t.Fatal(err)
25
+	}
24 26
 	clusterAdminClientConfig, err := testutil.GetClusterAdminClientConfig(clusterAdminKubeConfig)
25
-	checkErr(t, err)
27
+	if err != nil {
28
+		t.Fatal(err)
29
+	}
26 30
 	clusterAdminClient, err := testutil.GetClusterAdminClient(clusterAdminKubeConfig)
27
-	checkErr(t, err)
31
+	if err != nil {
32
+		t.Fatal(err)
33
+	}
28 34
 	_, err = testserver.CreateNewProject(clusterAdminClient, *clusterAdminClientConfig, namespace, "my-test-user")
29
-	checkErr(t, err)
35
+	if err != nil {
36
+		t.Fatal(err)
37
+	}
30 38
 	osClient, _, _, err := testutil.GetClientForUser(*clusterAdminClientConfig, "my-test-user")
31
-	checkErr(t, err)
39
+	if err != nil {
40
+		t.Fatal(err)
41
+	}
32 42
 
33 43
 	config := deploytest.OkDeploymentConfig(0)
34 44
 	config.Spec.Triggers = []deployapi.DeploymentTriggerPolicy{}
... ...
@@ -27,15 +27,25 @@ func TestTriggers_manual(t *testing.T) {
27 27
 
28 28
 	testutil.RequireEtcd(t)
29 29
 	_, clusterAdminKubeConfig, err := testserver.StartTestMaster()
30
-	checkErr(t, err)
30
+	if err != nil {
31
+		t.Fatal(err)
32
+	}
31 33
 	clusterAdminClientConfig, err := testutil.GetClusterAdminClientConfig(clusterAdminKubeConfig)
32
-	checkErr(t, err)
34
+	if err != nil {
35
+		t.Fatal(err)
36
+	}
33 37
 	clusterAdminClient, err := testutil.GetClusterAdminClient(clusterAdminKubeConfig)
34
-	checkErr(t, err)
38
+	if err != nil {
39
+		t.Fatal(err)
40
+	}
35 41
 	_, err = testserver.CreateNewProject(clusterAdminClient, *clusterAdminClientConfig, namespace, "my-test-user")
36
-	checkErr(t, err)
42
+	if err != nil {
43
+		t.Fatal(err)
44
+	}
37 45
 	osClient, kubeClient, _, err := testutil.GetClientForUser(*clusterAdminClientConfig, "my-test-user")
38
-	checkErr(t, err)
46
+	if err != nil {
47
+		t.Fatal(err)
48
+	}
39 49
 
40 50
 	config := deploytest.OkDeploymentConfig(0)
41 51
 	config.Namespace = namespace
... ...
@@ -346,15 +356,25 @@ func TestTriggers_configChange(t *testing.T) {
346 346
 
347 347
 	testutil.RequireEtcd(t)
348 348
 	_, clusterAdminKubeConfig, err := testserver.StartTestMaster()
349
-	checkErr(t, err)
349
+	if err != nil {
350
+		t.Fatal(err)
351
+	}
350 352
 	clusterAdminClientConfig, err := testutil.GetClusterAdminClientConfig(clusterAdminKubeConfig)
351
-	checkErr(t, err)
353
+	if err != nil {
354
+		t.Fatal(err)
355
+	}
352 356
 	clusterAdminClient, err := testutil.GetClusterAdminClient(clusterAdminKubeConfig)
353
-	checkErr(t, err)
357
+	if err != nil {
358
+		t.Fatal(err)
359
+	}
354 360
 	_, err = testserver.CreateNewProject(clusterAdminClient, *clusterAdminClientConfig, namespace, "my-test-user")
355
-	checkErr(t, err)
361
+	if err != nil {
362
+		t.Fatal(err)
363
+	}
356 364
 	osClient, kubeClient, _, err := testutil.GetClientForUser(*clusterAdminClientConfig, "my-test-user")
357
-	checkErr(t, err)
365
+	if err != nil {
366
+		t.Fatal(err)
367
+	}
358 368
 
359 369
 	config := deploytest.OkDeploymentConfig(0)
360 370
 	config.Namespace = namespace
... ...
@@ -12,9 +12,13 @@ import (
12 12
 func TestNamespaceLifecycleAdmission(t *testing.T) {
13 13
 	testutil.RequireEtcd(t)
14 14
 	_, clusterAdminKubeConfig, err := testserver.StartTestMaster()
15
-	checkErr(t, err)
15
+	if err != nil {
16
+		t.Fatal(err)
17
+	}
16 18
 	clusterAdminClient, err := testutil.GetClusterAdminKubeClient(clusterAdminKubeConfig)
17
-	checkErr(t, err)
19
+	if err != nil {
20
+		t.Fatal(err)
21
+	}
18 22
 
19 23
 	for _, ns := range []string{"default", "openshift", "openshift-infra"} {
20 24
 		if err := clusterAdminClient.Namespaces().Delete(ns); err == nil {
... ...
@@ -1,5 +1,3 @@
1
-// +build integration,docker
2
-
3 1
 package router
4 2
 
5 3
 // These certificates are example certificates generated by a fake cert authority.
... ...
@@ -1,5 +1,3 @@
1
-// +build integration,docker
2
-
3 1
 package router
4 2
 
5 3
 import (
... ...
@@ -42,7 +42,9 @@ func TestWebhookGitHubPushWithImage(t *testing.T) {
42 42
 	}
43 43
 
44 44
 	clusterAdminKubeClient, err := testutil.GetClusterAdminKubeClient(clusterAdminKubeConfig)
45
-	checkErr(t, err)
45
+	if err != nil {
46
+		t.Fatal(err)
47
+	}
46 48
 
47 49
 	if err := testserver.WaitForServiceAccounts(clusterAdminKubeClient, testutil.Namespace(), []string{bootstrappolicy.BuilderServiceAccountName, bootstrappolicy.DefaultServiceAccountName}); err != nil {
48 50
 		t.Errorf("unexpected error: %v", err)
... ...
@@ -137,7 +139,9 @@ func TestWebhookGitHubPushWithImageStream(t *testing.T) {
137 137
 	}
138 138
 
139 139
 	clusterAdminKubeClient, err := testutil.GetClusterAdminKubeClient(clusterAdminKubeConfig)
140
-	checkErr(t, err)
140
+	if err != nil {
141
+		t.Fatal(err)
142
+	}
141 143
 
142 144
 	err = testutil.CreateNamespace(clusterAdminKubeConfig, testutil.Namespace())
143 145
 	if err != nil {