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

Mark PVCDataSource featuregate as GA #88686

Merged
merged 1 commit into from Mar 4, 2020

Conversation

@j-griffith
Copy link
Contributor

j-griffith commented Feb 29, 2020

/kind feature

What this PR does / why we need it:

Marks VolumePVCDataSource as GA for 1.18

Which issue(s) this PR fixes:

Fixes #88814

Special notes for your reviewer:

VolumePVCDataSource moves to GA in 1.18 release

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

https://github.com/kubernetes/website/pull/19318
Copy link
Member

jsafrane left a comment

Please remove feature gate evaluation in

if utilfeature.DefaultFeatureGate.Enabled(features.VolumePVCDataSource) &&
- it's always true now.

@j-griffith j-griffith force-pushed the j-griffith:upgrade_cloning_to_ga branch from 9ca2e97 to 09fb34c Mar 2, 2020
@k8s-ci-robot k8s-ci-robot added size/S and removed size/M labels Mar 2, 2020
@jsafrane

This comment has been minimized.

Copy link
Member

jsafrane commented Mar 2, 2020

/lgtm
Failed e2e-gce-alpha-features is not related to this PR and is being fixed in #88727

@jsafrane

This comment has been minimized.

Copy link
Member

jsafrane commented Mar 2, 2020

/kind api-review
for changes in validation (i.e. removal of feature checks)

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Mar 2, 2020

@jsafrane: The label(s) kind/api-review cannot be applied, because the repository doesn't have them

In response to this:

/kind api-review
for changes in validation (i.e. removal of feature checks)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@jsafrane

This comment has been minimized.

Copy link
Member

jsafrane commented Mar 2, 2020

/label api-review
for changes in validation (i.e. removal of feature checks)

@smarterclayton

This comment has been minimized.

Copy link
Contributor

smarterclayton commented Mar 2, 2020

/assign @smarterclayton

@smarterclayton

This comment has been minimized.

Copy link
Contributor

smarterclayton commented Mar 2, 2020

Does the godoc on DataSource need to be updated? I see:

	// This field requires the VolumeSnapshotDataSource alpha feature gate to be
	// enabled and currently VolumeSnapshot is the only supported data source.
	// If the provisioner can support VolumeSnapshot data source, it will create
	// a new volume and data will be restored to the volume at the same time.
	// If the provisioner does not support VolumeSnapshot data source, volume will
	// not be created and the failure will be reported as an event.
	// In the future, we plan to support more data source types and the behavior
	// of the provisioner may change.
	// +optional
	DataSource *TypedLocalObjectReference `json:"dataSource,omitempty" protobuf:"bytes,7,opt,name=dataSource"`

and I would expect it to be updated.

@j-griffith j-griffith force-pushed the j-griffith:upgrade_cloning_to_ga branch from 09fb34c to 5dfa03e Mar 2, 2020
@j-griffith

This comment has been minimized.

Copy link
Contributor Author

j-griffith commented Mar 4, 2020

/test pull-kubernetes-e2e-gci-gce-autoscaling

@j-griffith

This comment has been minimized.

Copy link
Contributor Author

j-griffith commented Mar 4, 2020

/test pull-kubernetes-verify

@jsafrane

This comment has been minimized.

Copy link
Member

jsafrane commented Mar 4, 2020

almost there, but generated doc is out of sync:

Generated swagger type documentation is out of date:
--- /home/prow/go/src/k8s.io/kubernetes/./staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go	2020-03-04 04:26:30.111495230 +0000
+++ /tmp/swagger-docs.IZkoOW/src/k8s.io/kubernetes/./staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go	2020-03-04 05:09:42.551694271 +0000
@@ -1300,7 +1300,7 @@ var map_PersistentVolumeClaimSpec = map[
 	"resources":        "Resources represents the minimum resources the volume should have. More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#resources",
 	"volumeName":       "VolumeName is the binding reference to the PersistentVolume backing this claim.",
 	"storageClassName": "Name of the StorageClass required by the claim. More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#class-1",
-	"volumeMode":       "volumeMode defines what type of volume is required by the claim. Value of Filesystem is implied when not included in claim spec. This is a beta feature.",
+	"volumeMode":       "volumeMode defines what type of volume is required by the claim. Value of Filesystem is implied when not included in claim spec.",

I think hack/update-generated-swagger-docs.sh is the right tool to fix this.

@j-griffith

This comment has been minimized.

Copy link
Contributor Author

j-griffith commented Mar 4, 2020

almost there, but generated doc is out of sync:

Generated swagger type documentation is out of date:
--- /home/prow/go/src/k8s.io/kubernetes/./staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go	2020-03-04 04:26:30.111495230 +0000
+++ /tmp/swagger-docs.IZkoOW/src/k8s.io/kubernetes/./staging/src/k8s.io/api/core/v1/types_swagger_doc_generated.go	2020-03-04 05:09:42.551694271 +0000
@@ -1300,7 +1300,7 @@ var map_PersistentVolumeClaimSpec = map[
 	"resources":        "Resources represents the minimum resources the volume should have. More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#resources",
 	"volumeName":       "VolumeName is the binding reference to the PersistentVolume backing this claim.",
 	"storageClassName": "Name of the StorageClass required by the claim. More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#class-1",
-	"volumeMode":       "volumeMode defines what type of volume is required by the claim. Value of Filesystem is implied when not included in claim spec. This is a beta feature.",
+	"volumeMode":       "volumeMode defines what type of volume is required by the claim. Value of Filesystem is implied when not included in claim spec.",

I think hack/update-generated-swagger-docs.sh is the right tool to fix this.

It is, and tha'ts what I used (through make update anyway), then the rebase happened with a conflict in the proto file, I should've run it again.

@j-griffith j-griffith force-pushed the j-griffith:upgrade_cloning_to_ga branch from dcc6b8e to 27e145b Mar 4, 2020
@j-griffith

This comment has been minimized.

Copy link
Contributor Author

j-griffith commented Mar 4, 2020

/test pull-kubernetes-e2e-kind

@j-griffith

This comment has been minimized.

Copy link
Contributor Author

j-griffith commented Mar 4, 2020

/retest

@j-griffith j-griffith force-pushed the j-griffith:upgrade_cloning_to_ga branch from 27e145b to a2a91e9 Mar 4, 2020
// If the provisioner does not support VolumeSnapshot data source, volume will
// This field can be used to specify either:
// * An existing VolumeSnapshot object (snapshot.storage.k8s.io/VolumeSnapshot - Beta)
// * An existing PVC (PersistentVolumeClaim - GA)

This comment has been minimized.

Copy link
@smarterclayton

smarterclayton Mar 4, 2020

Contributor

You shouldn't have to say PVC is GA (since anything not called out as alpha or beta is implicitly GA). Remove the - GA part (but keep - Beta on snapshot).

@smarterclayton

This comment has been minimized.

Copy link
Contributor

smarterclayton commented Mar 4, 2020

One more tweak and I will approve this.

@smarterclayton

This comment has been minimized.

Copy link
Contributor

smarterclayton commented Mar 4, 2020

Also, the "Fixes" bug in the PR description looks wrong, should that be pointing to an enhancement?

@j-griffith

This comment has been minimized.

Copy link
Contributor Author

j-griffith commented Mar 4, 2020

Also, the "Fixes" bug in the PR description looks wrong, should that be pointing to an enhancement?

Opened an issue in the k8s project and linked it, thanks!

Updates the VolumePVCDataSource featuregate (cloning) to GA for the 1.18 k8s
release.
@j-griffith j-griffith force-pushed the j-griffith:upgrade_cloning_to_ga branch from a2a91e9 to 9044fbf Mar 4, 2020
@smarterclayton

This comment has been minimized.

Copy link
Contributor

smarterclayton commented Mar 4, 2020

/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Mar 4, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: j-griffith, smarterclayton

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jsafrane

This comment has been minimized.

Copy link
Member

jsafrane commented Mar 4, 2020

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Mar 4, 2020
@j-griffith

This comment has been minimized.

Copy link
Contributor Author

j-griffith commented Mar 4, 2020

/test pull-kubernetes-e2e-gci-gce-autoscaling
/test pull-kubernetes-e2e-kind

@saad-ali

This comment has been minimized.

Copy link
Member

saad-ali commented Mar 4, 2020

/milestone v1.18

@k8s-ci-robot k8s-ci-robot added this to the v1.18 milestone Mar 4, 2020
@saad-ali

This comment has been minimized.

Copy link
Member

saad-ali commented Mar 4, 2020

/priority important-soon

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Mar 4, 2020

@saad-ali: The label(s) priority/ cannot be applied, because the repository doesn't have them

In response to this:

/priority important-soon

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Mar 4, 2020

@j-griffith: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
pull-kubernetes-e2e-gci-gce-autoscaling 9044fbf link /test pull-kubernetes-e2e-gci-gce-autoscaling

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot merged commit e865c0b into kubernetes:master Mar 4, 2020
17 of 18 checks passed
17 of 18 checks passed
pull-kubernetes-e2e-gci-gce-autoscaling Job failed.
Details
cla/linuxfoundation j-griffith authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-alpha-features Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-kind Job succeeded.
Details
pull-kubernetes-e2e-kind-ipv6 Job succeeded.
Details
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.