-
Notifications
You must be signed in to change notification settings - Fork 68
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
Cleanup role for tripleo services #649
Cleanup role for tripleo services #649
Conversation
Skipping CI for Draft Pull Request. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jpodivin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/9c1662b4c35b49fb8ff66653788108da ✔️ openstack-k8s-operators-content-provider SUCCESS in 49m 41s |
Zuul encountered a syntax error while parsing its configuration in the Job edpm_tripleo_cleanup not defined The error appears in the following project stanza: project: in "openstack-k8s-operators/edpm-ansible/zuul.d/projects.yaml@main", line 2, column 3 |
3b73a6f
to
cc0a178
Compare
98d8941
to
fd7a1c1
Compare
recheck - new tests |
94844e6
to
b8602f9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me.
I rechecked the data-plane-adoption patch using the new playbook openstack-k8s-operators/data-plane-adoption#438 to see if the newer versions of this PR works in a real env.
We are in a beta freeze and I'm not sure we want to land this now. I let others from the dataplane team decide on that.
Tests and docs included Signed-off-by: Jiri Podivin <[email protected]>
edpm_remove_tripleo_unit_files: | ||
type: "bool" | ||
description: | | ||
Remove unit files after disabling services. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in two minds about this one. where do we draw the cleanup line? It is one thing to stop the service it is another to cleanup files/packages etc. (yes I realise you're not removing packages here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point I don't really care much either way. The cleanup procedure can be adjusted, or left with defaults. One way or another, given the number of files involving tripleo, we are going to miss something. After all, we have been adopting without cleanup up until now.
What is critical, is that no tripleo processes are running, or can spontaneously start up after adoption. As that could cause damage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think when we start doing operations on the data plane we are already beyond "point of no return" in terms of rollback so i don't think we'd be reinstating the old services. But i think it's nice that there is a variable to control the removal, thanks.
This passed in the adoption CI and the beta freeze is over, so IMO "merge and iterate if necessary" is the way :) |
/lgtm |
c420dec
into
openstack-k8s-operators:main
Just a role with documentation and tests. Actual invocations of the role, along with removals of other cleanup routines, will be included in follow up PRs.