CNTRLPLANE-2207: Upgrade to CAPI 1.11#8594
Conversation
|
@enxebre: This pull request references CNTRLPLANE-2207 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "5.0." or "openshift-5.0.", but it targets "openshift-4.22" instead. DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
|
supersedes #7590 |
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThis PR centralizes CRD inclusion and conversion wiring (crdIncludeFilter, CAPICRDOverrides), adds --disable-capi-conversion-webhook and operator webhook defaulting, narrows Makefile CRD generation, registers a hostedcluster conversion webhook, migrates Cluster API imports to api/core v1beta1, removes backwardcompat CAPI-image logic (adding ClusterImagePolicy normalization), updates go.mod dependency versions, extends nodepool completion to consult conversion-data, and updates related tests and e2e AWS capacity checks. Sequence Diagram(s)sequenceDiagram
participant CLI
participant Installer
participant CRDLoader as setupCRDs
participant CAPIO as crdassets.CAPICRDOverrides
participant CRD as CustomResourceDefinition
participant Operator
participant Webhook as conversion.Webhook
CLI->>Installer: parse --disable-capi-conversion-webhook
Installer->>CRDLoader: build CRD list & apply crdIncludeFilter
CRDLoader->>CAPIO: consult CAPICRDOverrides by CRD name
CAPIO->>CRDLoader: provide StorageVersion/NeedsConversion
CRDLoader->>CRD: set storage version and spec.conversion as needed
Installer->>Operator: set EnableWebhook based on flags/options
Operator->>Webhook: register /convert endpoint (hostedcluster)
🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8594 +/- ##
==========================================
+ Coverage 41.26% 41.29% +0.02%
==========================================
Files 755 755
Lines 93443 93456 +13
==========================================
+ Hits 38563 38593 +30
+ Misses 52148 52134 -14
+ Partials 2732 2729 -3
... and 1 file with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go (1)
2508-2543: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd unit coverage for the CAPI manager behavior change.
This now changes both the image-selection path and the rendered ClusterRoleBinding subjects, but there’s no matching unit coverage in the provided diff. A focused test around
reconcileCAPIManager/reconcileCAPIManagerClusterRoleBindingwould lock down the annotation-only image path and the extracapi-providersubject.As per coding guidelines, "Unit test any code changes and additions".
Also applies to: 2962-2984
🤖 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 `@hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go` around lines 2508 - 2543, Add focused unit tests for reconcileCAPIManager and reconcileCAPIManagerClusterRoleBinding: create tests that (1) verify image selection when the hyperv1.ClusterAPIManagerImage annotation is present (ensure capimanagerv2.NewComponent receives the annotation image path / capiManager.Reconcile is invoked with that image) and (2) validate the rendered ClusterRoleBinding subjects include the extra "capi-provider" subject introduced by reconcileCAPIManagerClusterRoleBinding (use clusterapi.CAPIManagerClusterRoleBinding to construct expected object and assert subjects list). Exercise both the annotation-only image path and the default image path, mocking or faking the createOrUpdate flow and controlPlaneNamespace lookups as needed so reconcileCAPIManager and reconcileCAPIManagerClusterRoleBinding behavior is asserted deterministically.test/e2e/autoscaling_test.go (1)
748-751:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe scale-from-zero workload is no longer constrained to the scale-from-zero NodePool.
Without targeting the labeled NodePool, pods can schedule on existing nodes and the test may pass/fail without actually validating scale-from-zero behavior.
Proposed fix
workload := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ Name: "scale-from-zero-workload", Namespace: "default", }, Spec: batchv1.JobSpec{ Template: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ + NodeSelector: map[string]string{ + "scale-from-zero-test": "true", + }, Containers: []corev1.Container{Also applies to: 889-928
🤖 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 `@test/e2e/autoscaling_test.go` around lines 748 - 751, The test's pods aren't constrained to the scale-from-zero NodePool because the workload spec doesn't target the NodePool's label; update the workload pod/Deployment/Job spec to include a nodeSelector or nodeAffinity that matches the NodePool label key "scale-from-zero-test" with value "true" so pods can only schedule on nodes from scaleFromZeroNP (ensure you modify the workload creation code that references scaleFromZeroNP and add the selector/affinity there); apply the same change to the other similar test block that uses scaleFromZeroNP further down in the file.
🧹 Nitpick comments (1)
hypershift-operator/controllers/nodepool/capi_test.go (1)
3283-3422: 💤 Low valueConsider adding
t.Parallel()for consistency with other tests in this file.Most other tests in this file use
t.Parallel(). Since this test has no shared state and uses table-driven subtests, it would benefit from parallel execution.func TestMachineDeploymentComplete(t *testing.T) { + t.Parallel() two := int32(2) three := int32(3)🤖 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 `@hypershift-operator/controllers/nodepool/capi_test.go` around lines 3283 - 3422, The TestMachineDeploymentComplete test should run in parallel like other tests: add t.Parallel() as the first statement inside the TestMachineDeploymentComplete function, and inside the t.Run closure capture the loop variable (e.g. tc := tc) then call t.Parallel() immediately after entering the subtest so each table-driven subtest runs concurrently; update the TestMachineDeploymentComplete function and its t.Run anonymous func accordingly (referencing TestMachineDeploymentComplete and the t.Run closure).
🤖 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.
Outside diff comments:
In `@hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go`:
- Around line 2508-2543: Add focused unit tests for reconcileCAPIManager and
reconcileCAPIManagerClusterRoleBinding: create tests that (1) verify image
selection when the hyperv1.ClusterAPIManagerImage annotation is present (ensure
capimanagerv2.NewComponent receives the annotation image path /
capiManager.Reconcile is invoked with that image) and (2) validate the rendered
ClusterRoleBinding subjects include the extra "capi-provider" subject introduced
by reconcileCAPIManagerClusterRoleBinding (use
clusterapi.CAPIManagerClusterRoleBinding to construct expected object and assert
subjects list). Exercise both the annotation-only image path and the default
image path, mocking or faking the createOrUpdate flow and controlPlaneNamespace
lookups as needed so reconcileCAPIManager and
reconcileCAPIManagerClusterRoleBinding behavior is asserted deterministically.
In `@test/e2e/autoscaling_test.go`:
- Around line 748-751: The test's pods aren't constrained to the scale-from-zero
NodePool because the workload spec doesn't target the NodePool's label; update
the workload pod/Deployment/Job spec to include a nodeSelector or nodeAffinity
that matches the NodePool label key "scale-from-zero-test" with value "true" so
pods can only schedule on nodes from scaleFromZeroNP (ensure you modify the
workload creation code that references scaleFromZeroNP and add the
selector/affinity there); apply the same change to the other similar test block
that uses scaleFromZeroNP further down in the file.
---
Nitpick comments:
In `@hypershift-operator/controllers/nodepool/capi_test.go`:
- Around line 3283-3422: The TestMachineDeploymentComplete test should run in
parallel like other tests: add t.Parallel() as the first statement inside the
TestMachineDeploymentComplete function, and inside the t.Run closure capture the
loop variable (e.g. tc := tc) then call t.Parallel() immediately after entering
the subtest so each table-driven subtest runs concurrently; update the
TestMachineDeploymentComplete function and its t.Run anonymous func accordingly
(referencing TestMachineDeploymentComplete and the t.Run closure).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 3dd6979b-e808-49a4-a4e9-6adff6e87c10
⛔ Files ignored due to path filters (230)
api/go.sumis excluded by!**/*.sumapi/vendor/modules.txtis excluded by!**/vendor/**api/vendor/sigs.k8s.io/structured-merge-diff/v6/value/reflectcache.gois excluded by!**/vendor/**cmd/install/assets/crds/cluster-api-provider-aws/infrastructure.cluster.x-k8s.io_awsclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-aws/infrastructure.cluster.x-k8s.io_awsmachinepools.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-aws/infrastructure.cluster.x-k8s.io_awsmachines.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-aws/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-azure/infrastructure.cluster.x-k8s.io_azureclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-azure/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-gcp/infrastructure.cluster.x-k8s.io_gcpclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-gcp/infrastructure.cluster.x-k8s.io_gcpclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-gcp/infrastructure.cluster.x-k8s.io_gcpmachines.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-gcp/infrastructure.cluster.x-k8s.io_gcpmachinetemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmpowervsclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmpowervsimages.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmvpcclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmvpcclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmvpcmachines.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-kubevirt/infrastructure.cluster.x-k8s.io_kubevirtclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-openstack/infrastructure.cluster.x-k8s.io_openstackclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-openstack/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/addons.cluster.x-k8s.io_clusterresourcesetbindings.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/addons.cluster.x-k8s.io_clusterresourcesets.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_clusterclasses.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_clusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinedeployments.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinedrainrules.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinehealthchecks.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinepools.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machines.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinesets.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/ipam.cluster.x-k8s.io_ipaddressclaims.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/ipam.cluster.x-k8s.io_ipaddresses.yamlis excluded by!cmd/install/assets/**/*.yamlgo.sumis excluded by!**/*.sumvendor/github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets/_metadata.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets/assets.jsonis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets/client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets/tsp-location.yamlis excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/Azure/go-ansiterm/osc_string_state.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/clients/instance/ibm-pi-instance.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/clients/instance/ibm-pi-network.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/clients/instance/ibm-pi-routes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/clients/instance/ibm-pi-sap-instance.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/clients/instance/ibm-pi-shared-processor-pool.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/clients/instance/ibm-pi-ssh-key.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/clients/instance/ibm-pi-virtual-serial-number.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/clients/instance/ibm-pi-vpn-policy.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/clients/instance/ibm-pi-vpn.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/clients/instance/ibm-pi-workspaces.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/errors/errors.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/helpers/constants.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/p_cloud_s_a_p/pcloud_sap_getall_parameters.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/p_cloud_virtual_serial_number/p_cloud_virtual_serial_number_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/p_cloud_virtual_serial_number/pcloud_pvminstances_virtualserialnumber_put_responses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/p_cloud_virtual_serial_number/pcloud_virtualserialnumber_softwaretiers_getall_parameters.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/p_cloud_virtual_serial_number/pcloud_virtualserialnumber_softwaretiers_getall_responses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/power_iaas_api_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/routes/routes_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/routes/v1_routes_delete_parameters.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/routes/v1_routes_delete_responses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/routes/v1_routes_get_parameters.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/routes/v1_routes_get_responses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/routes/v1_routes_getall_parameters.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/routes/v1_routes_getall_responses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/routes/v1_routes_post_parameters.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/routes/v1_routes_post_responses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/routes/v1_routes_put_parameters.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/routes/v1_routes_put_responses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/routes/v1_routes_report_get_parameters.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/routes/v1_routes_report_get_responses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/ssh_keys/ssh_keys_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/ssh_keys/v1_sshkeys_delete_parameters.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/ssh_keys/v1_sshkeys_delete_responses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/ssh_keys/v1_sshkeys_get_parameters.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/ssh_keys/v1_sshkeys_get_responses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/ssh_keys/v1_sshkeys_getall_parameters.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/ssh_keys/v1_sshkeys_getall_responses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/ssh_keys/v1_sshkeys_post_parameters.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/ssh_keys/v1_sshkeys_post_responses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/ssh_keys/v1_sshkeys_put_parameters.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/client/ssh_keys/v1_sshkeys_put_responses.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/create_server_virtual_serial_number.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/create_workspace_ssh_key.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/get_server_virtual_serial_number.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/network.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/network_create.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/network_reference.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/network_update.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/p_vm_instance_create.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/placement_group.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/placement_group_create.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/route.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/route_create.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/route_report.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/route_report_route.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/route_update.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/routes.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/s_a_p_create.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/s_a_p_profile.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/s_p_p_placement_group.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/s_p_p_placement_group_create.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/snapshot.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/snapshot_create.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/snapshot_update.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/software_tier.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/supported_software_tier.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/supported_software_tier_list.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/system.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/system_resources.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/update_server_virtual_serial_number.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/update_workspace_ssh_key.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/virtual_serial_number.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/workspace_ssh_key.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM-Cloud/power-go-client/power/models/workspace_ssh_keys.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/go-sdk-core/v5/core/authenticator_factory.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/go-sdk-core/v5/core/base_service.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/go-sdk-core/v5/core/constants.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/go-sdk-core/v5/core/container_authenticator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/go-sdk-core/v5/core/cp4d_authenticator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/go-sdk-core/v5/core/iam_assume_authenticator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/go-sdk-core/v5/core/iam_authenticator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/go-sdk-core/v5/core/mcsp_v1_authenticator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/go-sdk-core/v5/core/mcsp_v2_authenticator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/go-sdk-core/v5/core/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/ibm-cos-sdk-go/aws/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/networking-go-sdk/common/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/networking-go-sdk/transitgatewayapisv1/transit_gateway_apis_v1.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/platform-services-go-sdk/common/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/platform-services-go-sdk/globalcatalogv1/global_catalog_v1.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/platform-services-go-sdk/globaltaggingv1/global_tagging_v1.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/platform-services-go-sdk/iamidentityv1/iam_identity_v1.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/platform-services-go-sdk/iampolicymanagementv1/iam_policy_management_v1.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/vpc-go-sdk/common/version.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/IBM/vpc-go-sdk/vpcv1/vpc_v1.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cenkalti/backoff/v5/exponential.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/cenkalti/backoff/v5/retry.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/gabriel-vasile/mimetype/.golangci.ymlis excluded by!vendor/**,!**/vendor/**vendor/github.com/gabriel-vasile/mimetype/internal/json/json.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/gabriel-vasile/mimetype/internal/json/parser.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/gabriel-vasile/mimetype/internal/magic/archive.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/gabriel-vasile/mimetype/internal/magic/binary.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/gabriel-vasile/mimetype/internal/magic/geo.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/gabriel-vasile/mimetype/internal/magic/text.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/gabriel-vasile/mimetype/supported_mimes.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/gabriel-vasile/mimetype/tree.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/analysis/flatten.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/analysis/flatten_name.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/analysis/flatten_options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/analysis/internal/debug/debug.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/analysis/internal/flatten/replace/replace.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/analysis/internal/flatten/sortref/keys.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/analysis/mixin.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/analysis/schema.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/loads/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/loads/TODO.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/loads/loaders.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/loads/spec.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/bytestream.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/client/request.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/client/runtime.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/csv.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/csv_options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/logger/standard.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/middleware/context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/middleware/denco/router.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/middleware/go18.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/middleware/pre_go18.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/middleware/rapidoc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/middleware/redoc.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/middleware/request.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/middleware/router.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/middleware/spec.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/middleware/swaggerui.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/middleware/swaggerui_oauth2.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/middleware/ui_defaults.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/middleware/ui_options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/runtime/middleware/validation.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/spec/README.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/spec/expander.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/BENCHMARK.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/default_validator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/example_validator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/formats.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/helpers.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/object_validator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/options.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/pools.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/pools_debug.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/result.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/schema.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/schema_option.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/schema_props.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/slice_validator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/spec.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/spec_messages.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/type.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/go-openapi/validate/validator.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/gophercloud/gophercloud/v2/CHANGELOG.mdis excluded by!vendor/**,!**/vendor/**vendor/github.com/gophercloud/gophercloud/v2/Makefileis excluded by!vendor/**,!**/vendor/**vendor/github.com/gophercloud/gophercloud/v2/endpoint_search.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/gophercloud/gophercloud/v2/provider_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/gophercloud/gophercloud/v2/results.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/gophercloud/gophercloud/v2/service_client.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/gophercloud/gophercloud/v2/util.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**,!**/*.pb.govendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2.protois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2/options/openapiv2_protoopaque.pb.gois excluded by!**/*.pb.go,!vendor/**,!**/vendor/**,!**/*.pb.govendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/BUILD.bazelis excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/context.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/marshal_jsonpb.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/grpc-ecosystem/grpc-gateway/v2/runtime/mux.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/moby/term/term_unix.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift-online/ocm-common/pkg/resource/validations/kms_arn_regex_validation.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/cluster-api-provider-agent/api/v1alpha1/agentcluster_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/cluster-api-provider-agent/api/v1alpha1/agentmachine_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/cluster-api-provider-agent/api/v1alpha1/zz_generated.deepcopy.gois excluded by!vendor/**,!**/vendor/**,!**/zz_generated*.go,!**/zz_generated*vendor/github.com/openshift/cluster-api-provider-agent/api/v1beta1/agentcluster_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/cluster-api-provider-agent/api/v1beta1/agentmachine_types.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/cluster-api-provider-agent/api/v1beta1/zz_generated.deepcopy.gois excluded by!vendor/**,!**/vendor/**,!**/zz_generated*.go,!**/zz_generated*vendor/go.mongodb.org/mongo-driver/bson/bsoncodec/default_value_decoders.gois excluded by!vendor/**,!**/vendor/**vendor/go.mongodb.org/mongo-driver/bson/unmarshal.gois excluded by!vendor/**,!**/vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/LICENSEis excluded by!vendor/**,!**/vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/exporter.gois excluded by!vendor/**,!**/vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/attribute.gois excluded by!vendor/**,!**/vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/instrumentation.gois excluded by!vendor/**,!**/vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/resource.gois excluded by!vendor/**,!**/vendor/**vendor/go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/tracetransform/span.gois excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (70)
.codespellignore.golangci.ymlMakefileapi/go.modcmd/cluster/core/dump.gocmd/install/assets/crds/assets.gocmd/install/install.gocmd/install/install_test.gocontrib/gomaxprocs-webhook/Dockerfilecontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/kubeconfig.gocontrol-plane-operator/hostedclusterconfigoperator/api/scheme.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/setup.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/machine/machine.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/machine/machine_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/machine/setup.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/node/node.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/node/node_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/spotremediation/spotremediation.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/spotremediation/spotremediation_test.gogo.modhypershift-operator/controllers/hostedcluster/hostedcluster_controller.gohypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.gohypershift-operator/controllers/hostedcluster/hostedcluster_webhook.gohypershift-operator/controllers/hostedcluster/internal/platform/agent/agent.gohypershift-operator/controllers/hostedcluster/internal/platform/agent/agent_test.gohypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.gohypershift-operator/controllers/hostedcluster/internal/platform/azure/azure.gohypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp.gohypershift-operator/controllers/hostedcluster/internal/platform/ibmcloud/ibmcloud.gohypershift-operator/controllers/hostedcluster/internal/platform/ibmcloud/ibmcloud_test.gohypershift-operator/controllers/hostedcluster/internal/platform/kubevirt/kubevirt.gohypershift-operator/controllers/hostedcluster/internal/platform/kubevirt/kubevirt_test.gohypershift-operator/controllers/hostedcluster/internal/platform/openstack/openstack.gohypershift-operator/controllers/hostedcluster/internal/platform/openstack/openstack_test.gohypershift-operator/controllers/hostedcluster/internal/platform/platform.gohypershift-operator/controllers/hostedcluster/internal/platform/powervs/powervs.gohypershift-operator/controllers/manifests/controlplaneoperator/manifests.gohypershift-operator/controllers/nodepool/aws.gohypershift-operator/controllers/nodepool/aws_test.gohypershift-operator/controllers/nodepool/azure_test.gohypershift-operator/controllers/nodepool/capi.gohypershift-operator/controllers/nodepool/capi_test.gohypershift-operator/controllers/nodepool/conditions.gohypershift-operator/controllers/nodepool/conditions_test.gohypershift-operator/controllers/nodepool/gcp.gohypershift-operator/controllers/nodepool/metrics/metrics.gohypershift-operator/controllers/nodepool/nodepool_controller.gohypershift-operator/controllers/nodepool/nodepool_controller_test.gohypershift-operator/controllers/nodepool/powervs.gohypershift-operator/controllers/nodepool/scale_from_zero_test.gohypershift-operator/controllers/nodepool/version.gohypershift-operator/controllers/nodepool/version_test.gosupport/api/capi_types.gosupport/api/scheme.gosupport/backwardcompat/backwardcompat.gosupport/backwardcompat/backwardcompat_test.gosupport/k8sutil/resources.gosupport/upsert/upsert.gotest/e2e/autoscaling_test.gotest/e2e/nodepool_day2_tags_test.gotest/e2e/nodepool_kv_advanced_multinet_test.gotest/e2e/nodepool_osp_advanced_test.gotest/e2e/nodepool_rolling_upgrade_test.gotest/e2e/nodepool_spot_termination_handler_test.gotest/e2e/upgrade_hypershift_operator_test.gotest/e2e/util/util.gotest/e2e/v2/backuprestore/cleanup.go
💤 Files with no reviewable changes (3)
- support/backwardcompat/backwardcompat_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/platform.go
- support/backwardcompat/backwardcompat.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/install/install.go (1)
355-361: 💤 Low valueVerify intentional asymmetry with
EnableAuditLogPersistence.The replica scaling condition here doesn't include
opts.EnableAuditLogPersistence, but theEnableWebhookassignment at line 1256 does. This means if onlyEnableAuditLogPersistenceis true (withDisableCAPIConversionWebhook=trueand other webhooks disabled), the operator will run with 1 replica but have webhooks enabled.If this is intentional (audit log mutating webhook doesn't need HA), no change needed. If webhooks generally require 2 replicas for availability, consider adding
|| o.EnableAuditLogPersistencehere.🤖 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 `@cmd/install/install.go` around lines 355 - 361, The replica-setting branch for o.HyperShiftOperatorReplicas omits o.EnableAuditLogPersistence, causing inconsistent behavior versus the webhook enablement logic; update the conditional that sets o.HyperShiftOperatorReplicas (the case that currently checks o.EnableDefaultingWebhook || o.EnableConversionWebhook || o.EnableValidatingWebhook || !o.DisableCAPIConversionWebhook) to also include || o.EnableAuditLogPersistence so that when audit-log persistence enables the mutating webhook the operator scales to 2 replicas consistently with the webhook flags.
🤖 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 `@cmd/install/install.go`:
- Around line 355-361: The replica-setting branch for
o.HyperShiftOperatorReplicas omits o.EnableAuditLogPersistence, causing
inconsistent behavior versus the webhook enablement logic; update the
conditional that sets o.HyperShiftOperatorReplicas (the case that currently
checks o.EnableDefaultingWebhook || o.EnableConversionWebhook ||
o.EnableValidatingWebhook || !o.DisableCAPIConversionWebhook) to also include ||
o.EnableAuditLogPersistence so that when audit-log persistence enables the
mutating webhook the operator scales to 2 replicas consistently with the webhook
flags.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: c787e632-093a-43b4-8ac4-832bc81c3965
📒 Files selected for processing (1)
cmd/install/install.go
5e84119 to
3569ce9
Compare
3569ce9 to
819cca6
Compare
Signed-off-by: Borja Clemente <bclement@redhat.com>
Signed-off-by: Borja Clemente <bclement@redhat.com>
Remove the temporary hardocded CAPI image overrides now that hypershift supports CAPI 1.11 Signed-off-by: Borja Clemente <bclement@redhat.com>
For conversion to work, the CAPI provider needs to be able to access CRDs cluster-wide to list available versions. Signed-off-by: Borja Clemente <bclement@redhat.com>
Update TestScaleFromZero to support both CAPI 1.11+ native Status.Capacity and pre-1.11 annotation-based capacity information. In CAPI 1.11, cluster-api-provider-aws now populates Status.Capacity directly on AWSMachineTemplate, making the workaround annotations unnecessary. The HyperShift controller detects this and skips setting annotations when Status.Capacity is present. The test now: - First checks AWSMachineTemplate.Status.Capacity (CAPI 1.11+) - Falls back to MachineDeployment annotations (pre-CAPI 1.11) - Logs the capacity source for debugging This makes the test backward compatible and fixes the failure in PR openshift#7590.
Setting the MinReadySeconds default to 0 explicitly on the nodepool controller causes infinite reconciliaiton due to a lossy v1beta1 -> v1beta2 conversion and flipping value between 0 and nil. Removing the explicit setting should not have any other side effect since the zero value of the field is the same. Signed-off-by: Borja Clemente <bclement@redhat.com>
58135e0 to
9d70224
Compare
There was a problem hiding this comment.
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 `@hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go`:
- Around line 4373-4376: The code dereferences OVNKubernetesConfig.IPv6 fields
when only OVNKubernetesConfig is checked; update the conditional that sets
ipv6JoinSubnet and ipv6TransitSubnet to also verify
hc.Spec.OperatorConfiguration.ClusterNetworkOperator.OVNKubernetesConfig.IPv6 !=
nil (or guard access with a separate nil check) before reading
IPv6.InternalJoinSubnet and IPv6.InternalTransitSwitchSubnet so the reconciler
won’t panic on IPv4-only or absent IPv6 configs; ensure the logic that sets
ipv6JoinSubnet/ipv6TransitSubnet falls back to empty/default values when the
IPv6 struct is nil.
- Around line 2252-2257: kasServingCertHashFromEndpoint currently calls
tls.Dialer.DialContext using the reconcile ctx with no timeout; update this
probe to derive a short-lived context via context.WithTimeout (e.g., a few
seconds) and use that derived context when calling DialContext so the dial
cannot hang indefinitely. Locate kasServingCertHashFromEndpoint and the
tls.Dialer.DialContext call and wrap the dial in a context.WithTimeout/Cancel,
pass the new ctx into DialContext, and ensure you defer the cancel to release
resources.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: f889e68d-e39a-441c-84f2-1602dddc56ec
⛔ Files ignored due to path filters (35)
api/go.sumis excluded by!**/*.sumapi/vendor/modules.txtis excluded by!**/vendor/**api/vendor/sigs.k8s.io/structured-merge-diff/v6/value/reflectcache.gois excluded by!**/vendor/**cmd/install/assets/crds/cluster-api-provider-aws/infrastructure.cluster.x-k8s.io_awsclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-aws/infrastructure.cluster.x-k8s.io_awsmachinepools.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-aws/infrastructure.cluster.x-k8s.io_awsmachines.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-aws/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-azure/infrastructure.cluster.x-k8s.io_azureclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-azure/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-gcp/infrastructure.cluster.x-k8s.io_gcpclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-gcp/infrastructure.cluster.x-k8s.io_gcpclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-gcp/infrastructure.cluster.x-k8s.io_gcpmachines.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-gcp/infrastructure.cluster.x-k8s.io_gcpmachinetemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmpowervsclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmpowervsimages.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmvpcclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmvpcclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmvpcmachines.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-kubevirt/infrastructure.cluster.x-k8s.io_kubevirtclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-openstack/infrastructure.cluster.x-k8s.io_openstackclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-openstack/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/addons.cluster.x-k8s.io_clusterresourcesetbindings.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/addons.cluster.x-k8s.io_clusterresourcesets.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_clusterclasses.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_clusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinedeployments.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinedrainrules.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinehealthchecks.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinepools.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machines.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinesets.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/ipam.cluster.x-k8s.io_ipaddressclaims.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/ipam.cluster.x-k8s.io_ipaddresses.yamlis excluded by!cmd/install/assets/**/*.yamlgo.sumis excluded by!**/*.sum
📒 Files selected for processing (63)
.codespellignore.golangci.ymlMakefileapi/go.modcmd/cluster/core/dump.gocmd/install/assets/crds/assets.gocmd/install/install.gocmd/install/install_test.gocontrib/gomaxprocs-webhook/Dockerfilecontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/kubeconfig.gocontrol-plane-operator/hostedclusterconfigoperator/api/scheme.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/setup.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/machine/machine.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/machine/machine_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/machine/setup.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/node/node.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/node/node_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/spotremediation/spotremediation.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/spotremediation/spotremediation_test.gogo.modhypershift-operator/controllers/hostedcluster/hostedcluster_controller.gohypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.gohypershift-operator/controllers/hostedcluster/hostedcluster_webhook.gohypershift-operator/controllers/hostedcluster/internal/platform/agent/agent.gohypershift-operator/controllers/hostedcluster/internal/platform/agent/agent_test.gohypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.gohypershift-operator/controllers/hostedcluster/internal/platform/azure/azure.gohypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp.gohypershift-operator/controllers/hostedcluster/internal/platform/ibmcloud/ibmcloud.gohypershift-operator/controllers/hostedcluster/internal/platform/ibmcloud/ibmcloud_test.gohypershift-operator/controllers/hostedcluster/internal/platform/kubevirt/kubevirt.gohypershift-operator/controllers/hostedcluster/internal/platform/kubevirt/kubevirt_test.gohypershift-operator/controllers/hostedcluster/internal/platform/openstack/openstack.gohypershift-operator/controllers/hostedcluster/internal/platform/openstack/openstack_test.gohypershift-operator/controllers/hostedcluster/internal/platform/platform.gohypershift-operator/controllers/hostedcluster/internal/platform/powervs/powervs.gohypershift-operator/controllers/manifests/controlplaneoperator/manifests.gohypershift-operator/controllers/nodepool/aws.gohypershift-operator/controllers/nodepool/aws_test.gohypershift-operator/controllers/nodepool/azure_test.gohypershift-operator/controllers/nodepool/capi.gohypershift-operator/controllers/nodepool/capi_test.gohypershift-operator/controllers/nodepool/conditions.gohypershift-operator/controllers/nodepool/conditions_test.gohypershift-operator/controllers/nodepool/gcp.gohypershift-operator/controllers/nodepool/metrics/metrics.gohypershift-operator/controllers/nodepool/nodepool_controller.gohypershift-operator/controllers/nodepool/nodepool_controller_test.gohypershift-operator/controllers/nodepool/powervs.gohypershift-operator/controllers/nodepool/scale_from_zero_test.gohypershift-operator/controllers/nodepool/version.gohypershift-operator/controllers/nodepool/version_test.gokarpenter-operator/controllers/karpenter/karpenter_controller.gokarpenter-operator/controllers/karpenter/karpenter_controller_test.gosupport/api/capi_types.gosupport/api/scheme.gosupport/backwardcompat/backwardcompat.gosupport/backwardcompat/backwardcompat_test.gosupport/k8sutil/resources.gosupport/upsert/upsert.go
💤 Files with no reviewable changes (39)
- hypershift-operator/controllers/manifests/controlplaneoperator/manifests.go
- support/upsert/upsert.go
- support/k8sutil/resources.go
- hypershift-operator/controllers/nodepool/powervs.go
- hypershift-operator/controllers/nodepool/scale_from_zero_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go
- hypershift-operator/controllers/nodepool/metrics/metrics.go
- hypershift-operator/controllers/nodepool/aws.go
- karpenter-operator/controllers/karpenter/karpenter_controller_test.go
- hypershift-operator/controllers/nodepool/version.go
- hypershift-operator/controllers/hostedcluster/internal/platform/agent/agent_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/ibmcloud/ibmcloud_test.go
- hypershift-operator/controllers/nodepool/conditions_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/agent/agent.go
- hypershift-operator/controllers/nodepool/gcp.go
- hypershift-operator/controllers/hostedcluster/hostedcluster_webhook.go
- hypershift-operator/controllers/hostedcluster/internal/platform/kubevirt/kubevirt_test.go
- hypershift-operator/controllers/nodepool/aws_test.go
- support/api/capi_types.go
- hypershift-operator/controllers/hostedcluster/internal/platform/ibmcloud/ibmcloud.go
- hypershift-operator/controllers/nodepool/version_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/openstack/openstack_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/openstack/openstack.go
- hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/azure/azure.go
- hypershift-operator/controllers/hostedcluster/internal/platform/powervs/powervs.go
- support/api/scheme.go
- hypershift-operator/controllers/nodepool/azure_test.go
- hypershift-operator/controllers/nodepool/conditions.go
- karpenter-operator/controllers/karpenter/karpenter_controller.go
- support/backwardcompat/backwardcompat.go
- hypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp.go
- hypershift-operator/controllers/hostedcluster/internal/platform/platform.go
- hypershift-operator/controllers/nodepool/nodepool_controller_test.go
- hypershift-operator/controllers/nodepool/capi.go
- support/backwardcompat/backwardcompat_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/kubevirt/kubevirt.go
- hypershift-operator/controllers/nodepool/capi_test.go
- hypershift-operator/controllers/nodepool/nodepool_controller.go
✅ Files skipped from review due to trivial changes (3)
- .codespellignore
- cmd/cluster/core/dump.go
- control-plane-operator/hostedclusterconfigoperator/controllers/spotremediation/spotremediation.go
🚧 Files skipped from review as they are similar to previous changes (19)
- contrib/gomaxprocs-webhook/Dockerfile
- .golangci.yml
- api/go.mod
- control-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/spotremediation/spotremediation_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/machine/setup.go
- control-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps_test.go
- cmd/install/assets/crds/assets.go
- control-plane-operator/hostedclusterconfigoperator/controllers/node/node.go
- control-plane-operator/hostedclusterconfigoperator/controllers/machine/machine_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/node/node_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/setup.go
- Makefile
- control-plane-operator/controllers/hostedcontrolplane/v2/kas/kubeconfig.go
- control-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader.go
- control-plane-operator/hostedclusterconfigoperator/api/scheme.go
- cmd/install/install.go
- cmd/install/install_test.go
- go.mod
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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 `@hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go`:
- Around line 4373-4376: The code dereferences OVNKubernetesConfig.IPv6 fields
when only OVNKubernetesConfig is checked; update the conditional that sets
ipv6JoinSubnet and ipv6TransitSubnet to also verify
hc.Spec.OperatorConfiguration.ClusterNetworkOperator.OVNKubernetesConfig.IPv6 !=
nil (or guard access with a separate nil check) before reading
IPv6.InternalJoinSubnet and IPv6.InternalTransitSwitchSubnet so the reconciler
won’t panic on IPv4-only or absent IPv6 configs; ensure the logic that sets
ipv6JoinSubnet/ipv6TransitSubnet falls back to empty/default values when the
IPv6 struct is nil.
- Around line 2252-2257: kasServingCertHashFromEndpoint currently calls
tls.Dialer.DialContext using the reconcile ctx with no timeout; update this
probe to derive a short-lived context via context.WithTimeout (e.g., a few
seconds) and use that derived context when calling DialContext so the dial
cannot hang indefinitely. Locate kasServingCertHashFromEndpoint and the
tls.Dialer.DialContext call and wrap the dial in a context.WithTimeout/Cancel,
pass the new ctx into DialContext, and ensure you defer the cancel to release
resources.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: f889e68d-e39a-441c-84f2-1602dddc56ec
⛔ Files ignored due to path filters (35)
api/go.sumis excluded by!**/*.sumapi/vendor/modules.txtis excluded by!**/vendor/**api/vendor/sigs.k8s.io/structured-merge-diff/v6/value/reflectcache.gois excluded by!**/vendor/**cmd/install/assets/crds/cluster-api-provider-aws/infrastructure.cluster.x-k8s.io_awsclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-aws/infrastructure.cluster.x-k8s.io_awsmachinepools.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-aws/infrastructure.cluster.x-k8s.io_awsmachines.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-aws/infrastructure.cluster.x-k8s.io_awsmachinetemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-azure/infrastructure.cluster.x-k8s.io_azureclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-azure/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-gcp/infrastructure.cluster.x-k8s.io_gcpclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-gcp/infrastructure.cluster.x-k8s.io_gcpclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-gcp/infrastructure.cluster.x-k8s.io_gcpmachines.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-gcp/infrastructure.cluster.x-k8s.io_gcpmachinetemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmpowervsclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmpowervsclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmpowervsimages.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmvpcclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmvpcclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-ibmcloud/infrastructure.cluster.x-k8s.io_ibmvpcmachines.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-kubevirt/infrastructure.cluster.x-k8s.io_kubevirtclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-openstack/infrastructure.cluster.x-k8s.io_openstackclusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api-provider-openstack/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/addons.cluster.x-k8s.io_clusterresourcesetbindings.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/addons.cluster.x-k8s.io_clusterresourcesets.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_clusterclasses.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_clusters.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinedeployments.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinedrainrules.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinehealthchecks.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinepools.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machines.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/cluster.x-k8s.io_machinesets.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/ipam.cluster.x-k8s.io_ipaddressclaims.yamlis excluded by!cmd/install/assets/**/*.yamlcmd/install/assets/crds/cluster-api/ipam.cluster.x-k8s.io_ipaddresses.yamlis excluded by!cmd/install/assets/**/*.yamlgo.sumis excluded by!**/*.sum
📒 Files selected for processing (63)
.codespellignore.golangci.ymlMakefileapi/go.modcmd/cluster/core/dump.gocmd/install/assets/crds/assets.gocmd/install/install.gocmd/install/install_test.gocontrib/gomaxprocs-webhook/Dockerfilecontrol-plane-operator/controllers/hostedcontrolplane/v2/kas/kubeconfig.gocontrol-plane-operator/hostedclusterconfigoperator/api/scheme.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/setup.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/machine/machine.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/machine/machine_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/machine/setup.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/node/node.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/node/node_test.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/spotremediation/spotremediation.gocontrol-plane-operator/hostedclusterconfigoperator/controllers/spotremediation/spotremediation_test.gogo.modhypershift-operator/controllers/hostedcluster/hostedcluster_controller.gohypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.gohypershift-operator/controllers/hostedcluster/hostedcluster_webhook.gohypershift-operator/controllers/hostedcluster/internal/platform/agent/agent.gohypershift-operator/controllers/hostedcluster/internal/platform/agent/agent_test.gohypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.gohypershift-operator/controllers/hostedcluster/internal/platform/azure/azure.gohypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp.gohypershift-operator/controllers/hostedcluster/internal/platform/ibmcloud/ibmcloud.gohypershift-operator/controllers/hostedcluster/internal/platform/ibmcloud/ibmcloud_test.gohypershift-operator/controllers/hostedcluster/internal/platform/kubevirt/kubevirt.gohypershift-operator/controllers/hostedcluster/internal/platform/kubevirt/kubevirt_test.gohypershift-operator/controllers/hostedcluster/internal/platform/openstack/openstack.gohypershift-operator/controllers/hostedcluster/internal/platform/openstack/openstack_test.gohypershift-operator/controllers/hostedcluster/internal/platform/platform.gohypershift-operator/controllers/hostedcluster/internal/platform/powervs/powervs.gohypershift-operator/controllers/manifests/controlplaneoperator/manifests.gohypershift-operator/controllers/nodepool/aws.gohypershift-operator/controllers/nodepool/aws_test.gohypershift-operator/controllers/nodepool/azure_test.gohypershift-operator/controllers/nodepool/capi.gohypershift-operator/controllers/nodepool/capi_test.gohypershift-operator/controllers/nodepool/conditions.gohypershift-operator/controllers/nodepool/conditions_test.gohypershift-operator/controllers/nodepool/gcp.gohypershift-operator/controllers/nodepool/metrics/metrics.gohypershift-operator/controllers/nodepool/nodepool_controller.gohypershift-operator/controllers/nodepool/nodepool_controller_test.gohypershift-operator/controllers/nodepool/powervs.gohypershift-operator/controllers/nodepool/scale_from_zero_test.gohypershift-operator/controllers/nodepool/version.gohypershift-operator/controllers/nodepool/version_test.gokarpenter-operator/controllers/karpenter/karpenter_controller.gokarpenter-operator/controllers/karpenter/karpenter_controller_test.gosupport/api/capi_types.gosupport/api/scheme.gosupport/backwardcompat/backwardcompat.gosupport/backwardcompat/backwardcompat_test.gosupport/k8sutil/resources.gosupport/upsert/upsert.go
💤 Files with no reviewable changes (39)
- hypershift-operator/controllers/manifests/controlplaneoperator/manifests.go
- support/upsert/upsert.go
- support/k8sutil/resources.go
- hypershift-operator/controllers/nodepool/powervs.go
- hypershift-operator/controllers/nodepool/scale_from_zero_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go
- hypershift-operator/controllers/nodepool/metrics/metrics.go
- hypershift-operator/controllers/nodepool/aws.go
- karpenter-operator/controllers/karpenter/karpenter_controller_test.go
- hypershift-operator/controllers/nodepool/version.go
- hypershift-operator/controllers/hostedcluster/internal/platform/agent/agent_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/ibmcloud/ibmcloud_test.go
- hypershift-operator/controllers/nodepool/conditions_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/agent/agent.go
- hypershift-operator/controllers/nodepool/gcp.go
- hypershift-operator/controllers/hostedcluster/hostedcluster_webhook.go
- hypershift-operator/controllers/hostedcluster/internal/platform/kubevirt/kubevirt_test.go
- hypershift-operator/controllers/nodepool/aws_test.go
- support/api/capi_types.go
- hypershift-operator/controllers/hostedcluster/internal/platform/ibmcloud/ibmcloud.go
- hypershift-operator/controllers/nodepool/version_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/openstack/openstack_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/openstack/openstack.go
- hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/azure/azure.go
- hypershift-operator/controllers/hostedcluster/internal/platform/powervs/powervs.go
- support/api/scheme.go
- hypershift-operator/controllers/nodepool/azure_test.go
- hypershift-operator/controllers/nodepool/conditions.go
- karpenter-operator/controllers/karpenter/karpenter_controller.go
- support/backwardcompat/backwardcompat.go
- hypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp.go
- hypershift-operator/controllers/hostedcluster/internal/platform/platform.go
- hypershift-operator/controllers/nodepool/nodepool_controller_test.go
- hypershift-operator/controllers/nodepool/capi.go
- support/backwardcompat/backwardcompat_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/kubevirt/kubevirt.go
- hypershift-operator/controllers/nodepool/capi_test.go
- hypershift-operator/controllers/nodepool/nodepool_controller.go
✅ Files skipped from review due to trivial changes (3)
- .codespellignore
- cmd/cluster/core/dump.go
- control-plane-operator/hostedclusterconfigoperator/controllers/spotremediation/spotremediation.go
🚧 Files skipped from review as they are similar to previous changes (19)
- contrib/gomaxprocs-webhook/Dockerfile
- .golangci.yml
- api/go.mod
- control-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/spotremediation/spotremediation_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/machine/setup.go
- control-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps_test.go
- cmd/install/assets/crds/assets.go
- control-plane-operator/hostedclusterconfigoperator/controllers/node/node.go
- control-plane-operator/hostedclusterconfigoperator/controllers/machine/machine_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/node/node_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/setup.go
- Makefile
- control-plane-operator/controllers/hostedcontrolplane/v2/kas/kubeconfig.go
- control-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader.go
- control-plane-operator/hostedclusterconfigoperator/api/scheme.go
- cmd/install/install.go
- cmd/install/install_test.go
- go.mod
🛑 Comments failed to post (2)
hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go (2)
2252-2257:
⚠️ Potential issue | 🟠 Major | ⚡ Quick win🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail file="hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go" echo "# function body" sed -n '2252,2265p' "$file" echo echo "# call sites" rg -n 'kasServingCertHashFromEndpoint\s*\(' "$file" echo echo "# timeout-related calls in this file" rg -n 'WithTimeout|WithDeadline|DialContext' "$file"Repository: openshift/hypershift
Length of output: 1052
🏁 Script executed:
#!/bin/bash set -euo pipefail file="hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go" echo "# call site around 1805" sed -n '1785,1830p' "$file" echo echo "# call site around 2212" sed -n '2185,2265p' "$file" echo echo "# timeout/deadline helpers in the file (broader search)" rg -n --hidden --glob '!**/vendor/**' 'WithTimeout|WithDeadline|Deadline\(' "$file" echo echo "# nearby structured usage of ctx cancellation in reconcile (optional context)" rg -n 'Reconcile\(|ctx :=|context\.With' "$file"Repository: openshift/hypershift
Length of output: 5766
🏁 Script executed:
#!/bin/bash set -euo pipefail file="hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go" echo "# any WithTimeout/WithDeadline/Deadline usage in this file" rg -n 'WithTimeout|WithDeadline|Deadline\(' "$file" || true echo echo "# Reconcile entrypoint signature/body (find where ctx originates)" rg -n '^func \(r \*HostedClusterReconciler\) Reconcile\(' "$file" # print a small window after the function line line=$(rg -n '^func \(r \*HostedClusterReconciler\) Reconcile\(' "$file" | head -n1 | cut -d: -f1) start=$((line-5)) end=$((line+60)) sed -n "${start},${end}p" "$file"Repository: openshift/hypershift
Length of output: 2786
Add a dedicated timeout to the KAS certificate probe
kasServingCertHashFromEndpointdials withtls.Dialer.DialContext(ctx, ...)using the reconcilectxpassed fromReconcile, and this file doesn’t derive anyWithTimeout/WithDeadlinefor that probe. Wrap the dial incontext.WithTimeout(short duration like a few seconds) and pass the derived context toDialContext.🧰 Tools
🪛 ast-grep (0.43.0)
[warning] 2253-2256: MinVersion
is missing from this TLS configuration. By default, TLS 1.2 is currently used as the minimum when acting as a client, and TLS 1.0 when acting as a server. General purpose web applications should default to TLS 1.3 with all other protocols disabled. Only where it is known that a web server must support legacy clients with unsupported an insecure browsers (such as Internet Explorer 10), it may be necessary to enable TLS 1.0 to provide support. AddMinVersion: tls.VersionTLS13' to the TLS configuration to bump the minimum version to TLS 1.3.
Context: tls.Config{
InsecureSkipVerify: true,
ServerName: "kubernetes",
}
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
https://owasp.org/Top10/A02_2021-Cryptographic_Failures(missing-ssl-minversion-go)
🪛 OpenGrep (1.22.0)
[ERROR] 2254-2257: TLS certificate verification is disabled via InsecureSkipVerify. This allows man-in-the-middle attacks. Remove InsecureSkipVerify or set it to false.
(coderabbit.tls.go-insecure-skip-verify)
[ERROR] 2254-2257: TLS certificate verification is disabled via InsecureSkipVerify. This allows man-in-the-middle attacks. Remove InsecureSkipVerify or set it to false.
(coderabbit.tls.go-insecure-skip-verify)
🤖 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 `@hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go` around lines 2252 - 2257, kasServingCertHashFromEndpoint currently calls tls.Dialer.DialContext using the reconcile ctx with no timeout; update this probe to derive a short-lived context via context.WithTimeout (e.g., a few seconds) and use that derived context when calling DialContext so the dial cannot hang indefinitely. Locate kasServingCertHashFromEndpoint and the tls.Dialer.DialContext call and wrap the dial in a context.WithTimeout/Cancel, pass the new ctx into DialContext, and ensure you defer the cancel to release resources.
4373-4376:
⚠️ Potential issue | 🔴 Critical | ⚡ Quick winGuard the optional IPv6 OVN config before dereferencing it.
Line 4375 reads
OVNKubernetesConfig.IPv6.*after only checkingOVNKubernetesConfig != nil. On IPv4-only or otherwise unset IPv6 configs, this will panic the reconciler instead of returning validation output.🛡️ Proposed fix
var ipv6JoinSubnet, ipv6TransitSubnet string if hc.Spec.OperatorConfiguration != nil && hc.Spec.OperatorConfiguration.ClusterNetworkOperator != nil && - hc.Spec.OperatorConfiguration.ClusterNetworkOperator.OVNKubernetesConfig != nil { + hc.Spec.OperatorConfiguration.ClusterNetworkOperator.OVNKubernetesConfig != nil && + hc.Spec.OperatorConfiguration.ClusterNetworkOperator.OVNKubernetesConfig.IPv6 != nil { ipv6JoinSubnet = hc.Spec.OperatorConfiguration.ClusterNetworkOperator.OVNKubernetesConfig.IPv6.InternalJoinSubnet ipv6TransitSubnet = hc.Spec.OperatorConfiguration.ClusterNetworkOperator.OVNKubernetesConfig.IPv6.InternalTransitSwitchSubnet }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.var ipv6JoinSubnet, ipv6TransitSubnet string if hc.Spec.OperatorConfiguration != nil && hc.Spec.OperatorConfiguration.ClusterNetworkOperator != nil && hc.Spec.OperatorConfiguration.ClusterNetworkOperator.OVNKubernetesConfig != nil && hc.Spec.OperatorConfiguration.ClusterNetworkOperator.OVNKubernetesConfig.IPv6 != nil { ipv6JoinSubnet = hc.Spec.OperatorConfiguration.ClusterNetworkOperator.OVNKubernetesConfig.IPv6.InternalJoinSubnet ipv6TransitSubnet = hc.Spec.OperatorConfiguration.ClusterNetworkOperator.OVNKubernetesConfig.IPv6.InternalTransitSwitchSubnet }🤖 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 `@hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go` around lines 4373 - 4376, The code dereferences OVNKubernetesConfig.IPv6 fields when only OVNKubernetesConfig is checked; update the conditional that sets ipv6JoinSubnet and ipv6TransitSubnet to also verify hc.Spec.OperatorConfiguration.ClusterNetworkOperator.OVNKubernetesConfig.IPv6 != nil (or guard access with a separate nil check) before reading IPv6.InternalJoinSubnet and IPv6.InternalTransitSwitchSubnet so the reconciler won’t panic on IPv4-only or absent IPv6 configs; ensure the logic that sets ipv6JoinSubnet/ipv6TransitSubnet falls back to empty/default values when the IPv6 struct is nil.
…mplete check Replace the 1-second sleep workaround for OCPBUGS-77922 with a deterministic cross-check of the v1beta2 conversion-data annotation. In CAPI v1.11+, the v1beta1 UpdatedReplicas field maps from deprecated.v1beta1.updatedReplicas rather than the native upToDateReplicas, which can transiently disagree. When v1beta1 fields indicate completion, we now verify against the authoritative v1beta2 status in the conversion-data annotation before declaring complete. Jira: https://issues.redhat.com/browse/OCPBUGS-77922 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The word uptodate and all its casing variants are a false positive on codespell. They are defined as such in CAPI. Signed-off-by: Borja Clemente <bclement@redhat.com>
Add --disable-capi-conversion-webhook flag (default false) to allow disabling CAPI CRD conversion webhooks during the v1beta1/v1beta2 transition period. The flag feeds into EnableWebhook and the replica count computation. Signed-off-by: Alberto Garcia <algarcia@redhat.com>
Moves the CRD include filter logic from setupCRDs into a standalone crdIncludeFilter function to bring cyclomatic complexity under the gocyclo threshold of 30. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CAPI 1.11 moved api/v1beta1 to api/core/v1beta1. Update the karpenter controller import added on main by e148f7d. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
9d70224 to
1be104e
Compare
|
/test e2e-aws |
|
/pipeline required |
|
Scheduling tests matching the |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
Both the metrics-proxy and endpoint-resolver are running. The Now I have all the evidence. Let me compile the final report: Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryThe e2e-gke job failed with 6 test failures across two independent hosted clusters. The TestCreateClusterHABreakGlassCredentials cluster never came up at all — the Root CauseThere are two distinct failure paths: Failure 1: TestCreateClusterHABreakGlassCredentials — Hosted cluster never provisioned (CAPI-related) The HA Break Glass Credentials hosted cluster (
Critically, the TestCreateCluster cluster's capi-provider pod started at the same time ( Failure 2: TestCreateCluster/Main/EnsureMetricsForwarderWorking — Metrics scraping failure The TestCreateCluster hosted cluster provisioned successfully (nodes ready, cluster operators healthy), but the Failure 3: TestCreateCluster/Teardown — Credential expiry (consequential) The Teardown phase (4367s) failed because management cluster API credentials expired during the long-running test, causing Recommendations
Evidence
|
|
/test e2e-aws-4-22 |
|
/test e2e-aks |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
/hold cancel |
|
@enxebre: This PR has been marked as verified by DetailsIn 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@enxebre: The following test 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. |
What this PR does / why we need it:
Bumps hypershift to use CAPI
v1.11including the following tasks:v1.11compatible version ingo.mod.v1beta1as storage version.v1beta1<->v1beta2.Which issue(s) this PR fixes:
Fixes CNTRLPLANE-2207
Special notes for your reviewer:
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tests