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.

Sign up for GitHub

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

Jump to bottom

core: Continue processing PVs for network fencing when no node IPs found #13768

Merged
merged 3 commits into from
Feb 14, 2024
Merged

core: Continue processing PVs for network fencing when no node IPs found #13768

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e115c4
core: fix cephfs pvc network fencing
Madhu-1 Feb 14, 2024
eb97390
core: use error wraping in fenceCephFSVolume
Madhu-1 Feb 14, 2024
ca21bd9
core: retry other pvc if active client not found
Madhu-1 Feb 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 9 additions & 4 deletions pkg/operator/ceph/cluster/watcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ func (c *clientCluster) fenceCephFSVolume(

status, err := cephclient.StatusWithUser(c.context, clusterInfo)
if err != nil {
return fmt.Errorf("failed to get ceph status for check active mds. %v", err)
return pkgerror.Wrapf(err, "failed to get ceph status for check active mds")
}

var activeMDS string
Expand All @@ -438,13 +438,18 @@ func (c *clientCluster) fenceCephFSVolume(
return fmt.Errorf("failed to list watchers for cephfs pool/subvoumeName %s/%s. %v", cephFSPV.Spec.CSI.VolumeAttributes["pool"], cephFSPV.Spec.CSI.VolumeAttributes["subvolumeName"], err)
}
ips, err := cephFSMDSClientMarshal(buf, cephFSPV)
if err != nil || ips == nil {
return fmt.Errorf("failed to unmarshal cephfs mds output. %v", err)
if err != nil {
return pkgerror.Wrapf(err, "failed to unmarshal cephfs mds output")
}

if len(ips) == 0 {
logger.Infof("no active mds clients found for cephfs volume %q", cephFSPV.Name)
return nil
Copy link
Member

Choose a reason for hiding this comment

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

By returning nil, shouldn't this mean the fencing was successful? But the fencing is being skipped. How will the fencing be retried in the future if we are returning nil? Don't we need to return an error so the fencing will try again?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes true, we are doing the same for RBD

rook/pkg/operator/ceph/cluster/watcher.go

Lines 404 to 409 in 64cf8f2

if len(ips) != 0 {
err = c.createNetworkFence(ctx, rbdPV, node, cluster, ips, rbdDriver)
if err != nil {
return pkgerror.Wrapf(err, "failed to create network fence for node %q", node.Name)
}
}
, if we don't any client using that subvolume/rbdimage we are assuming that no client is connected and no need for blocklisting.

Copy link
Member

Choose a reason for hiding this comment

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

For both rbd and cephfs, the calling method is in a for loop. If nil is returned, the for loop breaks out. In the case of no IPs found, should the for loop keep iterating through other PVs instead of aborting? Or all PVs will have the same result of no IPs found?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, you are correct we should try for other PVC, added new comment for introducing new check. PTAL

}

err = c.createNetworkFence(ctx, cephFSPV, node, cluster, ips, cephfsDriver)
if err != nil {
return fmt.Errorf("failed to create network fence for node %q. %v", node.Name, err)
return pkgerror.Wrapf(err, "failed to create network fence for node %q", node.Name)
}

return nil
Expand Down
3 changes: 2 additions & 1 deletion pkg/operator/ceph/cluster/watcher_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func TestHandleNodeFailure(t *testing.T) {
case command == "ceph" && args[0] == "status":
return `{"entity":[{"addr": [{"addr": "10.244.0.12:0", "nonce":3247243972}]}], "client_metadata":{"root":"/"}}`, nil
case command == "ceph" && args[0] == "tell":
return `[{"entity":{"addr":{"addr":"10.244.0.12:0","nonce":3247243972}}, "client_metadata":{"root":"/"}}]`, nil
return `[{"entity":{"addr":{"addr":"10.244.0.12:0","nonce":3247243972}}, "client_metadata":{"root":"/volumes/csi/csi-vol-58469d41-f6c0-4720-b23a-0a0826b842ca"}}]`, nil

}
return "", errors.Errorf("unexpected rbd/ceph command %q", args)
Expand Down Expand Up @@ -250,6 +250,7 @@ func TestHandleNodeFailure(t *testing.T) {
VolumeHandle: "0001-0009-rook-ceph-0000000000000002-24862838-240d-4215-9183-abfc0e9e4001",
VolumeAttributes: map[string]string{
"fsName": "myfs",
"subvolumePath": "/volumes/csi/csi-vol-58469d41-f6c0-4720-b23a-0a0826b842ca",
"subvolumeName": "csi-vol-58469d41-f6c0-4720-b23a-0a0826b842ca",
},
},
Expand Down