rbd: use blocklist range cmd, fallback if it fails #3385

Rakshith-R · 2022-09-13T12:00:58Z

This commit adds blocklist range cmd feature,
while fallbacks to old blocklist one ip at a
time if the cmd is invalid(not available).

Signed-off-by: Rakshith R [email protected]

I0913 11:56:29.787215       1 utils.go:195] ID: 21 GRPC call: /fence.FenceController/UnfenceClusterNetwork
I0913 11:56:29.788168       1 utils.go:206] ID: 21 GRPC request: {"cidrs":[{"cidr":"10.90.89.66/32"}],"parameters":{"clusterID":"rook-ceph"},"secrets":"***stripped***"}
I0913 11:56:31.335384       1 cephcmds.go:105] ID: 21 command succeeded: ceph [osd blocklist range rm 10.90.89.66/32 --id csi-rbd-provisioner --keyfile=***stripped*** -m 10.99.96.253:6789]
I0913 11:56:31.335467       1 fencing.go:189] ID: 21 unblocked IP "10.90.89.66/32" successfully
I0913 11:56:31.336029       1 utils.go:212] ID: 21 GRPC response: {}
I0913 11:56:52.346352       1 utils.go:195] ID: 22 GRPC call: /fence.FenceController/FenceClusterNetwork
I0913 11:56:52.346578       1 utils.go:206] ID: 22 GRPC request: {"cidrs":[{"cidr":"10.90.89.66/32"}],"parameters":{"clusterID":"rook-ceph"},"secrets":"***stripped***"}
I0913 11:56:53.821075       1 cephcmds.go:105] ID: 22 command succeeded: ceph [osd blocklist range add 10.90.89.66/32 157784760 --id csi-rbd-provisioner --keyfile=***stripped*** -m 10.99.96.253:6789]
I0913 11:56:53.821230       1 fencing.go:91] ID: 22 blocklisted IP "10.90.89.66/32" successfully
I0913 11:56:53.822079       1 utils.go:212] ID: 22 GRPC response: {}

internal/csi-addons/networkfence/fencing.go

Madhu-1 · 2022-09-13T12:07:55Z

internal/csi-addons/networkfence/fencing.go

+		// iterating through IP range.
+		err := nf.addCephBlocklist(ctx, cidr, true)
+		if err == nil {
+			return nil


we should not return here its for loop.

Madhu-1 · 2022-09-13T12:08:33Z

internal/csi-addons/networkfence/fencing.go

+		// iterating through IP range.
+		err := nf.removeCephBlocklist(ctx, cidr, true)
+		if err == nil {
+			return nil


continue instead of return

Madhu-1 · 2022-09-13T12:08:59Z

internal/csi-addons/networkfence/fencing.go

+			return nil
+		}
+		if !strings.Contains(err.Error(), "invalid command") {
+			return fmt.Errorf("failed to blocklist range rm %q: %w", cidr, err)


Suggested change

return fmt.Errorf("failed to blocklist range rm %q: %w", cidr, err)

return fmt.Errorf("failed to remove blocklist range %q: %w", cidr, err)

Madhu-1 · 2022-09-13T12:09:08Z

internal/csi-addons/networkfence/fencing.go

+		if !strings.Contains(err.Error(), "invalid command") {
+			return fmt.Errorf("failed to blocklist range rm %q: %w", cidr, err)
+		}
+		log.DebugLog(ctx, "failed to use blocklist range cmd, falling back to single ip blocklisting: %s", err)


remove debug log

Madhu-1 · 2022-09-13T12:09:34Z

internal/csi-addons/networkfence/fencing.go

+			return nil
+		}
+		if !strings.Contains(err.Error(), "invalid command") {
+			return fmt.Errorf("failed to blocklist range add %q: %w", cidr, err)


Suggested change

return fmt.Errorf("failed to blocklist range add %q: %w", cidr, err)

return fmt.Errorf("failed to add blocklist range %q: %w", cidr, err)

nixpanic · 2022-09-13T12:12:38Z

internal/csi-addons/networkfence/fencing.go

@@ -93,6 +98,16 @@ func (nf *NetworkFence) addCephBlocklist(ctx context.Context, ip string) error {
 func (nf *NetworkFence) AddNetworkFence(ctx context.Context) error {
 	// for each CIDR block, convert it into a range of IPs so as to perform blocklisting operation.
 	for _, cidr := range nf.Cidr {
+		// try range blocklist cmd, if invalid fallback to
+		// iterating through IP range.
+		err := nf.addCephBlocklist(ctx, cidr, true)


Can you move this to a helper and use a NetworkFence.hasBlocklistRange bool? No need to pass it around that way, and it will only be tried once for the cidr in nf.Cidr.

made modifications so we only check fail with invalid command in the first iteration.

Madhu-1 · 2022-09-13T12:31:48Z

internal/csi-addons/networkfence/fencing.go

+		// try range blocklist cmd, if invalid fallback to
+		// iterating through IP range.
+		if hasBlocklistRangeSupport {
+			err := nf.addCephBlocklist(ctx, cidr, true)


instead of true and false pass hasBlocklistRangeSupport

pass it where ?

We would still need to parse and loop through the range in this layer.

instead of passing true to addCephBlocklist pass hasBlocklistRangeSupport

Madhu-1 · 2022-09-13T12:32:10Z

internal/csi-addons/networkfence/fencing.go

+			if err == nil {
+				continue
+			}
+			if !strings.Contains(err.Error(), "invalid command") {


define const of invalid command

Madhu-1 · 2022-09-13T12:35:15Z

@Rakshith-R planning to backport this to 3.7?

This commit adds blocklist range cmd feature, while fallbacks to old blocklist one ip at a time if the cmd is invalid(not available). Signed-off-by: Rakshith R <[email protected]>

Rakshith-R · 2022-09-13T12:39:08Z

@Rakshith-R planning to backport this to 3.7?

yes

Rakshith-R · 2022-09-13T12:51:34Z

/retest all

skipping e2e, since we dont have any for networkfence

Rakshith-R · 2022-09-13T12:54:52Z

/retest all

mergify bot added the component/rbd Issues related to RBD label Sep 13, 2022

Rakshith-R requested review from Madhu-1, nixpanic and a team September 13, 2022 12:05

Madhu-1 requested changes Sep 13, 2022

View reviewed changes

nixpanic reviewed Sep 13, 2022

View reviewed changes

Rakshith-R force-pushed the add-cidr-range branch 2 times, most recently from 4b06c7b to 082dba4 Compare September 13, 2022 12:29

Rakshith-R requested review from Madhu-1 and nixpanic September 13, 2022 12:30

Madhu-1 reviewed Sep 13, 2022

View reviewed changes

Rakshith-R added the backport-to-release-v3.7 label Sep 13, 2022

rbd: use blocklist range cmd, fallback if it fails

3a5f29d

This commit adds blocklist range cmd feature, while fallbacks to old blocklist one ip at a time if the cmd is invalid(not available). Signed-off-by: Rakshith R <[email protected]>

Rakshith-R force-pushed the add-cidr-range branch from 082dba4 to 3a5f29d Compare September 13, 2022 12:39

Rakshith-R requested a review from Madhu-1 September 13, 2022 12:39

Rakshith-R mentioned this pull request Sep 13, 2022

release 3.7.1 tracker issue #3335

Closed

Madhu-1 approved these changes Sep 13, 2022

View reviewed changes

Rakshith-R added the ci/skip/e2e skip running e2e CI jobs label Sep 13, 2022

yati1998 approved these changes Sep 13, 2022

View reviewed changes

mergify bot merged commit a57859d into ceph:devel Sep 13, 2022

mergify bot mentioned this pull request Sep 13, 2022

rbd: use blocklist range cmd, fallback if it fails (backport #3385) #3386

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rbd: use blocklist range cmd, fallback if it fails #3385

rbd: use blocklist range cmd, fallback if it fails #3385

Rakshith-R commented Sep 13, 2022

Madhu-1 Sep 13, 2022

Madhu-1 Sep 13, 2022

Madhu-1 Sep 13, 2022

Madhu-1 Sep 13, 2022

Madhu-1 Sep 13, 2022

nixpanic Sep 13, 2022

Rakshith-R Sep 13, 2022

Madhu-1 Sep 13, 2022

Rakshith-R Sep 13, 2022

Madhu-1 Sep 13, 2022

Madhu-1 Sep 13, 2022

Madhu-1 commented Sep 13, 2022

Rakshith-R commented Sep 13, 2022

Rakshith-R commented Sep 13, 2022

Rakshith-R commented Sep 13, 2022

	return fmt.Errorf("failed to blocklist range rm %q: %w", cidr, err)
	return fmt.Errorf("failed to remove blocklist range %q: %w", cidr, err)

	return fmt.Errorf("failed to blocklist range add %q: %w", cidr, err)
	return fmt.Errorf("failed to add blocklist range %q: %w", cidr, err)

rbd: use blocklist range cmd, fallback if it fails #3385

rbd: use blocklist range cmd, fallback if it fails #3385

Conversation

Rakshith-R commented Sep 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Madhu-1 commented Sep 13, 2022

Rakshith-R commented Sep 13, 2022

Rakshith-R commented Sep 13, 2022

Rakshith-R commented Sep 13, 2022