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

Add force detach #477

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add force detach
Add new controller capability:
* UNPUBLISH_FENCE

Add new node capabilitiy:
* FORCE_UNPUBLISH
  • Loading branch information
bswartz committed Jun 7, 2021
commit 71086dec6d2b4b8ab970f651d9169ece6779ebcd
25 changes: 25 additions & 0 deletions csi.proto
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,17 @@ message ControllerUnpublishVolumeRequest {
// This field is OPTIONAL. Refer to the `Secrets Requirements`
// section on how to use this field.
map<string, string> secrets = 3 [(csi_secret) = true];

// Indicates SP MUST make the volume inacessible to the node or nodes
Copy link
Contributor

@humblec humblec May 4, 2021

Choose a reason for hiding this comment

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

s/inacessible/inaccessible

Copy link
Contributor

Choose a reason for hiding this comment

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

@bswartz may be you missed this ? :)

// it is being unpublished from. Any attempt to read or write data
// to a volume from a node that has been fenced MUST NOT succeed,
// even if the volume remains staged and/or published on the node.
Copy link
Contributor

Choose a reason for hiding this comment

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

If there are a couple of pods accessing the volume on same node, we would have a state where more than one publish and just one stage. Considering the nodeunpublish request arrives for a particular volume handle and if its fenced on stage level, its not the desired outcome we intended here with this enhancement/spec, Isnt it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes the normal set of NodeUnpublish and NodeUnstage calls still have to be made for each volume on each node to return to the original state. The trigger for the quarantined state is the ControllerUnpublish call with the fence flag. The above statement just means that, without communicating with the node, the SP is expected to render the volume inaccessible to the node regardless of the node's state, and the node will be forced to clean up under conditions where it can't access the volume. The only way the node will be able to access the volume again is for the node-side cleanup to complete and then for a subsequent ControllerPublish to happen for that node again.

// CO MUST NOT set this field to true unless SP has the
// UNPUBLISH_FENCE controller capability.
// The SP MAY make the volume inaccessible even when this field is
// false.
// This is an OPTIONAL field.
bool fence = 4;
}

message ControllerUnpublishVolumeResponse {
Expand Down Expand Up @@ -1316,6 +1327,13 @@ message NodeUnstageVolumeRequest {
// system/filesystem, but, at a minimum, SP MUST accept a max path
// length of at least 128 bytes.
string staging_target_path = 2;

// Indicates that the SP should prefer to successfully unstage the
// volume, even if data loss would occur as a result.
// CO MUST NOT set this field to true unless SP has the
// FORCE_UNPUBLISH node capability.
// This in an OPTIONAL field.
bool force = 3;
}

message NodeUnstageVolumeResponse {
Expand Down Expand Up @@ -1400,6 +1418,13 @@ message NodeUnpublishVolumeRequest {
// system/filesystem, but, at a minimum, SP MUST accept a max path
// length of at least 128 bytes.
string target_path = 2;

// Indicates that the SP should prefer to successfully unpublish the
// volume, even if data loss would occur as a result.
// CO MUST NOT set this field to true unless SP has the
// FORCE_UNPUBLISH node capability.
// This in an OPTIONAL field.
bool force = 3;
}

message NodeUnpublishVolumeResponse {
Expand Down
Loading