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

rbd: Implement FenceClusterNetwork and UnfenceClusterNetwork #2738

Merged
merged 8 commits into from
Jan 7, 2022

Conversation

Yuggupta27
Copy link
Contributor

@Yuggupta27 Yuggupta27 commented Dec 22, 2021

Add implementation for FenceClusterNetwork and UnfenceClusterNetwork grpc calls.

  • FenceClusterNetwork: allows to blocks access to a CIDR block by
    creating a network fence using blocklisting.

Ceph operation: ceph osd blocklist add <ip>

  • UnfenceClusterNetwork: allows unblocking the access to a
    CIDR block by removing it from the network fence.

Ceph operation: ceph osd blocklist rm <ip>

Since the controller is not available yet, e2e will be added later.
Currently, all the grpcs have been manually tested and verified.


Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

  • /retest ci/centos/<job-name>: retest the <job-name> after unrelated
    failure (please report the failure too!)
  • /retest all: run this in case the CentOS CI failed to start/report any test
    progress or results

@Yuggupta27 Yuggupta27 changed the title Implement FenceClusterNetwork and UnfenceClusterNetwork rbd: Implement FenceClusterNetwork and UnfenceClusterNetwork Dec 22, 2021
@mergify mergify bot added the component/rbd Issues related to RBD label Dec 22, 2021
@mergify
Copy link
Contributor

mergify bot commented Dec 23, 2021

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

@Yuggupta27
Copy link
Contributor Author

/retest ci/centos/mini-e2e/k8s-1.22

@Yuggupta27
Copy link
Contributor Author

Tested the FenceClusterNetwork and UnfenceClusterNetwork grpcs locally using a custom grpc client for NetworkFence operation written on top of #2745.
Everything seems to be working as expected :)

@Yuggupta27 Yuggupta27 requested review from a team January 3, 2022 19:21
@Yuggupta27 Yuggupta27 added this to the release-3.5.0 milestone Jan 4, 2022
@Yuggupta27 Yuggupta27 force-pushed the fencing branch 2 times, most recently from a84448a to 17b9b66 Compare January 4, 2022 09:04
@Yuggupta27 Yuggupta27 requested a review from Madhu-1 January 4, 2022 09:16
Madhu-1
Madhu-1 previously approved these changes Jan 4, 2022
}

_, err := controller.FenceClusterNetwork(context.TODO(), req)
assert.Error(t, err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

unit tests should test for the specific error message in most of the cases to make sure things are working as expected.

humblec
humblec previously requested changes Jan 4, 2022
Copy link
Collaborator

@humblec humblec left a comment

Choose a reason for hiding this comment

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

few minor comments

@mergify mergify bot dismissed Madhu-1’s stale review January 4, 2022 10:08

Pull request has been modified.

@Yuggupta27 Yuggupta27 requested review from Madhu-1 and humblec January 4, 2022 10:08
Copy link
Member

@nixpanic nixpanic left a comment

Choose a reason for hiding this comment

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

just some initial remarks, not completely reviewed yet

@Yuggupta27 Yuggupta27 force-pushed the fencing branch 2 times, most recently from aa4af09 to 03308d5 Compare January 4, 2022 14:38
@Yuggupta27 Yuggupta27 requested a review from nixpanic January 4, 2022 14:38
@mergify mergify bot dismissed nixpanic’s stale review January 6, 2022 13:04

Pull request has been modified.

@Yuggupta27 Yuggupta27 requested a review from nixpanic January 6, 2022 13:04
nixpanic
nixpanic previously approved these changes Jan 6, 2022
@humblec humblec dismissed their stale review January 6, 2022 19:15

there are few more comments from my side, ptal..

@Yuggupta27 Yuggupta27 requested a review from humblec January 6, 2022 19:26
@mergify mergify bot dismissed nixpanic’s stale review January 6, 2022 19:26

Pull request has been modified.

@Yuggupta27 Yuggupta27 requested a review from nixpanic January 6, 2022 19:27
@Yuggupta27
Copy link
Contributor Author

/retest ci/centos/mini-e2e/k8s-1.22

@Madhu-1 Madhu-1 requested a review from a team January 7, 2022 06:15
Convert the CIDR block into a range of IPs,
and then add network fencing via "ceph osd blocklist"
for each IP in that range.

Signed-off-by: Yug Gupta <[email protected]>
implement FenceClusterNetwork grpc call which
allows to blocks access to a CIDR block by
creating a network fence.

Signed-off-by: Yug Gupta <[email protected]>
implement UnfenceClusterNetwork grpc call
which allows to unblock the access to a
CIDR block by removing it from network fence.

Signed-off-by: Yug Gupta <[email protected]>
@Yuggupta27
Copy link
Contributor Author

/retest ci/centos/mini-e2e-helm/k8s-1.22

@Yuggupta27 Yuggupta27 added the ci/retry/e2e Label to retry e2e retesting on approved PR's label Jan 7, 2022
@Yuggupta27
Copy link
Contributor Author

/retest ci/centos/mini-e2e-helm/k8s-1.22

@Yuggupta27
Copy link
Contributor Author

/retest ci/centos/mini-e2e-helm/k8s-1.22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/retry/e2e Label to retry e2e retesting on approved PR's component/rbd Issues related to RBD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants