Browse code

deploy: switch config change to a generic trigger controller

Michail Kargakis authored on 2016/06/14 18:37:52
Showing 4 changed files
... ...
@@ -160,6 +160,12 @@ func OkPodTemplate() *kapi.PodTemplateSpec {
160 160
 	}
161 161
 }
162 162
 
163
+func OkPodTemplateChanged() *kapi.PodTemplateSpec {
164
+	template := OkPodTemplate()
165
+	template.Spec.Containers[0].Image = DockerImageReference
166
+	return template
167
+}
168
+
163 169
 func OkPodTemplateMissingImage(missing ...string) *kapi.PodTemplateSpec {
164 170
 	set := sets.NewString(missing...)
165 171
 	template := OkPodTemplate()
... ...
@@ -194,19 +200,22 @@ func OkImageChangeTrigger() deployapi.DeploymentTriggerPolicy {
194 194
 	}
195 195
 }
196 196
 
197
+func OkTriggeredImageChange() deployapi.DeploymentTriggerPolicy {
198
+	ict := OkImageChangeTrigger()
199
+	ict.ImageChangeParams.LastTriggeredImage = DockerImageReference
200
+	return ict
201
+}
202
+
197 203
 func OkNonAutomaticICT() deployapi.DeploymentTriggerPolicy {
198
-	return deployapi.DeploymentTriggerPolicy{
199
-		Type: deployapi.DeploymentTriggerOnImageChange,
200
-		ImageChangeParams: &deployapi.DeploymentTriggerImageChangeParams{
201
-			ContainerNames: []string{
202
-				"container1",
203
-			},
204
-			From: kapi.ObjectReference{
205
-				Kind: "ImageStreamTag",
206
-				Name: imageapi.JoinImageStreamTag(ImageStreamName, imageapi.DefaultImageTag),
207
-			},
208
-		},
209
-	}
204
+	ict := OkImageChangeTrigger()
205
+	ict.ImageChangeParams.Automatic = false
206
+	return ict
207
+}
208
+
209
+func OkTriggeredNonAutomatic() deployapi.DeploymentTriggerPolicy {
210
+	ict := OkNonAutomaticICT()
211
+	ict.ImageChangeParams.LastTriggeredImage = DockerImageReference
212
+	return ict
210 213
 }
211 214
 
212 215
 func TestDeploymentConfig(config *deployapi.DeploymentConfig) *deployapi.DeploymentConfig {
... ...
@@ -3,27 +3,33 @@ package generictrigger
3 3
 import (
4 4
 	"fmt"
5 5
 
6
-	"github.com/golang/glog"
7
-
8 6
 	kapi "k8s.io/kubernetes/pkg/api"
9 7
 	kclient "k8s.io/kubernetes/pkg/client/unversioned"
8
+	"k8s.io/kubernetes/pkg/runtime"
10 9
 
11 10
 	osclient "github.com/openshift/origin/pkg/client"
12 11
 	deployapi "github.com/openshift/origin/pkg/deploy/api"
13 12
 	deployutil "github.com/openshift/origin/pkg/deploy/util"
14 13
 )
15 14
 
16
-// DeploymentTriggerController increments the version of a
17
-// DeploymentConfig which has a config change trigger when a pod template
18
-// change is detected.
19
-//
20
-// Use the DeploymentTriggerControllerFactory to create this controller.
15
+// DeploymentTriggerController processes all triggers for a deployment config
16
+// and kicks new deployments whenever possible.
21 17
 type DeploymentTriggerController struct {
22
-	client  osclient.Interface
23
-	kClient kclient.Interface
18
+	// dn is used to update deployment configs.
19
+	dn osclient.DeploymentConfigsNamespacer
20
+	// rn is used for getting the latest deployment for a config.
21
+	rn kclient.ReplicationControllersNamespacer
22
+	// codec is used for decoding a config out of a deployment.
23
+	codec runtime.Codec
24
+}
24 25
 
25
-	// decodeConfig knows how to decode the deploymentConfig from a deployment's annotations.
26
-	decodeConfig func(deployment *kapi.ReplicationController) (*deployapi.DeploymentConfig, error)
26
+// NewDeploymentTriggerController returns a new DeploymentTriggerController.
27
+func NewDeploymentTriggerController(oc osclient.Interface, kc kclient.Interface, codec runtime.Codec) *DeploymentTriggerController {
28
+	return &DeploymentTriggerController{
29
+		dn:    oc,
30
+		rn:    kc,
31
+		codec: codec,
32
+	}
27 33
 }
28 34
 
29 35
 // fatalError is an error which can't be retried.
... ...
@@ -33,9 +39,9 @@ func (e fatalError) Error() string {
33 33
 	return fmt.Sprintf("fatal error handling configuration: %s", string(e))
34 34
 }
35 35
 
36
-// Handle processes change triggers for config.
36
+// Handle processes deployment triggers for a deployment config.
37 37
 func (c *DeploymentTriggerController) Handle(config *deployapi.DeploymentConfig) error {
38
-	if !deployutil.HasChangeTrigger(config) || config.Spec.Paused {
38
+	if len(config.Spec.Triggers) == 0 || config.Spec.Paused {
39 39
 		return nil
40 40
 	}
41 41
 
... ...
@@ -46,26 +52,14 @@ func (c *DeploymentTriggerController) Handle(config *deployapi.DeploymentConfig)
46 46
 		return err
47 47
 	}
48 48
 
49
-	// If this is the initial deployment, then wait for any images that need to be resolved, otherwise
50
-	// automatically start a new deployment.
51
-	if config.Status.LatestVersion == 0 {
52
-		canTrigger, causes := canTrigger(config, decoded)
53
-		if !canTrigger {
54
-			// If we cannot trigger then we need to wait for the image change controller.
55
-			glog.V(5).Infof("Ignoring deployment config %q; template image needs to be resolved by the image change controller", deployutil.LabelForDeploymentConfig(config))
56
-			return nil
57
-		}
58
-		return c.updateStatus(config, causes)
59
-	}
49
+	canTrigger, causes := canTrigger(config, decoded)
60 50
 
61
-	// If this is not the initial deployment, check if there is any template difference between
62
-	// this and the decoded deploymentconfig.
63
-	if kapi.Semantic.DeepEqual(config.Spec.Template, decoded.Spec.Template) {
51
+	// Return if we cannot trigger a new deployment.
52
+	if !canTrigger {
64 53
 		return nil
65 54
 	}
66 55
 
67
-	_, causes := canTrigger(config, decoded)
68
-	return c.updateStatus(config, causes)
56
+	return c.update(config, causes)
69 57
 }
70 58
 
71 59
 // decodeFromLatest will try to return the decoded version of the current deploymentconfig found
... ...
@@ -77,7 +71,7 @@ func (c *DeploymentTriggerController) decodeFromLatest(config *deployapi.Deploym
77 77
 	}
78 78
 
79 79
 	latestDeploymentName := deployutil.LatestDeploymentNameForConfig(config)
80
-	deployment, err := c.kClient.ReplicationControllers(config.Namespace).Get(latestDeploymentName)
80
+	deployment, err := c.rn.ReplicationControllers(config.Namespace).Get(latestDeploymentName)
81 81
 	if err != nil {
82 82
 		// If there's no deployment for the latest config, we have no basis of
83 83
 		// comparison. It's the responsibility of the deployment config controller
... ...
@@ -85,18 +79,35 @@ func (c *DeploymentTriggerController) decodeFromLatest(config *deployapi.Deploym
85 85
 		return nil, fmt.Errorf("couldn't retrieve deployment for deployment config %q: %v", deployutil.LabelForDeploymentConfig(config), err)
86 86
 	}
87 87
 
88
-	return c.decodeConfig(deployment)
88
+	latest, err := deployutil.DecodeDeploymentConfig(deployment, c.codec)
89
+	if err != nil {
90
+		return nil, fatalError(err.Error())
91
+	}
92
+	return latest, nil
89 93
 }
90 94
 
91
-// canTrigger is used by the config change controller to determine if the provided config can
92
-// trigger its initial deployment. The only requirement is set for image change trigger (ICT)
93
-// deployments - all of the ICTs need to have LastTriggedImage set which means that the image
94
-// change controller did its job. The second return argument helps in separating between config
95
-// change and image change causes.
95
+// canTrigger is used by the trigger controller to determine if the provided config can trigger
96
+// a deployment.
97
+//
98
+// Image change triggers are processed first. It is required for all of them to point to images
99
+// that exist. Otherwise, this controller will wait for the images to land and be updated in the
100
+// triggers that point to them by the image change controller.
101
+//
102
+// Config change triggers are processed last. If all images are resolved and an automatic trigger
103
+// was updated, then it should be possible to trigger a new deployment without a config change
104
+// trigger. Otherwise, if a config change trigger exists and the config is not deployed yet or it
105
+// has a podtemplate change, then the controller should trigger a new deployment (assuming all
106
+// image change triggers can trigger).
96 107
 func canTrigger(config, decoded *deployapi.DeploymentConfig) (bool, []deployapi.DeploymentCause) {
97
-	ictCount, resolved := 0, 0
108
+	if decoded == nil {
109
+		// The decoded deployment config will never be nil here but a sanity check
110
+		// never hurts.
111
+		return false, nil
112
+	}
113
+	ictCount, resolved, canTriggerByImageChange := 0, 0, false
98 114
 	var causes []deployapi.DeploymentCause
99 115
 
116
+	// IMAGE CHANGE TRIGGERS
100 117
 	for _, t := range config.Spec.Triggers {
101 118
 		if t.Type != deployapi.DeploymentTriggerOnImageChange {
102 119
 			continue
... ...
@@ -111,6 +122,11 @@ func canTrigger(config, decoded *deployapi.DeploymentConfig) (bool, []deployapi.
111 111
 		}
112 112
 		resolved++
113 113
 
114
+		// Non-automatic triggers should not be able to trigger deployments.
115
+		if !t.ImageChangeParams.Automatic {
116
+			continue
117
+		}
118
+
114 119
 		// We need stronger checks in order to validate that this template
115 120
 		// change is an image change. Look at the deserialized config's
116 121
 		// triggers and compare with the present trigger.
... ...
@@ -118,6 +134,7 @@ func canTrigger(config, decoded *deployapi.DeploymentConfig) (bool, []deployapi.
118 118
 			continue
119 119
 		}
120 120
 
121
+		canTriggerByImageChange = true
121 122
 		causes = append(causes, deployapi.DeploymentCause{
122 123
 			Type: deployapi.DeploymentTriggerOnImageChange,
123 124
 			ImageTrigger: &deployapi.DeploymentCauseImageTrigger{
... ...
@@ -130,13 +147,31 @@ func canTrigger(config, decoded *deployapi.DeploymentConfig) (bool, []deployapi.
130 130
 		})
131 131
 	}
132 132
 
133
-	if len(causes) == 0 {
134
-		causes = []deployapi.DeploymentCause{{Type: deployapi.DeploymentTriggerOnConfigChange}}
133
+	// We need to wait for all images to resolve before triggering a new deployment.
134
+	if ictCount != resolved {
135
+		return false, nil
136
+	}
137
+
138
+	// CONFIG CHANGE TRIGGERS
139
+	canTriggerByConfigChange := false
140
+	// Our deployment config has a config change trigger and no image change has triggered.
141
+	// If an image change had happened, it would be enough to start a new deployment without
142
+	// caring about the config change trigger.
143
+	if deployutil.HasChangeTrigger(config) && !canTriggerByImageChange {
144
+		// This is the initial deployment or the config has a template change. We need to
145
+		// kick a new deployment.
146
+		if config.Status.LatestVersion == 0 || !kapi.Semantic.DeepEqual(config.Spec.Template, decoded.Spec.Template) {
147
+			canTriggerByConfigChange = true
148
+			causes = []deployapi.DeploymentCause{{Type: deployapi.DeploymentTriggerOnConfigChange}}
149
+		}
135 150
 	}
136 151
 
137
-	return ictCount == resolved, causes
152
+	return canTriggerByConfigChange || canTriggerByImageChange, causes
138 153
 }
139 154
 
155
+// triggeredByDifferentImage compares the provided image change parameters with those found in the
156
+// previous deployment config (the one we decoded from the annotations of its latest deployment)
157
+// and returns whether the two deployment configs have been triggered by a different image change.
140 158
 func triggeredByDifferentImage(ictParams deployapi.DeploymentTriggerImageChangeParams, previous deployapi.DeploymentConfig) bool {
141 159
 	for _, t := range previous.Spec.Triggers {
142 160
 		if t.Type != deployapi.DeploymentTriggerOnImageChange {
... ...
@@ -153,10 +188,12 @@ func triggeredByDifferentImage(ictParams deployapi.DeploymentTriggerImageChangeP
153 153
 	return false
154 154
 }
155 155
 
156
-func (c *DeploymentTriggerController) updateStatus(config *deployapi.DeploymentConfig, causes []deployapi.DeploymentCause) error {
156
+// update increments the latestVersion of the provided deployment config so the deployment config
157
+// controller can run a new deployment and also updates the details of the deployment config.
158
+func (c *DeploymentTriggerController) update(config *deployapi.DeploymentConfig, causes []deployapi.DeploymentCause) error {
157 159
 	config.Status.LatestVersion++
158 160
 	config.Status.Details = new(deployapi.DeploymentDetails)
159 161
 	config.Status.Details.Causes = causes
160
-	_, err := c.client.DeploymentConfigs(config.Namespace).UpdateStatus(config)
162
+	_, err := c.dn.DeploymentConfigs(config.Namespace).UpdateStatus(config)
161 163
 	return err
162 164
 }
... ...
@@ -14,18 +14,15 @@ import (
14 14
 	deployutil "github.com/openshift/origin/pkg/deploy/util"
15 15
 )
16 16
 
17
+var codec = kapi.Codecs.LegacyCodec(deployapi.SchemeGroupVersion)
18
+
17 19
 // TestHandle_newConfigNoTriggers ensures that a change to a config with no
18 20
 // triggers doesn't result in a new config version bump.
19 21
 func TestHandle_newConfigNoTriggers(t *testing.T) {
20 22
 	fake := &testclient.Fake{}
21 23
 	kFake := &ktestclient.Fake{}
22
-	controller := &DeploymentConfigChangeController{
23
-		client:  fake,
24
-		kClient: kFake,
25
-		decodeConfig: func(deployment *kapi.ReplicationController) (*deployapi.DeploymentConfig, error) {
26
-			return deployutil.DecodeDeploymentConfig(deployment, kapi.Codecs.LegacyCodec(deployapi.SchemeGroupVersion))
27
-		},
28
-	}
24
+
25
+	controller := NewDeploymentTriggerController(fake, kFake, codec)
29 26
 
30 27
 	config := testapi.OkDeploymentConfig(1)
31 28
 	config.Namespace = kapi.NamespaceDefault
... ...
@@ -54,13 +51,7 @@ func TestHandle_newConfigTriggers(t *testing.T) {
54 54
 	})
55 55
 	kFake := &ktestclient.Fake{}
56 56
 
57
-	controller := &DeploymentConfigChangeController{
58
-		client:  fake,
59
-		kClient: kFake,
60
-		decodeConfig: func(deployment *kapi.ReplicationController) (*deployapi.DeploymentConfig, error) {
61
-			return deployutil.DecodeDeploymentConfig(deployment, kapi.Codecs.LegacyCodec(deployapi.SchemeGroupVersion))
62
-		},
63
-	}
57
+	controller := NewDeploymentTriggerController(fake, kFake, codec)
64 58
 
65 59
 	config := testapi.OkDeploymentConfig(0)
66 60
 	config.Namespace = kapi.NamespaceDefault
... ...
@@ -132,13 +123,7 @@ func TestHandle_changeWithTemplateDiff(t *testing.T) {
132 132
 			return true, deployment, nil
133 133
 		})
134 134
 
135
-		controller := &DeploymentConfigChangeController{
136
-			client:  fake,
137
-			kClient: kFake,
138
-			decodeConfig: func(deployment *kapi.ReplicationController) (*deployapi.DeploymentConfig, error) {
139
-				return deployutil.DecodeDeploymentConfig(deployment, kapi.Codecs.LegacyCodec(deployapi.SchemeGroupVersion))
140
-			},
141
-		}
135
+		controller := NewDeploymentTriggerController(fake, kFake, codec)
142 136
 
143 137
 		s.modify(config)
144 138
 		if err := controller.Handle(config); err != nil {
... ...
@@ -184,13 +169,7 @@ func TestHandle_waitForImageController(t *testing.T) {
184 184
 		return true, nil, nil
185 185
 	})
186 186
 
187
-	controller := &DeploymentConfigChangeController{
188
-		client:  fake,
189
-		kClient: kFake,
190
-		decodeConfig: func(deployment *kapi.ReplicationController) (*deployapi.DeploymentConfig, error) {
191
-			return deployutil.DecodeDeploymentConfig(deployment, kapi.Codecs.LegacyCodec(deployapi.SchemeGroupVersion))
192
-		},
193
-	}
187
+	controller := NewDeploymentTriggerController(fake, kFake, codec)
194 188
 
195 189
 	config := testapi.OkDeploymentConfig(0)
196 190
 	config.Namespace = kapi.NamespaceDefault
... ...
@@ -257,13 +236,7 @@ func TestHandle_automaticImageUpdates(t *testing.T) {
257 257
 			return true, deployment, nil
258 258
 		})
259 259
 
260
-		controller := &DeploymentConfigChangeController{
261
-			client:  fake,
262
-			kClient: kFake,
263
-			decodeConfig: func(deployment *kapi.ReplicationController) (*deployapi.DeploymentConfig, error) {
264
-				return deployutil.DecodeDeploymentConfig(deployment, kapi.Codecs.LegacyCodec(deployapi.SchemeGroupVersion))
265
-			},
266
-		}
260
+		controller := NewDeploymentTriggerController(fake, kFake, codec)
267 261
 
268 262
 		config := testapi.OkDeploymentConfig(test.version)
269 263
 		config.Namespace = kapi.NamespaceDefault
... ...
@@ -284,3 +257,357 @@ func TestHandle_automaticImageUpdates(t *testing.T) {
284 284
 		}
285 285
 	}
286 286
 }
287
+
288
+func TestCanTrigger(t *testing.T) {
289
+	tests := []struct {
290
+		name string
291
+
292
+		config  *deployapi.DeploymentConfig
293
+		decoded *deployapi.DeploymentConfig
294
+
295
+		expected       bool
296
+		expectedCauses []deployapi.DeploymentCause
297
+	}{
298
+		{
299
+			name: "nil decoded config",
300
+
301
+			config:  testapi.OkDeploymentConfig(1),
302
+			decoded: nil,
303
+
304
+			expected:       false,
305
+			expectedCauses: nil,
306
+		},
307
+		{
308
+			name: "no trigger",
309
+
310
+			config: &deployapi.DeploymentConfig{
311
+				Spec: deployapi.DeploymentConfigSpec{
312
+					Template: testapi.OkPodTemplateChanged(),
313
+				},
314
+				Status: testapi.OkDeploymentConfigStatus(1),
315
+			},
316
+			decoded: &deployapi.DeploymentConfig{
317
+				Spec: deployapi.DeploymentConfigSpec{
318
+					Template: testapi.OkPodTemplate(),
319
+				},
320
+				Status: testapi.OkDeploymentConfigStatus(1),
321
+			},
322
+
323
+			expected:       false,
324
+			expectedCauses: nil,
325
+		},
326
+		{
327
+			name: "config change trigger only",
328
+
329
+			config: &deployapi.DeploymentConfig{
330
+				Spec: deployapi.DeploymentConfigSpec{
331
+					Template: testapi.OkPodTemplateChanged(),
332
+					Triggers: []deployapi.DeploymentTriggerPolicy{
333
+						testapi.OkConfigChangeTrigger(),
334
+					},
335
+				},
336
+				Status: testapi.OkDeploymentConfigStatus(1),
337
+			},
338
+			decoded: &deployapi.DeploymentConfig{
339
+				Spec: deployapi.DeploymentConfigSpec{
340
+					Template: testapi.OkPodTemplate(),
341
+					Triggers: []deployapi.DeploymentTriggerPolicy{
342
+						testapi.OkConfigChangeTrigger(),
343
+					},
344
+				},
345
+				Status: testapi.OkDeploymentConfigStatus(1),
346
+			},
347
+
348
+			expected:       true,
349
+			expectedCauses: testapi.OkConfigChangeDetails().Causes,
350
+		},
351
+		{
352
+			name: "config change trigger only [no change][initial]",
353
+
354
+			config: &deployapi.DeploymentConfig{
355
+				Spec: deployapi.DeploymentConfigSpec{
356
+					Template: testapi.OkPodTemplate(),
357
+					Triggers: []deployapi.DeploymentTriggerPolicy{
358
+						testapi.OkConfigChangeTrigger(),
359
+					},
360
+				},
361
+				Status: testapi.OkDeploymentConfigStatus(0),
362
+			},
363
+			decoded: &deployapi.DeploymentConfig{
364
+				Spec: deployapi.DeploymentConfigSpec{
365
+					Template: testapi.OkPodTemplate(),
366
+					Triggers: []deployapi.DeploymentTriggerPolicy{
367
+						testapi.OkConfigChangeTrigger(),
368
+					},
369
+				},
370
+				Status: testapi.OkDeploymentConfigStatus(0),
371
+			},
372
+
373
+			expected:       true,
374
+			expectedCauses: testapi.OkConfigChangeDetails().Causes,
375
+		},
376
+		{
377
+			name: "config change trigger only [no change]",
378
+
379
+			config: &deployapi.DeploymentConfig{
380
+				Spec: deployapi.DeploymentConfigSpec{
381
+					Template: testapi.OkPodTemplate(),
382
+					Triggers: []deployapi.DeploymentTriggerPolicy{
383
+						testapi.OkConfigChangeTrigger(),
384
+					},
385
+				},
386
+				Status: testapi.OkDeploymentConfigStatus(1),
387
+			},
388
+			decoded: &deployapi.DeploymentConfig{
389
+				Spec: deployapi.DeploymentConfigSpec{
390
+					Template: testapi.OkPodTemplate(),
391
+					Triggers: []deployapi.DeploymentTriggerPolicy{
392
+						testapi.OkConfigChangeTrigger(),
393
+					},
394
+				},
395
+				Status: testapi.OkDeploymentConfigStatus(1),
396
+			},
397
+
398
+			expected:       false,
399
+			expectedCauses: nil,
400
+		},
401
+		{
402
+			name: "image change trigger only [automatic=false]",
403
+
404
+			config: &deployapi.DeploymentConfig{
405
+				Spec: deployapi.DeploymentConfigSpec{
406
+					Template: testapi.OkPodTemplateChanged(), // Irrelevant change
407
+					Triggers: []deployapi.DeploymentTriggerPolicy{
408
+						testapi.OkNonAutomaticICT(), // Image still to be resolved but it's false anyway
409
+					},
410
+				},
411
+			},
412
+			decoded: &deployapi.DeploymentConfig{
413
+				Spec: deployapi.DeploymentConfigSpec{
414
+					Template: testapi.OkPodTemplate(),
415
+					Triggers: []deployapi.DeploymentTriggerPolicy{
416
+						testapi.OkNonAutomaticICT(),
417
+					},
418
+				},
419
+			},
420
+
421
+			expected:       false,
422
+			expectedCauses: nil,
423
+		},
424
+		{
425
+			name: "image change trigger only [automatic=false][image triggered]",
426
+
427
+			config: &deployapi.DeploymentConfig{
428
+				Spec: deployapi.DeploymentConfigSpec{
429
+					Template: testapi.OkPodTemplateChanged(), // Image has been updated in the template but automatic=false
430
+					Triggers: []deployapi.DeploymentTriggerPolicy{
431
+						testapi.OkTriggeredNonAutomatic(),
432
+					},
433
+				},
434
+			},
435
+			decoded: &deployapi.DeploymentConfig{
436
+				Spec: deployapi.DeploymentConfigSpec{
437
+					Template: testapi.OkPodTemplate(),
438
+					Triggers: []deployapi.DeploymentTriggerPolicy{
439
+						testapi.OkNonAutomaticICT(),
440
+					},
441
+				},
442
+			},
443
+
444
+			expected:       false,
445
+			expectedCauses: nil,
446
+		},
447
+		{
448
+			name: "image change trigger only [automatic=true]",
449
+
450
+			config: &deployapi.DeploymentConfig{
451
+				Spec: deployapi.DeploymentConfigSpec{
452
+					Template: testapi.OkPodTemplateChanged(),
453
+					Triggers: []deployapi.DeploymentTriggerPolicy{
454
+						testapi.OkTriggeredImageChange(),
455
+					},
456
+				},
457
+			},
458
+			decoded: &deployapi.DeploymentConfig{
459
+				Spec: deployapi.DeploymentConfigSpec{
460
+					Template: testapi.OkPodTemplate(),
461
+					Triggers: []deployapi.DeploymentTriggerPolicy{
462
+						testapi.OkImageChangeTrigger(),
463
+					},
464
+				},
465
+			},
466
+
467
+			expected:       true,
468
+			expectedCauses: testapi.OkImageChangeDetails().Causes,
469
+		},
470
+		{
471
+			name: "image change trigger only [automatic=true][no change]",
472
+
473
+			config: &deployapi.DeploymentConfig{
474
+				Spec: deployapi.DeploymentConfigSpec{
475
+					Template: testapi.OkPodTemplate(),
476
+					Triggers: []deployapi.DeploymentTriggerPolicy{
477
+						testapi.OkImageChangeTrigger(),
478
+					},
479
+				},
480
+			},
481
+			decoded: &deployapi.DeploymentConfig{
482
+				Spec: deployapi.DeploymentConfigSpec{
483
+					Template: testapi.OkPodTemplate(),
484
+					Triggers: []deployapi.DeploymentTriggerPolicy{
485
+						testapi.OkImageChangeTrigger(),
486
+					},
487
+				},
488
+			},
489
+
490
+			expected:       false,
491
+			expectedCauses: nil,
492
+		},
493
+		{
494
+			name: "config change and image change trigger [automatic=false][initial][image resolved]",
495
+
496
+			config: &deployapi.DeploymentConfig{
497
+				Spec: deployapi.DeploymentConfigSpec{
498
+					Template: testapi.OkPodTemplateChanged(),
499
+					Triggers: []deployapi.DeploymentTriggerPolicy{
500
+						testapi.OkConfigChangeTrigger(),
501
+						testapi.OkTriggeredNonAutomatic(),
502
+					},
503
+				},
504
+				Status: testapi.OkDeploymentConfigStatus(0),
505
+			},
506
+			decoded: &deployapi.DeploymentConfig{
507
+				Spec: deployapi.DeploymentConfigSpec{
508
+					Template: testapi.OkPodTemplate(),
509
+					Triggers: []deployapi.DeploymentTriggerPolicy{
510
+						testapi.OkConfigChangeTrigger(),
511
+						testapi.OkNonAutomaticICT(),
512
+					},
513
+				},
514
+				Status: testapi.OkDeploymentConfigStatus(0),
515
+			},
516
+
517
+			expected:       true,
518
+			expectedCauses: testapi.OkConfigChangeDetails().Causes,
519
+		},
520
+		{
521
+			name: "config change and image change trigger [automatic=false][initial]",
522
+
523
+			config: &deployapi.DeploymentConfig{
524
+				Spec: deployapi.DeploymentConfigSpec{
525
+					Template: testapi.OkPodTemplate(),
526
+					Triggers: []deployapi.DeploymentTriggerPolicy{
527
+						testapi.OkConfigChangeTrigger(),
528
+						testapi.OkNonAutomaticICT(), // Image is not resolved yet
529
+					},
530
+				},
531
+				Status: testapi.OkDeploymentConfigStatus(0),
532
+			},
533
+			decoded: &deployapi.DeploymentConfig{
534
+				Spec: deployapi.DeploymentConfigSpec{
535
+					Template: testapi.OkPodTemplate(),
536
+					Triggers: []deployapi.DeploymentTriggerPolicy{
537
+						testapi.OkConfigChangeTrigger(),
538
+						testapi.OkNonAutomaticICT(),
539
+					},
540
+				},
541
+				Status: testapi.OkDeploymentConfigStatus(0),
542
+			},
543
+
544
+			expected:       false,
545
+			expectedCauses: nil,
546
+		},
547
+		{
548
+			name: "config change and image change trigger [automatic=true][initial]",
549
+
550
+			config: &deployapi.DeploymentConfig{
551
+				Spec: deployapi.DeploymentConfigSpec{
552
+					Template: testapi.OkPodTemplateChanged(), // Pod template has changed but the image in the template is yet to be updated
553
+					Triggers: []deployapi.DeploymentTriggerPolicy{
554
+						testapi.OkConfigChangeTrigger(),
555
+						testapi.OkImageChangeTrigger(),
556
+					},
557
+				},
558
+				Status: testapi.OkDeploymentConfigStatus(0),
559
+			},
560
+			decoded: &deployapi.DeploymentConfig{
561
+				Spec: deployapi.DeploymentConfigSpec{
562
+					Template: testapi.OkPodTemplate(),
563
+					Triggers: []deployapi.DeploymentTriggerPolicy{
564
+						testapi.OkConfigChangeTrigger(),
565
+						testapi.OkImageChangeTrigger(),
566
+					},
567
+				},
568
+				Status: testapi.OkDeploymentConfigStatus(0),
569
+			},
570
+
571
+			expected:       false,
572
+			expectedCauses: nil,
573
+		},
574
+		{
575
+			name: "config change and image change trigger [automatic=true][initial][image triggered]",
576
+
577
+			config: &deployapi.DeploymentConfig{
578
+				Spec: deployapi.DeploymentConfigSpec{
579
+					Template: testapi.OkPodTemplateChanged(),
580
+					Triggers: []deployapi.DeploymentTriggerPolicy{
581
+						testapi.OkConfigChangeTrigger(),
582
+						testapi.OkTriggeredImageChange(),
583
+					},
584
+				},
585
+				Status: testapi.OkDeploymentConfigStatus(0),
586
+			},
587
+			decoded: &deployapi.DeploymentConfig{
588
+				Spec: deployapi.DeploymentConfigSpec{
589
+					Template: testapi.OkPodTemplate(),
590
+					Triggers: []deployapi.DeploymentTriggerPolicy{
591
+						testapi.OkConfigChangeTrigger(),
592
+						testapi.OkImageChangeTrigger(),
593
+					},
594
+				},
595
+				Status: testapi.OkDeploymentConfigStatus(0),
596
+			},
597
+
598
+			expected:       true,
599
+			expectedCauses: testapi.OkImageChangeDetails().Causes,
600
+		},
601
+		{
602
+			name: "config change and image change trigger [automatic=true][no change]",
603
+
604
+			config: &deployapi.DeploymentConfig{
605
+				Spec: deployapi.DeploymentConfigSpec{
606
+					Template: testapi.OkPodTemplate(),
607
+					Triggers: []deployapi.DeploymentTriggerPolicy{
608
+						testapi.OkConfigChangeTrigger(),
609
+						testapi.OkImageChangeTrigger(),
610
+					},
611
+				},
612
+				Status: testapi.OkDeploymentConfigStatus(1),
613
+			},
614
+			decoded: &deployapi.DeploymentConfig{
615
+				Spec: deployapi.DeploymentConfigSpec{
616
+					Template: testapi.OkPodTemplate(),
617
+					Triggers: []deployapi.DeploymentTriggerPolicy{
618
+						testapi.OkConfigChangeTrigger(),
619
+						testapi.OkImageChangeTrigger(),
620
+					},
621
+				},
622
+				Status: testapi.OkDeploymentConfigStatus(1),
623
+			},
624
+
625
+			expected:       false,
626
+			expectedCauses: nil,
627
+		},
628
+	}
629
+
630
+	for _, test := range tests {
631
+		got, gotCauses := canTrigger(test.config, test.decoded)
632
+		if test.expected != got {
633
+			t.Errorf("%s: expected to trigger: %t, got: %t", test.name, test.expected, got)
634
+			continue
635
+		}
636
+		if !kapi.Semantic.DeepEqual(test.expectedCauses, gotCauses) {
637
+			t.Errorf("%s: expected causes:\n%#v\ngot:\n%#v", test.name, test.expectedCauses, gotCauses)
638
+		}
639
+	}
640
+}
... ...
@@ -15,7 +15,6 @@ import (
15 15
 	osclient "github.com/openshift/origin/pkg/client"
16 16
 	controller "github.com/openshift/origin/pkg/controller"
17 17
 	deployapi "github.com/openshift/origin/pkg/deploy/api"
18
-	deployutil "github.com/openshift/origin/pkg/deploy/util"
19 18
 )
20 19
 
21 20
 // DeploymentTriggerControllerFactory can create a DeploymentTriggerController that watches all DeploymentConfigs.
... ...
@@ -44,13 +43,7 @@ func (factory *DeploymentTriggerControllerFactory) Create() controller.RunnableC
44 44
 	eventBroadcaster := record.NewBroadcaster()
45 45
 	eventBroadcaster.StartRecordingToSink(factory.KubeClient.Events(""))
46 46
 
47
-	triggerController := &DeploymentTriggerController{
48
-		client:  factory.Client,
49
-		kClient: factory.KubeClient,
50
-		decodeConfig: func(deployment *kapi.ReplicationController) (*deployapi.DeploymentConfig, error) {
51
-			return deployutil.DecodeDeploymentConfig(deployment, factory.Codec)
52
-		},
53
-	}
47
+	triggerController := NewDeploymentTriggerController(factory.Client, factory.KubeClient, factory.Codec)
54 48
 
55 49
 	return &controller.RetryController{
56 50
 		Queue: queue,