Add multi-arch annotation to DICT #3521
Conversation
kubevirt-bot
added
release-note
|
hco-e2e-kv-smoke-gcp lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-kv-smoke-azure 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-sigs/prow repository. |
nunnatsa
force-pushed
the
golden-images-multi-arch
branch
3 times, most recently
from
1267d48 to
051dc5a
Compare
|
hco-e2e-upgrade-prev-operator-sdk-sno-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-azure, ci/prow/hco-e2e-operator-sdk-gcp, ci/prow/hco-e2e-operator-sdk-sno-azure, ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure, ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-azure 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-sigs/prow repository. |
|
hco-e2e-consecutive-operator-sdk-upgrades-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-consecutive-operator-sdk-upgrades-azure, ci/prow/hco-e2e-upgrade-operator-sdk-azure, ci/prow/hco-e2e-upgrade-operator-sdk-sno-azure 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-sigs/prow repository. |
|
hco-e2e-operator-sdk-sno-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-sno-azure, ci/prow/hco-e2e-upgrade-operator-sdk-sno-azure, ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure 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-sigs/prow repository. |
|
hco-e2e-operator-sdk-gcp, hco-e2e-operator-sdk-aws lanes succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-azure, ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-azure 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-sigs/prow repository. |
|
hco-e2e-upgrade-operator-sdk-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-operator-sdk-azure 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-sigs/prow repository. |
|
hco-e2e-consecutive-operator-sdk-upgrades-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-consecutive-operator-sdk-upgrades-azure 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-sigs/prow repository. |
nunnatsa
force-pushed
the
golden-images-multi-arch
branch
from
051dc5a to
c8df2db
Compare
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
hco-e2e-operator-sdk-gcp lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-aws, ci/prow/hco-e2e-operator-sdk-azure 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-sigs/prow repository. |
|
hco-e2e-upgrade-operator-sdk-sno-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-sno-azure, ci/prow/hco-e2e-upgrade-operator-sdk-sno-azure, ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-azure 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-sigs/prow repository. |
|
hco-e2e-upgrade-prev-operator-sdk-aws lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-consecutive-operator-sdk-upgrades-azure, ci/prow/hco-e2e-upgrade-operator-sdk-azure, ci/prow/hco-e2e-upgrade-prev-operator-sdk-azure 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-sigs/prow repository. |
| - metadata: | ||
| annotations: | ||
| cdi.kubevirt.io/storage.bind.immediate.requested: "true" | ||
| ssp.kubevirt.io/dict.architectures: arm64,s390x,amd64 |
There was a problem hiding this comment.
why different order than the first two?
There was a problem hiding this comment.
- the order is meaningless
- this is done by the tool, introduced in Introduce the annotate-dicts tool #3523 - this tool takes the architectures from the manifest image, by the order of the manifest, meaning, this is the order in the manifest.
| if len(hcoDicts) == 0 { | ||
| return nil | ||
| } | ||
|
|
||
| res := make([]sspv1beta2.DataImportCronTemplate, len(hcoDicts)) | ||
|
|
||
| for i, hcoDict := range hcoDicts { | ||
| res[i] = hcoDictToSSP(hcoDict) | ||
| res[i] = hcoDictToSSP(hc, hcoDict) |
There was a problem hiding this comment.
res doesn't sound for me as a meaningful variable name in this scope.
i would name it something like sspDictSlice or similar.
side note: dict can be confused with "dictionary", not a short for DataImportCronTemplate. but since it's go and not python, i think it's legit here.
There was a problem hiding this comment.
resis there for years. Not part of this PR.- res is a very (very very) common short name for "result". For me it's very clear.
- dict == dictionary - in python. It's a known short name for DataImportCronTemplate in this project, as well as in CDI and SSP, again - for years.
If it's very important for you, I'll change it.
There was a problem hiding this comment.
it's clear that res is short for "result", but since it's the return value, it doesn't add much information for what it contains. but anyway, it's a short function so the reader can look few lines above at the function declaration. so i think it's fine.
controllers/operands/ssp_test.go
Outdated
| } | ||
| }) | ||
|
|
||
| It("should not drop the ssp.kubevirt.io/dict.architectures annotation, when heterogeneous cluster is enabled also for custom DICTst", func() { |
There was a problem hiding this comment.
| It("should not drop the ssp.kubevirt.io/dict.architectures annotation, when heterogeneous cluster is enabled also for custom DICTst", func() { | |
| It("should not drop the ssp.kubevirt.io/dict.architectures annotation, when heterogeneous cluster is enabled also for custom DICTs", func() { |
| @@ -349,7 +351,7 @@ func (d dataImportTemplateSlice) Len() int { return len(d) } | |||
| func (d dataImportTemplateSlice) Swap(i, j int) { d[i], d[j] = d[j], d[i] } | |||
| func (d dataImportTemplateSlice) Less(i, j int) bool { return d[i].Name < d[j].Name } | |||
|
|
|||
| func hcoDictToSSP(hcoDict hcov1beta1.DataImportCronTemplate) sspv1beta2.DataImportCronTemplate { | |||
| func hcoDictToSSP(hc *hcov1beta1.HyperConverged, hcoDict hcov1beta1.DataImportCronTemplate) sspv1beta2.DataImportCronTemplate { | |||
| spec := cdiv1beta1.DataImportCronSpec{} | |||
| if hcoDict.Spec != nil { | |||
There was a problem hiding this comment.
not part of the PR, but should we continue as usual if the spec of the DICT is nil?
There was a problem hiding this comment.
yes. There a use-case for it. The user want to disable a DICT, they can add an annotation to it. In this case, we don't use and don't require the spec.
controllers/operands/ssp_test.go
Outdated
| }) | ||
| }) | ||
|
|
||
| It("should drop the ssp.kubevirt.io/dict.architectures annotation, when heterogeneous cluster is disabled (default)", func() { |
There was a problem hiding this comment.
what does it mean "when heterogeneous cluster is disabled"?
i think the intention was:
"when EnableMultiArchCommonBootImageImport is disabled"
?
There was a problem hiding this comment.
re-wrote test names.
controllers/operands/ssp_test.go
Outdated
| } | ||
| }) | ||
|
|
||
| It("should drop the ssp.kubevirt.io/dict.architectures annotation, when heterogeneous cluster is disabled (default) also for custom DICTst", func() { |
There was a problem hiding this comment.
| It("should drop the ssp.kubevirt.io/dict.architectures annotation, when heterogeneous cluster is disabled (default) also for custom DICTst", func() { | |
| It("should drop the ssp.kubevirt.io/dict.architectures annotation, when heterogeneous cluster is disabled (default) also for custom DICTs", func() { |
If the enableMultiArchCommonBootImageImport feature gate is "false" (or missing), do not copy the `"ssp.kubevirt.io/dict.architectures"` annotation to the SSP CR. If the feature gate is set to "true, do copy the annotation. Signed-off-by: Nahshon Unna-Tsameret <[email protected]>
Added the ssp.kubevirt.io/dict.architectures annotation to the DataImportCronTemplate objects in assets/dataImportCronTemplates/dataImportCronTemplates.yaml, by running `make annotate-dicts`. Signed-off-by: Nahshon Unna-Tsameret <[email protected]>
Signed-off-by: Nahshon Unna-Tsameret <[email protected]>
nunnatsa
force-pushed
the
golden-images-multi-arch
branch
from
6e2e83c to
76ea0e1
Compare
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: orenc1 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
|
hco-e2e-operator-sdk-gcp lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-aws, ci/prow/hco-e2e-operator-sdk-azure 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-sigs/prow repository. |
|
hco-e2e-operator-sdk-sno-azure lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-operator-sdk-sno-aws 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-sigs/prow repository. |
|
hco-e2e-upgrade-prev-operator-sdk-azure lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-upgrade-prev-operator-sdk-aws 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-sigs/prow repository. |
|
hco-e2e-consecutive-operator-sdk-upgrades-azure lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-consecutive-operator-sdk-upgrades-aws, ci/prow/hco-e2e-upgrade-operator-sdk-aws, ci/prow/hco-e2e-upgrade-operator-sdk-sno-aws, ci/prow/hco-e2e-upgrade-prev-operator-sdk-sno-aws 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-sigs/prow repository. |
|
hco-e2e-kv-smoke-gcp lane succeeded. |
|
@hco-bot: Overrode contexts on behalf of hco-bot: ci/prow/hco-e2e-kv-smoke-azure 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-sigs/prow repository. |
What this PR does / why we need it:
Add the new
enableMultiArchCommonBootImageImportfeature gate to the HyperConverged CR, with default value offalse.Add the new
ssp.kubevirt.io/dict.architecturesannotation to the DataImportCronTemplate object in the dataImportCronTemplates.yaml file, with a comma separated list of supported architectures in the specific image.When adding a DataImportCronTemplate object to the SSP CR, do not copy the new
ssp.kubevirt.io/dict.architecturesannotation from the file to the object, unless the newenableMultiArchCommonBootImageImportfeature gate is set to true. Then do copy it.Jira Ticket:
Release note: