Skip to content

GetCapacityResponse: promote maximum_volume_size#499

Merged
saad-ali merged 1 commit into
container-storage-interface:masterfrom
pohly:maximum-volume-size
Mar 16, 2022
Merged

GetCapacityResponse: promote maximum_volume_size#499
saad-ali merged 1 commit into
container-storage-interface:masterfrom
pohly:maximum-volume-size

Conversation

@pohly

@pohly pohly commented Mar 15, 2022

Copy link
Copy Markdown
Contributor

Kubernetes and several CSI drivers (for example, PMEM-CSI) have been using that field for a while now
and no changes to the semantic are expected, therefore the alpha status can and
should be removed.

The minimum_volume_size field is not used by Kubernetes. The semantic is
probably stable, but further practical experience with it might be desirable
before promoting it.

@humblec

humblec commented Mar 15, 2022

Copy link
Copy Markdown
Contributor

may be its good to give some references in the PR description to its usage in kube or csi drivers as we are promoting this field.

@pohly

pohly commented Mar 15, 2022

Copy link
Copy Markdown
Contributor Author

I've added some links.

@xing-yang

Copy link
Copy Markdown
Contributor

/lgtm

@xing-yang

Copy link
Copy Markdown
Contributor

/assign @saad-ali

@humblec

humblec commented Mar 15, 2022

Copy link
Copy Markdown
Contributor

lgtm..

@jdef

jdef commented Mar 15, 2022

Copy link
Copy Markdown
Member

RIP travis.org

Since June 15th, 2021, the building on travis-ci.org is ceased. Please use travis-ci.com from now on.

Please regenerate the Go bindings since you're changing a protobuf descriptor.

Kubernetes and several CSI drivers have been using that field for a while now
and no changes to the semantic are expected, therefore the alpha status can and
should be removed.

The minimum_volume_size field is not used by Kubernetes. The semantic is
probably stable, but further practical experience with it might be desirable
before promoting it.
@pohly pohly force-pushed the maximum-volume-size branch from b8056f8 to 3175f6b Compare March 15, 2022 16:13
@pohly

pohly commented Mar 15, 2022

Copy link
Copy Markdown
Contributor Author

It looks like updating lib/go/csi/csi.pb.go was also skipped in d64255d. Some of the changes that I got are from that commit.

The reason I missed the lib/go/csi update is that I expected the top-level "make" to update everything. Would it make sense to add that?

@jdef

jdef commented Mar 15, 2022

Copy link
Copy Markdown
Member

The reason I missed the lib/go/csi update is that I expected the top-level "make" to update everything. Would it make sense to add that?

probably, maybe in a follow-up PR?

@pohly

pohly commented Mar 16, 2022

Copy link
Copy Markdown
Contributor Author

Can this be merged and included in a new release?

@pohly

pohly commented Mar 16, 2022

Copy link
Copy Markdown
Contributor Author

The fix for the Makefile is in #500. I did not include the updated lib/go/csi/csi.pb.go there because that's already pending here, but it might be cleaner to merge that change via the other PR.

@xing-yang

Copy link
Copy Markdown
Contributor

CC @saad-ali

@saad-ali

Copy link
Copy Markdown
Member

/lgtm
/approve

@saad-ali saad-ali merged commit 09b5d07 into container-storage-interface:master Mar 16, 2022
@saad-ali

Copy link
Copy Markdown
Member

RIP travis.org

Since June 15th, 2021, the building on travis-ci.org is ceased. Please use travis-ci.com from now on.

Opened #501

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.

5 participants