Skip to content

Enable DeclarativeHotplugVolume by default#4306

Open
dsanatar wants to merge 1 commit into
kubevirt:mainfrom
dsanatar:enable-declarative-hotplug
Open

Enable DeclarativeHotplugVolume by default#4306
dsanatar wants to merge 1 commit into
kubevirt:mainfrom
dsanatar:enable-declarative-hotplug

Conversation

@dsanatar

@dsanatar dsanatar commented Jun 11, 2026

Copy link
Copy Markdown

What this PR does / why we need it:

DeclarativeHotplugVolumes supersedes the now deprecated HotplugVolumes. This PR intends to enable Declarative by default removing the logic that previously defaulted to the using legacy HotplugVolumes when Declarative wasn't explicitly enabled

Addresses: https://redhat.atlassian.net/browse/CNV-81481

Reviewer Checklist

Reviewers are supposed to review the PR for every aspect below one by one. To check an item means the PR is either "OK" or "Not Applicable" in terms of that item. All items are supposed to be checked before merging a PR.

  • PR Message
  • Commit Messages
  • How to test
  • Unit Tests
  • Functional Tests
  • User Documentation
  • Developer Documentation
  • Upgrade Scenario
  • Uninstallation Scenario
  • Backward Compatibility
  • Troubleshooting Friendly

Jira Ticket:

https://redhat.atlassian.net/browse/CNV-69390

Release note:

Enable DeclarativeHotplugVolume by default

@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jun 11, 2026
@kubevirt-bot

Copy link
Copy Markdown
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 avlitman for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details 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

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2fe47f9b-039b-4a9f-8839-5db97ec1a1de

📥 Commits

Reviewing files that changed from the base of the PR and between 16c7db0 and 0629126.

⛔ Files ignored due to path filters (4)
  • api/v1/zz_generated.openapi.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.defaults.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.featuregates_conversion.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.openapi.go is excluded by !**/zz_generated.*
📒 Files selected for processing (18)
  • api/v1/hyperconverged_types.go
  • api/v1beta1/hyperconverged_types.go
  • config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml
  • controllers/handlers/kubevirt.go
  • controllers/handlers/kubevirt_test.go
  • controllers/hyperconverged/hyperconverged_controller_test.go
  • deploy/crds/hco00.crd.yaml
  • deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml
  • deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml
  • docs/api-v1.md
  • docs/api.md
  • docs/cluster-configuration-v1beta1-api.md
  • docs/cluster-configuration.md
  • pkg/featuregatedetails/feature-gates.json
  • pkg/internal/kvfeaturegates/kv-beta-feature-gates.json
  • tests/func-tests/defaults_test.go
  • tools/csv-merger/generated-crd.yaml
  • tools/manifest-templator/generated-crd.yaml
✅ Files skipped from review due to trivial changes (5)
  • controllers/hyperconverged/hyperconverged_controller_test.go
  • docs/cluster-configuration-v1beta1-api.md
  • docs/api.md
  • api/v1/hyperconverged_types.go
  • docs/cluster-configuration.md
🚧 Files skipped from review as they are similar to previous changes (12)
  • tests/func-tests/defaults_test.go
  • controllers/handlers/kubevirt.go
  • api/v1beta1/hyperconverged_types.go
  • pkg/internal/kvfeaturegates/kv-beta-feature-gates.json
  • pkg/featuregatedetails/feature-gates.json
  • tools/manifest-templator/generated-crd.yaml
  • config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml
  • deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml
  • deploy/crds/hco00.crd.yaml
  • deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml
  • tools/csv-merger/generated-crd.yaml
  • controllers/handlers/kubevirt_test.go

📝 Walkthrough

Walkthrough

declarativeHotplugVolumes is promoted from alpha/Developer Preview to beta across the API types, CRD manifests, feature metadata, docs, and tests. Its default is changed to true in the v1/v1beta1 API defaults and generated CRDs. Controller feature-gate selection now appends kvDeclarativeHotplugVolumesGate only when the gate is enabled and no longer falls back to kvHotplugVolumesGate. The KubeVirt beta feature-gates metadata marks DeclarativeHotplugVolumes as beta and HotplugVolumes as deprecated.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Warning

Review ran into problems

🔥 Problems

Linked repositories: Your configuration references 13 linked repositories, but your current plan allows 0. Analyzed ``, skipped kubevirt/kubevirt, `kubevirt/containerized-data-importer`, `kubevirt/ssp-operator`, `kubevirt/cluster-network-addons-operator`, `kubevirt/hostpath-provisioner-operator`, `kubevirt/application-aware-quota`, `kubevirt/managed-tenant-quota`, `kubevirt/hyperconverged-cluster-operator`, `kubevirt/monitoring`, `kubevirt/community`, `kubevirt/enhancements`, `kubevirt/project-infra`, `kubevirt/kubevirtci`.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
controllers/hyperconverged/hyperconverged_controller_test.go (1)

210-210: ⚡ Quick win

Use a shared constant for the DeclarativeHotplugVolumes feature gate name.

The test uses the string literal "DeclarativeHotplugVolumes" directly. According to the coding guidelines for this PR, hotplug-volume feature gate handling should use shared constants rather than duplicated string literals. This reduces the risk of typos and improves maintainability.

Consider referencing a constant from the handlers package (e.g., kvDeclarativeHotplugVolumesGate or similar) that should be defined in controllers/handlers/kubevirt.go.

As per coding guidelines: "For this PR, ensure hotplug-volume feature gate handling uses shared constants (e.g., feature gate names/keys) rather than duplicated string literals"

🤖 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 `@controllers/hyperconverged/hyperconverged_controller_test.go` at line 210,
Replace the hard-coded feature gate string "DeclarativeHotplugVolumes" in the
test with the shared constant defined in the handlers package (e.g., use the
kvDeclarativeHotplugVolumesGate constant from controllers/handlers/kubevirt.go);
update the import to reference the handlers package and use
handlers.kvDeclarativeHotplugVolumesGate (or the actual exported constant name)
wherever the string literal is used so the test relies on the single shared
constant for the hotplug-volume feature gate.

Source: Coding guidelines

🤖 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 `@config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml`:
- Around line 6543-6546: Update the docs to reflect that the CRD field
declarativeHotplugVolumes is GA, always enabled, and no longer used: search for
declarativeHotplugVolumes in docs/cluster-configuration-v1beta1-api.md,
docs/api-v1.md and docs/cluster-configuration.md and replace any "Developer
Preview"/"alpha" labels and "Default: false" notes with text stating it is
GA/always enabled and no longer used (or add a short note pointing to the
follow-up commit if you prefer). Ensure the declarativeHotplugVolumes
row/section content, labels, and default value are consistent with the CRD
description and remove any Developer Preview warnings.

---

Nitpick comments:
In `@controllers/hyperconverged/hyperconverged_controller_test.go`:
- Line 210: Replace the hard-coded feature gate string
"DeclarativeHotplugVolumes" in the test with the shared constant defined in the
handlers package (e.g., use the kvDeclarativeHotplugVolumesGate constant from
controllers/handlers/kubevirt.go); update the import to reference the handlers
package and use handlers.kvDeclarativeHotplugVolumesGate (or the actual exported
constant name) wherever the string literal is used so the test relies on the
single shared constant for the hotplug-volume feature gate.
🪄 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: 62e9667d-ad8c-4c96-8eda-b3f412ecd5f0

📥 Commits

Reviewing files that changed from the base of the PR and between 1bf9421 and e7b650f.

⛔ Files ignored due to path filters (4)
  • api/v1/zz_generated.openapi.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.defaults.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.featuregates_conversion.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.openapi.go is excluded by !**/zz_generated.*
📒 Files selected for processing (14)
  • api/v1/hyperconverged_types.go
  • api/v1beta1/hyperconverged_types.go
  • config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml
  • controllers/handlers/kubevirt.go
  • controllers/handlers/kubevirt_test.go
  • controllers/hyperconverged/hyperconverged_controller_test.go
  • deploy/crds/hco00.crd.yaml
  • deploy/hco.cr.yaml
  • deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml
  • deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml
  • pkg/featuregatedetails/feature-gates.json
  • tests/func-tests/defaults_test.go
  • tools/csv-merger/generated-crd.yaml
  • tools/manifest-templator/generated-crd.yaml
💤 Files with no reviewable changes (4)
  • api/v1/hyperconverged_types.go
  • tests/func-tests/defaults_test.go
  • pkg/featuregatedetails/feature-gates.json
  • deploy/hco.cr.yaml

Comment on lines 6543 to 6546
declarativeHotplugVolumes:
default: false
description: |-
DeclarativeHotplugVolumes enables the use of the declarative volume hotplug feature in KubeVirt.
When set to true, the "DeclarativeHotplugVolumes" feature gate is enabled instead of "HotplugVolumes".
When set to false or nil, the "HotplugVolumes" feature gate is enabled (default behavior).
This feature is in Developer Preview.
description: DeclarativeHotplugVolumes is now GA and always enabled.
This field is no longer used.
type: boolean

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify documentation files are updated to reflect GA status
echo "=== Checking for stale 'Developer Preview' or 'Default: false' references for declarativeHotplugVolumes ==="
rg -n -i "declarativeHotplugVolumes" --type md -A 5 -B 2

Repository: kubevirt/hyperconverged-cluster-operator

Length of output: 10173


GA promo for declarativeHotplugVolumes: fix stale docs
The CRD now says declarativeHotplugVolumes is GA and always enabled and no longer used, but docs still describe it as Developer Preview with Default: false:

  • docs/cluster-configuration-v1beta1-api.md (lines 227-233)
  • docs/api-v1.md (declarativeHotplugVolumes row still marked Developer Preview/alpha)
  • docs/cluster-configuration.md (Developer Preview note)

Update these to match the GA/always-enabled/no-longer-used behavior (or point to the follow-up commit). LGTM on the CRD schema change.

🤖 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 `@config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml` around lines 6543 -
6546, Update the docs to reflect that the CRD field declarativeHotplugVolumes is
GA, always enabled, and no longer used: search for declarativeHotplugVolumes in
docs/cluster-configuration-v1beta1-api.md, docs/api-v1.md and
docs/cluster-configuration.md and replace any "Developer Preview"/"alpha" labels
and "Default: false" notes with text stating it is GA/always enabled and no
longer used (or add a short note pointing to the follow-up commit if you
prefer). Ensure the declarativeHotplugVolumes row/section content, labels, and
default value are consistent with the CRD description and remove any Developer
Preview warnings.

@nunnatsa

Copy link
Copy Markdown
Collaborator

Hi @dsanatar .

DeclarativeHotplugVolumes is not GA in KubeVirt, but it's beta.

/hold

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 11, 2026
@dsanatar

Copy link
Copy Markdown
Author

DeclarativeHotplugVolumes is not GA in KubeVirt, but it's beta.

Hi @nunnatsa, yes I understand this is unconventional however, we are planning to announce this feature as GA in the upcoming 5.0 release and we don't want to require users to still opt in to use the feature.

@dsanatar

Copy link
Copy Markdown
Author

/cc @akalenyu

@kubevirt-bot kubevirt-bot requested a review from akalenyu June 11, 2026 18:32
@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 15, 2026
@dsanatar dsanatar force-pushed the enable-declarative-hotplug branch 2 times, most recently from f1951fe to fa40a79 Compare June 16, 2026 18:32
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 16, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@api/v1beta1/hyperconverged_types.go`:
- Line 633: The field `declarativeHotplugVolumes` has conflicting default values
across different locations in the hyperconverged_types.go file: the kubebuilder
default annotation sets it to true, while the `HyperConvergedSpec.FeatureGates`
default map at an earlier location in the file still declares it as false. Unify
these defaults by updating the `declarativeHotplugVolumes` default value in the
`HyperConvergedSpec.FeatureGates` default map to true so it matches the
kubebuilder default annotation setting.

In `@tools/manifest-templator/generated-crd.yaml`:
- Line 5374: The generated CRD file has `declarativeHotplugVolumes: true` as the
default at lines 5374 and 6551, but the API source-of-truth in
`api/v1beta1/hyperconverged_types.go` still defines it with a default of
`false`. Update the kubebuilder marker or default specification for
`declarativeHotplugVolumes` in the `hyperconverged_types.go` API definition to
set the default to `true`, then regenerate the CRD artifact from the updated API
source to ensure consistency between the API and generated manifest.
🪄 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: b3eb2a6b-6e6c-43ce-b6cc-8beb4958be96

📥 Commits

Reviewing files that changed from the base of the PR and between e7b650f and f1951fe.

⛔ Files ignored due to path filters (4)
  • api/v1/zz_generated.openapi.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.defaults.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.featuregates_conversion.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.openapi.go is excluded by !**/zz_generated.*
📒 Files selected for processing (15)
  • api/v1/hyperconverged_types.go
  • api/v1beta1/hyperconverged_types.go
  • config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml
  • controllers/handlers/kubevirt.go
  • controllers/handlers/kubevirt_test.go
  • controllers/hyperconverged/hyperconverged_controller_test.go
  • deploy/crds/hco00.crd.yaml
  • deploy/hco.cr.yaml
  • deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml
  • deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml
  • docs/api-v1.md
  • docs/api.md
  • pkg/featuregatedetails/feature-gates.json
  • tools/csv-merger/generated-crd.yaml
  • tools/manifest-templator/generated-crd.yaml
💤 Files with no reviewable changes (1)
  • controllers/handlers/kubevirt.go
✅ Files skipped from review due to trivial changes (2)
  • deploy/hco.cr.yaml
  • docs/api.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • controllers/hyperconverged/hyperconverged_controller_test.go

Comment thread api/v1beta1/hyperconverged_types.go Outdated
Comment thread tools/manifest-templator/generated-crd.yaml

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
api/v1beta1/hyperconverged_types.go (1)

633-633: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Conflicting declarativeHotplugVolumes defaults still exist in v1beta1.

Line 633 sets spec.featureGates.declarativeHotplugVolumes to true, but Line 70 still defaults the same gate to false in HyperConvergedSpec.FeatureGates. Please unify both defaults to true to avoid an ambiguous API default contract.

Suggested minimal fix
- // +kubebuilder:default={"downwardMetrics": false, "deployKubeSecondaryDNS": false, "disableMDevConfiguration": false, "persistentReservation": false, "enableMultiArchBootImageImport": false, "decentralizedLiveMigration": true, "declarativeHotplugVolumes": false, "videoConfig": true, "objectGraph": false, "incrementalBackup": false, "containerPathVolumes": false}
+ // +kubebuilder:default={"downwardMetrics": false, "deployKubeSecondaryDNS": false, "disableMDevConfiguration": false, "persistentReservation": false, "enableMultiArchBootImageImport": false, "decentralizedLiveMigration": true, "declarativeHotplugVolumes": true, "videoConfig": true, "objectGraph": false, "incrementalBackup": false, "containerPathVolumes": false}

As per coding guidelines, v1beta1 behavior and defaulting changes must preserve a clear backward-compatible API contract.

🤖 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 `@api/v1beta1/hyperconverged_types.go` at line 633, The kubebuilder default
annotation at line 633 sets declarativeHotplugVolumes to true, but the
HyperConvergedSpec.FeatureGates definition at line 70 still defaults it to
false, creating conflicting defaults in the same v1beta1 API. Update the
declarativeHotplugVolumes field definition in HyperConvergedSpec.FeatureGates at
line 70 to default to true, matching the kubebuilder annotation default at line
633, to ensure a clear and unified API contract.

Source: Coding guidelines

🤖 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.

Duplicate comments:
In `@api/v1beta1/hyperconverged_types.go`:
- Line 633: The kubebuilder default annotation at line 633 sets
declarativeHotplugVolumes to true, but the HyperConvergedSpec.FeatureGates
definition at line 70 still defaults it to false, creating conflicting defaults
in the same v1beta1 API. Update the declarativeHotplugVolumes field definition
in HyperConvergedSpec.FeatureGates at line 70 to default to true, matching the
kubebuilder annotation default at line 633, to ensure a clear and unified API
contract.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c82b57ec-6e58-4fdb-8cfd-bae06c2f988d

📥 Commits

Reviewing files that changed from the base of the PR and between f1951fe and fa40a79.

⛔ Files ignored due to path filters (4)
  • api/v1/zz_generated.openapi.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.defaults.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.featuregates_conversion.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.openapi.go is excluded by !**/zz_generated.*
📒 Files selected for processing (14)
  • api/v1/hyperconverged_types.go
  • api/v1beta1/hyperconverged_types.go
  • config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml
  • controllers/handlers/kubevirt.go
  • controllers/handlers/kubevirt_test.go
  • controllers/hyperconverged/hyperconverged_controller_test.go
  • deploy/crds/hco00.crd.yaml
  • deploy/hco.cr.yaml
  • deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml
  • deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml
  • docs/api-v1.md
  • docs/api.md
  • pkg/featuregatedetails/feature-gates.json
  • tools/csv-merger/generated-crd.yaml
💤 Files with no reviewable changes (1)
  • controllers/handlers/kubevirt.go
✅ Files skipped from review due to trivial changes (3)
  • deploy/hco.cr.yaml
  • docs/api.md
  • docs/api-v1.md
🚧 Files skipped from review as they are similar to previous changes (7)
  • pkg/featuregatedetails/feature-gates.json
  • controllers/hyperconverged/hyperconverged_controller_test.go
  • api/v1/hyperconverged_types.go
  • deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml
  • tools/csv-merger/generated-crd.yaml
  • controllers/handlers/kubevirt_test.go
  • deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml

@coveralls

coveralls commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 28598671859

Coverage decreased (-0.004%) to 81.424%

Details

  • Coverage decreased (-0.004%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 13921
Covered Lines: 11335
Line Coverage: 81.42%
Coverage Strength: 2.18 hits per line

💛 - Coveralls

@dsanatar dsanatar force-pushed the enable-declarative-hotplug branch from fa40a79 to 5906245 Compare June 16, 2026 18:41
Comment on lines -921 to -923
} else {
// Default behavior: use the original HotplugVolumes featuregate
fgs = append(fgs, kvHotplugVolumesGate)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we still need this, if the FG is disabled.

@nunnatsa nunnatsa Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This FG is in alpha phase in the current version v1.9.0-beta.0. When the next version will be released, where the FG will be in beta and HotplugVolumes FG will be deprecated, we could continue.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

if the DeclarativeHotplugVolumes gate is explicitly disabled, we do not want to fall back to the legacy HotplugVolumes anymore since it will be deprecated. But yes these changes reflect the feature gate graduations that will be included in the 1.9 release.

@nunnatsa

nunnatsa commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

To make the test work, please update the default functional test in

"declarativeHotplugVolumes": BeFalseBecause("the declarativeHotplugVolumes feature gate should be disabled by default"),

Change BeFalesBecause() to BeTrueBecause()

@nunnatsa

Copy link
Copy Markdown
Collaborator

Please update the phase and the documentation here:

**Note**: This feature is in Developer Preview.
**Default**: `Disabled`
**Graduation Status**: Alpha

and here:

**Note**: This feature is in Developer Preview.
**Default**: `false`
**Graduation Status**: Alpha

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 17, 2026
@dsanatar dsanatar force-pushed the enable-declarative-hotplug branch from 5906245 to 6df69fe Compare June 22, 2026 14:41
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 22, 2026
@dsanatar dsanatar force-pushed the enable-declarative-hotplug branch from 6df69fe to 16c7db0 Compare June 22, 2026 14:43

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@pkg/internal/kvfeaturegates/kv-beta-feature-gates.json`:
- Line 56: To address the deprecation of HotplugVolumes, first verify that
DeclarativeHotplugVolumes is being automatically enabled as a replacement for
users upgrading from versions where HotplugVolumes was previously enabled, and
implement any necessary upgrade migration logic to handle this transition.
Remove or repurpose the kvHotplugVolumesGate constant definition if it is no
longer referenced in the gate addition logic. Add test cases covering the
deprecation scenario to ensure the transition works correctly. Finally, update
the PR description to explicitly document the backward compatibility strategy,
confirming how existing HotplugVolumes users will be transitioned and that
DeclarativeHotplugVolumes will be guaranteed enabled post-upgrade.
🪄 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: dd522caa-5a09-480d-a5e5-fe521843c749

📥 Commits

Reviewing files that changed from the base of the PR and between 6df69fe and 16c7db0.

⛔ Files ignored due to path filters (4)
  • api/v1/zz_generated.openapi.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.defaults.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.featuregates_conversion.go is excluded by !**/zz_generated.*
  • api/v1beta1/zz_generated.openapi.go is excluded by !**/zz_generated.*
📒 Files selected for processing (18)
  • api/v1/hyperconverged_types.go
  • api/v1beta1/hyperconverged_types.go
  • config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml
  • controllers/handlers/kubevirt.go
  • controllers/handlers/kubevirt_test.go
  • controllers/hyperconverged/hyperconverged_controller_test.go
  • deploy/crds/hco00.crd.yaml
  • deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml
  • deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml
  • docs/api-v1.md
  • docs/api.md
  • docs/cluster-configuration-v1beta1-api.md
  • docs/cluster-configuration.md
  • pkg/featuregatedetails/feature-gates.json
  • pkg/internal/kvfeaturegates/kv-beta-feature-gates.json
  • tests/func-tests/defaults_test.go
  • tools/csv-merger/generated-crd.yaml
  • tools/manifest-templator/generated-crd.yaml
💤 Files with no reviewable changes (1)
  • controllers/handlers/kubevirt.go
✅ Files skipped from review due to trivial changes (6)
  • docs/api-v1.md
  • docs/cluster-configuration-v1beta1-api.md
  • docs/api.md
  • docs/cluster-configuration.md
  • pkg/featuregatedetails/feature-gates.json
  • api/v1/hyperconverged_types.go
🚧 Files skipped from review as they are similar to previous changes (10)
  • tests/func-tests/defaults_test.go
  • controllers/hyperconverged/hyperconverged_controller_test.go
  • api/v1beta1/hyperconverged_types.go
  • deploy/crds/hco00.crd.yaml
  • controllers/handlers/kubevirt_test.go
  • tools/csv-merger/generated-crd.yaml
  • tools/manifest-templator/generated-crd.yaml
  • deploy/olm-catalog/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml
  • config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml
  • deploy/index-image/community-kubevirt-hyperconverged/1.19.0/manifests/hco00.crd.yaml

{
"name": "HotplugVolumes",
"state": "Alpha"
"state": "Deprecated"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify HotplugVolumes deprecation status and upgrade handling

# 1. Check if HotplugVolumes is marked as deprecated in KubeVirt upstream
echo "=== Checking HotplugVolumes state in KubeVirt ==="
rg -n "HotplugVolumes" --type json --type go -C 5

# 2. Search for upgrade logic that handles HotplugVolumes → DeclarativeHotplugVolumes transition
echo -e "\n=== Checking for upgrade migration logic ==="
rg -n "HotplugVolumes.*DeclarativeHotplugVolumes|upgrade.*hotplug" --type go -C 3

# 3. Check if there are any tests for the deprecation scenario
echo -e "\n=== Checking for deprecation tests ==="
rg -n "Deprecated.*HotplugVolumes|HotplugVolumes.*deprecated" --type go -C 3

Repository: kubevirt/hyperconverged-cluster-operator

Length of output: 30392


Verify upgrade and backward compatibility handling for HotplugVolumes deprecation.

The "Deprecated" state for HotplugVolumes is a valid KubeVirt phase recognized by the schema. However, the change raises unresolved backward compatibility concerns per the coding guidelines.

Findings:

  • Tests confirm the intended behavior: DeclarativeHotplugVolumes is included, HotplugVolumes is excluded from the beta gates list (kubevirt_test.go:1897, 2026).
  • No upgrade migration logic was found in the codebase to handle the transition for users who previously had HotplugVolumes enabled.
  • No tests exist for the deprecation scenario.

Issues:

  1. Upgrade scenario: Users upgrading from a version where HotplugVolumes was enabled will silently lose that gate. The PR must document and test how this transition is handled—whether DeclarativeHotplugVolumes automatically substitutes for it or whether manual intervention is required.

  2. Backward compatibility: Per the coding guidelines, changes must be backward compatible or explicitly justify why not. This change lacks explicit justification or a documented compatibility strategy.

  3. Dead code: The constant kvHotplugVolumesGate (kubevirt.go:95) is defined but no longer referenced in gate addition logic. Clarify whether removal is part of this PR.

The PR description should explicitly address: how existing HotplugVolumes users are transitioned, whether DeclarativeHotplugVolumes is guaranteed enabled by default post-upgrade, and confirmation of community agreement on this deprecation timeline.

🤖 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/internal/kvfeaturegates/kv-beta-feature-gates.json` at line 56, To
address the deprecation of HotplugVolumes, first verify that
DeclarativeHotplugVolumes is being automatically enabled as a replacement for
users upgrading from versions where HotplugVolumes was previously enabled, and
implement any necessary upgrade migration logic to handle this transition.
Remove or repurpose the kvHotplugVolumesGate constant definition if it is no
longer referenced in the gate addition logic. Add test cases covering the
deprecation scenario to ensure the transition works correctly. Finally, update
the PR description to explicitly document the backward compatibility strategy,
confirming how existing HotplugVolumes users will be transitioned and that
DeclarativeHotplugVolumes will be guaranteed enabled post-upgrade.

Source: Coding guidelines

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2026
in kubevirt 1.9 declarative has been graduated to beta
and the legacy feature gate HotplugVolumes has been deprecated.
change logic to default to declarative.

Signed-off-by: dsanatar <dsanatar@redhat.com>
@dsanatar dsanatar force-pushed the enable-declarative-hotplug branch from 16c7db0 to 0629126 Compare July 2, 2026 14:40
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 2, 2026
@kubevirt-prow

kubevirt-prow Bot commented Jul 2, 2026

Copy link
Copy Markdown

[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 avlitman for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details 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

@sonarqubecloud

sonarqubecloud Bot commented Jul 2, 2026

Copy link
Copy Markdown

@openshift-ci

openshift-ci Bot commented Jul 2, 2026

Copy link
Copy Markdown

@dsanatar: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/hco-e2e-upgrade-operator-sdk-sno-aws 0629126 link false /test hco-e2e-upgrade-operator-sdk-sno-aws
ci/prow/hco-e2e-consecutive-operator-sdk-upgrades-aws 0629126 link true /test hco-e2e-consecutive-operator-sdk-upgrades-aws
ci/prow/hco-e2e-upgrade-prev-operator-sdk-aws 0629126 link true /test hco-e2e-upgrade-prev-operator-sdk-aws
ci/prow/hco-e2e-operator-sdk-aws 0629126 link true /test hco-e2e-operator-sdk-aws
ci/prow/hco-e2e-upgrade-operator-sdk-aws 0629126 link true /test hco-e2e-upgrade-operator-sdk-aws

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

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

Labels

dco-signoff: yes Indicates the PR's author has DCO signed all their commits. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants