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

@j-griffith j-griffith 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.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Apr 11, 2019
@j-griffith
Copy link
Contributor Author

/sig storage

@k8s-ci-robot k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Apr 11, 2019
@k8s-ci-robot k8s-ci-robot added area/kubectl sig/cli Categorizes an issue or PR as relevant to SIG CLI. labels Apr 11, 2019
@msau42
Copy link
Member

msau42 commented Apr 17, 2019

/hold
until datasource is beta

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 17, 2019
@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 16, 2019
@msau42
Copy link
Member

msau42 commented Jul 16, 2019

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 16, 2019
@msau42
Copy link
Member

msau42 commented Jul 16, 2019

/hold cancel
targeting beta in 1.16

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 16, 2019
@@ -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 {
Copy link
Member

Choose a reason for hiding this comment

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

can we add a unit test for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, added!

@j-griffith j-griffith force-pushed the add_datasource_to_pvc_describe branch from 403f70e to a21a410 Compare July 17, 2019 04:23
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 17, 2019
@j-griffith
Copy link
Contributor Author

/retest

@msau42
Copy link
Member

msau42 commented Jul 17, 2019

/lgtm
/assign @mengqiy
/retest

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 17, 2019
@j-griffith j-griffith force-pushed the add_datasource_to_pvc_describe branch from a21a410 to 5cc2ea7 Compare August 10, 2019 14:36
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 10, 2019
@j-griffith
Copy link
Contributor Author

same changes, rebased to the move to staging

@j-griffith
Copy link
Contributor Author

/retest

@j-griffith
Copy link
Contributor Author

/kind bug

@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. and removed needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Aug 10, 2019
@msau42
Copy link
Member

msau42 commented Aug 12, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 12, 2019
Copy link
Member

@mengqiy mengqiy left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 12, 2019
@j-griffith
Copy link
Contributor Author

/retestt

@fejta-bot
Copy link

/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
Copy link

/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
Copy link

/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
Copy link

/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
@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)
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

wking pushed a commit to wking/kubernetes that referenced this pull request Jul 21, 2020
…_pvc_describe

Add DataSource to PVC describe

Kubernetes-commit: 9dad928
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/kubectl cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide DataSource info in PVC describe
6 participants