Skip to content

CNTRLPLANE-3160: Drop AutoNodeKarpenter feature gate and promote EC2NodeClass to v1 #8166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
enxebre merged 5 commits into from
Apr 30, 2026
Merged

CNTRLPLANE-3160: Drop AutoNodeKarpenter feature gate and promote EC2NodeClass to v1 #8166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
df33c35
feat(api): drop AutoNodeKarpenter feature gate
enxebre Apr 20, 2026
9ce89af
feat(api): promote OpenshiftEC2NodeClass API from v1beta1 to v1
enxebre Apr 20, 2026
52f7f8c
test: add envtest validation suites for AutoNode and EC2NodeClass
enxebre Apr 20, 2026
d64c222
chore(generated): regenerate CRDs, clients, vendor, and docs
enxebre Apr 20, 2026
366f255
ci: add karpenter test path to envtest GHA workflow triggers
enxebre Apr 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/envtest-kube.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:
if [ "$EVENT_NAME" = "pull_request" ]; then
ref="$PR_DIFF_REF"
fi
if git diff --name-only "$ref" | grep -qE '^(api/|test/envtest/|cmd/install/assets/crds/hypershift-operator/tests/|\.github/workflows/envtest-kube\.yaml$)'; then
if git diff --name-only "$ref" | grep -qE '^(api/|test/envtest/|cmd/install/assets/crds/hypershift-operator/tests/|karpenter-operator/controllers/karpenter/assets/tests/|\.github/workflows/envtest-kube\.yaml$)'; then
echo "should_run=true"
else
echo "should_run=false"
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/envtest-ocp.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:
if [ "$EVENT_NAME" = "pull_request" ]; then
ref="$PR_DIFF_REF"
fi
if git diff --name-only "$ref" | grep -qE '^(api/|test/envtest/|cmd/install/assets/crds/hypershift-operator/tests/|\.github/workflows/envtest-ocp\.yaml$)'; then
if git diff --name-only "$ref" | grep -qE '^(api/|test/envtest/|cmd/install/assets/crds/hypershift-operator/tests/|karpenter-operator/controllers/karpenter/assets/tests/|\.github/workflows/envtest-ocp\.yaml$)'; then
echo "should_run=true"
else
echo "should_run=false"
Expand Down
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ karpenter-api: $(CONTROLLER_GEN) $(YQ)
karpenter-operator/hack/crds-sync.sh
karpenter-operator/hack/adjust-cel.sh
$(CONTROLLER_GEN) $(CRD_OPTIONS) paths="./api/karpenter/..." output:crd:artifacts:config=karpenter-operator/controllers/karpenter/assets
cp karpenter-operator/controllers/karpenter/assets/karpenter.hypershift.openshift.io_openshiftec2nodeclasses.yaml karpenter-operator/controllers/karpenter/assets/zz_generated.crd-manifests/openshiftec2nodeclasses.crd.yaml

@coderabbitai coderabbitai Bot Apr 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Guard the CRD copy with directory creation.

Line 192 assumes .../zz_generated.crd-manifests/ already exists. If it doesn’t, make karpenter-api fails at this step.

Suggested hardening 
 	$(CONTROLLER_GEN) $(CRD_OPTIONS) paths="./api/karpenter/..." output:crd:artifacts:config=karpenter-operator/controllers/karpenter/assets
+	mkdir -p karpenter-operator/controllers/karpenter/assets/zz_generated.crd-manifests
 	cp karpenter-operator/controllers/karpenter/assets/karpenter.hypershift.openshift.io_openshiftec2nodeclasses.yaml karpenter-operator/controllers/karpenter/assets/zz_generated.crd-manifests/openshiftec2nodeclasses.crd.yaml
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Makefile` at line 192, The Makefile copy step assumes the destination
directory exists; update the recipe that runs the cp command (used by the
karpenter-api target) to create the destination dir first by running mkdir -p
for the zz_generated.crd-manifests directory (or the directory portion of the
target path) before the cp into
karpenter-operator/controllers/karpenter/assets/zz_generated.crd-manifests/openshiftec2nodeclasses.crd.yaml
so the copy never fails when the folder is missing.


.PHONY: control-plane-operator
control-plane-operator:
Expand Down
5 changes: 0 additions & 5 deletions api/.golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,6 @@ linters:
- kubeapilinter
path: hypershift/v1beta1/openstack.go
text: 'arrayofstruct: OpenStackPlatformSpec.Subnets is an array of structs, but the struct has no required fields. At least one field should be marked as required to prevent ambiguous YAML configurations'
- linters:
- kubeapilinter
path: karpenter/v1beta1/karpenter_types.go

@jparrill jparrill Apr 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we add v1 to this path?

@enxebre enxebre Apr 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the exception is not actually needed, the struct already has // +kubebuilder:validation:MinProperties=1

@enxebre enxebre Apr 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this triggered an offline discussion for me with Joel. This would still hit the linter on a new field or a field change. "Not having a specific requirement but at least one" is not an ideal choice when the struct belongs to a slice. The reasoning is that with all fields optional, a user could accidentally write two separate items thinking they're writing one (or the other way around). I think we'll accepted that here.

@JoelSpeed JoelSpeed Apr 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

To put the slice thing into an example

- foo: some
- bar: thing

vs

- foo: some
  bar: thing

Is a very subtle difference that you might glance over, but has a very different meaning to the API.

This has caught people out in security contexts before with pretty nasty results

text: 'arrayofstruct: OpenshiftEC2NodeClassSpec.CapacityReservationSelectorTerms is an array of structs, but the struct has no required fields. At least one field should be marked as required to prevent ambiguous YAML configurations'

# conditions (1 issue)
- linters:
- kubeapilinter
Expand Down
3 changes: 0 additions & 3 deletions api/hypershift/v1beta1/featuregates/featureGate-Hypershift-Default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
"featureGates": [
{
"disabled": [
{
"name": "AutoNodeKarpenter",
},
{
"name": "AROHCPManagedIdentities"
},
Expand Down
3 changes: 0 additions & 3 deletions api/hypershift/v1beta1/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@
{
"disabled": [],
"enabled": [
{
"name": "AutoNodeKarpenter",
},
{
"name": "ExternalOIDC"
},
Expand Down
3 changes: 0 additions & 3 deletions api/hypershift/v1beta1/featuregates/featureGate-SelfManagedHA-Default.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
"featureGates": [
{
"disabled": [
{
"name": "AutoNodeKarpenter",
},
{
"name": "ExternalOIDCWithUpstreamParity"
},
Expand Down
3 changes: 0 additions & 3 deletions api/hypershift/v1beta1/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@
{
"disabled": [],
"enabled": [
{
"name": "AutoNodeKarpenter",
},
{
"name": "ExternalOIDC"
},
Expand Down
4 changes: 1 addition & 3 deletions api/hypershift/v1beta1/hosted_controlplane.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,8 @@ type HostedControlPlaneSpec struct {
// using the specified provisioner (e.g. Karpenter) instead of requiring
// manual NodePool management.
//
// +openshift:enable:FeatureGate=AutoNodeKarpenter
// +optional
AutoNode *AutoNode `json:"autoNode,omitempty"`
AutoNode AutoNode `json:"autoNode,omitzero"`

@jparrill jparrill Apr 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just to know more, is this change expected by promoting a featureGate or this is due to the new api linter?

@enxebre enxebre Apr 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

is expected that any API follow best practices, specially if it's GA. The linter enforces those best practices.


// nodeSelector when specified, must be true for the pods managed by the HostedCluster to be scheduled.
//
Expand Down Expand Up @@ -414,7 +413,6 @@ type HostedControlPlaneStatus struct {
NodeCount *int `json:"nodeCount,omitempty"`

// autoNode contains the observed state of the autoNode (Karpenter) provisioner.
// +openshift:enable:FeatureGate=AutoNodeKarpenter
// +optional
AutoNode AutoNodeStatus `json:"autoNode,omitzero"`

Expand Down
8 changes: 3 additions & 5 deletions api/hypershift/v1beta1/hostedcluster_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,9 +642,8 @@ type HostedClusterSpec struct {
// autoNode specifies the configuration for automatic node provisioning and lifecycle management.
// When set, the provisioner(e.g. Karpenter) will be used to provision nodes for targeted workloads.
//
// +openshift:enable:FeatureGate=AutoNodeKarpenter
// +optional
AutoNode *AutoNode `json:"autoNode,omitempty"`
AutoNode AutoNode `json:"autoNode,omitzero"`

// etcd specifies configuration for the control plane etcd cluster. The
// default managementType is Managed. Once set, the managementType cannot be
Expand Down Expand Up @@ -1398,7 +1397,7 @@ type ProvisionerConfig struct {
//
// +optional
// +unionMember
Karpenter *KarpenterConfig `json:"karpenter,omitempty"`
Karpenter KarpenterConfig `json:"karpenter,omitzero"`
}

// KarpenterConfig specifies the configuration for the Karpenter provisioner
Expand All @@ -1418,7 +1417,7 @@ type KarpenterConfig struct {
//
// +optional
// +unionMember
AWS *KarpenterAWSConfig `json:"aws,omitempty"`
AWS KarpenterAWSConfig `json:"aws,omitzero"`

@jparrill jparrill Apr 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same comment as above

}

// KarpenterAWSConfig specifies AWS-specific configuration for the Karpenter provisioner.
Expand Down Expand Up @@ -2174,7 +2173,6 @@ type HostedClusterStatus struct {
Platform *PlatformStatus `json:"platform,omitempty"`

// autoNode contains the observed state of the autoNode (Karpenter) provisioner.
// +openshift:enable:FeatureGate=AutoNodeKarpenter
// +optional
AutoNode AutoNodeStatus `json:"autoNode,omitzero"`

Expand Down
26 changes: 5 additions & 21 deletions api/hypershift/v1beta1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions api/hypershift/v1beta1/zz_generated.featuregated-crd-manifests.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading