Skip to content

[FEATURE POC] Add support for EC2 active vCPU options#178

Open
davivcgarcia wants to merge 2 commits intoopenshift:mainfrom
davivcgarcia:main
Open

[FEATURE POC] Add support for EC2 active vCPU options#178
davivcgarcia wants to merge 2 commits intoopenshift:mainfrom
davivcgarcia:main

Conversation

@davivcgarcia
Copy link

@davivcgarcia davivcgarcia commented Mar 4, 2026

Summary

Hey folks! 👋

This PR extends AWSMachineProviderConfig.CPUOptions to expose EC2 Active vCPU controls — specifically coreCount and threadsPerCore — allowing OpenShift admins to fine-tune CPU topology when launching instances.

These map directly to the CpuOptionsRequest fields in the EC2 RunInstances API and give users control over Simultaneous Multithreading (SMT).

Why this matters for OpenShift on EC2

Controlling SMT at the instance level is valuable for several real-world scenarios:

  • Performance-sensitive workloads: Some HPC, ML training, and latency-critical applications perform better with SMT disabled (threadsPerCore: 1), as it eliminates thread contention on shared core resources. This is especially relevant for compute-optimized instance types like c5, c6i, and hpc6a.

  • Licensing optimization: Many commercial software licenses (e.g., Oracle, SQL Server) are priced per-vCPU. Disabling hyperthreading cuts the visible vCPU count in half, directly reducing licensing costs while retaining the same number of physical cores.

  • Security and isolation: Disabling SMT mitigates certain side-channel attacks (e.g., MDS, L1TF) that exploit shared resources between hardware threads. This is particularly relevant for clusters handling sensitive data or running in regulated environments.

  • Right-sizing capacity: Specifying coreCount allows launching an instance type with fewer active cores than the default, enabling cost savings when workloads don't need the full compute capacity of the chosen instance family.

AWS documentation references

Example usage

providerSpec:
  value:
    cpuOptions:
      coreCount: 4
      threadsPerCore: 1   # disables SMT (hyperthreading)

What changed

  • Vendored API types: Added coreCount (*int64, min 1) and threadsPerCore (*int64, enum 1 or 2) to the existing CPUOptions struct, plus deepcopy and swagger doc updates.
  • Provider logic: Extended getCPUOptionsRequest() in instances.go to pass the new fields through to ec2.CpuOptionsRequest.
  • Tests: Added 4 new test cases covering each field individually, both together, and all CPUOptions fields combined. All existing tests continue to pass.

Both fields are optional and default to nil (no opinion), so this is fully backward-compatible — existing Machine specs are unaffected.


Note

This is not a production-grade PR. It was co-built using Agentic Coding (with Claude Code) as a proof-of-concept to explore how AI-assisted development works for extending Kubernetes operators. The vendored API type changes would normally originate from the openshift/api repository first.

Test plan

  • go vet ./pkg/... ./cmd/... passes
  • go build ./cmd/... compiles
  • Full test suite passes with -race (go test ./pkg/... ./cmd/...)
  • New TestGetCPUOptionsRequest cases all pass
  • E2E validation on a live cluster with an SMT-capable instance type

Summary by CodeRabbit

Release Notes

  • New Features

    • Added support for configuring CPU CoreCount and ThreadsPerCore options in AWS machine provider settings, enabling fine-grained control over EC2 instance CPU allocation.
  • Tests

    • Expanded test coverage to validate CPU options configuration across multiple scenarios, including individual and combined option settings.

davivcgarcia and others added 2 commits March 4, 2026 11:23
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 836f6c74-a055-408f-9226-cf0cb2eecd48

📥 Commits

Reviewing files that changed from the base of the PR and between 0dd849f and d4bbe7c.

⛔ Files ignored due to path filters (3)
  • vendor/github.com/openshift/api/machine/v1beta1/types_awsprovider.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/machine/v1beta1/zz_generated.deepcopy.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/machine/v1beta1/zz_generated.swagger_doc_generated.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
📒 Files selected for processing (2)
  • pkg/actuators/machine/instances.go
  • pkg/actuators/machine/instances_test.go

Walkthrough

This change adds support for propagating CPU options (CoreCount and ThreadsPerCore) from AWSMachineProviderConfig into EC2 CpuOptionsRequest. Corresponding test cases are added to cover the new functionality.

Changes

Cohort / File(s) Summary
CPU Options Propagation
pkg/actuators/machine/instances.go
Adds logic to propagate CoreCount and ThreadsPerCore from AWSMachineProviderConfig.CPUOptions to EC2 CpuOptionsRequest fields when set.
CPU Options Tests
pkg/actuators/machine/instances_test.go
Expands TestGetCPUOptionsRequest with test cases for CoreCount, ThreadsPerCore, and combinations including ConfidentialCompute to verify proper translation to CpuOptionsRequest.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main feature being added: support for EC2 active vCPU options (CoreCount and ThreadsPerCore), which is the primary change across both modified files.
Stable And Deterministic Test Names ✅ Passed The newly added test cases in TestGetCPUOptionsRequest use static literal names and include no dynamic values, ensuring test titles remain stable and deterministic.
Test Structure And Quality ✅ Passed TestGetCPUOptionsRequest follows all quality requirements: single responsibility per test case, appropriate structure for unit testing, no timeouts needed, assertions follow established codebase conventions, and patterns are consistent with similar tests in the repository.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@openshift-ci openshift-ci bot requested review from damdo and nrb March 4, 2026 11:38
@openshift-ci openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 4, 2026
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 4, 2026

Hi @davivcgarcia. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 4, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign theobarberbany for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@davivcgarcia
Copy link
Author

I've found out that OpenShift currently offers the possibility to disable SMT, via MachineConfig or via PerformanceProfile. These mechanisms are based on changing Kernel arguments, so it disable SMT at operating system level. This PR and my proposal is about bringing the option to do the same at AWS hypervisor (Nitro) level.

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

Labels

needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant