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.

Sign up for GitHub

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

Jump to bottom

Nuke /var/lib/dockershim/sandbox/* while nodes are drained #6110

Merged
merged 1 commit into from
Nov 14, 2017
Merged

Nuke /var/lib/dockershim/sandbox/* while nodes are drained #6110

merged 1 commit into from
Nov 14, 2017

Conversation

sdodson
Copy link
Member

@sdodson sdodson commented Nov 14, 2017 •
edited
Loading

Fixes an issue where 3.6 nodes created checkpoint files which could lead to problems after upgrading to 3.7.
As implemented this will happen during all node upgrades regardless of installed or target version.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1513054

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 14, 2017
@sdodson
Copy link
Member Author

sdodson commented Nov 14, 2017

FYI: @eparis @derekwaynecarr @sjenning

@sdodson
Copy link
Member Author

sdodson commented Nov 14, 2017

/assign mtnbikenc

@sdodson
Copy link
Member Author

sdodson commented Nov 14, 2017

/kind bug
/priority p1

@openshift-ci-robot openshift-ci-robot added kind/bug Categorizes issue or PR as related to a bug. priority/P1 labels Nov 14, 2017
mtnbikenc
mtnbikenc approved these changes Nov 14, 2017
View reviewed changes
Copy link
Member

@mtnbikenc mtnbikenc left a comment

Choose a reason for hiding this comment

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

Is there a bug filed for this? Otherwise,
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 14, 2017
@eparis
Copy link
Member

eparis commented Nov 14, 2017

https://bugzilla.redhat.com/show_bug.cgi?id=1513054

eparis
eparis reviewed Nov 14, 2017
View reviewed changes
roles/openshift_node_upgrade/tasks/main.yml Outdated
@@ -98,6 +98,11 @@
failed_when: false
when: not openshift.common.is_containerized | bool

# Works around a 3.6 bug that left checkpoint files around
- name: Clean up dockershim data
command: rm -rf /var/lib/dockershim/*
Copy link
Member

Choose a reason for hiding this comment

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

/var/lib/dockershim/sandbox/*

Copy link
Member

Choose a reason for hiding this comment

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

For my own personal edification, why not use the files module and state=absent?

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

@openshift-merge-robot openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 14, 2017
@sdodson sdodson changed the title Nuke /var/lib/dockershim/* while nodes are drained Nuke /var/lib/dockershim/sandbox/* while nodes are drained Nov 14, 2017
@eparis
Copy link
Member

eparis commented Nov 14, 2017

so long as the node is drained when this runs, it's fine/good to run on any version.

@sdodson sdodson force-pushed the dockershim branch 2 times, most recently from 6e06e2c to 89bdef0 Compare November 14, 2017 16:48
@mtnbikenc
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 14, 2017
@sdodson
Copy link
Member Author

sdodson commented Nov 14, 2017

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 14, 2017
@sdodson
Copy link
Member Author

sdodson commented Nov 14, 2017

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 14, 2017
@sdodson
Copy link
Member Author

sdodson commented Nov 14, 2017

/test upgrade

@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 14, 2017 •
edited
Loading

@sdodson: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/openshift-jenkins/upgrade 89bdef0 link /test upgrade

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 14, 2017
@sdodson
Copy link
Member Author

sdodson commented Nov 14, 2017

command and shell weren't working as expected, switched to file module. I've verified that /var/lib/dockershim/sandbox is re-created post upgrade as new pods are created.

mtnbikenc
mtnbikenc approved these changes Nov 14, 2017
View reviewed changes
Copy link
Member

@mtnbikenc mtnbikenc left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 14, 2017
@sdodson sdodson merged commit df85c04 into openshift:master Nov 14, 2017
@sdodson
Copy link
Member Author

sdodson commented Nov 14, 2017

/cherrypick release-3.7

@openshift-cherrypick-robot
Copy link

@sdodson: New pull request created: #6113

In response to this:

/cherrypick release-3.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Nov 14, 2017
Merged
sdodson added a commit that referenced this pull request Nov 14, 2017
@sdodson
Merge pull request #6113 from openshift-cherrypick-robot/cherry-pick-…
ffa5aaa 
…6110-to-release-3.7

Automated cherry-pick of #6110 on release-3.7
@danwinship danwinship mentioned this pull request Dec 11, 2017
Merged
@sdodson sdodson deleted the dockershim branch May 11, 2018 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eparis eparis eparis left review comments

@mtnbikenc mtnbikenc mtnbikenc approved these changes

Assignees

@mtnbikenc mtnbikenc

Labels
kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. priority/P1 retest-not-required size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sdodson @eparis @mtnbikenc @openshift-ci-robot @openshift-cherrypick-robot @openshift-merge-robot