Skip to content

OCPSTRAT-1677: Add Spot instances documentation - #8485

Merged
enxebre merged 4 commits into
openshift:mainfrom
enxebre:enxebre/spot-instances-docs
Jun 3, 2026
Merged

OCPSTRAT-1677: Add Spot instances documentation#8485
enxebre merged 4 commits into
openshift:mainfrom
enxebre:enxebre/spot-instances-docs

Conversation

@enxebre

@enxebre enxebre commented May 12, 2026

Copy link
Copy Markdown
Member

Summary

  • Add documentation for AWS Spot instance support in the automated machine management section
  • Covers prerequisites (SQS queue, EventBridge rules, IAM permissions), NodePool creation with marketType: Spot, optional maxPrice, and termination handling behavior
  • Documents the graceful termination flow (NTH + spot remediation controller), fallback without SQS, and MachineHealthCheck details

Test plan

  • Verify docs render correctly with mkdocs serve
  • Review YAML examples for accuracy against current API types

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Added a how-to for AWS Spot instances in HyperShift: prerequisites (SQS queue and EventBridge interruption signals), configuring clusters to receive termination notifications, creating Spot-backed node pools with optional max price, runtime labeling/tagging, recommended graceful termination flow using the AWS Node Termination Handler and remediation, fallback when no queue is configured, guidance on MachineHealthCheck per Spot pool, and operational constraints.

Add documentation for AWS Spot instance support in the
automated machine management section covering prerequisites,
SQS queue setup, NodePool creation, and termination handling
behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.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 May 12, 2026
@openshift-ci-robot

Copy link
Copy Markdown

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

Details

In response to this:

Summary

  • Add documentation for AWS Spot instance support in the automated machine management section
  • Covers prerequisites (SQS queue, EventBridge rules, IAM permissions), NodePool creation with marketType: Spot, optional maxPrice, and termination handling behavior
  • Documents the graceful termination flow (NTH + spot remediation controller), fallback without SQS, and MachineHealthCheck details

Test plan

  • Verify docs render correctly with mkdocs serve
  • Review YAML examples for accuracy against current API types

🤖 Generated with Claude Code

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.

@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)
  • docs/content/reference/aggregated-docs.md is excluded by !docs/content/reference/aggregated-docs.md

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: cf4b4230-0a4b-4f27-9889-d1773dd5f833

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

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Added a new documentation page for AWS Spot instances in HyperShift that covers prerequisites (SQS queue, EventBridge rules, and queue policies), configuration steps for HostedCluster and NodePool, runtime behavior including graceful termination flow via AWS Node Termination Handler, MachineHealthCheck setup per Spot NodePool, and operational constraints. Updated the MkDocs navigation configuration to include a link to this new documentation page under the automated machine management section.

🚥 Pre-merge checks | ✅ 12
✅ Passed checks (12 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: adding AWS Spot instances documentation to the automated machine management section.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 This PR is documentation-only (adds spot-instances.md and updates mkdocs.yml), containing no test files. The check for Ginkgo test name stability is not applicable as no tests are modified or added.
Test Structure And Quality ✅ Passed PR contains only documentation changes (markdown + mkdocs config); no Ginkgo test code present, so test quality check is not applicable.
Microshift Test Compatibility ✅ Passed PR adds documentation only (markdown file + mkdocs config). No new Ginkgo e2e tests are added, so the MicroShift test compatibility check is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed Documentation-only PR with no Ginkgo e2e tests added; SNO compatibility check does not apply to non-test changes.
Topology-Aware Scheduling Compatibility ✅ Passed This PR is documentation-only (193 lines of guide + 1 mkdocs config line). The check applies to code, operators, and controllers; it does not flag documentation files.
Ote Binary Stdout Contract ✅ Passed PR contains only documentation (spot-instances.md) and MkDocs configuration changes; no executable code that could violate OTE Binary Stdout Contract.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed This PR adds only documentation and configuration files (spot-instances.md and mkdocs.yml); no Ginkgo e2e tests are present, making the IPv6/disconnected network compatibility check inapplicable.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@enxebre enxebre changed the title NO-JIRA: Add Spot instances documentation CNTRLPLANE-1388: Add Spot instances documentation May 12, 2026
@openshift-ci-robot

openshift-ci-robot commented May 12, 2026

Copy link
Copy Markdown

@enxebre: This pull request references CNTRLPLANE-1388 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 epic to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

  • Add documentation for AWS Spot instance support in the automated machine management section
  • Covers prerequisites (SQS queue, EventBridge rules, IAM permissions), NodePool creation with marketType: Spot, optional maxPrice, and termination handling behavior
  • Documents the graceful termination flow (NTH + spot remediation controller), fallback without SQS, and MachineHealthCheck details

Test plan

  • Verify docs render correctly with mkdocs serve
  • Review YAML examples for accuracy against current API types

🤖 Generated with Claude Code

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@enxebre enxebre changed the title CNTRLPLANE-1388: Add Spot instances documentation OCPSTRAT-1677: Add Spot instances documentation May 12, 2026
@openshift-ci-robot

openshift-ci-robot commented May 12, 2026

Copy link
Copy Markdown

@enxebre: This pull request references OCPSTRAT-1677 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 feature to target either version "5.0." or "openshift-5.0.", but it targets "openshift-4.22" instead.

Details

In response to this:

Summary

  • Add documentation for AWS Spot instance support in the automated machine management section
  • Covers prerequisites (SQS queue, EventBridge rules, IAM permissions), NodePool creation with marketType: Spot, optional maxPrice, and termination handling behavior
  • Documents the graceful termination flow (NTH + spot remediation controller), fallback without SQS, and MachineHealthCheck details

Test plan

  • Verify docs render correctly with mkdocs serve
  • Review YAML examples for accuracy against current API types

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
  • Added comprehensive guide for configuring and managing AWS Spot instances in HyperShift, including prerequisites, cluster and NodePool setup, graceful termination strategies with health monitoring, and key operational constraints.

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 requested review from cblecker and muraee May 12, 2026 09:32
@openshift-ci

openshift-ci Bot commented May 12, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre

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 approved Indicates a PR has been approved by an approver from all required OWNERS files. area/documentation Indicates the PR includes changes for documentation and removed do-not-merge/needs-area labels May 12, 2026

@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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
docs/content/how-to/automated-machine-management/spot-instances.md (1)

164-177: ⚡ Quick win

Add a flow diagram for termination handling paths.

This section explains service interaction flow (AWS → SQS/EventBridge → NTH → remediation/MHC), but it’s only prose today. Please add a small mermaid (or ASCII) diagram so operators can parse normal vs fallback paths faster.

As per coding guidelines, "Provide service architecture diagrams (mermaid or ASCII format) for design documentation".

🤖 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 `@docs/content/how-to/automated-machine-management/spot-instances.md` around
lines 164 - 177, Add a small mermaid (or ASCII) flow diagram illustrating both
graceful and fallback termination paths into the "Graceful termination with SQS
(recommended)" section: show AWS -> EventBridge -> SQS -> NTH -> cordon/drain ->
taint (aws-node-termination-handler/spot-itn) -> machine annotation
(hypershift.openshift.io/spot-interruption-signal) -> Machine delete ->
replacement provisioning, and the fallback path AWS -> abrupt terminate ->
Machine Failed -> MachineHealthCheck remediation; reference
HostedCluster.terminationHandlerQueueURL, NodePool with marketType: Spot, NTH,
Machine, and MachineHealthCheck in the diagram caption so operators can quickly
map diagram nodes to config symbols.
🤖 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 `@docs/content/how-to/automated-machine-management/spot-instances.md`:
- Line 166: The phrase "control plane component" should use a hyphenated
compound modifier; update the documentation sentence that reads "deploys the
Termination Handler as a control plane component" to "deploys the Termination
Handler as a control-plane component" (affecting the sentence referencing
terminationHandlerQueueURL, HostedCluster, NodePool, marketType: Spot, and
Termination Handler).

---

Nitpick comments:
In `@docs/content/how-to/automated-machine-management/spot-instances.md`:
- Around line 164-177: Add a small mermaid (or ASCII) flow diagram illustrating
both graceful and fallback termination paths into the "Graceful termination with
SQS (recommended)" section: show AWS -> EventBridge -> SQS -> NTH ->
cordon/drain -> taint (aws-node-termination-handler/spot-itn) -> machine
annotation (hypershift.openshift.io/spot-interruption-signal) -> Machine delete
-> replacement provisioning, and the fallback path AWS -> abrupt terminate ->
Machine Failed -> MachineHealthCheck remediation; reference
HostedCluster.terminationHandlerQueueURL, NodePool with marketType: Spot, NTH,
Machine, and MachineHealthCheck in the diagram caption so operators can quickly
map diagram nodes to config symbols.
🪄 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: 73217af9-146e-43af-8ed8-803168420fbe

📥 Commits

Reviewing files that changed from the base of the PR and between b7c62b0 and d33aca0.

⛔ Files ignored due to path filters (1)
  • docs/content/reference/aggregated-docs.md is excluded by !docs/content/reference/aggregated-docs.md
📒 Files selected for processing (2)
  • docs/content/how-to/automated-machine-management/spot-instances.md
  • docs/mkdocs.yml


### Graceful termination with SQS (recommended)

When `terminationHandlerQueueURL` is set on the HostedCluster and at least one NodePool has `marketType: Spot`, HyperShift automatically deploys the Termination Handler as a control plane component. The termination flow is:

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 | ⚡ Quick win

Use hyphenated compound modifier in “control-plane component”.

At Line 166, prefer “control-plane component” for correct technical style.

🤖 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 `@docs/content/how-to/automated-machine-management/spot-instances.md` at line
166, The phrase "control plane component" should use a hyphenated compound
modifier; update the documentation sentence that reads "deploys the Termination
Handler as a control plane component" to "deploys the Termination Handler as a
control-plane component" (affecting the sentence referencing
terminationHandlerQueueURL, HostedCluster, NodePool, marketType: Spot, and
Termination Handler).

@cblecker

Copy link
Copy Markdown
Member

/uncc

@openshift-ci
openshift-ci Bot removed the request for review from cblecker May 16, 2026 02:27
marketType: Spot
```

### Setting a maximum price (optional)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@enxebre do we want to mention this in the downstream docs since it seems like customers shouldn't set it?

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 is a valid choice, I removed the statement


### Graceful termination with SQS (recommended)

When `terminationHandlerQueueURL` is set on the HostedCluster and at least one NodePool has `marketType: Spot`, HyperShift automatically deploys the Termination Handler as a control plane component. The termination flow is:

@muraee muraee May 21, 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.

Suggested change
When `terminationHandlerQueueURL` is set on the HostedCluster and at least one NodePool has `marketType: Spot`, HyperShift automatically deploys the Termination Handler as a control plane component. The termination flow is:
When `terminationHandlerQueueURL` is set on the HostedCluster and at least one NodePool has `marketType: Spot`, HyperShift automatically deploys the Node Termination Handler (NTH) as a control plane component. The termination flow is:

it would be good to mention the abbreviation here first, since we are using it later

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
github-actions Bot temporarily deployed to docs-preview/pr-8485 May 21, 2026 10:03 Inactive
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
github-actions Bot temporarily deployed to docs-preview/pr-8485 May 21, 2026 10:05 Inactive
@muraee

muraee commented May 21, 2026

Copy link
Copy Markdown
Contributor

/lgtm

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

Copy link
Copy Markdown
Contributor

Pipeline controller notification

No second-stage tests were triggered for this PR.

This can happen when:

  • The changed files don't match any pipeline_run_if_changed patterns
  • All files match pipeline_skip_if_only_changed patterns
  • No pipeline-controlled jobs are defined for the main branch

Use /test ? to see all available tests.

@hypershift-jira-solve-ci

hypershift-jira-solve-ci Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

I now have the complete root cause. Here is the report:

Test Failure Analysis Complete

Job Information

Test Failure Analysis

Error

docs/content/reference/aggregated-docs.md: needs update
Process completed with exit code 1

Summary

The Verify job runs make generate update which regenerates docs/content/reference/aggregated-docs.md using the docs-aggregator tool (hack/tools/docs-aggregator/main.go). This tool reads all markdown files under docs/content/, strips markdown links/images, and concatenates them into a single file. After regeneration, the job checks for uncommitted changes via git diff. The CI-regenerated aggregated-docs.md differs from the version committed in the PR because the PR author manually edited aggregated-docs.md with content that does not match the source file docs/content/how-to/automated-machine-management/spot-instances.md. The fix is to run make generate update locally and commit the result.

Root Cause

The PR adds three files:

  1. docs/content/how-to/automated-machine-management/spot-instances.md (source document, 193 lines)
  2. docs/content/reference/aggregated-docs.md (aggregated file, 199 new lines)
  3. docs/mkdocs.yml (nav entry, 1 line)

The aggregated-docs.md file is auto-generated by hack/tools/docs-aggregator/main.go during make generate update. The PR author manually edited aggregated-docs.md with content that diverges from the source spot-instances.md file in three specific places:

  1. Line 15 (Prerequisites section): Source says "The AWS Node Termination Handler (NTH) deployed by HyperShift..." but the manually-edited aggregated version says "The AWS Node Termination Handler deployed by HyperShift..." — the (NTH) abbreviation was removed.

  2. Line 146 (Max price section): Source says "You can request Spot Instances at the Spot price, capped at the On-Demand price, or you can specify the maximum amount you're willing to pay:" but the aggregated version has entirely different wording: "You can optionally set a maximum hourly price for Spot instances. When omitted, you pay the current Spot price (capped at the on-demand price). AWS recommends **not** setting a maximum price to reduce interruption frequency:".

  3. Line 166 (Graceful termination section): Source says "...deploys the Node Termination Handler (NTH) as a control plane component..." but aggregated version says "...deploys the Termination Handler as a control plane component..." — both "Node" prefix and (NTH) abbreviation were removed.

When CI runs make generate update, the docs-aggregator regenerates aggregated-docs.md from the source spot-instances.md, producing output that matches the source file (with link-stripping applied, though this file has no markdown links). This regenerated version differs from the manually-edited version committed in the PR, causing git update-index --refresh to detect the file as modified and fail the check.

Recommendations
  1. Run make generate update locally and commit the result:

    make generate update
    git add docs/content/reference/aggregated-docs.md
    git commit -m "Regenerate aggregated-docs.md"
  2. Do not manually edit aggregated-docs.md — this file is auto-generated by the docs-aggregator tool. Any manual edits will be overwritten by CI and cause the verify job to fail.

  3. If different wording is desired in the aggregated output, change the source file (docs/content/how-to/automated-machine-management/spot-instances.md) instead, then regenerate. The aggregated file simply mirrors all source docs.

  4. Consider adding a comment at the top of aggregated-docs.md (if not already present) indicating that it is auto-generated and should not be manually edited.

Evidence
Evidence Detail
Failed step Step 8: git update-index --refresh — detects uncommitted changes after make generate update
Error message docs/content/reference/aggregated-docs.md: needs update
Generator tool hack/tools/docs-aggregator/main.go — run via make generate update
Generator log Successfully aggregated 296 documentation files to docs/content/reference/aggregated-docs.md
Mismatch 1 Line 15: Source has (NTH) abbreviation, aggregated version omits it
Mismatch 2 Line 146: Entirely different wording for max price description between source and aggregated
Mismatch 3 Line 166: Source has Node Termination Handler (NTH), aggregated has Termination Handler
Root cause PR author manually edited aggregated-docs.md with different content than spot-instances.md
Fix Run make generate update locally and commit the regenerated file

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-ci openshift-ci Bot removed the lgtm Indicates that a PR is ready to be merged. label Jun 2, 2026
@enxebre enxebre added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jun 2, 2026
@devguyio

devguyio commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

/lgtm yolo

@openshift-ci

openshift-ci Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

@enxebre: 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.

@muraee

muraee commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

/lgtm

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

Copy link
Copy Markdown
Contributor

Pipeline controller notification

No second-stage tests were triggered for this PR.

This can happen when:

  • The changed files don't match any pipeline_run_if_changed patterns
  • All files match pipeline_skip_if_only_changed patterns
  • No pipeline-controlled jobs are defined for the main branch

Use /test ? to see all available tests.

@enxebre

enxebre commented Jun 3, 2026

Copy link
Copy Markdown
Member Author

/verified by @enxebre

@enxebre
enxebre merged commit 435c545 into openshift:main Jun 3, 2026
18 of 19 checks passed
@openshift-ci-robot

Copy link
Copy Markdown

@enxebre: This PR has been marked as verified by @enxebre.

Details

In response to this:

/verified by @enxebre

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 verified Signifies that the PR passed pre-merge verification criteria label Jun 3, 2026
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/documentation Indicates the PR includes changes for documentation 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. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants