Skip to content

AUTOSCALE-870: add KarpenterOperator tech-preview FeatureGate#8976

Open
maxcao13 wants to merge 1 commit into
openshift:mainfrom
maxcao13:karpenter-operator-fg
Open

AUTOSCALE-870: add KarpenterOperator tech-preview FeatureGate#8976
maxcao13 wants to merge 1 commit into
openshift:mainfrom
maxcao13:karpenter-operator-fg

Conversation

@maxcao13

@maxcao13 maxcao13 commented Jul 9, 2026

Copy link
Copy Markdown
Member

What this PR does / why we need it:

Adds KarpenterOperator as a FeatureGate to HyperShift, behind the TechPreview HCP featureset. For now, this feature gate is a no-op.

Refer to openshift/enhancements#2007 for more details on the function of this feature gate.

This is the first part of refactoring karpenter-operator out of HyperShift, and replacing it with the standalone component here: https://github.com/openshift/karpenter-operator

Which issue(s) this PR fixes:

Fixes https://redhat.atlassian.net/browse/AUTOSCALE-870

Special notes for your reviewer:

Since this will automatically be enabled anytime someone uses techpreview feature set for HO, we have to be careful that any features we add behind this feature gate to not break anyone's existing HO deployment and HCP clusters.

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Summary by CodeRabbit

  • New Features

    • Added a new feature gate for Karpenter Operator support.
    • Made the feature available for the Tech Preview No Upgrade feature set.
  • Tests

    • Updated feature gate coverage to verify the new gate is disabled by default and enabled in the Tech Preview No Upgrade set.
    • Added validation for the new feature gate name.

Adds KarpenterOperator as a FeatureGate to HyperShift, behind the TechPreview HCP featureset.
For now, this feature gate is a no-op.

Signed-off-by: Max Cao <macao@redhat.com>
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jul 9, 2026
@openshift-ci-robot

openshift-ci-robot commented Jul 9, 2026

Copy link
Copy Markdown

@maxcao13: This pull request references AUTOSCALE-870 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.

Details

In response to this:

What this PR does / why we need it:

Adds KarpenterOperator as a FeatureGate to HyperShift, behind the TechPreview HCP featureset. For now, this feature gate is a no-op.

Refer to openshift/enhancements#2007 for more details on the function of this feature gate.

This is the first part of refactoring karpenter-operator out of HyperShift, and replacing it with the standalone component here: https://github.com/openshift/karpenter-operator

Which issue(s) this PR fixes:

Fixes https://redhat.atlassian.net/browse/AUTOSCALE-870

Special notes for your reviewer:

Since this will automatically be enabled anytime someone uses techpreview feature set for HO, we have to be careful that any features we add behind this feature gate to not break anyone's existing HO deployment and HCP clusters.

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

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 openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 9, 2026
@openshift-ci

openshift-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci Bot added do-not-merge/needs-area area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release and removed do-not-merge/needs-area labels Jul 9, 2026
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This change introduces a new feature gate constant, KarpenterOperator, in the HyperShift operator's feature gate module. The feature is enabled for the TechPreviewNoUpgrade feature set and disabled by default and for DevPreviewNoUpgrade. It is registered into the internal allFeatures collection. Corresponding test cases were updated to verify the expected enabled/disabled state of KarpenterOperator across all three feature sets and to confirm its string constant value.

🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed Changed tests use static, descriptive names only; no dynamic pod/UUID/timestamp-like values appear in any test titles.
Test Structure And Quality ✅ Passed Changed tests are plain unit tests, not Ginkgo specs; no cluster setup/cleanup or Eventually waits, and assertions are localized and consistent.
Topology-Aware Scheduling Compatibility ✅ Passed PR only adds a feature gate and tests; the karpenter-operator manifest has no node selectors, affinity, or spread constraints.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR only changes feature-gate unit tests and constants; no new Ginkgo e2e tests, IPv4 assumptions, or external connectivity requirements found.
No-Weak-Crypto ✅ Passed Changed files only add a feature gate constant and tests; no weak crypto, custom crypto, or secret comparisons appear in the diff.
Container-Privileges ✅ Passed Diff only adds a feature gate constant and tests; no container/K8s manifests or privilege/securityContext settings were changed.
No-Sensitive-Data-In-Logs ✅ Passed PR diff only adds a feature-gate constant and tests; no new log statements or sensitive-data strings appear in the patch.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding the KarpenterOperator tech-preview FeatureGate.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

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.

🧹 Nitpick comments (1)
hypershift-operator/featuregate/feature.go (1)

37-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Placeholder "beta: x.y.z" left in the comment.

The alpha: line has a real version, but beta: x.y.z is unfilled placeholder text. Either remove it until a beta stage is planned or fill in the actual target version.

🤖 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/featuregate/feature.go` around lines 37 - 42, The
KarpenterOperator feature gate comment still contains placeholder beta version
text, so update the documentation in feature.go to either remove the beta
annotation entirely or replace the beta: x.y.z placeholder with the real target
version. Keep the existing KarpenterOperator symbol and its alpha metadata, and
make the comment consistent with the actual release stage.
🤖 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 `@hypershift-operator/featuregate/feature.go`:
- Around line 37-42: The KarpenterOperator feature gate comment still contains
placeholder beta version text, so update the documentation in feature.go to
either remove the beta annotation entirely or replace the beta: x.y.z
placeholder with the real target version. Keep the existing KarpenterOperator
symbol and its alpha metadata, and make the comment consistent with the actual
release stage.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: d289020f-3399-4bb0-b600-16e9d9ec1d2c

📥 Commits

Reviewing files that changed from the base of the PR and between 54d2400 and 1ec7c7f.

📒 Files selected for processing (2)
  • hypershift-operator/featuregate/feature.go
  • hypershift-operator/featuregate/feature_test.go

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.60%. Comparing base (54d2400) to head (1ec7c7f).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8976   +/-   ##
=======================================
  Coverage   43.60%   43.60%           
=======================================
  Files         771      771           
  Lines       95806    95807    +1     
=======================================
+ Hits        41778    41779    +1     
  Misses      51119    51119           
  Partials     2909     2909           
Files with missing lines Coverage Δ
hypershift-operator/featuregate/feature.go 81.81% <100.00%> (+0.86%) ⬆️
Flag Coverage Δ
cmd-support 37.22% <ø> (ø)
cpo-hostedcontrolplane 45.91% <ø> (ø)
cpo-other 45.10% <ø> (ø)
hypershift-operator 53.66% <100.00%> (+<0.01%) ⬆️
other 32.08% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@maxcao13 maxcao13 marked this pull request as ready for review July 9, 2026 19:13
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 9, 2026
@openshift-ci openshift-ci Bot requested review from csrwng and devguyio July 9, 2026 19:13

@csrwng csrwng left a comment

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.

Clean, well-tested addition. Follows the established feature gate pattern exactly — constant, feature variable with TechPreviewNoUpgrade, init registration, and full test coverage across all three feature sets. LGTM.

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jul 9, 2026
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aks-4-22
/test e2e-aws-4-22
/test e2e-aks
/test e2e-aws
/test e2e-aws-upgrade-hypershift-operator
/test e2e-azure-v2-self-managed
/test e2e-kubevirt-aws-ovn-reduced
/test e2e-v2-aws
/test e2e-v2-gke

@openshift-ci

openshift-ci Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csrwng, maxcao13

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 9, 2026
@cwbotbot

cwbotbot commented Jul 9, 2026

Copy link
Copy Markdown

Test Results

e2e-aws

Failed Tests

Total failed tests: 2

  • TestKarpenter
  • TestKarpenter/Teardown

e2e-aks

@maxcao13

Copy link
Copy Markdown
Member Author

/retest-required

@openshift-ci

openshift-ci Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

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

Test name Commit Details Required Rerun command
ci/prow/e2e-azure-v2-self-managed 1ec7c7f link true /test e2e-azure-v2-self-managed
ci/prow/e2e-aws 1ec7c7f link true /test e2e-aws
ci/prow/e2e-aks-4-22 1ec7c7f link true /test e2e-aks-4-22

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@hypershift-jira-solve-ci

hypershift-jira-solve-ci Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

All Karpenter functional tests passed. The failure is purely in the teardown/cleanup phase. Now I have enough evidence for the report.

Test Failure Analysis Complete

Job Information

Test Failure Analysis

Error

fixture.go:338: Failed to wait for infra resources in guest cluster to be deleted: context deadline exceeded
fixture.go:345: Failed to clean up 10 remaining resources for guest cluster
hypershift_framework.go:576: Failed to destroy cluster, will retry: hostedcluster wasn't finalized, aborting delete: context deadline exceeded

Summary

All Karpenter functional tests passed (ValidateHostedCluster, Main with all parallel provisioning subtests, EnsureHostedCluster). The sole failure is in TestKarpenter/Teardown (1397.53s), caused by the hosted cluster karpenter-x27cc not finalizing within the grace period on the first destroy attempt, followed by residual AWS infrastructure resources (1 NLB + 9 EBS volumes) not being cleaned up within the 15-minute post-delete validation window. The NLB belonged to the openshift-ingress/router-default service, and the EBS volumes were associated with 6 different Karpenter nodepools and 2 CAPA Machines. This is a teardown infrastructure cleanup timeout — not a product regression from the PR's FeatureGate changes.

Root Cause

The failure chain has two stages:

Stage 1: HostedCluster finalization timeout. The first destroyCluster call timed out because the HostedCluster e2e-clusters-c2p2m/karpenter-x27cc wasn't finalized within the 15-minute ClusterGracePeriod (see fixture.go:241). This is logged as: "Failed to destroy cluster, will retry: hostedcluster wasn't finalized, aborting delete: context deadline exceeded". The cluster was eventually destroyed on the retry attempt (hypershift_framework.go:591: Destroyed cluster).

Stage 2: Post-delete AWS resource validation timeout. After the cluster was destroyed, the validateAWSGuestResourcesDeletedFunc (fixture.go:287-352) polls AWS every 20 seconds for up to 15 minutes, checking for remaining resources tagged with kubernetes.io/cluster/karpenter-x27cc=owned. Ten resources survived:

Resource Type Count Source
NLB (elasticloadbalancing) 1 openshift-ingress/router-default service
EBS volumes (Karpenter nodepools) 7 nodepools: on-demand (×2), capacity-reservation-test, version-test, arbitrary-subnet-test, instance-profile-test, kubelet-config-test
EBS volumes (CAPA Machines) 2 Machine nodes: karpenter-x27cc-us-east-1b-pg4f2-gbqzl, karpenter-x27cc-us-east-1b-pg4f2-wlp2j

The hasGuestResources function in fixture.go:363 filters EC2 volumes — it only counts them as "guest resources" if they carry a kubernetes.io/created-for/pv/name tag (PV-backed volumes). The Karpenter nodepool and Machine volumes lack this PV tag. However, the NLB (elasticloadbalancing) always triggers hasGuestResources=true, which is what prevented the poll from succeeding within the 15-minute window.

The NLB for openshift-ingress/router-default is typically deleted by the cloud controller when the hosted cluster's ingress controller is cleaned up. The delayed finalization in Stage 1 consumed time that was needed for AWS to fully deprovision the NLB, causing a cascade of timeouts.

Contributing factor: An AWS account-level KeyPairLimitExceeded error (Maximum of 5000 keypairs reached) prevented machine journal dumps but is marked nonfatal and did not directly cause the test failure.

This is a test infrastructure/timing issue, not a product bug introduced by the PR's KarpenterOperator FeatureGate changes. All functional Karpenter tests passed, including the full parallel provisioning suite.

Recommendations
  1. Retry / re-trigger the job. This is a teardown timing issue, not a code regression. The PR's functional changes (KarpenterOperator FeatureGate) all passed validation. Re-running the job should succeed if the cluster finalizes faster.

  2. Investigate slow HostedCluster finalization. The first destroy attempt timed out on finalization. If this pattern recurs, investigate what finalizers on the HostedCluster take the longest — the Karpenter test creates multiple nodepools with different configurations (6 distinct Karpenter nodepools), and draining/deleting all of them may take longer than the 15-minute grace period.

  3. Consider increasing the ClusterGracePeriod for Karpenter tests. Karpenter tests create more AWS resources (multiple nodepools, EC2NodeClasses) than typical HyperShift e2e tests. The fixed 15-minute grace period in fixture.go:241 may be too tight for the Karpenter teardown scenario, especially when there are many concurrent test clusters being destroyed.

  4. Address the KeyPairLimitExceeded warning. The AWS CI account has hit the 5,000 keypair limit. While nonfatal, this prevents machine journal dumps for diagnostics. A periodic cleanup of stale keypairs in the CI AWS account would resolve this.

Evidence
Evidence Detail
Failed test TestKarpenter/Teardown (1397.53s) — only failure in the entire 645-test suite
All functional tests PASS — ValidateHostedCluster, Main (all 7 parallel provisioning subtests), EnsureHostedCluster
First destroy attempt hypershift_framework.go:576: Failed to destroy cluster, will retry: hostedcluster wasn't finalized, aborting delete: context deadline exceeded
Cluster eventually destroyed hypershift_framework.go:591: Destroyed cluster. Namespace: e2e-clusters-c2p2m, name: karpenter-x27cc
Residual NLB arn:aws:elasticloadbalancing:us-east-1:820196288204:loadbalancer/net/a7ea2a93c77bc4fd0a16372155de590c/53ab6356c432c7d6 (openshift-ingress/router-default)
Residual EBS volumes 9 volumes across 6 Karpenter nodepools + 2 CAPA Machines, all tagged kubernetes.io/cluster/karpenter-x27cc=owned
Cleanup validation timeout fixture.go:338: Failed to wait for infra resources in guest cluster to be deleted: context deadline exceeded (15-minute poll at fixture.go:303)
hasGuestResources logic fixture.go:363-382 — only counts EC2 volumes with PV tag, but NLB triggers true unconditionally
AWS account issue (nonfatal) KeyPairLimitExceeded: Maximum of 5000 keypairs reached — prevented journal dumps only
Cluster name karpenter-x27cc in namespace e2e-clusters-c2p2m
Test summary DONE 645 tests, 30 skipped, 2 failures (TestKarpenter + TestKarpenter/Teardown)

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants