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 DataSource to PVC describe #76463

Merged

Conversation

@j-griffith
Copy link
Contributor

commented Apr 11, 2019

/kind bug

What this PR does / why we need it:
This change adds non nil DataSource entries of a PVC to the describe pvc output. Currently there's no real way to know for sure if you submitted an invalid or unsupported DataSource Kind if it was accepted or not.

For example, using the Hostpath driver, I can enter any info I want as DataSource Kind and Name and the volume is succesfully created/bound. Of course the info I supplied is filtered out, but I don't have any way of knowing that via the API.

In the case of valid entries (ie VolumeSnapshot and a supporting CSI Plugin) again there's no information after creation to make it clear to me that the PVC was in fact valid and created with the DataSource that I specified.

Which issue(s) this PR fixes:

Fixes #76460

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Non nil DataSource entries on PVC's are now displayed as part of `describe pvc` output.
@j-griffith

This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2019

/sig storage

@msau42

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

/hold
until datasource is beta

@fejta-bot

This comment has been minimized.

Copy link

commented Jul 16, 2019

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@msau42

This comment has been minimized.

Copy link
Member

commented Jul 16, 2019

/remove-lifecycle stale

@msau42

This comment has been minimized.

Copy link
Member

commented Jul 16, 2019

/hold cancel
targeting beta in 1.16

@@ -1519,6 +1519,11 @@ func describePersistentVolumeClaim(pvc *corev1.PersistentVolumeClaim, events *co
if pvc.Spec.VolumeMode != nil {
w.Write(LEVEL_0, "VolumeMode:\t%v\n", *pvc.Spec.VolumeMode)
}
if pvc.Spec.DataSource != nil {

This comment has been minimized.

Copy link
@msau42

msau42 Jul 16, 2019

Member

can we add a unit test for this?

This comment has been minimized.

Copy link
@j-griffith

j-griffith Jul 17, 2019

Author Contributor

Yup, added!

@j-griffith j-griffith force-pushed the j-griffith:add_datasource_to_pvc_describe branch from 403f70e to a21a410 Jul 17, 2019

@k8s-ci-robot k8s-ci-robot added size/M and removed size/XS labels Jul 17, 2019

@j-griffith

This comment has been minimized.

Copy link
Contributor Author

commented Jul 17, 2019

/retest

@msau42

This comment has been minimized.

Copy link
Member

commented Jul 17, 2019

/lgtm
/assign @mengqiy
/retest

@j-griffith j-griffith force-pushed the j-griffith:add_datasource_to_pvc_describe branch from a21a410 to 5cc2ea7 Aug 10, 2019

@k8s-ci-robot k8s-ci-robot removed the lgtm label Aug 10, 2019

@j-griffith

This comment has been minimized.

Copy link
Contributor Author

commented Aug 10, 2019

same changes, rebased to the move to staging

@j-griffith

This comment has been minimized.

Copy link
Contributor Author

commented Aug 10, 2019

/retest

@j-griffith

This comment has been minimized.

Copy link
Contributor Author

commented Aug 10, 2019

/kind bug

@k8s-ci-robot k8s-ci-robot added kind/bug and removed needs-kind labels Aug 10, 2019

@msau42

This comment has been minimized.

Copy link
Member

commented Aug 12, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm label Aug 12, 2019

@mengqiy
Copy link
Contributor

left a comment

/lgtm
/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

[APPROVALNOTIFIER] This PR is APPROVED

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

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

@j-griffith

This comment has been minimized.

Copy link
Contributor Author

commented Aug 12, 2019

/retestt

@fejta-bot

This comment has been minimized.

Copy link

commented Aug 13, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

3 similar comments
@fejta-bot

This comment has been minimized.

Copy link

commented Aug 13, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot

This comment has been minimized.

Copy link

commented Aug 13, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@fejta-bot

This comment has been minimized.

Copy link

commented Aug 13, 2019

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit 9dad928 into kubernetes:master Aug 13, 2019

23 checks passed

cla/linuxfoundation j-griffith authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
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-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
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
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details

@k8s-ci-robot k8s-ci-robot added this to the v1.16 milestone Aug 13, 2019

@@ -1534,6 +1534,11 @@ func describePersistentVolumeClaim(pvc *corev1.PersistentVolumeClaim, events *co
if pvc.Spec.VolumeMode != nil {
w.Write(LEVEL_0, "VolumeMode:\t%v\n", *pvc.Spec.VolumeMode)
}
if pvc.Spec.DataSource != nil {
w.Write(LEVEL_0, "DataSource:\n")
w.Write(LEVEL_1, "Name:\t%v\n", pvc.Spec.DataSource.Name)

This comment has been minimized.

Copy link
@cwdsuzhou

cwdsuzhou Sep 6, 2019

Member

I am confused why not print pvc.Spec.DataSource.APIGroup at the same time

This comment has been minimized.

Copy link
@msau42

msau42 Sep 6, 2019

Member

Yes, I think printing out api group makes sense too. Can you open a PR to add it? Right now, since only cloning is beta and on by default, then this can only be PVC type.

This comment has been minimized.

Copy link
@cwdsuzhou

cwdsuzhou Sep 8, 2019

Member

Yes, I think printing out api group makes sense too. Can you open a PR to add it? Right now, since only cloning is beta and on by default, then this can only be PVC type.

Sure, I will open a pr.

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.