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

Remove "optional" for GetVolumeGroupSnapshot to avoid confusion #580

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xing-yang
Copy link
Contributor

What type of PR is this?
cleanup

What this PR does / why we need it:
This PR removes "optional" from the following description about GetVolumeGroupSnapshot. Technically this sentence is correct as GetVolumeGroupSnapshot is optional. However, it adds confusion as the sentence immediately following this says it must be implemented if a plugin has CREATE_DELETE_GET_VOLUME_GROUP_SNAPSHOT capability. For volume group snapshots, there is a single capability for create/delete/get volume group snapshots. If that capability is supported, all there RPCs create/delete/get must be implemented.

This optional RPC MAY be called by the CO to fetch current information about a volume group snapshot.

A Controller Plugin MUST implement this `GetVolumeGroupSnapshot` RPC call if it has `CREATE_DELETE_GET_VOLUME_GROUP_SNAPSHOT` capability.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce an API-breaking change?:

none

@xing-yang
Copy link
Contributor Author

/assign @bswartz

@jdef
Copy link
Member

jdef commented Mar 2, 2025 via email

@xing-yang
Copy link
Contributor Author

I found similar language for ControllerGetVolume RPC. The only difference is that GET_VOLUME capability is only for ControllerGetVolume RPC, while CREATE_DELETE_GET_VOLUME_GROUP_SNAPSHOT capability is for Create/Delete/GetVolumeGroupSnapshot RPCs.

Should we remove "optional" for ControllerGetVolume as well?

This optional RPC MAY be called by the CO to fetch current information about a volume.

A Controller Plugin MUST implement this ControllerGetVolume RPC call if it has GET_VOLUME capability.

@bswartz
Copy link
Contributor

bswartz commented Mar 5, 2025

Should we remove "optional" for ControllerGetVolume as well?

Yes, we should.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants