Common: changed civetweb line in rgw section(conf) #1731

SirishaGuduru · 2017-07-31T11:03:54Z

Resolves issue: Multiple RGW Ceph.conf Issue #1258

In multi-RGW setup, in ceph.conf the RGW sections
contain identical bind IP in civetweb line. So this
modification fixes that issue and puts the right IP
for each RGW.

Closes: #1767
Signed-off-by: SirishaGuduru SGuduru@walmartlabs.com

leseb · 2017-07-31T12:12:45Z

jenkins test this please

leseb

You have to re-use this entire block in your rgw code: https://github.com/SirishaGuduru/ceph-ansible/blob/341af20e6820a924df11cb983aaa674a07954606/roles/ceph-common/templates/ceph.conf.j2#L61-L80

SirishaGuduru · 2017-08-02T08:34:57Z

@leseb, In this section "https://github.com/SirishaGuduru/ceph-ansible/blob/341af20e6820a924df11cb983aaa674a07954606/roles/ceph-common/templates/ceph.conf.j2#L67-L73" monitor address is basically the addresses of mon nodes. If we have RGWs residing on different nodes other than mon nodes, how will this work? And this condition gives the same IP for all RGW sections.

leseb · 2017-08-02T08:59:34Z

@SirishaGuduru sorry I might have been not very clear. You really need to re-use this entire block https://github.com/SirishaGuduru/ceph-ansible/blob/341af20e6820a924df11cb983aaa674a07954606/roles/ceph-common/templates/ceph.conf.j2#L67-L80, leave the block thing but you have to create a set of variables for rgw:

raadosgw_interface
radosgw_address

Then, you need to make these variables mandatory by adding check in the rgw role, see examples from the mon here: https://github.com/ceph/ceph-ansible/blob/149f21b2eff8d37c979148fb838bb73bfd5b00a6/roles/ceph-mon/tasks/check_mandatory_vars.yml

Hope it's clearer now.
Thanks!

SirishaGuduru · 2017-08-03T06:18:54Z

@leseb, its clear now. I will look at how mon is implemented and try doing the same.

leseb · 2017-08-03T08:39:52Z

@SirishaGuduru thanks!

leseb

You don't need to delete manually, you need to rebase your branch on master and then push force

SirishaGuduru · 2017-08-04T08:34:35Z

@leseb , I did try rebasing master. There was a merge conflict with this file on my branch. Hence had to do a commit. Also since, my PR has changes in this file.

leseb · 2017-08-04T08:45:44Z

@SirishaGuduru if there is a conflict you have to resolve it then you can pursue you rebase and then push cleanly.

guits · 2017-08-07T09:30:52Z

@SirishaGuduru I think you forgot to modify the roles/ceph-defaults/defaults/main.yml.

You shouldn't edit *.yml.sample files directly.
Basically you edit roles/ceph-*/defaults/main.yml and then you execute generate_group_vars_sample.sh at root to generate the *.yml.sample files

SirishaGuduru · 2017-08-07T09:39:40Z

@guits, sorry I didn't change this roles/ceph-defaults/defaults/main.yml as I didn't realize the recent commits, but when I run this generate_group_vars_sample.sh, some other sample files are also being changed. Is there a way I can just run this for the changes in all.yml.sample (because my changes are only in that file)?

guits · 2017-08-07T09:47:47Z

As far as I know, if you rebased well on master, you shouldn't have any other changes except yours when running generate_group_vars_sample.sh

SirishaGuduru · 2017-08-07T10:07:56Z

@guits, I'm well on par with master, except for my commit in my branch. But still when I run generate_group_vars_sample.sh, I see modified: group_vars/all.yml.sample modified: group_vars/osds.yml.sample modified: group_vars/rhcs.yml.sample
where group_vars/all.yml.sample is with my changes.

guits · 2017-08-07T11:38:52Z

@SirishaGuduru it doesn't matter, commit the changes and push them, unless I see something wrong it should be ok.

SirishaGuduru · 2017-08-08T05:05:26Z

@leseb, @guits, I have made necessary changes. Please review.

guits · 2017-08-08T06:48:51Z

roles/ceph-defaults/defaults/main.yml

@@ -294,7 +294,15 @@ radosgw_civetweb_num_threads: 100
 # For additional civetweb configuration options available such as SSL, logging,
 # keepalive, and timeout settings, please see the civetweb docs at
 # https://github.com/civetweb/civetweb/blob/master/docs/UserManual.md
-radosgw_civetweb_options: "port={{ radosgw_civetweb_bind_ip }}:{{ radosgw_civetweb_port }} num_threads={{ radosgw_civetweb_num_threads }}"
+#radosgw_civetweb_options: "num_threads={{ radosgw_civetweb_num_threads }}"


all the variables declared here should be uncommented.

SirishaGuduru · 2017-08-10T04:20:08Z

@leseb, @guits can you please review ithis?

guits · 2017-08-10T06:43:22Z

@SirishaGuduru
Tests need to be updated, could you update them?

SirishaGuduru · 2017-08-11T05:42:44Z

@guits will do that. Do you have any specific tests that I need to target on? or should I go with how monitor_interface is used in tests?

guits · 2017-08-11T06:53:32Z

@SirishaGuduru yes, reproduce the same thing which has been made for monitor_interface/monitor_address for each test, please

guits · 2017-08-17T20:01:58Z

jenkins test luminous-ansible2.3-update_dmcrypt

guits · 2017-08-17T20:02:56Z

jenkins test luminous-ansible2.3-docker_dmcrypt_journal_collocation

SirishaGuduru · 2017-08-18T05:03:53Z

@guits, does this look good?

SirishaGuduru · 2017-08-18T05:41:22Z

@gfidente, in this PR I have implemented radosgw_address_block just as "monitor_address_block" is done.

gfidente · 2017-08-18T10:29:55Z

@SirishaGuduru yes this is great indeed, I pointed to this PR from issue #1767 ; I don't think it addresses setting the civetweb bind ip address for the docker container; also see PR #1773

leseb

One thing only, please run ./generate_group_vars_sample.sh. Do not edit group_vars/rhcs.yml.sample. Just let the script run and do its business :). Thanks! Once we have that I'll merge.

SirishaGuduru · 2017-08-22T02:58:46Z

@leseb , I did not edit this file manually. This got modified when I ran generate_group_vars_sample.sh

leseb

Can you rebase on master and then run ./generate_group_vars_sample.sh. We are almost there, thanks!

leseb · 2017-08-22T12:04:25Z

roles/ceph-rgw/tasks/check_mandatory_vars.yml

@@ -0,0 +1,8 @@
+---


Having a check in roles/ceph-common/tasks/checks/check_mandatory_vars.yml is sufficient, so you can remove this one.

Resolves issue: Multiple RGW Ceph.conf Issue ceph#1258 In multi-RGW setup, in ceph.conf the RGW sections contain identical bind IP in civetweb line. So this modification fixes that issue and puts the right IP for each RGW. Signed-off-by: SirishaGuduru SGuduru@walmartlabs.com Modified ceph-defaults and ran generate_group_vars_sample.sh group_vars/osds.yml.sample and group_vars/rhcs.yml.sample are not part of the changes. But they got modified when generate_group_vars_sample.sh is ran to generate group_vars/ all.yml.sample. Uncommented added variables in ceph-defaults Updated tests by adding value for radosgw_interface Added radosgw_interface to centos cluster tests Modified ceph-rgw role,rebased and ran generate_group_vars_sample.sh In ceph-rgw role removed check_mandatory_vars.yml. Rebased on master. Ran generate_group_vars_sample.sh and then the below files got modified.

leseb · 2017-08-23T09:49:37Z

jenkins test luminous-ansible2.3-bluestore_dmcrypt_journal

leseb · 2017-08-23T09:49:48Z

jenkins test luminous-ansible2.3-update_docker_cluster

leseb · 2017-08-23T09:49:55Z

jenkins test luminous-ansible2.3-xenial_cluster

leseb · 2017-08-23T09:50:07Z

jenkins test luminous-ansible2.3-update_dmcrypt

leseb · 2017-08-23T09:53:45Z

jenkins test ansible2.3-bluestore_docker_dmcrypt_journal_collocation

leseb · 2017-08-23T09:55:55Z

@SirishaGuduru did you run ./generate_group_vars_sample.sh?
Your group vars files are wrong, if I pull your branch and run it, I get the following diff: https://pastebin.com/qQKddyRM

Thanks!

leseb approved these changes Jul 31, 2017

View reviewed changes

leseb requested changes Jul 31, 2017

View reviewed changes

leseb requested changes Aug 4, 2017

View reviewed changes

SirishaGuduru force-pushed the rgw-civetwebIP-conf branch from 8d859fc to 8224d73 Compare August 4, 2017 08:50

SirishaGuduru closed this Aug 7, 2017

SirishaGuduru force-pushed the rgw-civetwebIP-conf branch from aa52db4 to 3b6d1bf Compare August 7, 2017 09:15

SirishaGuduru reopened this Aug 7, 2017

guits requested changes Aug 8, 2017

View reviewed changes

SirishaGuduru force-pushed the rgw-civetwebIP-conf branch from ccad501 to 4ed17aa Compare August 8, 2017 09:25

SirishaGuduru force-pushed the rgw-civetwebIP-conf branch from 5ccd5de to feccd20 Compare August 17, 2017 06:19

gfidente mentioned this pull request Aug 17, 2017

Set mds_address_block and radosgw_address_block like monitor_address_block #1767

Closed

leseb requested changes Aug 21, 2017

View reviewed changes

SirishaGuduru force-pushed the rgw-civetwebIP-conf branch from 8e34389 to fe15042 Compare August 22, 2017 04:56

leseb requested changes Aug 22, 2017

View reviewed changes

SirishaGuduru force-pushed the rgw-civetwebIP-conf branch 3 times, most recently from e7fee01 to 4289864 Compare August 23, 2017 09:31

SirishaGuduru force-pushed the rgw-civetwebIP-conf branch from 4289864 to 1359869 Compare August 23, 2017 09:33

leseb merged commit d9b3d4a into ceph:master Aug 23, 2017

Common: changed civetweb line in rgw section(conf) #1731

Common: changed civetweb line in rgw section(conf) #1731

Conversation

SirishaGuduru commented Jul 31, 2017 • edited by leseb

leseb commented Jul 31, 2017

leseb left a comment

Choose a reason for hiding this comment

SirishaGuduru commented Aug 2, 2017

leseb commented Aug 2, 2017

SirishaGuduru commented Aug 3, 2017

leseb commented Aug 3, 2017

leseb left a comment

Choose a reason for hiding this comment

SirishaGuduru commented Aug 4, 2017 • edited

leseb commented Aug 4, 2017

guits commented Aug 7, 2017 • edited

SirishaGuduru commented Aug 7, 2017 • edited

guits commented Aug 7, 2017

SirishaGuduru commented Aug 7, 2017

guits commented Aug 7, 2017

SirishaGuduru commented Aug 8, 2017

guits Aug 8, 2017

Choose a reason for hiding this comment

SirishaGuduru commented Aug 10, 2017

guits commented Aug 10, 2017

SirishaGuduru commented Aug 11, 2017

guits commented Aug 11, 2017

guits commented Aug 17, 2017

guits commented Aug 17, 2017

SirishaGuduru commented Aug 18, 2017

SirishaGuduru commented Aug 18, 2017

gfidente commented Aug 18, 2017

leseb left a comment

Choose a reason for hiding this comment

SirishaGuduru commented Aug 22, 2017

leseb left a comment

Choose a reason for hiding this comment

leseb Aug 22, 2017

Choose a reason for hiding this comment

leseb commented Aug 23, 2017

leseb commented Aug 23, 2017

leseb commented Aug 23, 2017

leseb commented Aug 23, 2017

leseb commented Aug 23, 2017

leseb commented Aug 23, 2017

SirishaGuduru commented Jul 31, 2017 •

edited by leseb

SirishaGuduru commented Aug 4, 2017 •

edited

guits commented Aug 7, 2017 •

edited

SirishaGuduru commented Aug 7, 2017 •

edited