Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deploy: remove deployer pods on cancellation #9291

Merged
merged 1 commit into from
Jun 14, 2016
Merged

deploy: remove deployer pods on cancellation #9291

merged 1 commit into from
Jun 14, 2016

Conversation

0xmichalis
Copy link
Contributor

@0xmichalis
Copy link
Contributor Author

Opened #9296 which is related

@smarterclayton
Copy link
Contributor

[test]

@0xmichalis
Copy link
Contributor Author

• Failure [24.149 seconds]
deploymentconfigs
/data/src/github.com/openshift/origin/test/extended/deployments/deployments.go:309
  when run iteratively
  /data/src/github.com/openshift/origin/test/extended/deployments/deployments.go:131
    should only deploy the last deployment [Conformance] [It]
    /data/src/github.com/openshift/origin/test/extended/deployments/deployments.go:130

    Expected error:
        <*errors.errorString | 0xc8201c1630>: {
            s: "found multiple running deployments: [deployment-simple-1 deployment-simple-2 deployment-simple-3 deployment-simple-4]",
        }
        found multiple running deployments: [deployment-simple-1 deployment-simple-2 deployment-simple-3 deployment-simple-4]
    not to have occurred

    /data/src/github.com/openshift/origin/test/extended/deployments/deployments.go:119

The failure is real but I think it has to do with the way the deployer pods are listed in one shot in the extended utilities for the deployments. Looking...

@0xmichalis
Copy link
Contributor Author

Worth noting that I don't see anywhere in the output the defered function that dumps the dc and its rcs and deployer pods.

@0xmichalis
Copy link
Contributor Author

Ok, so we cannot just transition the deployment to Failed when we delete the deployers because the deployment config controller will notice it and create a new deployment maybe before the previous deployer pod is deleted. We can transition to Failed once we make sure the deployer pod does not exist. I think we will need a pod cache to requeue on DELETE events (we will need to requeue on every event for #9296 anyway).

@0xmichalis
Copy link
Contributor Author

Green on the extended test, broken unit test

return err
case err != nil:
// We'll try again later on resync. Continue to process cancellations.
glog.V(2).Infof("Error getting deployer pod %s for deployment %s: %#v", deployerPodName, deployutil.LabelForDeployment(deployment), err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v(4) probably.

@smarterclayton
Copy link
Contributor

Changes look ok

@0xmichalis
Copy link
Contributor Author

yum flake, repushed to fix the glog level

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 7a7770e

@smarterclayton
Copy link
Contributor

Lgtm [merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented Jun 14, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4829/) (Image: devenv-rhel7_4371)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 7a7770e

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/4829/)

@openshift-bot openshift-bot merged commit abc8da1 into openshift:master Jun 14, 2016
@0xmichalis 0xmichalis deleted the delete-cancelled-deployers branch June 14, 2016 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants