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

rgw: extend automatic rgw pool creation capability #4445

Closed
wants to merge 1 commit into from

Conversation

alimaredia
Copy link
Contributor

Signed-off-by: Ali Maredia amaredia@redhat.com

@alimaredia alimaredia changed the title rgw: extend automatic rgw pool creation capability [DNM] rgw: extend automatic rgw pool creation capability Sep 10, 2019
@alimaredia alimaredia added the DNM Do NOT merge label Sep 10, 2019
roles/ceph-rgw/tasks/rgw_create_pools.yml Outdated Show resolved Hide resolved
roles/ceph-rgw/tasks/rgw_create_pools.yml Outdated Show resolved Hide resolved
group_vars/rgws.yml.sample Outdated Show resolved Hide resolved
roles/ceph-rgw/defaults/main.yml Outdated Show resolved Hide resolved
roles/ceph-rgw/tasks/rgw_create_pools.yml Outdated Show resolved Hide resolved
roles/ceph-rgw/tasks/rgw_create_pools.yml Outdated Show resolved Hide resolved
@guits
Copy link
Collaborator

guits commented Sep 18, 2019

@alimaredia when you have a moment 🙂

@alimaredia alimaredia force-pushed the wip-rgw-multisite-create-pools branch 6 times, most recently from 6f435c9 to 0663a38 Compare September 20, 2019 21:49
@alimaredia
Copy link
Contributor Author

@guits @dsavineau: This PR should be ready for final review. I made some changes in the tests directory, let me know if there's anything else that needs to be done there.

@alimaredia alimaredia changed the title [DNM] rgw: extend automatic rgw pool creation capability rgw: extend automatic rgw pool creation capability Sep 23, 2019
@alimaredia alimaredia removed the DNM Do NOT merge label Sep 23, 2019
when:
- item.value.pool_type is defined
- item.value.pool_type == 'ec'
ignore_errors: yes
Copy link
Collaborator

Choose a reason for hiding this comment

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

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason we had this was if the playbook gets run again. When the playbook gets run again, it will try to rm the ec_profile and we didn't want it to fail if the ec_profile was already created and being used.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@alimaredia If we really want to stick to ignore errors, failed_when: false should be enough

- item.value.pool_type == 'ec'

- name: set crush rule
command: "{{ container_exec_cmd }} ceph --connect-timeout 5 --cluster {{ cluster }} osd crush rule create-erasure {{ item.key }} {{ item.value.ec_profile }}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about this command?
If the playbook is run again, will it return 0 even though it's already created? Otherwise, we might need a failed_when: false here as well

Copy link
Collaborator

Choose a reason for hiding this comment

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

roles/ceph-rgw/tasks/main.yml Show resolved Hide resolved
roles/ceph-validate/tasks/check_rgw_pools.yml Outdated Show resolved Hide resolved
Signed-off-by: Ali Maredia <amaredia@redhat.com>
@alimaredia alimaredia force-pushed the wip-rgw-multisite-create-pools branch from 3f02b2a to 325b2a9 Compare October 2, 2019 14:45
@dsavineau
Copy link
Contributor

Closing in flavor of #4688

@dsavineau dsavineau closed this Oct 28, 2019
@dsavineau dsavineau deleted the wip-rgw-multisite-create-pools branch October 28, 2019 19:35
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

3 participants