Browse code

Consider existing deployments correctly when creating a new deployment

Abhishek Gupta authored on 2015/06/09 07:19:33
Showing 2 changed files
... ...
@@ -62,7 +62,14 @@ func (c *DeploymentConfigController) Handle(config *deployapi.DeploymentConfig)
62 62
 		return fmt.Errorf("couldn't list Deployments for DeploymentConfig %s: %v", deployutil.LabelForDeploymentConfig(config), err)
63 63
 	}
64 64
 	var inflightDeployment *kapi.ReplicationController
65
+	latestDeploymentExists := false
65 66
 	for _, deployment := range existingDeployments.Items {
67
+		// check if this is the latest deployment
68
+		// we'll return after we've dealt with the multiple-active-deployments case
69
+		if deployutil.DeploymentVersionFor(&deployment) == config.LatestVersion {
70
+			latestDeploymentExists = true
71
+		}
72
+
66 73
 		deploymentStatus := deployutil.DeploymentStatusFor(&deployment)
67 74
 		switch deploymentStatus {
68 75
 		case deployapi.DeploymentStatusFailed,
... ...
@@ -91,14 +98,14 @@ func (c *DeploymentConfigController) Handle(config *deployapi.DeploymentConfig)
91 91
 		}
92 92
 	}
93 93
 
94
+	// if the latest deployment exists then nothing else needs to be done
95
+	if latestDeploymentExists {
96
+		return nil
97
+	}
98
+
94 99
 	// check to see if there are inflight deployments
95 100
 	if inflightDeployment != nil {
96
-		// check if this is the latest and only deployment
97
-		// if so, nothing needs to be done
98
-		if deployutil.DeploymentVersionFor(inflightDeployment) == config.LatestVersion {
99
-			return nil
100
-		}
101
-		// if this is an earlier deployment, raise a transientError so that the deployment config can be re-queued
101
+		// raise a transientError so that the deployment config can be re-queued
102 102
 		glog.V(4).Infof("Found previous inflight Deployment for %s - will requeue", deployutil.LabelForDeploymentConfig(config))
103 103
 		return transientError(fmt.Sprintf("found previous inflight Deployment for %s - requeuing", deployutil.LabelForDeploymentConfig(config)))
104 104
 	}
... ...
@@ -135,7 +142,6 @@ func (c *DeploymentConfigController) Handle(config *deployapi.DeploymentConfig)
135 135
 		// If the deployment was already created, just move on. The cache could be stale, or another
136 136
 		// process could have already handled this update.
137 137
 		if errors.IsAlreadyExists(err) {
138
-			c.recorder.Eventf(config, "alreadyExists", "Deployment already exists for DeploymentConfig: %s", deployutil.LabelForDeploymentConfig(config))
139 138
 			glog.V(4).Infof("Deployment already exists for DeploymentConfig %s", deployutil.LabelForDeploymentConfig(config))
140 139
 			return nil
141 140
 		}
... ...
@@ -293,38 +293,44 @@ func TestHandle_existingDeployments(t *testing.T) {
293 293
 	}
294 294
 
295 295
 	type scenario struct {
296
-		version   int
297
-		existing  []existing
298
-		errorType reflect.Type
296
+		version          int
297
+		existing         []existing
298
+		errorType        reflect.Type
299
+		expectDeployment bool
299 300
 	}
300 301
 
301 302
 	transientErrorType := reflect.TypeOf(transientError(""))
302 303
 	scenarios := []scenario{
303 304
 		// No existing deployments
304
-		{1, []existing{}, nil},
305
+		{1, []existing{}, nil, true},
305 306
 		// A single existing completed deployment
306
-		{2, []existing{{1, deployapi.DeploymentStatusComplete, false}}, nil},
307
+		{2, []existing{{1, deployapi.DeploymentStatusComplete, false}}, nil, true},
307 308
 		// A single existing failed deployment
308
-		{2, []existing{{1, deployapi.DeploymentStatusFailed, false}}, nil},
309
+		{2, []existing{{1, deployapi.DeploymentStatusFailed, false}}, nil, true},
309 310
 		// Multiple existing completed/failed deployments
310
-		{3, []existing{{2, deployapi.DeploymentStatusFailed, false}, {1, deployapi.DeploymentStatusComplete, false}}, nil},
311
+		{3, []existing{{2, deployapi.DeploymentStatusFailed, false}, {1, deployapi.DeploymentStatusComplete, false}}, nil, true},
311 312
 
312 313
 		// A single existing deployment in the default state
313
-		{2, []existing{{1, "", false}}, transientErrorType},
314
+		{2, []existing{{1, "", false}}, transientErrorType, false},
314 315
 		// A single existing new deployment
315
-		{2, []existing{{1, deployapi.DeploymentStatusNew, false}}, transientErrorType},
316
+		{2, []existing{{1, deployapi.DeploymentStatusNew, false}}, transientErrorType, false},
316 317
 		// A single existing pending deployment
317
-		{2, []existing{{1, deployapi.DeploymentStatusPending, false}}, transientErrorType},
318
+		{2, []existing{{1, deployapi.DeploymentStatusPending, false}}, transientErrorType, false},
318 319
 		// A single existing running deployment
319
-		{2, []existing{{1, deployapi.DeploymentStatusRunning, false}}, transientErrorType},
320
+		{2, []existing{{1, deployapi.DeploymentStatusRunning, false}}, transientErrorType, false},
320 321
 		// Multiple existing deployments with one in new/pending/running
321
-		{4, []existing{{3, deployapi.DeploymentStatusRunning, false}, {2, deployapi.DeploymentStatusComplete, false}, {1, deployapi.DeploymentStatusFailed, false}}, transientErrorType},
322
+		{4, []existing{{3, deployapi.DeploymentStatusRunning, false}, {2, deployapi.DeploymentStatusComplete, false}, {1, deployapi.DeploymentStatusFailed, false}}, transientErrorType, false},
323
+
324
+		// Latest deployment exists and has already failed/completed
325
+		{2, []existing{{2, deployapi.DeploymentStatusFailed, false}, {1, deployapi.DeploymentStatusComplete, false}}, nil, false},
326
+		// Latest deployment exists and is in new/pending/running state
327
+		{2, []existing{{2, deployapi.DeploymentStatusRunning, false}, {1, deployapi.DeploymentStatusComplete, false}}, nil, false},
322 328
 
323 329
 		// Multiple existing deployments with more than one in new/pending/running
324
-		{4, []existing{{3, deployapi.DeploymentStatusNew, false}, {2, deployapi.DeploymentStatusRunning, true}, {1, deployapi.DeploymentStatusFailed, false}}, transientErrorType},
330
+		{4, []existing{{3, deployapi.DeploymentStatusNew, false}, {2, deployapi.DeploymentStatusRunning, true}, {1, deployapi.DeploymentStatusFailed, false}}, transientErrorType, false},
325 331
 		// Multiple existing deployments with more than one in new/pending/running
326 332
 		// Latest deployment has already failed
327
-		{6, []existing{{5, deployapi.DeploymentStatusFailed, false}, {4, deployapi.DeploymentStatusRunning, false}, {3, deployapi.DeploymentStatusNew, true}, {2, deployapi.DeploymentStatusComplete, false}, {1, deployapi.DeploymentStatusNew, true}}, transientErrorType},
333
+		{6, []existing{{5, deployapi.DeploymentStatusFailed, false}, {4, deployapi.DeploymentStatusRunning, false}, {3, deployapi.DeploymentStatusNew, true}, {2, deployapi.DeploymentStatusComplete, false}, {1, deployapi.DeploymentStatusNew, true}}, transientErrorType, false},
328 334
 	}
329 335
 
330 336
 	for _, scenario := range scenarios {
... ...
@@ -341,13 +347,14 @@ func TestHandle_existingDeployments(t *testing.T) {
341 341
 		}
342 342
 		err := controller.Handle(config)
343 343
 
344
+		if scenario.expectDeployment && deployed == nil {
345
+			t.Fatalf("expected a deployment")
346
+		}
347
+
344 348
 		if scenario.errorType == nil {
345 349
 			if err != nil {
346 350
 				t.Fatalf("unexpected error: %v", err)
347 351
 			}
348
-			if deployed == nil {
349
-				t.Fatalf("expected a deployment")
350
-			}
351 352
 		} else {
352 353
 			if err == nil {
353 354
 				t.Fatalf("expected error")