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

Jump to bottom

purge: fix purge cluster failed when container images does not exist #4882

Merged
merged 1 commit into from Jan 31, 2020
Merged

purge: fix purge cluster failed when container images does not exist #4882

merged 1 commit into from Jan 31, 2020

Conversation

wujie1993
Copy link
Contributor

@wujie1993 wujie1993 commented Jan 5, 2020 •
edited

fix purge cluster failed when container images does not exist

Signed-off-by: wujie1993 qq594jj@gmail.com

dsavineau
dsavineau suggested changes Jan 7, 2020
View reviewed changes
Copy link
Contributor

@dsavineau dsavineau left a comment

Choose a reason for hiding this comment

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

I don't think this is the right way to do that.
Instead we should use the dashboard_enabled variable (boolean) in a conditional block.
This should also be present in the purge-container-cluster.yml playbook.

@wujie1993 wujie1993 changed the title purge-cluster.yml: fix purge cluster failed purge: fix purge cluster failed when container images does not exist Jan 8, 2020
@wujie1993
Copy link
Contributor Author

Thanks for you reply @dsavineau .Do you mean we should purge node-exporter and grafana-server only when dashboard_enabled is set to true?

@dsavineau
Copy link
Contributor

Thanks for you reply @dsavineau .Do you mean we should purge node-exporter and grafana-server only when dashboard_enabled is set to true?

exactly

@dsavineau
Copy link
Contributor

jenkins test centos-non_container-all_daemons

@dsavineau
Copy link
Contributor

jenkins test centos-non_container-update

dsavineau
dsavineau suggested changes Jan 9, 2020
View reviewed changes
Copy link
Contributor

@dsavineau dsavineau left a comment

Choose a reason for hiding this comment

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

Except my comment inline about the failed_when statement, this looks good.

The only thing you should do is to squash your commits.

infrastructure-playbooks/purge-container-cluster.yml Outdated Show resolved Hide resolved
@dsavineau dsavineau requested a review from guits January 9, 2020 14:52
@wujie1993 wujie1993 force-pushed the fix_purge_cluster_failed branch 2 times, most recently from fcba0b8 to 65816e0 Compare January 10, 2020 07:31
@wujie1993
purge: fix purge cluster failed
263ec30 
Fix purge cluster failed when local container images does not exist.

Purge node-exporter and grafana-server only when dashboard_enabled is set to True.

Signed-off-by: wujie1993 qq594jj@gmail.com
@dsavineau
Copy link
Contributor

jenkins test centos-container-all_daemons

@dsavineau dsavineau mentioned this pull request Jan 15, 2020
Closed
@dsavineau dsavineau mentioned this pull request Jan 27, 2020
Closed
@dsavineau dsavineau added this to the v4.1 milestone Jan 31, 2020
@dsavineau dsavineau merged commit d8b0b3c into ceph:master Jan 31, 2020
@mergify mergify bot mentioned this pull request Jan 31, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dsavineau dsavineau dsavineau approved these changes

@guits guits Awaiting requested review from guits

Assignees
No one assigned
Labels
backport-stable-4.0 enhancement
Projects
None yet
Milestone
v4.1
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@wujie1993 @dsavineau