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
Uninstall playbook for Glusterfs #6918
Uninstall playbook for Glusterfs #6918
Conversation
Can one of the admins verify this patch?
|
@jarrpa Please review |
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.
- The desired solution is to just do a standalone uninstall playbook, not a conditional variable, so get rid of the
openshift_storage_glusterfs_state
variable. - With this implementation we don't need the
wipe
variable anymore. Remove that as a separate commit in this PR, and make sure to keep all of the functionality it enables. - There's no need to have
| default
after every use ofglusterfs_name
, that's already done.
40cfa5c
to
94608cc
Compare
So, you mean to call uninstall playbook directly like:
Sure, duplicate code needs to be removed.
Sure. will remove it. |
5cd4619
to
07e7ffb
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.
A few things:
- Rename the
glusterfs_uninstall.yml
files to justuninstall.yml
. - Don't forget to update
registry.yml
to use the new facts file.
@@ -0,0 +1,96 @@ | |||
--- | |||
|
|||
- include_tasks: glusterfs_config_facts.yml |
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.
- block:
- include_tasks: glusterfs_config_facts.yml
- include_tasks: glusterfs_uninstall.yml
when: 'glusterfs' in groups and groups['glusterfs'] | length > 0
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.
Done
- kind: "svc" | ||
name: "heketi-storage-endpoints" | ||
- kind: "secret" | ||
name: "heketi-{{ glusterfs_name }}-topology-secret" |
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.
It would be good if we could have an optional section to wipe pv, vg, lv
. So development would be easier when reusing same nodes... as now we would need to go and delete residual disk configuration.
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.
@mjudeikis Sure - doing it..
- block: | ||
- include_tasks: glusterfs_config_facts.yml | ||
- include_tasks: glusterfs_uninstall.yml | ||
when: groups.glusterfs | length > 0 |
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.
The group may not always be defined, so check that with 'glusterfs_registry' in groups and groups['glusterfs_registry'] | length > 0
.
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 have updated it - please check
- kind: daemonset | ||
name: "glusterfs-{{ glusterfs_name }}" | ||
- kind: storageclass | ||
name: "glusterfs-{{ glusterfs_name }}" |
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.
We should also delete the glusterblock-provisioner pod. I believe that's part of a DeploymentConfig.
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 dont' see glusterblock provisioner starting up as part of default CNS setup. Also, all the resources in glusterfs namespace gets deleted.
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.
It should be deploying by default, because you have to specify --no-block
to not deploy it. Regardless, since we COULD deploy it it should be covered by the uninstall.
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.
Note that we'll need to cover the uninstall of gluster-s3, but I say we can do that later.
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.
sure - will check about the block part
- name: Run glusterfs uninstall role | ||
include_role: | ||
name: openshift_storage_glusterfs | ||
tasks_from: uninstall.yml |
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.
You've changed the file name here but not the actual file's name.
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.
Now, we have two files at roles/openshift_storage_glusterfs/tasks/ 1. uninstall.yml 2. glusterfs_uninstall.yml
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.
uninstall.yml is the top level file there.
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.
...OH RIGHT! I forget my own words sometimes, sorry. :)
Signed-off-by: Saravanakumar Arumugam <sarumuga@redhat.com>
c7c73b0
to
81033d5
Compare
- name: Run glusterfs uninstall role | ||
include_role: | ||
name: openshift_storage_glusterfs | ||
tasks_from: uninstall.yml |
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.
...OH RIGHT! I forget my own words sometimes, sorry. :)
- kind: daemonset | ||
name: "glusterfs-{{ glusterfs_name }}" | ||
- kind: storageclass | ||
name: "glusterfs-{{ glusterfs_name }}" |
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.
It should be deploying by default, because you have to specify --no-block
to not deploy it. Regardless, since we COULD deploy it it should be covered by the uninstall.
register: devices_info | ||
delegate_to: "{{ item }}" | ||
with_items: "{{ glusterfs_nodes | default([]) }}" | ||
failed_when: False |
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.
You should use a variable to allow for specifying whether this gets done or not. I saw appropriate the wipe
variable for this.
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.
Done
- block: | ||
- include_tasks: glusterfs_config_facts.yml | ||
- include_tasks: glusterfs_uninstall.yml | ||
when: "'glusterfs' in groups and groups['glusterfs'] | length > 0" |
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.
Move the indentation two spaces to the left. This applies it to the whole block, not the last task.
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.
Done
- block: | ||
- include_tasks: glusterfs_registry_facts.yml | ||
- include_tasks: glusterfs_uninstall.yml | ||
when: "'glusterfs_registry' in groups and groups['glusterfs_registry'] | length > 0" |
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.
Move the indentation two spaces to the left. This applies it to the whole block, not the last task.
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.
Done
- kind: daemonset | ||
name: "glusterfs-{{ glusterfs_name }}" | ||
- kind: storageclass | ||
name: "glusterfs-{{ glusterfs_name }}" |
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.
Note that we'll need to cover the uninstall of gluster-s3, but I say we can do that later.
/ok-to-test |
/test all [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue. |
@SaravanaStorageNetwork: The following test failed, say
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. |
This reimpliments openshift#6402 which was missed by openshift#6918. Signed-off-by: Jose A. Rivera <jarrpa@redhat.com>
This reimpliments openshift#6402 which was missed by openshift#6918. Signed-off-by: Jose A. Rivera <jarrpa@redhat.com>
…ll_playbook Automatic merge from submit-queue. Uninstall playbook for Glusterfs Uninstall playbook for Glusterfs
Automatic merge from submit-queue. GlusterFS: Fix uninstall regression This reimpliments openshift#6402 which was missed by openshift#6918. Signed-off-by: Jose A. Rivera <jarrpa@redhat.com>
…ll_playbook Automatic merge from submit-queue. Uninstall playbook for Glusterfs Uninstall playbook for Glusterfs
Automatic merge from submit-queue. GlusterFS: Fix uninstall regression This reimpliments openshift#6402 which was missed by openshift#6918. Signed-off-by: Jose A. Rivera <jarrpa@redhat.com>
…ll_playbook Automatic merge from submit-queue. Uninstall playbook for Glusterfs Uninstall playbook for Glusterfs
Automatic merge from submit-queue. GlusterFS: Fix uninstall regression This reimpliments openshift#6402 which was missed by openshift#6918. Signed-off-by: Jose A. Rivera <jarrpa@redhat.com>
…ll_playbook Automatic merge from submit-queue. Uninstall playbook for Glusterfs Uninstall playbook for Glusterfs
Automatic merge from submit-queue. GlusterFS: Fix uninstall regression This reimpliments openshift#6402 which was missed by openshift#6918. Signed-off-by: Jose A. Rivera <jarrpa@redhat.com>
…ll_playbook Automatic merge from submit-queue. Uninstall playbook for Glusterfs Uninstall playbook for Glusterfs
Automatic merge from submit-queue. GlusterFS: Fix uninstall regression This reimpliments openshift#6402 which was missed by openshift#6918. Signed-off-by: Jose A. Rivera <jarrpa@redhat.com>
…ll_playbook Automatic merge from submit-queue. Uninstall playbook for Glusterfs Uninstall playbook for Glusterfs
Automatic merge from submit-queue. GlusterFS: Fix uninstall regression This reimpliments openshift#6402 which was missed by openshift#6918. Signed-off-by: Jose A. Rivera <jarrpa@redhat.com>
…ll_playbook Automatic merge from submit-queue. Uninstall playbook for Glusterfs Uninstall playbook for Glusterfs
Automatic merge from submit-queue. GlusterFS: Fix uninstall regression This reimpliments openshift#6402 which was missed by openshift#6918. Signed-off-by: Jose A. Rivera <jarrpa@redhat.com>
…ll_playbook Automatic merge from submit-queue. Uninstall playbook for Glusterfs Uninstall playbook for Glusterfs
Automatic merge from submit-queue. GlusterFS: Fix uninstall regression This reimpliments openshift#6402 which was missed by openshift#6918. Signed-off-by: Jose A. Rivera <jarrpa@redhat.com>
Automatic merge from submit-queue. GlusterFS: Fix uninstall regression This reimpliments openshift#6402 which was missed by openshift#6918. Signed-off-by: Jose A. Rivera <jarrpa@redhat.com>
This reimpliments openshift#6402 which was missed by openshift#6918. Signed-off-by: Jose A. Rivera <jarrpa@redhat.com>
Uninstall playbook for Glusterfs