CNTRLPLANE-3603: Migrate clients from CAPI v1beta1 to v1beta2#8717
Draft
clebs wants to merge 3 commits into
Draft
CNTRLPLANE-3603: Migrate clients from CAPI v1beta1 to v1beta2#8717clebs wants to merge 3 commits into
clebs wants to merge 3 commits into
Conversation
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@clebs: This pull request references CNTRLPLANE-3603 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 the "5.0.0" version, but no target version was set. 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. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
Contributor
|
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:
📝 WalkthroughWalkthroughThis PR migrates code and tests from Cluster API v1beta1 to v1beta2 across APIs, controllers, platform integrations, schemes, manifests, and utilities. It adds HostedControlPlane.Status.Initialization and HostedControlPlaneInitializationStatus, sets Initialization.ControlPlaneInitialized in HostedControlPlane reconciliation, patches CAPI Cluster Status.Initialization.InfrastructureProvisioned, replaces several CAPI spec/status shapes (ContractVersionedObjectReference, pointer Spec.Paused, seconds-based timeout fields), removes CAPI conversion webhook plumbing, and updates golangci lint exclusions. Sequence Diagram(s)sequenceDiagram
participant HostedControlPlane
participant HostedControlPlaneController
participant HostedClusterController
participant ClusterAPI
participant Tests
HostedControlPlane->>HostedControlPlaneController: expose Status.Initialization.ControlPlaneInitialized
HostedControlPlaneController->>HostedClusterController: reconciliation observes InfrastructureReady
HostedClusterController->>ClusterAPI: patch status.initialization.infrastructureProvisioned=true
ClusterAPI->>Tests: tests updated to use v1beta2 types and condition shapes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: clebs The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
openshift-ci
Bot
added
area/api
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8717 +/- ##
==========================================
- Coverage 41.66% 41.65% -0.01%
==========================================
Files 758 758
Lines 93929 93933 +4
==========================================
- Hits 39135 39131 -4
- Misses 52046 52052 +6
- Partials 2748 2750 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
api/hypershift/v1beta1/hosted_controlplane.go (1)
440-440: ⚡ Quick winUse consistent marker syntax for default value.
This file uses
+kubebuilder:default=on lines 322, 331, and 338. For consistency, change+default=falseto+kubebuilder:default=false.📝 Suggested change
- // +default=false + // +kubebuilder:default=false🤖 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/hypershift/v1beta1/hosted_controlplane.go` at line 440, Change the marker annotation for the boolean default to use the same kubebuilder syntax as the other fields: replace the comment line `+default=false` with `+kubebuilder:default=false` on the corresponding boolean field in the HostedControlPlane spec (the same block that contains the other `+kubebuilder:default=` annotations around lines where the spec fields are defined), ensuring consistency with the markers used on the other fields.
🤖 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
`@control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go`:
- Around line 681-685: Don't set
hostedControlPlane.Status.Initialization.ControlPlaneInitialized
unconditionally; instead gate it on the availability checks already computed by
reconcileAvailabilityStatus. Replace the unconditional assignment of
hostedControlPlane.Status.Initialization.ControlPlaneInitialized = ptr.To(true)
with logic that sets it to true only when the reconciler determines
hyperv1.HostedControlPlaneAvailable is true (and specifically
KubeAPIServerAvailable is true / kubeconfig exists and LB health/component
checks passed); otherwise leave it false or nil. Locate the assignment and use
the results/conditions produced by reconcileAvailabilityStatus (or the same
booleans it computes) to decide the value.
In
`@control-plane-operator/hostedclusterconfigoperator/controllers/machine/machine_test.go`:
- Around line 467-468: The test case title "With Failing machine with internal
addresses and passthrow service should mark endpointslices as not ready/not
serving" does not match the setup which uses
pairOfDualStackMachines(capiv1.MachinePhaseRunning,
capiv1.MachinePhaseDeleting); either rename the test title to refer to a
"Deleting machine" to match the current setup, or change the second machine
phase to capiv1.MachinePhaseFailed so the scenario truly models a failing
machine; update the test case entry (the name string and/or the machines call)
accordingly, keeping the rest of the assertions intact.
In `@hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go`:
- Around line 3043-3044: The ClusterRole that includes Verbs:
[]string{"get","list","patch","watch"} is granting the capi-provider subject
cluster-wide patch (write) access to CRDs which contradicts the later code that
assumes a read-only binding for capi-provider; either remove "patch" from that
ClusterRole's Verbs so capi-provider only gets get/list/watch, or create a
separate ClusterRole (e.g., "capi-provider-crd-reader") with Verbs:
[]string{"get","list","watch"} and bind capi-provider to that instead, leaving
the original ClusterRole with "patch" bound only to trusted controller
subjects—update the RoleBinding/ClusterRoleBinding creation for the
capi-provider subject accordingly so the access model matches the assumptions in
the subsequent code paths (lines around where capi-provider is referenced).
In `@hypershift-operator/controllers/nodepool/capi.go`:
- Around line 249-253: The cleanupMachineTemplates function currently uses
api.Scheme.VersionsForGroupKind(...)[0] to build a single apiVersion and only
lists MachineTemplates for that version; change it to iterate over all versions
returned by api.Scheme.VersionsForGroupKind(schema.GroupKind{Group:
ref.APIGroup, Kind: ref.Kind}) and for each version construct
schema.GroupVersion{Group: ref.APIGroup, Version: ver.Version}.String(), then
list and delete MachineTemplates for each apiVersion (rather than only
versions[0]). Keep using the existing capiv1.ContractVersionedObjectReference
writers (no change needed to the reference type).
---
Nitpick comments:
In `@api/hypershift/v1beta1/hosted_controlplane.go`:
- Line 440: Change the marker annotation for the boolean default to use the same
kubebuilder syntax as the other fields: replace the comment line
`+default=false` with `+kubebuilder:default=false` on the corresponding boolean
field in the HostedControlPlane spec (the same block that contains the other
`+kubebuilder:default=` annotations around lines where the spec fields are
defined), ensuring consistency with the markers used on the other fields.
🪄 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: 5cdbc23e-aa7c-4f55-a404-d1a9a463f044
⛔ Files ignored due to path filters (1)
vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hosted_controlplane.gois excluded by!vendor/**,!**/vendor/**
📒 Files selected for processing (63)
api/.golangci.ymlapi/hypershift/v1beta1/hosted_controlplane.gocmd/cluster/core/dump.gocontrol-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.gocontrol-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.gohypershift-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/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/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 (2)
- hypershift-operator/controllers/hostedcluster/hostedcluster_webhook.go
- support/api/scheme.go
Comment on lines
+681
to
+685
| // Set status.initialization.controlPlaneInitialized for CAPI 1.11 v1beta2 contract. | ||
| // CAPI reads this field from the ControlPlane provider object to determine if the | ||
| // control plane is initialized (ControlPlaneInitialized condition on the CAPI Cluster). | ||
| hostedControlPlane.Status.Initialization.ControlPlaneInitialized = ptr.To(true) | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locations writing/readings ControlPlaneInitialized =="
rg -n -C3 'ControlPlaneInitialized|Status\.Initialization|Status\.Initialized' --type go
echo
echo "== Reconcile flow around assignment in hostedcontrolplane controller =="
rg -n -C6 'Status\.Initialization\.ControlPlaneInitialized|Status\.Initialized\s*=' control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
echo
echo "== CAPI/HostedCluster reconciliation references that depend on initialization contracts =="
rg -n -C4 'ControlPlaneInitialized|InfrastructureReady|initialized' hypershift-operator/controllers/hostedcluster --type goRepository: openshift/hypershift
Length of output: 49187
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Other writes to hostedControlPlane.Status.Initialized in hostedcontrolplane_controller.go =="
rg -n 'Status\.Initialized\s*=' control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
echo
echo "== Writes to hostedControlPlane.Status.Initialization.ControlPlaneInitialized =="
rg -n 'Initialization\.ControlPlaneInitialized\s*=' control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
echo
echo "== Read access patterns of HCP.Status.Initialized in hypershift-operator HostedCluster controller =="
rg -n 'Status\.Initialized' hypershift-operator/controllers/hostedcluster --type go
echo
echo "== Read/contract references of status.initialized / ControlPlaneInitialized in repo =="
rg -n 'status\.initialized|ControlPlaneInitialized' --type go --type ts --type py --type yaml
echo
echo "== Inspect surrounding code for the assignment block (hostedcontrolplane_controller.go:650-710) =="
sed -n '630,710p' control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go | cat -nRepository: openshift/hypershift
Length of output: 14059
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go"
echo "== Where reconcileAvailabilityAndReadyStatus is defined =="
rg -n 'func \(r \*.*\) reconcileAvailabilityAndReadyStatus\(' "$FILE"
echo
echo "== Body of reconcileAvailabilityAndReadyStatus (up to next func) =="
python3 - <<'PY'
import re, itertools
path="control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go"
start_pat=re.compile(r'func \(r \*.*\) reconcileAvailabilityAndReadyStatus\(')
with open(path,'r') as f:
lines=f.readlines()
start=None
for i,l in enumerate(lines):
if start_pat.search(l):
start=i
break
assert start is not None
# print next ~250 lines until next "func " at col 0
out=[]
for j in range(start, min(start+350, len(lines))):
if j>start and lines[j].startswith("func "):
break
out.append(f"{j+1}:{lines[j].rstrip()}")
print("\n".join(out))
PY
echo
echo "== Any other writes to ControlPlaneInitialized in control-plane-operator =="
rg -n 'ControlPlaneInitialized\s*=\s*ptr\.To\((true|false)\)' --type go control-plane-operator
echo
echo "== What conditions are set around KubeAPIServerAvailable / InfrastructureReady etc. in this controller =="
rg -n 'KubeAPIServerAvailable|InfrastructureReady|ControlPlaneAvailable|ControlPlaneInitialized' --type go "$FILE"Repository: openshift/hypershift
Length of output: 2946
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== CAPI contract wording for ControlPlaneInitialized (v1beta2) =="
sed -n '145,190p' vendor/sigs.k8s.io/cluster-api/api/core/v1beta2/cluster_types.go | cat -n
echo
sed -n '1360,1390p' vendor/sigs.k8s.io/cluster-api/api/core/v1beta2/cluster_types.go | cat -n
echo
echo "== Find reconcileAvailabilityStatus definition =="
rg -n 'func reconcileAvailabilityStatus\(' control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go control-plane-operator/controllers/hostedcontrolplane -S
echo
echo "== Print reconcileAvailabilityStatus implementation =="
python3 - <<'PY'
import subprocess, re, pathlib, sys
import os, json
root="control-plane-operator/controllers/hostedcontrolplane"
# find exact file with reconcileAvailabilityStatus
res=subprocess.check_output(["bash","-lc",f"rg -n 'func reconcileAvailabilityStatus\\(' {root} -S | head -n 1"]).decode().strip()
print("match:",res)
file=res.split(":")[0]
line=int(res.split(":")[1])
print("file:",file,"line:",line)
# print 200 lines starting at line-1
start=max(1,line-1)
cmd=f"sed -n '{start},{start+220}p' {file}"
print(subprocess.check_output(["bash","-lc",cmd]).decode())
PYRepository: openshift/hypershift
Length of output: 15930
Gate CAPI controlPlaneInitialized on API-server readiness (avoid unconditional true)
hostedControlPlane.Status.Initialization.ControlPlaneInitialized = ptr.To(true) is set unconditionally, but the CAPI v1beta2 contract says controlPlaneInitialized should be “functional enough to accept requests” and is used to orchestrate provisioning that depends on a functional API server. This reconciler already computes hyperv1.HostedControlPlaneAvailable via reconcileAvailabilityStatus (which can be False when kubeconfig is missing, KubeAPIServerAvailable is False, load balancer healthz fails, or components aren’t available). Setting ControlPlaneInitialized to true even when those checks aren’t satisfied risks CAPI starting too early.
🤖 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
`@control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go`
around lines 681 - 685, Don't set
hostedControlPlane.Status.Initialization.ControlPlaneInitialized
unconditionally; instead gate it on the availability checks already computed by
reconcileAvailabilityStatus. Replace the unconditional assignment of
hostedControlPlane.Status.Initialization.ControlPlaneInitialized = ptr.To(true)
with logic that sets it to true only when the reconciler determines
hyperv1.HostedControlPlaneAvailable is true (and specifically
KubeAPIServerAvailable is true / kubeconfig exists and LB health/component
checks passed); otherwise leave it false or nil. Locate the assignment and use
the results/conditions produced by reconcileAvailabilityStatus (or the same
booleans it computes) to decide the value.
Comment on lines
+3043
to
3044
| Verbs: []string{"get", "list", "patch", "watch"}, | ||
| }, |
Contributor
There was a problem hiding this comment.
Don't keep capi-provider on the patched CRD role.
Adding patch here means the shared binding now gives capi-provider cluster-wide write access to CRDs, while Lines 3063-3065 still assume the role is read-only. Split the provider onto its own get/list/watch role or stop binding it to this ClusterRole.
Also applies to: 3063-3070
🤖 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 3043 - 3044, The ClusterRole that includes Verbs:
[]string{"get","list","patch","watch"} is granting the capi-provider subject
cluster-wide patch (write) access to CRDs which contradicts the later code that
assumes a read-only binding for capi-provider; either remove "patch" from that
ClusterRole's Verbs so capi-provider only gets get/list/watch, or create a
separate ClusterRole (e.g., "capi-provider-crd-reader") with Verbs:
[]string{"get","list","watch"} and bind capi-provider to that instead, leaving
the original ClusterRole with "patch" bound only to trusted controller
subjects—update the RoleBinding/ClusterRoleBinding creation for the
capi-provider subject accordingly so the access model matches the assumptions in
the subsequent code paths (lines around where capi-provider is referenced).
Comment on lines
+249
to
+253
| versions := api.Scheme.VersionsForGroupKind(schema.GroupKind{Group: ref.APIGroup, Kind: ref.Kind}) | ||
| if len(versions) == 0 { | ||
| return fmt.Errorf("no versions registered for GroupKind %s/%s", ref.APIGroup, ref.Kind) | ||
| } | ||
| apiVersion := schema.GroupVersion{Group: ref.APIGroup, Version: versions[0].Version}.String() |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file under review
FILE="hypershift-operator/controllers/nodepool/capi.go"
test -f "$FILE" || { echo "Missing $FILE"; exit 1; }
echo "== Snippet around lines 230-290 =="
nl -ba "$FILE" | sed -n '230,290p'
echo "== Snippet around lines 430-480 =="
nl -ba "$FILE" | sed -n '430,480p'
echo "== Snippet around lines 920-990 =="
nl -ba "$FILE" | sed -n '920,990p'
echo "== Finding ContractVersionedObjectReference usages in this file =="
rg -n "ContractVersionedObjectReference" "$FILE" || true
echo "== Locate struct definition in module cache / repo =="
# Try to locate in repo/vendor first
if fd -t f "types.go" . >/dev/null 2>&1; then
:
fi
# Search in repository for the type definition
rg -n "type +ContractVersionedObjectReference" -S . || true
# If not found, search in Go module cache paths (best-effort; sandbox may limit)
go env GOMODCACHE >/dev/null 2>&1 || true
GOMODCACHE="$(go env GOMODCACHE 2>/dev/null || true)"
if [ -n "${GOMODCACHE:-}" ] && [ -d "$GOMODCACHE" ]; then
echo "== Searching in GOMODCACHE for ContractVersionedObjectReference =="
rg -n "type +ContractVersionedObjectReference" "$GOMODCACHE" -S --hidden --no-ignore 2>/dev/null | head -n 20 || true
fi
echo "== Search for VersionsForGroupKind usage =="
rg -n "VersionsForGroupKind" "$FILE" || trueRepository: openshift/hypershift
Length of output: 160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="hypershift-operator/controllers/nodepool/capi.go"
test -f "$FILE" || { echo "Missing $FILE"; exit 1; }
echo "== Snippet around lines 240-275 =="
sed -n '240,275p' "$FILE"
echo "== Snippet around lines 435-465 =="
sed -n '435,465p' "$FILE"
echo "== Snippet around lines 935-970 =="
sed -n '935,970p' "$FILE"
echo "== Finding ContractVersionedObjectReference usages in this file =="
rg -n "ContractVersionedObjectReference" "$FILE" || true
echo "== Finding VersionsForGroupKind usage in this file =="
rg -n "VersionsForGroupKind" "$FILE" || true
echo "== Attempt to locate ContractVersionedObjectReference type in repo (module/vendor) =="
# Prefer repo search
rg -n "type +ContractVersionedObjectReference" . || true
# If nothing, attempt in module cache
if command -v go >/dev/null 2>&1; then
GOMODCACHE="$(go env GOMODCACHE 2>/dev/null || true)"
if [ -n "${GOMODCACHE:-}" ] && [ -d "$GOMODCACHE" ]; then
rg -n "type +ContractVersionedObjectReference" "$GOMODCACHE" -S --hidden --no-ignore 2>/dev/null | head -n 20 || true
fi
fiRepository: openshift/hypershift
Length of output: 4835
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="vendor/sigs.k8s.io/cluster-api/api/core/v1beta2/common_types.go"
test -f "$FILE" || { echo "Missing $FILE"; exit 1; }
# Print around the struct definition
sed -n '330,430p' "$FILE"
# Also search within the vendor file for the struct fields
rg -n "type +ContractVersionedObjectReference +struct" -n "$FILE"Repository: openshift/hypershift
Length of output: 3141
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="hypershift-operator/controllers/nodepool/capi.go"
test -f "$FILE" || { echo "Missing $FILE"; exit 1; }
rg -n "func \\(c \\*.*\\) cleanupMachineTemplates" "$FILE" || true
rg -n "cleanupMachineTemplates" "$FILE" || true
# Print the whole function (or as much as reasonable)
# Find start line number
START="$(rg -n "cleanupMachineTemplates" "$FILE" | head -n1 | cut -d: -f1 || true)"
if [ -n "${START:-}" ]; then
# print 120 lines from start
END=$((START+140))
sed -n "${START},${END}p" "$FILE"
fi
# Show the comment near line 248
sed -n '235,265p' "$FILE"
# Search for SetAPIVersion usage in this file
rg -n "SetAPIVersion\\(" "$FILE" || trueRepository: openshift/hypershift
Length of output: 6767
Fix MachineTemplate cleanup to delete across all template API versions (avoid VersionsForGroupKind(...)[0])
cleanupMachineTemplates reconstructs a single apiVersion from api.Scheme.VersionsForGroupKind(...)[0] and lists MachineTemplates for only that APIVersion, so any MachineTemplates stored under other versions for the same GroupKind won’t be discovered/deleted. Update it to list/delete for all versions returned by VersionsForGroupKind (instead of index 0).
The writer sites creating capiv1.ContractVersionedObjectReference don’t need to change: in vendored cluster-api v1beta2 (vendor/sigs.k8s.io/cluster-api/api/core/v1beta2/common_types.go), the type has only Kind, Name, and APIGroup (no Version field), so the version can’t be persisted there.
🤖 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.go` around lines 249 - 253, The
cleanupMachineTemplates function currently uses
api.Scheme.VersionsForGroupKind(...)[0] to build a single apiVersion and only
lists MachineTemplates for that version; change it to iterate over all versions
returned by api.Scheme.VersionsForGroupKind(schema.GroupKind{Group:
ref.APIGroup, Kind: ref.Kind}) and for each version construct
schema.GroupVersion{Group: ref.APIGroup, Version: ver.Version}.String(), then
list and delete MachineTemplates for each apiVersion (rather than only
versions[0]). Keep using the existing capiv1.ContractVersionedObjectReference
writers (no change needed to the reference type).
openshift-ci
Bot
added
the
area/documentation
openshift-ci
Bot
added
the
needs-rebase
4 tasks
openshift-ci
Bot
removed
the
needs-rebase
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hypershift-operator/controllers/nodepool/capi.go`:
- Around line 672-679: The durationToSeconds helper currently casts d.Seconds()
directly to int32 which can overflow or accept negative values; change
durationToSeconds to validate and clamp the seconds: if d is nil return nil;
compute secs := d.Seconds(); if secs < 0 set secs = 0; if secs >
float64(math.MaxInt32) set secs = float64(math.MaxInt32); then convert to int32
and return its pointer. Replace any other direct casts of d.Seconds() to int32
(the conversions referenced at lines 745-746) to use durationToSeconds (or apply
the same clamp logic) so all user-sourced durations are bounds-checked and
non-negative before assigning to *int32 fields.
🪄 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: 997bcf03-5270-4813-8542-4baba563e281
⛔ Files ignored due to path filters (48)
api/hypershift/v1beta1/zz_generated.deepcopy.gois excluded by!**/zz_generated*.go,!**/zz_generated*api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ClusterUpdateAcceptRisks.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ClusterVersionOperatorConfiguration.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ExternalOIDC.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ExternalOIDCWithUIDAndExtraClaimMappings.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ExternalOIDCWithUpstreamParity.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/GCPPlatform.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/HCPEtcdBackup.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/ImageStreamImportMode.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/KMSEncryptionProvider.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/OpenStack.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests/hostedcontrolplanes.hypershift.openshift.io/TLSAdherence.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**client/applyconfiguration/hypershift/v1beta1/hostedcontrolplaneinitializationstatus.gois excluded by!client/**client/applyconfiguration/hypershift/v1beta1/hostedcontrolplanestatus.gois excluded by!client/**client/applyconfiguration/utils.gois excluded by!client/**cmd/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/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/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamlcmd/install/assets/crds/hypershift-operator/zz_generated.crd-manifests/hostedcontrolplanes-Hypershift-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/**,!cmd/install/assets/**/*.yamldocs/content/reference/aggregated-docs.mdis excluded by!docs/content/reference/aggregated-docs.mddocs/content/reference/api.mdis excluded by!docs/content/reference/api.mdvendor/github.com/openshift/hypershift/api/hypershift/v1beta1/hosted_controlplane.gois excluded by!vendor/**,!**/vendor/**vendor/github.com/openshift/hypershift/api/hypershift/v1beta1/zz_generated.deepcopy.gois excluded by!vendor/**,!**/vendor/**,!**/zz_generated*.go,!**/zz_generated*vendor/modules.txtis excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/addons/v1beta1/.import-restrictionsis excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/addons/v1beta1/clusterresourceset_types.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/addons/v1beta1/clusterresourcesetbinding_types.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/addons/v1beta1/condition_consts.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/addons/v1beta1/conversion.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/addons/v1beta1/doc.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/addons/v1beta1/groupversion_info.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/addons/v1beta1/v1beta2_condition_consts.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/addons/v1beta1/zz_generated.conversion.gois excluded by!vendor/**,!**/vendor/**,!**/zz_generated*.go,!**/zz_generated*vendor/sigs.k8s.io/cluster-api/api/addons/v1beta1/zz_generated.deepcopy.gois excluded by!vendor/**,!**/vendor/**,!**/zz_generated*.go,!**/zz_generated*vendor/sigs.k8s.io/cluster-api/api/ipam/v1beta1/.import-restrictionsis excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/ipam/v1beta1/condition_consts.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/ipam/v1beta1/conversion.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/ipam/v1beta1/doc.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/ipam/v1beta1/groupversion_info.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/ipam/v1beta1/ipaddress_types.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/ipam/v1beta1/ipaddressclaim_types.gois excluded by!vendor/**,!**/vendor/**vendor/sigs.k8s.io/cluster-api/api/ipam/v1beta1/zz_generated.conversion.gois excluded by!vendor/**,!**/vendor/**,!**/zz_generated*.go,!**/zz_generated*vendor/sigs.k8s.io/cluster-api/api/ipam/v1beta1/zz_generated.deepcopy.gois excluded by!vendor/**,!**/vendor/**,!**/zz_generated*.go,!**/zz_generated*
📒 Files selected for processing (63)
api/.golangci.ymlapi/hypershift/v1beta1/hosted_controlplane.gocmd/cluster/core/dump.gocontrol-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.gocontrol-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.gohypershift-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/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/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 (2)
- hypershift-operator/controllers/hostedcluster/hostedcluster_webhook.go
- support/api/scheme.go
✅ Files skipped from review due to trivial changes (4)
- hypershift-operator/controllers/hostedcluster/internal/platform/kubevirt/kubevirt_test.go
- hypershift-operator/controllers/nodepool/metrics/metrics.go
- control-plane-operator/hostedclusterconfigoperator/controllers/node/node.go
- control-plane-operator/controllers/hostedcontrolplane/hostedcontrolplane_controller.go
🚧 Files skipped from review as they are similar to previous changes (49)
- control-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/setup.go
- hypershift-operator/controllers/nodepool/scale_from_zero_test.go
- support/upsert/upsert.go
- hypershift-operator/controllers/manifests/controlplaneoperator/manifests.go
- control-plane-operator/hostedclusterconfigoperator/controllers/machine/machine.go
- test/e2e/nodepool_spot_termination_handler_test.go
- control-plane-operator/hostedclusterconfigoperator/api/scheme.go
- hypershift-operator/controllers/hostedcluster/internal/platform/aws/aws.go
- api/.golangci.yml
- test/e2e/nodepool_rolling_upgrade_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/powervs/powervs.go
- hypershift-operator/controllers/nodepool/conditions_test.go
- test/e2e/nodepool_day2_tags_test.go
- test/e2e/v2/backuprestore/cleanup.go
- hypershift-operator/controllers/nodepool/azure_test.go
- karpenter-operator/controllers/karpenter/karpenter_controller.go
- test/e2e/util/util.go
- hypershift-operator/controllers/nodepool/powervs.go
- test/e2e/autoscaling_test.go
- hypershift-operator/controllers/nodepool/aws.go
- test/e2e/nodepool_kv_advanced_multinet_test.go
- hypershift-operator/controllers/hostedcluster/internal/platform/gcp/gcp.go
- hypershift-operator/controllers/hostedcluster/internal/platform/ibmcloud/ibmcloud.go
- support/k8sutil/resources.go
- hypershift-operator/controllers/hostedcluster/internal/platform/agent/agent_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/spotremediation/spotremediation.go
- control-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader.go
- hypershift-operator/controllers/hostedcluster/internal/platform/azure/azure.go
- control-plane-operator/hostedclusterconfigoperator/controllers/machine/setup.go
- hypershift-operator/controllers/hostedcluster/internal/platform/openstack/openstack.go
- control-plane-operator/controllers/hostedcontrolplane/v2/kas/kubeconfig.go
- support/api/capi_types.go
- api/hypershift/v1beta1/hosted_controlplane.go
- hypershift-operator/controllers/hostedcluster/internal/platform/kubevirt/kubevirt.go
- hypershift-operator/controllers/hostedcluster/internal/platform/openstack/openstack_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/globalps/globalps_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/machine/machine_test.go
- cmd/cluster/core/dump.go
- test/e2e/upgrade_hypershift_operator_test.go
- hypershift-operator/controllers/nodepool/aws_test.go
- test/e2e/nodepool_osp_advanced_test.go
- control-plane-operator/hostedclusterconfigoperator/controllers/inplaceupgrader/inplaceupgrader_test.go
- hypershift-operator/controllers/hostedcluster/hostedcluster_controller_test.go
- hypershift-operator/controllers/nodepool/nodepool_controller.go
- hypershift-operator/controllers/nodepool/conditions.go
- hypershift-operator/controllers/nodepool/version_test.go
- hypershift-operator/controllers/hostedcluster/hostedcluster_controller.go
- hypershift-operator/controllers/nodepool/nodepool_controller_test.go
- hypershift-operator/controllers/nodepool/capi_test.go
Comment on lines
+672
to
+679
| // durationToSeconds converts a *metav1.Duration to *int32 seconds. | ||
| // CAPI v1beta2 uses integer seconds for timeouts instead of metav1.Duration. | ||
| func durationToSeconds(d *metav1.Duration) *int32 { | ||
| if d == nil { | ||
| return nil | ||
| } | ||
| s := int32(d.Seconds()) | ||
| return &s |
Contributor
There was a problem hiding this comment.
Guard duration→int32 conversions against overflow/invalid values.
Line 678 and Lines 745-746 narrow duration seconds to int32 without bounds checks. Annotation/CR-sourced durations can exceed int32 range (or be negative), which can wrap/produce invalid timeout seconds.
🛠️ Proposed fix
+const maxInt32Seconds = int64(1<<31 - 1)
+
// durationToSeconds converts a *metav1.Duration to *int32 seconds.
// CAPI v1beta2 uses integer seconds for timeouts instead of metav1.Duration.
func durationToSeconds(d *metav1.Duration) *int32 {
if d == nil {
return nil
}
- s := int32(d.Seconds())
+ seconds := d.Duration / time.Second
+ if seconds < 0 {
+ seconds = 0
+ }
+ if seconds > maxInt32Seconds {
+ seconds = maxInt32Seconds
+ }
+ s := int32(seconds)
return &s
}
@@
- timeoutSec := int32(timeOut.Seconds())
- startupSec := int32(nodeStartupTimeout.Seconds())
+ timeoutSec := durationToSeconds(&metav1.Duration{Duration: timeOut})
+ startupSec := durationToSeconds(&metav1.Duration{Duration: nodeStartupTimeout})
mhc.Spec = capiv1.MachineHealthCheckSpec{
@@
- Checks: capiv1.MachineHealthCheckChecks{
- NodeStartupTimeoutSeconds: &startupSec,
+ Checks: capiv1.MachineHealthCheckChecks{
+ NodeStartupTimeoutSeconds: startupSec,
UnhealthyNodeConditions: []capiv1.UnhealthyNodeCondition{
{
Type: corev1.NodeReady,
Status: corev1.ConditionFalse,
- TimeoutSeconds: &timeoutSec,
+ TimeoutSeconds: timeoutSec,
},
{
Type: corev1.NodeReady,
Status: corev1.ConditionUnknown,
- TimeoutSeconds: &timeoutSec,
+ TimeoutSeconds: timeoutSec,
},
},
},As per coding guidelines, “Integer overflow: bounds-check user-supplied sizes” applies here because these timeout values are derived from user-configurable inputs.
Also applies to: 745-746
🤖 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.go` around lines 672 - 679, The
durationToSeconds helper currently casts d.Seconds() directly to int32 which can
overflow or accept negative values; change durationToSeconds to validate and
clamp the seconds: if d is nil return nil; compute secs := d.Seconds(); if secs
< 0 set secs = 0; if secs > float64(math.MaxInt32) set secs =
float64(math.MaxInt32); then convert to int32 and return its pointer. Replace
any other direct casts of d.Seconds() to int32 (the conversions referenced at
lines 745-746) to use durationToSeconds (or apply the same clamp logic) so all
user-sourced durations are bounds-checked and non-negative before assigning to
*int32 fields.
Source: Coding guidelines
Member
Author
|
/test e2e-aws |
Test Resultse2e-aws
|
Member
Author
|
/test e2e-aws |
Contributor
|
@clebs: 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. |
Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryThis is a CI infrastructure failure, not a test failure caused by PR #8717. The job never built any code or ran any tests. During the input resolution phase, ci-operator attempted to resolve the Root CauseThe OpenShift CI release controller at Failure chain:
Key indicators this is infrastructure, not PR-related:
Recommendations
Evidence
|
Bump Cluster API core imports from v1beta1 to v1beta2 across all
consumers. Provider imports (Azure, GCP, OpenStack, AWS, IBM Cloud)
remain on v1beta1 as their ControlPlaneEndpoint field still references
core v1beta1.APIEndpoint — no provider has migrated this yet.
Key type changes:
- corev1.ObjectReference → capiv1.ContractVersionedObjectReference
- Machine.Status.NodeRef: pointer → MachineNodeReference (use .IsDefined())
- Cluster.Spec.Paused: bool → *bool
- Version/FailureDomain: *string → string
- Status replica fields: int32 → *int32 (access via ptr.Deref)
- Strategy → Rollout.Strategy
- NodeDrainTimeout → Deletion.NodeDrainTimeoutSeconds
- MHC: UnhealthyConditions → Checks.UnhealthyNodeConditions,
MaxUnhealthy → Remediation.TriggerIf.UnhealthyLessThanOrEqualTo
- Conditions: capiv1.Condition → metav1.Condition
- ReadyCondition → MachinesReadyCondition (on MachineDeployment/MachineSet)
- machineConditionResult.Status: corev1.ConditionStatus → metav1.ConditionStatus
- MachineDeploymentComplete: use UpToDateReplicas + ReadyReplicas
- Constants renamed with V1Beta1 suffix (e.g. WaitingForNodeRefReason)
Control plane changes:
- Add HCP Status.Initialization.ControlPlaneInitialized
- Add patchInfrastructureInitializationProvisioned for CAPI v1beta2 contract
- Remove conversion webhook (no longer needed with single API version)
- Remove v1beta1 scheme registration (core, addons, ipam)
Signed-off-by: Borja Clemente <bclement@redhat.com>
- ContractVersionedObjectReference (APIGroup instead of APIVersion, no Namespace) - FailureDomain: *string nil → empty string - Version: *string → string - Status replica fields: int32 → *int32 via ptr.To - Conditions: []capiv1.Condition → []metav1.Condition - MHC: restructured Checks/Remediation fields - MachineDeploymentComplete rewritten for v1beta2 native fields - MachinePhaseFailed (deprecated) → MachinePhaseDeleting - machineConditionResult expectations: corev1 → metav1 ConditionStatus - Constants renamed with V1Beta1 suffix - Fix bare v1beta2 import and misleading alias in e2e tests Signed-off-by: Borja Clemente <bclement@redhat.com>
Update vendored dependencies after updating clients. Signed-off-by: Borja Clemente <bclement@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
As a the next step after bumping CAPI to 1.11 in CNTRLPLANE-2207, we are updating all clients from
v1beta1tov1beta2.Provider specific code can not be updated yet since all providers are still using
v1beta1in their types.Which issue(s) this PR fixes:
Fixes CNTRLPLANE-3603
Special notes for your reviewer:
Checklist:
Summary by CodeRabbit
New Features
Refactor
Behavior