fix: set boot source labels for DVs created by windows pipelines #845
Conversation
kubevirt-bot
added
release-note
| @@ -44,6 +44,10 @@ spec: | |||
| description: Namespace of the base DataVolume which is created. | |||
| type: string | |||
| default: kubevirt-os-images | |||
| - default: windows.10 | |||
| description: The result DV will be set as default boot source for given preference. | |||
There was a problem hiding this comment.
Basically it works the other way round. You set a given instancetype/preference to be the default of the DV. There can be multiple DVs using the same default instancetype/preference. Also I think both should be configurable.
ksimon1
force-pushed
the
windows-default-boot-source
branch
from
4253f12
to
90de668
Compare
| @@ -44,6 +44,11 @@ spec: | |||
| description: Name of Windows ISO datavolume | |||
| name: isoDVName | |||
| type: string | |||
| - default: windows.11.virtio | |||
| description: Name of the preference which will be used for this DV by default. | |||
| name: defaultPreferenceLabel | |||
There was a problem hiding this comment.
Why cannot preferenceName be reused instead of introducing defaultPreferenceLabel?
There was a problem hiding this comment.
preferenceName parameter is used for information about VM which is created to install the iso. The defaultPreferenceLabel is used for result DV. These two parameters might be different. E.g. for installation you want to use windows-11-some-fency-feature-enabled, but the result you want to use with different preference (e.g. windows-11-regular-user-disk)
There was a problem hiding this comment.
I agree that the two parameters are technically different, but it might be simple and intuitive to the user, if the DV would reflect the instancetype and preference which were used to create the DV. Avoiding the additional parameter would also keep the "surface" to the user smaller. Since this pipeline targets users who starts learning about kubevirt-tekton-tasks, I would vote to keep it as simple as possible, even at the price of flexibility.
|
/retest |
ksimon1
force-pushed
the
windows-default-boot-source
branch
2 times, most recently
from
52e368a
to
98e82a7
Compare
| @@ -55,6 +71,11 @@ spec: | |||
| apiVersion: cdi.kubevirt.io/v1beta1 | |||
| kind: DataVolume | |||
| metadata: | |||
| labels: | |||
There was a problem hiding this comment.
What do you think about conditionally adding the kind, only if the param was set? The default will always be clusterwide.
| description: Name of the instance type which will be used for this DV by default. | ||
| name: defaultInstancetypeLabel | ||
| type: string | ||
| - default: virtualmachineclusterinstancetype |
There was a problem hiding this comment.
You could make this an optional param without a default value.
ksimon1
force-pushed
the
windows-default-boot-source
branch
2 times, most recently
from
940a80e
to
e9b32ca
Compare
Set default instanceType and preferences labels to result DVs created by windows pipelines. With this fix, DVs will have default preferences when user triggers pipeline. Default values will be taken from already existing parameters instanceTypeName, instanceTypeKind, preferenceName, virtualMachinePreferenceKind Signed-off-by: Karel Simon <ksimon@redhat.com>
ksimon1
force-pushed
the
windows-default-boot-source
branch
from
e9b32ca
to
1894c31
Compare
|
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 0xFelix 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 |
kubevirt-bot
added
the
approved
|
/lgtm |
|
/cherry-pick release-v0.18 |
|
@ksimon1: new pull request created: #859 In response to this:
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. |
|
/cherry-pick release-v0.19 |
|
@ksimon1: new pull request created: #860 In response to this:
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. |
What this PR does / why we need it:
fix: set boot source labels for DVs created by windows pipelines
Set default instanceType and preferences labels to result DVs created by windows pipelines. With this fix, preferences will have default windows boot source when user triggers pipeline.
Which issue(s) this PR fixes:
Fixes https://issues.redhat.com/browse/CNV-34661
Release note: