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

Create lvm exclude filter #107

Merged
merged 8 commits into from Sep 29, 2020
Merged

Create lvm exclude filter #107

merged 8 commits into from Sep 29, 2020

Conversation

pkesavap
Copy link
Member

@pkesavap pkesavap commented Sep 29, 2020

Before RHHI-V Deployment , certain tasks (that might involve brick creation, or Day2 operation like creation of new volume, expanding the cluster or volume,) require removal of existing LVM filter

to avoid errors like " Device /dev/sdf excluded by a filter."

Post RHHI-V Deployment , certain tasks (that might involve brick creation, or Day2 operation like creation of new volume, expanding the cluster or volume,) require  removal of   existing LVM filter

to avoid errors like " Device /dev/sdf excluded by a filter."
Post RHHI-V Deployment , certain tasks (that might involve brick creation, or Day2 operation like creation of new volume, expanding the cluster or volume,) require  removal of   existing LVM filter

to avoid errors like " Device /dev/sdf excluded by a filter."
Create regenerate_new_lvm_filter_rules.yml
@pkesavap
Copy link
Member Author

This PR contains two independent task

  1. Create lvm exclude filter
    this tasks backups existing filter and removes the current filter
  2. Create regenerate_new_lvm_filter_rules.yml
    this task regenerates new lvm filter rules

The reason why its created as two independent task, is that it can be further called or included in gluster-ansible tasks wherever applicable, so that code need not be repeated.

@pkesavap
Copy link
Member Author

Requesting review :- @gobindadas @satheesaran

- name: Backup lvm.conf file
copy:
src: /etc/lvm/lvm.conf
dest: /backup
Copy link
Member

Choose a reason for hiding this comment

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

Will this directory be available? isn't it better to backup to same location with different name?

Copy link
Member Author

Choose a reason for hiding this comment

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

okay,

Choose a reason for hiding this comment

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

This part of the code is not required. This information is added in the doc, as the user is prone to make such mistakes while editing manually, as this is covered in ansible, this code block is safe to be ignored

Copy link
Member Author

Choose a reason for hiding this comment

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

@satheesaran , meaning backup is not necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@sabose
Copy link
Member

sabose commented Sep 29, 2020

@pkesavap "Post RHHI-V Deployment " -> post or before the deployment?

Moved the task to first and end of file
Removed backing up task
@pkesavap
Copy link
Member Author

pkesavap commented Sep 29, 2020

@pkesavap "Post RHHI-V Deployment " -> post or before the deployment?

done, set to before

@pkesavap
Copy link
Member Author

pkesavap commented Sep 29, 2020

@satheesaran @gobindadas have updated the discussed changes,

@gobindadas
Copy link
Collaborator

Looks good



- name: Exclue LVM Filter rules
import_tasks: lvm_exclude_filter.yml
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to be move to very first, otherwise blacklist mpath also will fail.

copy:
src: /etc/lvm/lvm.conf
dest: /backup

- name: Remove the existing LVM filter
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this fail if the pattern is not exist incase of deployment failure in middle before restore?
Should we need to add ignore_error=true ?

@gobindadas gobindadas merged commit cccc202 into master Sep 29, 2020
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.

None yet

4 participants