Browse code
deployer controller: ensure phase direction
kargakis authored on 2016/04/08 17:20:30Showing 5 changed files
- pkg/deploy/controller/deployerpod/controller.go
- pkg/deploy/controller/deployerpod/controller_test.go
- pkg/deploy/controller/deployment/controller.go
- pkg/deploy/util/util.go
- pkg/deploy/util/util_test.go
| ... | ... |
@@ -97,9 +97,8 @@ func (c *DeployerPodController) Handle(pod *kapi.Pod) error {
|
| 97 | 97 |
|
| 98 | 98 |
switch pod.Status.Phase {
|
| 99 | 99 |
case kapi.PodRunning: |
| 100 |
- if !deployutil.IsTerminatedDeployment(deployment) {
|
|
| 101 |
- nextStatus = deployapi.DeploymentStatusRunning |
|
| 102 |
- } |
|
| 100 |
+ nextStatus = deployapi.DeploymentStatusRunning |
|
| 101 |
+ |
|
| 103 | 102 |
case kapi.PodSucceeded: |
| 104 | 103 |
nextStatus = deployapi.DeploymentStatusComplete |
| 105 | 104 |
|
| ... | ... |
@@ -137,7 +136,7 @@ func (c *DeployerPodController) Handle(pod *kapi.Pod) error {
|
| 137 | 137 |
} |
| 138 | 138 |
} |
| 139 | 139 |
|
| 140 |
- if currentStatus != nextStatus {
|
|
| 140 |
+ if deployutil.CanTransitionPhase(currentStatus, nextStatus) {
|
|
| 141 | 141 |
deployment.Annotations[deployapi.DeploymentStatusAnnotation] = string(nextStatus) |
| 142 | 142 |
if _, err := c.kClient.ReplicationControllers(deployment.Namespace).Update(deployment); err != nil {
|
| 143 | 143 |
if kerrors.IsNotFound(err) {
|
| ... | ... |
@@ -352,7 +352,8 @@ func TestHandle_podTerminatedFailNoContainerStatusTest(t *testing.T) {
|
| 352 | 352 |
// TestHandle_cleanupDesiredReplicasAnnotation ensures that the desired replicas annotation |
| 353 | 353 |
// will be cleaned up in a complete deployment and stay around in a failed deployment |
| 354 | 354 |
func TestHandle_cleanupDesiredReplicasAnnotation(t *testing.T) {
|
| 355 |
- deployment, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(1), kapi.Codecs.LegacyCodec(deployapi.SchemeGroupVersion)) |
|
| 355 |
+ // shared fixtures shouldn't be used in unit tests |
|
| 356 |
+ shared, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(1), kapi.Codecs.LegacyCodec(deployapi.SchemeGroupVersion)) |
|
| 356 | 357 |
|
| 357 | 358 |
tests := []struct {
|
| 358 | 359 |
name string |
| ... | ... |
@@ -361,17 +362,18 @@ func TestHandle_cleanupDesiredReplicasAnnotation(t *testing.T) {
|
| 361 | 361 |
}{
|
| 362 | 362 |
{
|
| 363 | 363 |
name: "complete deployment - cleaned up annotation", |
| 364 |
- pod: succeededPod(deployment), |
|
| 364 |
+ pod: succeededPod(shared), |
|
| 365 | 365 |
expected: false, |
| 366 | 366 |
}, |
| 367 | 367 |
{
|
| 368 | 368 |
name: "failed deployment - annotation stays", |
| 369 |
- pod: terminatedPod(deployment), |
|
| 369 |
+ pod: terminatedPod(shared), |
|
| 370 | 370 |
expected: true, |
| 371 | 371 |
}, |
| 372 | 372 |
} |
| 373 | 373 |
|
| 374 | 374 |
for _, test := range tests {
|
| 375 |
+ deployment, _ := deployutil.MakeDeployment(deploytest.OkDeploymentConfig(1), kapi.Codecs.LegacyCodec(deployapi.SchemeGroupVersion)) |
|
| 375 | 376 |
var updatedDeployment *kapi.ReplicationController |
| 376 | 377 |
deployment.Annotations[deployapi.DesiredReplicasAnnotation] = "1" |
| 377 | 378 |
|
| ... | ... |
@@ -221,7 +221,7 @@ func (c *DeploymentController) Handle(deployment *kapi.ReplicationController) er |
| 221 | 221 |
} |
| 222 | 222 |
} |
| 223 | 223 |
|
| 224 |
- if currentStatus != nextStatus || deploymentScaled {
|
|
| 224 |
+ if deployutil.CanTransitionPhase(currentStatus, nextStatus) || deploymentScaled {
|
|
| 225 | 225 |
deployment.Annotations[deployapi.DeploymentStatusAnnotation] = string(nextStatus) |
| 226 | 226 |
if _, err := c.deploymentClient.updateDeployment(deployment.Namespace, deployment); err != nil {
|
| 227 | 227 |
if config, decodeErr := c.decodeConfig(deployment); decodeErr == nil {
|
| ... | ... |
@@ -279,6 +279,33 @@ func IsTerminatedDeployment(deployment *api.ReplicationController) bool {
|
| 279 | 279 |
return current == deployapi.DeploymentStatusComplete || current == deployapi.DeploymentStatusFailed |
| 280 | 280 |
} |
| 281 | 281 |
|
| 282 |
+// CanTransitionPhase returns whether it is allowed to go from the current to the next phase. |
|
| 283 |
+func CanTransitionPhase(current, next deployapi.DeploymentStatus) bool {
|
|
| 284 |
+ switch current {
|
|
| 285 |
+ case deployapi.DeploymentStatusNew: |
|
| 286 |
+ switch next {
|
|
| 287 |
+ case deployapi.DeploymentStatusPending, |
|
| 288 |
+ deployapi.DeploymentStatusRunning, |
|
| 289 |
+ deployapi.DeploymentStatusFailed, |
|
| 290 |
+ deployapi.DeploymentStatusComplete: |
|
| 291 |
+ return true |
|
| 292 |
+ } |
|
| 293 |
+ case deployapi.DeploymentStatusPending: |
|
| 294 |
+ switch next {
|
|
| 295 |
+ case deployapi.DeploymentStatusRunning, |
|
| 296 |
+ deployapi.DeploymentStatusFailed, |
|
| 297 |
+ deployapi.DeploymentStatusComplete: |
|
| 298 |
+ return true |
|
| 299 |
+ } |
|
| 300 |
+ case deployapi.DeploymentStatusRunning: |
|
| 301 |
+ switch next {
|
|
| 302 |
+ case deployapi.DeploymentStatusFailed, deployapi.DeploymentStatusComplete: |
|
| 303 |
+ return true |
|
| 304 |
+ } |
|
| 305 |
+ } |
|
| 306 |
+ return false |
|
| 307 |
+} |
|
| 308 |
+ |
|
| 282 | 309 |
// annotationFor returns the annotation with key for obj. |
| 283 | 310 |
func annotationFor(obj runtime.Object, key string) string {
|
| 284 | 311 |
meta, err := api.ObjectMetaFor(obj) |
| ... | ... |
@@ -184,3 +184,169 @@ func TestSort(t *testing.T) {
|
| 184 | 184 |
t.Errorf("Unexpected sort order")
|
| 185 | 185 |
} |
| 186 | 186 |
} |
| 187 |
+ |
|
| 188 |
+func TestCanTransitionPhase(t *testing.T) {
|
|
| 189 |
+ tests := []struct {
|
|
| 190 |
+ name string |
|
| 191 |
+ current, next deployapi.DeploymentStatus |
|
| 192 |
+ expected bool |
|
| 193 |
+ }{
|
|
| 194 |
+ {
|
|
| 195 |
+ name: "New->New", |
|
| 196 |
+ current: deployapi.DeploymentStatusNew, |
|
| 197 |
+ next: deployapi.DeploymentStatusNew, |
|
| 198 |
+ expected: false, |
|
| 199 |
+ }, |
|
| 200 |
+ {
|
|
| 201 |
+ name: "New->Pending", |
|
| 202 |
+ current: deployapi.DeploymentStatusNew, |
|
| 203 |
+ next: deployapi.DeploymentStatusPending, |
|
| 204 |
+ expected: true, |
|
| 205 |
+ }, |
|
| 206 |
+ {
|
|
| 207 |
+ name: "New->Running", |
|
| 208 |
+ current: deployapi.DeploymentStatusNew, |
|
| 209 |
+ next: deployapi.DeploymentStatusRunning, |
|
| 210 |
+ expected: true, |
|
| 211 |
+ }, |
|
| 212 |
+ {
|
|
| 213 |
+ name: "New->Complete", |
|
| 214 |
+ current: deployapi.DeploymentStatusNew, |
|
| 215 |
+ next: deployapi.DeploymentStatusComplete, |
|
| 216 |
+ expected: true, |
|
| 217 |
+ }, |
|
| 218 |
+ {
|
|
| 219 |
+ name: "New->Failed", |
|
| 220 |
+ current: deployapi.DeploymentStatusNew, |
|
| 221 |
+ next: deployapi.DeploymentStatusFailed, |
|
| 222 |
+ expected: true, |
|
| 223 |
+ }, |
|
| 224 |
+ {
|
|
| 225 |
+ name: "Pending->New", |
|
| 226 |
+ current: deployapi.DeploymentStatusPending, |
|
| 227 |
+ next: deployapi.DeploymentStatusNew, |
|
| 228 |
+ expected: false, |
|
| 229 |
+ }, |
|
| 230 |
+ {
|
|
| 231 |
+ name: "Pending->Pending", |
|
| 232 |
+ current: deployapi.DeploymentStatusPending, |
|
| 233 |
+ next: deployapi.DeploymentStatusPending, |
|
| 234 |
+ expected: false, |
|
| 235 |
+ }, |
|
| 236 |
+ {
|
|
| 237 |
+ name: "Pending->Running", |
|
| 238 |
+ current: deployapi.DeploymentStatusPending, |
|
| 239 |
+ next: deployapi.DeploymentStatusRunning, |
|
| 240 |
+ expected: true, |
|
| 241 |
+ }, |
|
| 242 |
+ {
|
|
| 243 |
+ name: "Pending->Failed", |
|
| 244 |
+ current: deployapi.DeploymentStatusPending, |
|
| 245 |
+ next: deployapi.DeploymentStatusFailed, |
|
| 246 |
+ expected: true, |
|
| 247 |
+ }, |
|
| 248 |
+ {
|
|
| 249 |
+ name: "Pending->Complete", |
|
| 250 |
+ current: deployapi.DeploymentStatusPending, |
|
| 251 |
+ next: deployapi.DeploymentStatusComplete, |
|
| 252 |
+ expected: true, |
|
| 253 |
+ }, |
|
| 254 |
+ {
|
|
| 255 |
+ name: "Running->New", |
|
| 256 |
+ current: deployapi.DeploymentStatusRunning, |
|
| 257 |
+ next: deployapi.DeploymentStatusNew, |
|
| 258 |
+ expected: false, |
|
| 259 |
+ }, |
|
| 260 |
+ {
|
|
| 261 |
+ name: "Running->Pending", |
|
| 262 |
+ current: deployapi.DeploymentStatusRunning, |
|
| 263 |
+ next: deployapi.DeploymentStatusPending, |
|
| 264 |
+ expected: false, |
|
| 265 |
+ }, |
|
| 266 |
+ {
|
|
| 267 |
+ name: "Running->Running", |
|
| 268 |
+ current: deployapi.DeploymentStatusRunning, |
|
| 269 |
+ next: deployapi.DeploymentStatusRunning, |
|
| 270 |
+ expected: false, |
|
| 271 |
+ }, |
|
| 272 |
+ {
|
|
| 273 |
+ name: "Running->Failed", |
|
| 274 |
+ current: deployapi.DeploymentStatusRunning, |
|
| 275 |
+ next: deployapi.DeploymentStatusFailed, |
|
| 276 |
+ expected: true, |
|
| 277 |
+ }, |
|
| 278 |
+ {
|
|
| 279 |
+ name: "Running->Complete", |
|
| 280 |
+ current: deployapi.DeploymentStatusRunning, |
|
| 281 |
+ next: deployapi.DeploymentStatusComplete, |
|
| 282 |
+ expected: true, |
|
| 283 |
+ }, |
|
| 284 |
+ {
|
|
| 285 |
+ name: "Complete->New", |
|
| 286 |
+ current: deployapi.DeploymentStatusComplete, |
|
| 287 |
+ next: deployapi.DeploymentStatusNew, |
|
| 288 |
+ expected: false, |
|
| 289 |
+ }, |
|
| 290 |
+ {
|
|
| 291 |
+ name: "Complete->Pending", |
|
| 292 |
+ current: deployapi.DeploymentStatusComplete, |
|
| 293 |
+ next: deployapi.DeploymentStatusPending, |
|
| 294 |
+ expected: false, |
|
| 295 |
+ }, |
|
| 296 |
+ {
|
|
| 297 |
+ name: "Complete->Running", |
|
| 298 |
+ current: deployapi.DeploymentStatusComplete, |
|
| 299 |
+ next: deployapi.DeploymentStatusRunning, |
|
| 300 |
+ expected: false, |
|
| 301 |
+ }, |
|
| 302 |
+ {
|
|
| 303 |
+ name: "Complete->Failed", |
|
| 304 |
+ current: deployapi.DeploymentStatusComplete, |
|
| 305 |
+ next: deployapi.DeploymentStatusFailed, |
|
| 306 |
+ expected: false, |
|
| 307 |
+ }, |
|
| 308 |
+ {
|
|
| 309 |
+ name: "Complete->Complete", |
|
| 310 |
+ current: deployapi.DeploymentStatusComplete, |
|
| 311 |
+ next: deployapi.DeploymentStatusComplete, |
|
| 312 |
+ expected: false, |
|
| 313 |
+ }, |
|
| 314 |
+ {
|
|
| 315 |
+ name: "Failed->New", |
|
| 316 |
+ current: deployapi.DeploymentStatusFailed, |
|
| 317 |
+ next: deployapi.DeploymentStatusNew, |
|
| 318 |
+ expected: false, |
|
| 319 |
+ }, |
|
| 320 |
+ {
|
|
| 321 |
+ name: "Failed->Pending", |
|
| 322 |
+ current: deployapi.DeploymentStatusFailed, |
|
| 323 |
+ next: deployapi.DeploymentStatusPending, |
|
| 324 |
+ expected: false, |
|
| 325 |
+ }, |
|
| 326 |
+ {
|
|
| 327 |
+ name: "Failed->Running", |
|
| 328 |
+ current: deployapi.DeploymentStatusFailed, |
|
| 329 |
+ next: deployapi.DeploymentStatusRunning, |
|
| 330 |
+ expected: false, |
|
| 331 |
+ }, |
|
| 332 |
+ {
|
|
| 333 |
+ name: "Failed->Complete", |
|
| 334 |
+ current: deployapi.DeploymentStatusFailed, |
|
| 335 |
+ next: deployapi.DeploymentStatusComplete, |
|
| 336 |
+ expected: false, |
|
| 337 |
+ }, |
|
| 338 |
+ {
|
|
| 339 |
+ name: "Failed->Failed", |
|
| 340 |
+ current: deployapi.DeploymentStatusFailed, |
|
| 341 |
+ next: deployapi.DeploymentStatusFailed, |
|
| 342 |
+ expected: false, |
|
| 343 |
+ }, |
|
| 344 |
+ } |
|
| 345 |
+ |
|
| 346 |
+ for _, test := range tests {
|
|
| 347 |
+ got := CanTransitionPhase(test.current, test.next) |
|
| 348 |
+ if got != test.expected {
|
|
| 349 |
+ t.Errorf("%s: expected %t, got %t", test.name, test.expected, got)
|
|
| 350 |
+ } |
|
| 351 |
+ } |
|
| 352 |
+} |