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.

Sign up for GitHub

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

Jump to bottom

docs(CNV-53188): improve disk-uploader usage guidance #576

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

docs(CNV-53188): improve disk-uploader usage guidance #576

wants to merge 1 commit into from

Conversation

codingben
Copy link
Member

What this PR does / why we need it:

Add more details to explain an usage of
EXPORT_SOURCE_KIND. Also, Add an example of the
Secret object, that will be referenced by
SECRET_NAME.

Release note:

None

@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Dec 16, 2024
@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign 0xfelix for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Dec 16, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: codingben
Once this PR has been reviewed and has the lgtm label, please assign 0xfelix for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@codingben
Copy link
Member Author

/cc @0xFelix @ksimon1 @dominikholler @geetikakay

@codingben
Copy link
Member Author

/cc @jcanocan @lyarwood

@openshift-ci openshift-ci bot requested review from jcanocan and lyarwood December 16, 2024 15:33
@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 6, 2025
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 7, 2025
@codingben
Copy link
Member Author

@ksimon1 @dominikholler Can you please review it?

@codingben
docs(CNV-53188): improve disk-uploader usage guidance
48d897a 
Add more details to explain an usage of
EXPORT_SOURCE_KIND. Also, Add an example of the
Secret object, that will be referenced by
SECRET_NAME.

Signed-off-by: Ben Oukhanov <[email protected]>
jcanocan
jcanocan reviewed Jan 8, 2025
View reviewed changes
release/tasks/disk-uploader/README.md
@@ -12,7 +12,7 @@ When user runs [KubeVirt Tekton Tasks](https://github.com/kubevirt/kubevirt-tekt

### Parameters

- **EXPORT_SOURCE_KIND**: The name of the export source kind
- **EXPORT_SOURCE_KIND**: Specify the export source kind (vm, vmsnapshot, pvc)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- **EXPORT_SOURCE_KIND**: Specify the export source kind (vm, vmsnapshot, pvc)
- **EXPORT_SOURCE_KIND**: Specify the export source kind (VM, VMsnapshot, PVC)

Please, use capitals to indicate that they are acronyms.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure about this. vm, vmsnapshot or pvc is the expected value. It won't work if the user will set another value than [1].

[1] https://github.com/kubevirt/kubevirt-tekton-tasks/blob/main/modules/disk-uploader/pkg/vmexport/vmexport.go#L21-L25

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see, so those are the expected values of the field. Maybe you can clarify it, at least confused me.

release/tasks/disk-uploader/disk-uploader.yaml
@@ -24,7 +24,7 @@ spec:
to be used in multiple Kubernetes clusters.
params:
- name: EXPORT_SOURCE_KIND
description: The name of the export source kind
description: Specify the export source kind (vm, vmsnapshot, pvc)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

templates/disk-uploader/manifests/disk-uploader.yaml
@@ -24,7 +24,7 @@ spec:
to be used in multiple Kubernetes clusters.
params:
- name: EXPORT_SOURCE_KIND
description: The name of the export source kind
description: Specify the export source kind (vm, vmsnapshot, pvc)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akrejcir akrejcir Awaiting requested review from akrejcir

@0xFelix 0xFelix Awaiting requested review from 0xFelix

@geetikakay geetikakay Awaiting requested review from geetikakay

@lyarwood lyarwood Awaiting requested review from lyarwood

@jcanocan jcanocan Awaiting requested review from jcanocan

@ksimon1 ksimon1 Awaiting requested review from ksimon1

@dominikholler dominikholler Awaiting requested review from dominikholler

Assignees
No one assigned
Labels
dco-signoff: yes Indicates the PR's author has DCO signed all their commits. release-note-none Denotes a PR that doesn't merit a release note. size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@codingben @kubevirt-bot @dominikholler @ksimon1 @jcanocan