1. origin
  2. Commit 7fd2ddcd23d7b6050ebc4c66c5fdb2613135cbab
Browse code

Diff whole template for deployment config changes

When detecting triggered deployment config changes, diff the entire
pod template instead of just the spec contained in the template. This
is so that changes to labels and other ObjectMeta on the template will
result in a new deployment.

Dan Mace authored on 2015/07/22 03:52:15
Showing 2 changed files
pkg/deploy/controller/configchange/controller.go
History View file @ 7fd2ddc
... ... 
@@ -80,12 +80,13 @@ func (c *DeploymentConfigChangeController) Handle(config *deployapi.DeploymentCo
80 80 
 		return fatalError(fmt.Sprintf("error decoding DeploymentConfig from Deployment %s for DeploymentConfig %s: %v", deployutil.LabelForDeployment(deployment), deployutil.LabelForDeploymentConfig(config), err))
81 81 
 	}
82 82
83 
-	newSpec, oldSpec := config.Template.ControllerTemplate.Template.Spec, deployedConfig.Template.ControllerTemplate.Template.Spec
84 
-	if kapi.Semantic.DeepEqual(oldSpec, newSpec) {
83 
+	// Detect template diffs, and return early if there aren't any changes.
84 
+	if kapi.Semantic.DeepEqual(config.Template.ControllerTemplate.Template, deployedConfig.Template.ControllerTemplate.Template) {
85 85 
 		glog.V(2).Infof("Ignoring DeploymentConfig change for %s (latestVersion=%d); same as Deployment %s", deployutil.LabelForDeploymentConfig(config), config.LatestVersion, deployutil.LabelForDeployment(deployment))
86 86 
 		return nil
87 87 
 	}
88 88
89 
+	// There was a template diff, so generate a new config version.
89 90 
 	fromVersion, toVersion, err := c.generateDeployment(config)
90 91 
 	if err != nil {
91 92 
 		if kerrors.IsConflict(err) {
pkg/deploy/controller/configchange/controller_test.go
History View file @ 7fd2ddc
... ... 
@@ -89,93 +89,84 @@ func TestHandle_newConfigTriggers(t *testing.T) {
89 89 
 // config with a config change trigger results in a version bump and cause
90 90 
 // update.
91 91 
 func TestHandle_changeWithTemplateDiff(t *testing.T) {
92 
-	var updated *deployapi.DeploymentConfig
93 
-
94 
-	controller := &DeploymentConfigChangeController{
95 
-		decodeConfig: func(deployment *kapi.ReplicationController) (*deployapi.DeploymentConfig, error) {
96 
-			return deployutil.DecodeDeploymentConfig(deployment, api.Codec)
97 
-		},
98 
-		changeStrategy: &changeStrategyImpl{
99 
-			generateDeploymentConfigFunc: func(namespace, name string) (*deployapi.DeploymentConfig, error) {
100 
-				return deployapitest.OkDeploymentConfig(2), nil
92 
+	scenarios := []struct {
93 
+		name           string
94 
+		modify         func(*deployapi.DeploymentConfig)
95 
+		changeExpected bool
96 
+	}{
97 
+		{
98 
+			name:           "container name change",
99 
+			changeExpected: true,
100 
+			modify: func(config *deployapi.DeploymentConfig) {
101 
+				config.Template.ControllerTemplate.Template.Spec.Containers[1].Name = "modified"
101 102 
 			},
102 
-			updateDeploymentConfigFunc: func(namespace string, config *deployapi.DeploymentConfig) (*deployapi.DeploymentConfig, error) {
103 
-				updated = config
104 
-				return config, nil
105 
-			},
106 
-			getDeploymentFunc: func(namespace, name string) (*kapi.ReplicationController, error) {
107 
-				deployment, _ := deployutil.MakeDeployment(deployapitest.OkDeploymentConfig(1), kapi.Codec)
108 
-				return deployment, nil
103 
+		},
104 
+		{
105 
+			name:           "template label change",
106 
+			changeExpected: true,
107 
+			modify: func(config *deployapi.DeploymentConfig) {
108 
+				config.Template.ControllerTemplate.Template.Labels["newkey"] = "value"
109 109 
 			},
110 110 
 		},
111 
+		{
112 
+			name:           "no diff",
113 
+			changeExpected: false,
114 
+			modify:         func(config *deployapi.DeploymentConfig) {},
115 
+		},
111 116 
 	}
112 117
113 
-	config := deployapitest.OkDeploymentConfig(1)
114 
-	config.Triggers = []deployapi.DeploymentTriggerPolicy{deployapitest.OkConfigChangeTrigger()}
115 
-	config.Template.ControllerTemplate.Template.Spec.Containers[1].Name = "modified"
116 
-	err := controller.Handle(config)
117 
-
118 
-	if err != nil {
119 
-		t.Fatalf("unexpected error: %v", err)
120 
-	}
121 
-
122 
-	if updated == nil {
123 
-		t.Fatalf("expected config to be updated")
124 
-	}
125 
-
126 
-	if e, a := 2, updated.LatestVersion; e != a {
127 
-		t.Fatalf("expected update to latestversion=%d, got %d", e, a)
128 
-	}
129 
-
130 
-	if updated.Details == nil {
131 
-		t.Fatalf("expected config change details to be set")
132 
-	} else if updated.Details.Causes == nil {
133 
-		t.Fatalf("expected config change causes to be set")
134 
-	} else if updated.Details.Causes[0].Type != deployapi.DeploymentTriggerOnConfigChange {
135 
-		t.Fatalf("expected config change cause to be set to config change trigger, got %s", updated.Details.Causes[0].Type)
136 
-	}
137 
-}
138 
-
139 
-// TestHandle_changeWithoutTemplateDiff ensures that an updated config with no
140 
-// pod template diff results in the config version remaining the same.
141 
-func TestHandle_changeWithoutTemplateDiff(t *testing.T) {
142 
-	config := deployapitest.OkDeploymentConfig(1)
143 
-	config.Triggers = []deployapi.DeploymentTriggerPolicy{deployapitest.OkConfigChangeTrigger()}
118 
+	for _, s := range scenarios {
119 
+		t.Logf("running scenario: %s", s.name)
144 120
145 
-	generated := false
146 
-	updated := false
121 
+		config := deployapitest.OkDeploymentConfig(1)
122 
+		config.Triggers = []deployapi.DeploymentTriggerPolicy{deployapitest.OkConfigChangeTrigger()}
123 
+		deployment, _ := deployutil.MakeDeployment(config, kapi.Codec)
124 
+		var updated *deployapi.DeploymentConfig
147 125
148 
-	controller := &DeploymentConfigChangeController{
149 
-		decodeConfig: func(deployment *kapi.ReplicationController) (*deployapi.DeploymentConfig, error) {
150 
-			return deployutil.DecodeDeploymentConfig(deployment, api.Codec)
151 
-		},
152 
-		changeStrategy: &changeStrategyImpl{
153 
-			generateDeploymentConfigFunc: func(namespace, name string) (*deployapi.DeploymentConfig, error) {
154 
-				generated = true
155 
-				return config, nil
156 
-			},
157 
-			updateDeploymentConfigFunc: func(namespace string, config *deployapi.DeploymentConfig) (*deployapi.DeploymentConfig, error) {
158 
-				updated = true
159 
-				return config, nil
126 
+		controller := &DeploymentConfigChangeController{
127 
+			decodeConfig: func(deployment *kapi.ReplicationController) (*deployapi.DeploymentConfig, error) {
128 
+				return deployutil.DecodeDeploymentConfig(deployment, api.Codec)
160 129 
 			},
161 
-			getDeploymentFunc: func(namespace, name string) (*kapi.ReplicationController, error) {
162 
-				deployment, _ := deployutil.MakeDeployment(deployapitest.OkDeploymentConfig(1), kapi.Codec)
163 
-				return deployment, nil
130 
+			changeStrategy: &changeStrategyImpl{
131 
+				generateDeploymentConfigFunc: func(namespace, name string) (*deployapi.DeploymentConfig, error) {
132 
+					return deployapitest.OkDeploymentConfig(2), nil
133 
+				},
134 
+				updateDeploymentConfigFunc: func(namespace string, config *deployapi.DeploymentConfig) (*deployapi.DeploymentConfig, error) {
135 
+					updated = config
136 
+					return config, nil
137 
+				},
138 
+				getDeploymentFunc: func(namespace, name string) (*kapi.ReplicationController, error) {
139 
+					return deployment, nil
140 
+				},
164 141 
 			},
165 
-		},
166 
-	}
167 
-
168 
-	err := controller.Handle(config)
169 
-
170 
-	if err != nil {
171 
-		t.Fatalf("unexpected error: %v", err)
172 
-	}
173 
-
174 
-	if generated {
175 
-		t.Error("Unexpected generation of deploymentConfig")
176 
-	}
177 
-
178 
-	if updated {
179 
-		t.Error("Unexpected update of deploymentConfig")
142 
+		}
143 
+
144 
+		s.modify(config)
145 
+		err := controller.Handle(config)
146 
+		if err != nil {
147 
+			t.Errorf("unexpected error: %v", err)
148 
+		}
149 
+
150 
+		if s.changeExpected {
151 
+			if updated == nil {
152 
+				t.Errorf("expected config to be updated")
153 
+				continue
154 
+			}
155 
+			if e, a := 2, updated.LatestVersion; e != a {
156 
+				t.Errorf("expected update to latestversion=%d, got %d", e, a)
157 
+			}
158 
+
159 
+			if updated.Details == nil {
160 
+				t.Errorf("expected config change details to be set")
161 
+			} else if updated.Details.Causes == nil {
162 
+				t.Errorf("expected config change causes to be set")
163 
+			} else if updated.Details.Causes[0].Type != deployapi.DeploymentTriggerOnConfigChange {
164 
+				t.Errorf("expected config change cause to be set to config change trigger, got %s", updated.Details.Causes[0].Type)
165 
+			}
166 
+		} else {
167 
+			if updated != nil {
168 
+				t.Errorf("unexpected update to version %d", updated.LatestVersion)
169 
+			}
170 
+		}
180 171 
 	}
181 172 
 }