OCPNODE-4601: Add {system,container}GomaxprocsBehavior field by haircommander · Pull Request #2934 · openshift/api

haircommander · 2026-07-14T17:21:07Z

according to openshift/enhancements#2047

openshift-merge-bot · 2026-07-14T17:21:10Z

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 · 2026-07-14T17:21:12Z

@haircommander: This pull request references OCPNODE-4601 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:

according to openshift/enhancements#2047

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 · 2026-07-14T17:21:19Z

Hello @haircommander! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

coderabbitai · 2026-07-14T17:21:33Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds the GomaxprocsInjection feature gate and preview availability. Extends kubelet and container runtime APIs with feature-gated GOMAXPROCS behavior fields and enum values. Updates CRD schemas, feature-gate manifests, documentation, and validation tests.

Suggested reviewers: cheesesashimi, everettraven

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
No-Weak-Crypto	❌ Error	FAIL: the diff adds `DES-CBC3-SHA` in `machineconfiguration/v1/zz_generated.featuregated-crd-manifests/kubeletconfigs.machineconfiguration.openshift.io/GomaxprocsInjection.yaml`, a 3DES cipher suite.	Remove `DES-CBC3-SHA` from the TLS profile/cipher lists in the source schema and regenerate the affected manifests.
Microshift Test Compatibility	⚠️ Warning	New Ginkgo API-validation suites exercise machineconfiguration.openshift.io/v1, which MicroShift doesn’t serve, and they lack any MicroShift skip/tag guard.	Add a [Skipped:MicroShift] label, an [apigroup:machineconfiguration.openshift.io] tag, or an exutil.IsMicroShiftCluster() g.Skip() guard.

✅ Passed checks (13 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: adding the system and container GomaxprocsBehavior fields.
Description check	✅ Passed	The description is brief but clearly points to the related enhancement PR driving these changes.
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	No changed Ginkgo titles contain dynamic data; the new test-case names are static, descriptive strings and the PR adds no _test.go titles at all.
Test Structure And Quality	✅ Passed	PASS: These are declarative CRD validation YAML tests, not Ginkgo; each case targets one behavior and matches existing onCreate/expectedError patterns.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the changes are CRD/feature-gate YAML and generated schema artifacts, so the SNO check is not applicable.
Topology-Aware Scheduling Compatibility	✅ Passed	PR only adds feature gates and CRD/API fields/tests; no deployment manifests, replicas, nodeSelectors, anti-affinity, or topology-aware scheduling logic changed.
Ote Binary Stdout Contract	✅ Passed	Changed Go files only add schema/types/docs; no main/init/TestMain/suite setup or stdout-writing calls were introduced.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No Ginkgo e2e tests were added; the new YAML CRD validation tests contain no IPv4 or external-network assumptions.
Container-Privileges	✅ Passed	No changed manifest or schema introduces privileged flags/capabilities; the PR only adds GomaxprocsBehavior CRD fields and schema tests.
No-Sensitive-Data-In-Logs	✅ Passed	The changed files add feature gates, CRD fields, and tests only; no new logging calls or sensitive data strings were introduced.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

haircommander · 2026-07-14T17:27:53Z

PTAL @yuqi-zhang

coderabbitai

Actionable comments posted: 1

🤖 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 `@machineconfiguration/v1/types.go`:
- Around line 778-779: Update the GOMAXPROCS documentation comments near the
Disabled setting, including the corresponding description around the other
referenced lines, so they no longer claim the default always equals the node’s
CPU count; remove the parenthetical or qualify it for the supported Go 1.25.0
toolchain. Regenerate the CRD descriptions after updating the source comments.

🪄 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: ad9d8da8-3047-42a4-a9c7-0ade39a5304c

📥 Commits

Reviewing files that changed from the base of the PR and between 8bc26b0 and 7a7164b.

⛔ Files ignored due to path filters (10)

machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_containerruntimeconfigs-CustomNoUpgrade.crd.yaml is excluded by !**/zz_generated.crd-manifests/*
machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_containerruntimeconfigs-DevPreviewNoUpgrade.crd.yaml is excluded by !**/zz_generated.crd-manifests/*
machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_containerruntimeconfigs-TechPreviewNoUpgrade.crd.yaml is excluded by !**/zz_generated.crd-manifests/*
machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_kubeletconfigs-CustomNoUpgrade.crd.yaml is excluded by !**/zz_generated.crd-manifests/*
machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_kubeletconfigs-DevPreviewNoUpgrade.crd.yaml is excluded by !**/zz_generated.crd-manifests/*
machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_kubeletconfigs-TechPreviewNoUpgrade.crd.yaml is excluded by !**/zz_generated.crd-manifests/*
machineconfiguration/v1/zz_generated.featuregated-crd-manifests.yaml is excluded by !**/zz_generated*
machineconfiguration/v1/zz_generated.featuregated-crd-manifests/containerruntimeconfigs.machineconfiguration.openshift.io/GomaxprocsInjection.yaml is excluded by !**/zz_generated.featuregated-crd-manifests/**
machineconfiguration/v1/zz_generated.featuregated-crd-manifests/kubeletconfigs.machineconfiguration.openshift.io/GomaxprocsInjection.yaml is excluded by !**/zz_generated.featuregated-crd-manifests/**
machineconfiguration/v1/zz_generated.swagger_doc_generated.go is excluded by !**/zz_generated*

📒 Files selected for processing (17)

features.md
features/features.go
machineconfiguration/v1/types.go
payload-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-CustomNoUpgrade.crd.yaml
payload-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-DevPreviewNoUpgrade.crd.yaml
payload-manifests/crds/0000_80_machine-config_01_containerruntimeconfigs-TechPreviewNoUpgrade.crd.yaml
payload-manifests/crds/0000_80_machine-config_01_kubeletconfigs-CustomNoUpgrade.crd.yaml
payload-manifests/crds/0000_80_machine-config_01_kubeletconfigs-DevPreviewNoUpgrade.crd.yaml
payload-manifests/crds/0000_80_machine-config_01_kubeletconfigs-TechPreviewNoUpgrade.crd.yaml
payload-manifests/featuregates/featureGate-4-10-Hypershift-Default.yaml
payload-manifests/featuregates/featureGate-4-10-Hypershift-DevPreviewNoUpgrade.yaml
payload-manifests/featuregates/featureGate-4-10-Hypershift-OKD.yaml
payload-manifests/featuregates/featureGate-4-10-Hypershift-TechPreviewNoUpgrade.yaml
payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-Default.yaml
payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-DevPreviewNoUpgrade.yaml
payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-OKD.yaml
payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-TechPreviewNoUpgrade.yaml

yuqi-zhang

Generally lgtm based on the enhancement - main issue is that this is lacking tests

yuqi-zhang · 2026-07-17T21:30:14Z

 					mustRegister()
+
+	FeatureGateGomaxprocsInjection = newFeatureGate("GomaxprocsInjection").
+					reportProblemsToJiraComponent("machine-config-operator").


Should we use node as the primary component here?

yuqi-zhang · 2026-07-17T21:31:58Z

Let's also add some tests to https://github.com/openshift/api/tree/2cee386cfdcf4c00938f0683cc4555d5120944a8/machineconfiguration/v1/tests

yuqi-zhang · 2026-07-17T21:33:15Z

 )

+// ContainerGomaxprocsBehaviorType specifies whether CRI-O should inject GOMAXPROCS into containers
+type ContainerGomaxprocsBehaviorType string


We can probably merge Container and System behaviour types into one, unless you foresee them branching in the future?

I don't think you necessarily need to define the default as a const as well - the general pattern is that we document it in the godocs, and then have the implementing controller do the necessary default implementation.

haircommander · 2026-07-27T18:38:19Z

/retest

haircommander · 2026-07-27T18:38:44Z

any additional thoughts @yuqi-zhang ?

yuqi-zhang

changes lgtm, thanks, marking as shadow review complete

openshift-ci · 2026-07-27T19:52:14Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yuqi-zhang
Once this PR has been reviewed and has the lgtm label, please assign everettraven 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:

OWNERS

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

everettraven

One minor comment and a question.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

openshift-ci · 2026-07-28T16:40:11Z

@haircommander: The following test 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/verify-hypershift-integration	`dce4c29`	link	true	`/test verify-hypershift-integration`

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.

haircommander · 2026-07-28T17:28:24Z

any additional thoughts @everettraven

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

openshift-ci Bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 14, 2026

openshift-ci Bot requested review from cheesesashimi and everettraven July 14, 2026 17:23

coderabbitai Bot reviewed Jul 14, 2026

View reviewed changes

Comment thread machineconfiguration/v1/types.go Outdated

yuqi-zhang reviewed Jul 17, 2026

View reviewed changes

haircommander force-pushed the gomaxprocs branch from 2cee386 to 1246adf Compare July 22, 2026 20:31

openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2026

haircommander force-pushed the gomaxprocs branch from 1246adf to fd3eecb Compare July 22, 2026 20:37

openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2026

haircommander force-pushed the gomaxprocs branch from fd3eecb to 99b07a0 Compare July 27, 2026 19:38

yuqi-zhang approved these changes Jul 27, 2026

View reviewed changes

everettraven reviewed Jul 28, 2026

View reviewed changes

Comment thread machineconfiguration/v1/types.go Outdated

Comment thread machineconfiguration/v1/types.go

haircommander and others added 2 commits July 28, 2026 11:45

Add container-level GOMAXPROCS injection support

4d57999

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Add system service-level GOMAXPROCS configuration support

dce4c29

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

haircommander force-pushed the gomaxprocs branch from 99b07a0 to dce4c29 Compare July 28, 2026 15:51

Uh oh!

Conversation

haircommander commented Jul 14, 2026

Uh oh!

openshift-merge-bot Bot commented Jul 14, 2026

Uh oh!

openshift-ci-robot commented Jul 14, 2026 • edited by openshift-ci Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci Bot commented Jul 14, 2026

Uh oh!

coderabbitai Bot commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Pre-merge checks failed

❌ Failed checks (1 error, 1 warning)

Uh oh!

haircommander commented Jul 14, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yuqi-zhang left a comment

Choose a reason for hiding this comment

Uh oh!

yuqi-zhang Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

haircommander Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

yuqi-zhang Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

yuqi-zhang Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

haircommander Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

haircommander commented Jul 27, 2026

Uh oh!

haircommander commented Jul 27, 2026

Uh oh!

yuqi-zhang left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented Jul 27, 2026

Uh oh!

everettraven left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

openshift-ci Bot commented Jul 28, 2026

Uh oh!

haircommander commented Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

openshift-ci-robot commented Jul 14, 2026 •

edited by openshift-ci Bot

Loading

coderabbitai Bot commented Jul 14, 2026 •

edited

Loading