Skip to content

NO-JIRA: test(autoscaling): make GPU annotation optional in scale-from-zero test - #7484

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
jhjaggars:fix-scale-from-zero-test-gpu-annotation
Jan 17, 2026
Merged

NO-JIRA: test(autoscaling): make GPU annotation optional in scale-from-zero test#7484
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
jhjaggars:fix-scale-from-zero-test-gpu-annotation

Conversation

@jhjaggars

@jhjaggars jhjaggars commented Jan 15, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

This PR fixes a test failure in TestNodePoolAutoscalingScaleFromZero that was failing when testing with non-GPU instance types.

The test unconditionally expected the machine.openshift.io/GPU annotation to be present on the MachineDeployment. However, the scale-from-zero controller only sets this annotation when the instance type has GPUs (see hypershift-operator/controllers/nodepool/scale_from_zero.go:126-132). When testing with non-GPU instances (like m5.large), the GPU annotation is intentionally not set, causing the test to fail.

Changes:

  1. Removed mandatory GPU annotation check - The test now only requires vCPU, memoryMb, and labels annotations (which are always set)
  2. Updated logging to gracefully handle missing GPU annotation in log output

Which issue(s) this PR fixes:

Fixes test failures seen in openshift/release#72805 which enables scale-from-zero testing in CI.

Special notes for your reviewer:

  • The controller behavior is correct - verified from CI artifacts that vCPU, memoryMb, and labels annotations are properly set
  • GPU annotation is correctly omitted for non-GPU instances per the implementation in scale_from_zero.go
  • make staticcheck passes

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs. (N/A - test-only change)
  • This change includes unit tests. (N/A - fixes existing e2e test)

@coderabbitai

coderabbitai Bot commented Jan 15, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Review skipped — only excluded labels are configured. (1)
  • do-not-merge/work-in-progress

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

GPU capacity annotation check in MachineDeployment verification is now optional. When absent, a default fallback string is applied, and log output uses this computed value instead of the raw annotation.

Changes

Cohort / File(s) Summary
GPU Annotation Optionality
test/e2e/autoscaling_test.go
Made GPU capacity annotation optional with fallback default value "none (non-GPU instance)" for logging when annotation is missing; updated log output to use computed value instead of raw annotation

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


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

@jhjaggars
jhjaggars marked this pull request as draft January 15, 2026 14:36
@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 Jan 15, 2026
@openshift-ci
openshift-ci Bot requested review from bryan-cox and devguyio January 15, 2026 14:38
@openshift-ci openshift-ci Bot added area/testing Indicates the PR includes changes for e2e testing and removed do-not-merge/needs-area labels Jan 15, 2026
@jhjaggars
jhjaggars marked this pull request as ready for review January 15, 2026 14:46
@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 Jan 15, 2026
@openshift-ci
openshift-ci Bot requested a review from csrwng January 15, 2026 14:48
@jhjaggars
jhjaggars force-pushed the fix-scale-from-zero-test-gpu-annotation branch from d8d5b74 to 1a886dd Compare January 15, 2026 15:43
@bryan-cox

Copy link
Copy Markdown
Member

/approve

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jan 15, 2026
@openshift-ci

openshift-ci Bot commented Jan 15, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bryan-cox, jhjaggars

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 Jan 15, 2026
@bryan-cox

Copy link
Copy Markdown
Member

/lgtm cancel

@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 15, 2026
@LiangquanLi930

Copy link
Copy Markdown
Contributor

/retest-required

@LiangquanLi930

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jan 16, 2026
@jhjaggars

Copy link
Copy Markdown
Contributor Author

/retest

@jhjaggars

Copy link
Copy Markdown
Contributor Author

/verified via e2e tests later

@openshift-ci-robot

Copy link
Copy Markdown

@jhjaggars: /verified later <@username> requires at least one GitHub @username to be specified (it can be a comma delimited list). It indicates the engineer(s) that will be performing the verification. See https://docs.ci.openshift.org/docs/architecture/jira/#premerge-verification for more information.

Details

In response to this:

/verified via e2e tests later

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.

@jhjaggars

Copy link
Copy Markdown
Contributor Author

/verified @jhjaggars via e2etest later

@openshift-ci-robot

Copy link
Copy Markdown

@jhjaggars: /verified later <@username> requires at least one GitHub @username to be specified (it can be a comma delimited list). It indicates the engineer(s) that will be performing the verification. See https://docs.ci.openshift.org/docs/architecture/jira/#premerge-verification for more information.

Details

In response to this:

/verified @jhjaggars via e2etest later

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.

@jhjaggars

Copy link
Copy Markdown
Contributor Author

/verified later @jhjaggars

@openshift-ci-robot openshift-ci-robot added verified-later verified Signifies that the PR passed pre-merge verification criteria labels Jan 16, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@jhjaggars: This PR has been marked to be verified later by @jhjaggars.

Details

In response to this:

/verified later @jhjaggars

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.

The GPU annotation is only set by the scale-from-zero controller when
instance types have GPUs (scale_from_zero.go:126-132). For non-GPU
instances, the annotation is intentionally not set.

This commit updates the test to not require the GPU annotation, while
still requiring the other capacity annotations (vCPU, memoryMb, labels)
that are always present. The logging is also updated to handle the
missing GPU annotation gracefully.

This fixes test failures in CI where non-GPU instance types (like
m5.large) are used.

Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
@jhjaggars
jhjaggars force-pushed the fix-scale-from-zero-test-gpu-annotation branch from 1a886dd to 1c97c7d Compare January 16, 2026 18:28
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 16, 2026
@bryan-cox

Copy link
Copy Markdown
Member

/lgtm

Just rebased

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jan 16, 2026
@jhjaggars

Copy link
Copy Markdown
Contributor Author

/retest

1 similar comment
@jhjaggars

Copy link
Copy Markdown
Contributor Author

/retest

@LiangquanLi930

Copy link
Copy Markdown
Contributor

/test e2e-aws-4-21

@openshift-ci

openshift-ci Bot commented Jan 17, 2026

Copy link
Copy Markdown
Contributor

@jhjaggars: all tests passed!

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.

@jhjaggars jhjaggars changed the title test(autoscaling): make GPU annotation optional in scale-from-zero test NO-JIRA: test(autoscaling): make GPU annotation optional in scale-from-zero test Jan 17, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@jhjaggars: This pull request explicitly references no jira issue.

Details

In response to this:

What this PR does / why we need it:

This PR fixes a test failure in TestNodePoolAutoscalingScaleFromZero that was failing when testing with non-GPU instance types.

The test unconditionally expected the machine.openshift.io/GPU annotation to be present on the MachineDeployment. However, the scale-from-zero controller only sets this annotation when the instance type has GPUs (see hypershift-operator/controllers/nodepool/scale_from_zero.go:126-132). When testing with non-GPU instances (like m5.large), the GPU annotation is intentionally not set, causing the test to fail.

Changes:

  1. Removed mandatory GPU annotation check - The test now only requires vCPU, memoryMb, and labels annotations (which are always set)
  2. Updated logging to gracefully handle missing GPU annotation in log output

Which issue(s) this PR fixes:

Fixes test failures seen in openshift/release#72805 which enables scale-from-zero testing in CI.

Special notes for your reviewer:

  • The controller behavior is correct - verified from CI artifacts that vCPU, memoryMb, and labels annotations are properly set
  • GPU annotation is correctly omitted for non-GPU instances per the implementation in scale_from_zero.go
  • make staticcheck passes

Checklist:

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs. (N/A - test-only change)
  • This change includes unit tests. (N/A - fixes existing e2e test)

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-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 17, 2026
@openshift-merge-bot
openshift-merge-bot Bot merged commit 9ee4ac7 into openshift:main Jan 17, 2026
20 checks passed
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/testing Indicates the PR includes changes for e2e testing 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. verified Signifies that the PR passed pre-merge verification criteria verified-later

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants