mgr/dashboard: rgw policy group management api

[nizamial09]

Contribution Guidelines

• To sign and title your commits, please refer to Submitting Patches to Ceph.

• If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

• When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

• Tracker (select at least one)
  • References tracker ticket
  • Very recent bug; references commit where it was introduced
  • New feature (ticket optional)
  • Doc update (no ticket needed)
  • Code cleanup (no ticket needed)
• Component impact
  • Affects Dashboard, opened tracker ticket
  • Affects Orchestrator, opened tracker ticket
  • No impact that needs to be tracked
• Documentation (select at least one)
  • Updates relevant documentation
  • No doc update is appropriate
• Tests (select at least one)
  • Includes unit test(s)
  • Includes integration test(s)
  • Includes bug reproducer
  • No tests

Show available Jenkins commands

• jenkins retest this please
• jenkins test classic perf
• jenkins test crimson perf
• jenkins test signed
• jenkins test make check
• jenkins test make check arm64
• jenkins test submodules
• jenkins test dashboard
• jenkins test dashboard cephadm
• jenkins test api
• jenkins test docs
• jenkins render docs
• jenkins test ceph-volume all
• jenkins test ceph-volume tox
• jenkins test windows
• jenkins test rook e2e

[Pegonzal]

LGTM @nizamial09, once the PR is out of draft if there are no big changes it gets my approval.

I left a bunch of comments, but very minor things.
A part from those, should the SYNC INFO be added as well? DOC

[Pegonzal]

From the docs I see that the optional parameters have not been added to create_sync_pipe and remove_sync_pipe

I suppose this is intentional, but pointing it out just in case

[nizamial09]

Yup, it was intentional. For now, I just went with the parameters that are required for the UI's basic functionality. As we move towards the advanced functionalities like filtering buckets in pipe, we can extend this method with those options. IMO, that way we can also test those things individually.

[nizamial09]

@Pegonzal thanks for the reviews. Addressed most of them and left some comments inline for the things i didn't address.

[nizamial09]

A part from those, should the SYNC INFO be added as well? DOC

not now as the sync info is a bit hard to understand as it is. I heard it might get refactored later on. So probably we can check later.

[Pegonzal]

@nizamial09 I don't think it's necessary to update the period when doing bucket related updates. Only when the zonegroup is being modified

"Note that the bucket needs to exist before being able to set this policy, and admin commands that modify bucket policies need to run on the master zone, however, they do not require period update." - From https://docs.ceph.com/en/latest/radosgw/multisite-sync-policy/#example-3-mirror-a-specific-bucket

A part from this, the PR looks good to me, thank you!

[nizamial09]

however, they do not require period update.

I agree, they do not require period update. But period update is necessary when different zones are coming into picture and we need the current policies changes to be carried over to the other zones as well. For now I removed the period commit from this API because its a time consuming process and ideally the group creation action itself shouldn't be failed on waiting for the period commit to be done. I'll think about it and add it separately.

[Pegonzal]

however, they do not require period update.

I agree, they do not require period update. But period update is necessary when different zones are coming into picture and we need the current policies changes to be carried over to the other zones as well. For now I removed the period commit from this API because its a time consuming process and ideally the group creation action itself shouldn't be failed on waiting for the period commit to be done. I'll think about it and add it separately.

From my undesrstanding you just have to update the period when you are not dealing with buckets, so
can we just updated conditionally like so?:

if not bucket_name:        
            self.update_period()

[nizamial09]

can we just updated conditionally like so?:

I am going to isolate the period commit into a separate task rather than executing along with these actions because it can cause timeouts. I'll take it up in a second PR.