POC: Expose virtio-win ISO via virt-artifacts-server#4373
Conversation
kubevirt-prow
Bot
added
release-note
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Coverage Report for CI Build 28525587818Coverage increased (+0.7%) to 82.097%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughThis PR adds a virtio-win ISO link to Estimated code review effort: 3 (Moderate) | ~25 minutes Related PRs: None provided. Suggested labels: enhancement, tests, area/console-cli-download Suggested reviewers: Reviewers familiar with Poem: Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 13 linked repositories, but your current plan allows 0. Analyzed ``, skipped Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/components/components.go (1)
279-347: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winExtract repeated literals into shared constants.
"virtio-win-data"is duplicated three times (Lines 297, 327, 340) and"768Mi"is repeated five times across requests/limits/size-limit. SonarCloud flags the volume-name duplication; both should be shared constants to avoid drift if either value changes.♻️ Proposed fix
+const ( + virtioWinDataVolumeName = "virtio-win-data" + virtioWinISOStorageSize = "768Mi" +) + func GetDeploymentSpecCliDownloads(params *DeploymentOperatorParams) appsv1.DeploymentSpec { return appsv1.DeploymentSpec{ ... InitContainers: []corev1.Container{ { Name: "virtio-win-init", ... Resources: corev1.ResourceRequirements{ Requests: map[corev1.ResourceName]resource.Quantity{ corev1.ResourceCPU: resource.MustParse("10m"), corev1.ResourceMemory: resource.MustParse("32Mi"), - corev1.ResourceEphemeralStorage: resource.MustParse("768Mi"), + corev1.ResourceEphemeralStorage: resource.MustParse(virtioWinISOStorageSize), }, Limits: map[corev1.ResourceName]resource.Quantity{ - corev1.ResourceEphemeralStorage: resource.MustParse("768Mi"), + corev1.ResourceEphemeralStorage: resource.MustParse(virtioWinISOStorageSize), }, }, VolumeMounts: []corev1.VolumeMount{ { - Name: "virtio-win-data", + Name: virtioWinDataVolumeName, MountPath: "/virtio-win-data", }, }, ... }, }, Containers: []corev1.Container{ { ... VolumeMounts: []corev1.VolumeMount{ { - Name: "virtio-win-data", + Name: virtioWinDataVolumeName, MountPath: "/home/server/src/virtio-win", ReadOnly: true, }, }, ... }, }, Volumes: []corev1.Volume{ { - Name: "virtio-win-data", + Name: virtioWinDataVolumeName, VolumeSource: corev1.VolumeSource{ EmptyDir: &corev1.EmptyDirVolumeSource{ - SizeLimit: ptr.To(resource.MustParse("768Mi")), + SizeLimit: ptr.To(resource.MustParse(virtioWinISOStorageSize)), }, }, }, },As per path instructions, "prefer shared constants for feature-gate names, annotation/label keys, and any repeated string literals". Also flagged by static analysis.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/components/components.go` around lines 279 - 347, The pod spec in components.go repeats the same volume name and size literals in the init container, server container, and volume definition. Extract the duplicated "virtio-win-data" and "768Mi" values into shared constants near the related builders/helpers, then reference those constants from the container volume mounts, resource requests/limits, and EmptyDir SizeLimit in the affected pod/template construction code.Sources: Path instructions, Linters/SAST tools
🧹 Nitpick comments (2)
tests/func-tests/cli_download_test.go (1)
81-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: extract the link-lookup loop into a small helper.
Lines 90-96 duplicate the "find link by substring" pattern used elsewhere in this file. A small helper (e.g.,
findLinkByHref(links, substr string) string) would reduce duplication if more link-specific tests are added later. As per path instructions, prefer helper functions over inline matcher-heavy logic in tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/func-tests/cli_download_test.go` around lines 81 - 118, The virtio-win download test in the ConsoleCLIDownload checks repeats the same “find link by substring” logic used elsewhere, so factor that lookup into a small helper and reuse it here. Add or reuse a helper like the existing link-finding pattern near the `ConsoleCLIDownload` test code, then call it from the `It("should expose virtio-win ISO for download")` block instead of keeping the inline loop.Source: Path instructions
pkg/components/components.go (1)
279-303: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider setting
TerminationMessagePolicyon the init container for troubleshooting.The
servercontainer setsTerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError(Line 335) butvirtio-win-initdoes not. If thecpcommand fails, defaulting to log output would help diagnose the failure faster.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/components/components.go` around lines 279 - 303, The virtio-win init container is missing a termination message policy, which makes failures harder to troubleshoot. Update the `virtio-win-init` container definition in `components.go` to set `TerminationMessagePolicy` the same way as the `server` container, using `corev1.TerminationMessageFallbackToLogsOnError`, so `cp` errors surface in logs automatically.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/func-tests/cli_download_test.go`:
- Around line 99-107: The ISO HEAD request in the cli download test can hang
indefinitely because the local httpClient is created without a timeout. Update
the httpClient setup in the download test to include a reasonable Timeout, using
the same pattern as the "URL Download customization" test, so the Head call on
virtioWinLink fails fast if the server is unresponsive.
---
Outside diff comments:
In `@pkg/components/components.go`:
- Around line 279-347: The pod spec in components.go repeats the same volume
name and size literals in the init container, server container, and volume
definition. Extract the duplicated "virtio-win-data" and "768Mi" values into
shared constants near the related builders/helpers, then reference those
constants from the container volume mounts, resource requests/limits, and
EmptyDir SizeLimit in the affected pod/template construction code.
---
Nitpick comments:
In `@pkg/components/components.go`:
- Around line 279-303: The virtio-win init container is missing a termination
message policy, which makes failures harder to troubleshoot. Update the
`virtio-win-init` container definition in `components.go` to set
`TerminationMessagePolicy` the same way as the `server` container, using
`corev1.TerminationMessageFallbackToLogsOnError`, so `cp` errors surface in logs
automatically.
In `@tests/func-tests/cli_download_test.go`:
- Around line 81-118: The virtio-win download test in the ConsoleCLIDownload
checks repeats the same “find link by substring” logic used elsewhere, so factor
that lookup into a small helper and reuse it here. Add or reuse a helper like
the existing link-finding pattern near the `ConsoleCLIDownload` test code, then
call it from the `It("should expose virtio-win ISO for download")` block instead
of keeping the inline loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b902b353-c055-403c-900a-cef42fb76524
📒 Files selected for processing (6)
controllers/handlers/cliDownload.gocontrollers/handlers/cliDownload_test.gopkg/components/components.gopkg/components/components_suite_test.gopkg/components/components_test.gotests/func-tests/cli_download_test.go
tiraboschi
force-pushed
the
poc_virtiowin_download
branch
3 times, most recently
from
239691b to
326c5c2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/operator.yaml`:
- Around line 293-317: The new virtio-win-init initContainer is missing a
hardened root filesystem setting. Update the securityContext for the
virtio-win-init container in the deployment manifest to set
readOnlyRootFilesystem to true, alongside the existing capability drop and
privilege escalation settings, since it only writes to the mounted
virtio-win-data volume.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 64eaa81b-1cd7-417e-a8fb-639b53641583
📒 Files selected for processing (9)
controllers/handlers/cliDownload.gocontrollers/handlers/cliDownload_test.godeploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/kubevirt-hyperconverged-operator.v1.19.0.clusterserviceversion.yamldeploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/kubevirt-hyperconverged-operator.v1.19.0.clusterserviceversion.yamldeploy/operator.yamlpkg/components/components.gopkg/components/components_suite_test.gopkg/components/components_test.gotests/func-tests/cli_download_test.go
✅ Files skipped from review due to trivial changes (3)
- pkg/components/components_suite_test.go
- controllers/handlers/cliDownload_test.go
- controllers/handlers/cliDownload.go
tiraboschi
force-pushed
the
poc_virtiowin_download
branch
from
326c5c2 to
c202ec8
Compare
|
@coderabbitai could you please review it again? I reworked it to rely on image volume (GA in k8s 1.36) |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd -a components.go pkg/components | xargs -I{} echo {}
echo "---components.go---"
cat -n pkg/components/components.go | sed -n '1,50p'
echo "---search virtio---"
rg -n -i 'virtio' pkg/components/components.go
echo "---full relevant function---"
ast-grep run --pattern 'func GetDeploymentSpecCliDownloads($$$) $$$' --lang go pkg/components/components.goLength of output: 7601 🤖 web_search executed:
Length of output: 12007 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---CSV files search for virtio-win-data---"
rg -n "virtio-win-data\|virtio-win-init\|ImageVolumeSource\|emptyDir" -i deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/kubevirt-hyperconverged-operator.v1.19.0.clusterserviceversion.yaml | head -50
echo "---operator.yaml---"
rg -n -i "virtio-win" deploy/operator.yaml
echo "---components_test.go---"
cat -n pkg/components/components_test.go
echo "---cliDownload.go---"
cat -n controllers/handlers/cliDownload.go | sed -n '1,80p'
echo "---check minimum kube version supported---"
rg -n -i "kubeVersion" deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/*.yaml | head -5Length of output: 1043 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---CSV files search---"
rg -n -i 'virtio-win-data|virtio-win-init|ImageVolumeSource|emptyDir' deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/kubevirt-hyperconverged-operator.v1.19.0.clusterserviceversion.yaml || true
echo "---operator.yaml---"
rg -n -i "virtio-win" deploy/operator.yaml || true
echo "---components_test.go---"
cat -n pkg/components/components_test.go || true
echo "---cliDownload.go---"
cat -n controllers/handlers/cliDownload.go | sed -n '1,80p'
echo "---kubeVersion in CSV---"
rg -n -i "kubeVersion|minKubeVersion" deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/*.yaml || true
echo "---check pullPolicy field default in ImageVolumeSource---"
rg -n "ImageVolumeSource" -A5 $(go env GOPATH 2>/dev/null)/pkg/mod/k8s.io/api*/core/v1/types.go 2>/dev/null || true
echo "---go.mod k8s version---"
rg -n "k8s.io/api " go.modLength of output: 7224 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---context around virtio-win-data in generated CSV (index-image)---"
sed -n '4300,4400p' deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/kubevirt-hyperconverged-operator.v1.19.0.clusterserviceversion.yaml
echo "---context around emptyDir at 4637---"
sed -n '4600,4650p' deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/kubevirt-hyperconverged-operator.v1.19.0.clusterserviceversion.yaml
echo "---olm-catalog csv search---"
rg -n -i 'virtio-win-data|virtio-win-init|image:|emptyDir|reference:' deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/kubevirt-hyperconverged-operator.v1.19.0.clusterserviceversion.yaml | rg -i 'virtio' -A2 -B2 || true
echo "---git diff stat for this PR (compare with main)---"
git log --oneline -5Length of output: 8930 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---search for supported OCP/kubernetes version docs---"
rg -n -i 'minimum.*(kubernetes|openshift|ocp)|supported.*(kubernetes|openshift|ocp)' README.md docs/*.md 2>/dev/null | head -30
fd -e md -i readme
echo "---search for maxKubeVersion / edge cases---"
rg -n -i 'ImageVolumeSource|image volume' -g '!*.yaml' . 2>/dev/null | grep -v vendor | head -20
echo "---check vendor for corev1.ImageVolumeSource definition to see PullPolicy default---"
find / -path /proc -prune -o -type d -name 'k8s.io' -print 2>/dev/null | headLength of output: 2613 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -i 'olm.skipRange|com.redhat.openshift.versions|kubeVersion' deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/*.yaml deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/metadata/*.yaml 2>/dev/null || true
fd annotations.yaml deploy/olm-catalog | head
cat deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/metadata/annotations.yaml 2>/dev/null || trueLength of output: 1839 Hi Two things I'd like to flag before this is safe to merge:
Everything else — the test coverage, the e2e HEAD-request check, and the ✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
deploy/operator.yaml (1)
298-301: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider explicit
pullPolicyon the virtio-win image volume.The
virtio-win-datavolume'sImageVolumeSourceonly setsreference, with nopullPolicy. Since the image is referenced by digest, the kubelet default pull policy is effectivelyIfNotPresentalready, but explicitly setting it avoids relying on that implicit behavior and matches the concern already raised in this PR's review thread for this large image.♻️ Proposed fix
volumes: - image: reference: quay.io/kubevirt/virtio-container-disk@sha256:6b823946eb7493347a93d6daa16b4101a68e16a0ee1766e01ac3adf82e4b5838 + pullPolicy: IfNotPresent name: virtio-win-data🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/operator.yaml` around lines 298 - 301, The virtio-win image volume currently sets only the image reference in the `virtio-win-data` `ImageVolumeSource`, so add an explicit `pullPolicy` there to make the intent clear and match the existing review concerns. Update the `volumes` entry for `virtio-win-data` in `operator.yaml` by setting the pull policy alongside the digest-based `reference`, using the same image volume configuration block to keep the behavior explicit and consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@deploy/operator.yaml`:
- Around line 298-301: The virtio-win image volume currently sets only the image
reference in the `virtio-win-data` `ImageVolumeSource`, so add an explicit
`pullPolicy` there to make the intent clear and match the existing review
concerns. Update the `volumes` entry for `virtio-win-data` in `operator.yaml` by
setting the pull policy alongside the digest-based `reference`, using the same
image volume configuration block to keep the behavior explicit and consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d472c3ee-5153-452c-93df-3ba5fa29f503
📒 Files selected for processing (9)
controllers/handlers/cliDownload.gocontrollers/handlers/cliDownload_test.godeploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/kubevirt-hyperconverged-operator.v1.19.0.clusterserviceversion.yamldeploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/kubevirt-hyperconverged-operator.v1.19.0.clusterserviceversion.yamldeploy/operator.yamlpkg/components/components.gopkg/components/components_suite_test.gopkg/components/components_test.gotests/func-tests/cli_download_test.go
✅ Files skipped from review due to trivial changes (2)
- controllers/handlers/cliDownload.go
- controllers/handlers/cliDownload_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/components/components_suite_test.go
- pkg/components/components_test.go
- pkg/components/components.go
tiraboschi
force-pushed
the
poc_virtiowin_download
branch
from
c202ec8 to
a9cabc2
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tools/csv-merger/csv-merger.go (1)
257-258: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider naming the hardcoded version.
"1.36.0"is embedded directly as a literal. Extracting it to a named constant (e.g.,minKubeVersionForImageVolumes) would make the reason for the bump self-documenting and easier to update if the requirement changes again.As per path instructions, go.md recommends "Use shared constants for feature gate names, annotation keys, and label values; avoid duplicated string literals when introducing new ones in this PR's code paths."
♻️ Proposed refactor
+// minKubeVersionForImageVolumes is the minimum Kubernetes version required +// for the ImageVolumeSource used to mount the virtio-win ISO (GA in k8s v1.36). +const minKubeVersionForImageVolumes = "1.36.0" + if *testImagesNVRs != "" { csvBase.Annotations["test-images-nvrs"] = *testImagesNVRs } - csvBase.Spec.MinKubeVersion = "1.36.0" + csvBase.Spec.MinKubeVersion = minKubeVersionForImageVolumes🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/csv-merger/csv-merger.go` around lines 257 - 258, The hardcoded minimum Kubernetes version in csv-merger.go should be named instead of embedded as a literal. Extract "1.36.0" into a shared constant near the csv merging logic and assign csvBase.Spec.MinKubeVersion from that constant, so the version bump is self-documenting and easy to update later. Use a descriptive symbol name such as minKubeVersionForImageVolumes and keep the change localized to the csvBase.Spec.MinKubeVersion assignment path.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tools/csv-merger/csv-merger.go`:
- Around line 257-258: The hardcoded minimum Kubernetes version in csv-merger.go
should be named instead of embedded as a literal. Extract "1.36.0" into a shared
constant near the csv merging logic and assign csvBase.Spec.MinKubeVersion from
that constant, so the version bump is self-documenting and easy to update later.
Use a descriptive symbol name such as minKubeVersionForImageVolumes and keep the
change localized to the csvBase.Spec.MinKubeVersion assignment path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: dc13fd3e-6498-44af-ad52-af25410fc0fd
📒 Files selected for processing (10)
controllers/handlers/cliDownload.gocontrollers/handlers/cliDownload_test.godeploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/kubevirt-hyperconverged-operator.v1.19.0.clusterserviceversion.yamldeploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/kubevirt-hyperconverged-operator.v1.19.0.clusterserviceversion.yamldeploy/operator.yamlpkg/components/components.gopkg/components/components_suite_test.gopkg/components/components_test.gotests/func-tests/cli_download_test.gotools/csv-merger/csv-merger.go
✅ Files skipped from review due to trivial changes (2)
- controllers/handlers/cliDownload_test.go
- controllers/handlers/cliDownload.go
🚧 Files skipped from review as they are similar to previous changes (5)
- deploy/operator.yaml
- pkg/components/components_test.go
- deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/kubevirt-hyperconverged-operator.v1.19.0.clusterserviceversion.yaml
- pkg/components/components_suite_test.go
- pkg/components/components.go
The virtio-win image is already part of the CNV payload but only referenced as a string in a ConfigMap. This change makes the ISO directly downloadable by mounting it into the virt-artifacts-server pod using a Kubernetes image volume (stable in k8s 1.36 / OCP 4.23). The image volume approach mounts the OCI image directly as a read-only volume via the container runtime — no init container, no copy, no ephemeral storage overhead. A subPath mount exposes only the ISO file (/disk/virtio-win.iso) at the server's document root. The virt-artifacts-server is the natural home for this artifact: despite the deployment being named "cli-download", the underlying server image is "virt-artifacts-server" — a general-purpose artifact server, not a CLI-specific one. The virtio-win ISO is a downloadable binary artifact that users need, just like virtctl. Spinning up a dedicated deployment (with its own Service, Route, TLS, ServiceAccount) for a single static file would add operational overhead without architectural benefit. Changes: - Add image volume referencing the virtio-win container image - Mount virtio-win.iso via subPath into the server container - Add virtio-win ISO download link to ConsoleCLIDownload - Set minKubeVersion to 1.36.0 in CSV spec (required for image volumes) - Add unit tests for the deployment spec changes - Add e2e test to verify the ISO is downloadable Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Simone Tiraboschi <stirabos@redhat.com>
tiraboschi
force-pushed
the
poc_virtiowin_download
branch
from
a9cabc2 to
48d9b56
Compare
|
|
@coderabbitai could you please review it again? |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---components.go ImageVolumeSource block---"
rg -n -B5 -A15 'ImageVolumeSource' pkg/components/components.go
echo "---csv-merger.go minKubeVersion---"
rg -n -B3 -A3 'minKubeVersion|MinKubeVersion' tools/csv-merger/csv-merger.go
echo "---CSV files minKubeVersion---"
rg -n 'minKubeVersion' deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/*.yaml deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/*.yaml deploy/operator.yaml || trueLength of output: 10731 Confirmed — both concerns from my previous review have been addressed.
Nothing else stands out — the volume/mount wiring, tests, and generated manifests are all consistent. Nicely done cleaning this up. ✅ Action performedReview finished.
|
|
/retest |
|
@tiraboschi: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
2 participants
What this PR does / why we need it:
The virtio-win image is already part of the CNV payload but only referenced as a string in a ConfigMap. This change makes the ISO directly downloadable by mounting it into the virt-artifacts-server pod using a Kubernetes image volume (GA in k8s 1.36 / OCP 4.23).
The image volume approach mounts the OCI image directly as a read-only volume via the container runtime — no init container, no copy, no ephemeral storage overhead. A subPath mount exposes only the ISO file (/disk/virtio-win.iso) at the server's document root.
The virt-artifacts-server is the natural home for this artifact: despite the deployment being named "cli-download", the underlying server image is "virt-artifacts-server" — a general-purpose artifact server, not a CLI-specific one. The virtio-win ISO is a downloadable binary artifact that users need, just like virtctl. Spinning up a dedicated deployment (with its own Service, Route, TLS, ServiceAccount) for a single static file would add operational overhead without architectural benefit.
Changes:
Reviewer Checklist
Jira Ticket:
Release note: