Skip to content

Commit

Permalink
Fix buggy ownership logic for Pods in Deployment awaiter (#311)
Browse files Browse the repository at this point in the history
The Deployment awaiter was erroneously overwriting aggregated
Pod errors if they had the same name due to buggy ownership checking.
Fixed the ownership logic and the relevant tests.
  • Loading branch information
lblackstone authored Dec 4, 2018
1 parent 52fad2d commit 8b1457a
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 9 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ install:
before_script:
- "${PULUMI_SCRIPTS}/ci/ensure-dependencies"
script:
- travis_wait 30 make travis_${TRAVIS_EVENT_TYPE}
- travis_wait 60 make travis_${TRAVIS_EVENT_TYPE}
after_failure:
- "${PULUMI_SCRIPTS}/ci/upload-failed-tests"
notifications:
Expand Down
14 changes: 8 additions & 6 deletions pkg/await/apps_deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package await
import (
"fmt"
"reflect"
"strings"
"time"

"github.com/golang/glog"
Expand Down Expand Up @@ -459,7 +458,7 @@ func (dia *deploymentInitAwaiter) checkReplicaSetStatus() {
// we would now have fewer replicas than we had requested in the `Deployment` we last submitted
// when we last ran `pulumi up`.
rawSpecReplicas, specReplicasExists := openapi.Pluck(rs.Object, "spec", "replicas")
specReplicas, _ := rawSpecReplicas.(float64)
specReplicas, _ := rawSpecReplicas.(int64)
if !specReplicasExists {
specReplicas = 1
}
Expand Down Expand Up @@ -505,8 +504,6 @@ func (dia *deploymentInitAwaiter) changeTriggeredRollout() bool {
}

func (dia *deploymentInitAwaiter) processPodEvent(event watch.Event) {
inputDeploymentName := dia.config.currentInputs.GetName()

pod, isUnstructured := event.Object.(*unstructured.Unstructured)
if !isUnstructured {
glog.V(3).Infof("Pod watch received unknown object type '%s'",
Expand All @@ -515,10 +512,11 @@ func (dia *deploymentInitAwaiter) processPodEvent(event watch.Event) {
}

// Check whether this Pod was created by our Deployment.
podName := pod.GetName()
if !strings.HasPrefix(podName+"-", inputDeploymentName) {
currentReplicaSet := dia.replicaSets[dia.currentGeneration]
if !isOwnedBy(pod, currentReplicaSet) {
return
}
podName := pod.GetName()

// If Pod was deleted, remove it from our aggregated checkers.
if event.Type == watch.Deleted {
Expand Down Expand Up @@ -653,6 +651,10 @@ func canonicalizeDeploymentAPIVersion(ver string) string {
}

func isOwnedBy(obj, possibleOwner *unstructured.Unstructured) bool {
if possibleOwner == nil {
return false
}

var possibleOwnerAPIVersion string

// Canonicalize apiVersion for Deployments.
Expand Down
6 changes: 4 additions & 2 deletions pkg/await/apps_deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ func Test_Apps_Deployment(t *testing.T) {
// Failed Pod should show up in the errors.
pods <- watchAddedEvent(deployedFailedPod(inputNamespace, readyPodName, replicaSetGeneratedName))

// // Unrelated successful Pod should generate no errors.
// Unrelated successful Pod should generate no errors.
pods <- watchAddedEvent(deployedReadyPod(inputNamespace, readyPodName, "bar"))

// Timeout. Failure.
Expand All @@ -496,6 +496,7 @@ func Test_Apps_Deployment(t *testing.T) {
object: deploymentProgressing(inputNamespace, deploymentInputName, revision1),
subErrors: []string{
"Minimum number of live Pods was not attained",
`1 Pods failed to run because: [ImagePullBackOff] Back-off pulling image "sdkjlsdlkj"`,
}},
},
{
Expand All @@ -511,7 +512,7 @@ func Test_Apps_Deployment(t *testing.T) {
// Failed Pod should show up in the errors.
pods <- watchAddedEvent(deployedFailedPod(inputNamespace, readyPodName, replicaSetGeneratedName))

// // Unrelated successful Pod should generate no errors.
// Unrelated successful Pod should generate no errors.
pods <- watchAddedEvent(deployedReadyPod(inputNamespace, readyPodName, "bar"))

// Timeout. Failure.
Expand All @@ -521,6 +522,7 @@ func Test_Apps_Deployment(t *testing.T) {
object: deploymentProgressing(inputNamespace, deploymentInputName, revision2),
subErrors: []string{
"Minimum number of live Pods was not attained",
`1 Pods failed to run because: [ImagePullBackOff] Back-off pulling image "sdkjlsdlkj"`,
}},
},
{
Expand Down

0 comments on commit 8b1457a

Please sign in to comment.