Skip to content

AUTOSCALE-812: Add NodeSelectorAdjuster admission plugin for standalone clusters#2695

Open
joelsmith wants to merge 1 commit into
openshift:masterfrom
joelsmith:node-selector-adjuster
Open

AUTOSCALE-812: Add NodeSelectorAdjuster admission plugin for standalone clusters#2695
joelsmith wants to merge 1 commit into
openshift:masterfrom
joelsmith:node-selector-adjuster

Conversation

@joelsmith

@joelsmith joelsmith commented Jun 17, 2026

Copy link
Copy Markdown

Add a new kube-apiserver admission plugin that adds the node-role.kubernetes.io/control-plane node selector to qualifying control-plane-adjacent Day 2 operator pods at admission time.

On standalone OpenShift clusters, operators like the VPA operator must run on master nodes for security reasons (e.g., limiting blast radius of privilege escalation from a worker-node compromise). HCP clusters have a different security posture (using the cluster rather than the node as a security boundary) and no master nodes exist in the guest cluster, so the plugin must not activate. The plugin detects standalone clusters by checking POD_NAMESPACE=openshift-kube-apiserver at start-up.

Currently the VPA operator pod (identified by the label k8s-app=vertical-pod-autoscaler-operator) is the only registered workload. The design is extensible to other control-plane-adjacent operators by adding label matchers to requiresNodeSelectorAdjustment().

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

Summary by CodeRabbit

Release Notes

  • New Features

    • Added automatic pod scheduling adjustment for control-plane node placement in standalone deployments.
  • Tests

    • Added comprehensive unit tests for pod scheduling mutation and environment detection logic.

…dalone clusters

Add a new kube-apiserver admission plugin that adds the
node-role.kubernetes.io/control-plane node selector to qualifying
control-plane-adjacent Day 2 operator pods at admission time.

On standalone OpenShift clusters, operators like the VPA operator must
run on master nodes for security reasons (e.g., limiting blast radius of
privilege escalation from a worker-node compromise). HCP clusters have
a different security posture (using the cluster rather than the node
as a security boundary) and no master nodes exist in the guest cluster,
so the plugin must not activate. The plugin detects standalone clusters
by checking POD_NAMESPACE=openshift-kube-apiserver at start-up.

Currently the VPA operator pod (identified by the label
k8s-app=vertical-pod-autoscaler-operator) is the only registered
workload. The design is extensible to other control-plane-adjacent
operators by adding label matchers to requiresNodeSelectorAdjustment().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-ci-robot openshift-ci-robot added the backports/unvalidated-commits Indicates that not all commits come to merged upstream PRs. label Jun 17, 2026
@openshift-ci-robot

openshift-ci-robot commented Jun 17, 2026

Copy link
Copy Markdown

@joelsmith: This pull request references AUTOSCALE-812 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:

Add a new kube-apiserver admission plugin that adds the node-role.kubernetes.io/control-plane node selector to qualifying control-plane-adjacent Day 2 operator pods at admission time.

On standalone OpenShift clusters, operators like the VPA operator must run on master nodes for security reasons (e.g., limiting blast radius of privilege escalation from a worker-node compromise). HCP clusters have a different security posture (using the cluster rather than the node as a security boundary) and no master nodes exist in the guest cluster, so the plugin must not activate. The plugin detects standalone clusters by checking POD_NAMESPACE=openshift-kube-apiserver at start-up.

Currently the VPA operator pod (identified by the label k8s-app=vertical-pod-autoscaler-operator) is the only registered workload. The design is extensible to other control-plane-adjacent operators by adding label matchers to requiresNodeSelectorAdjustment().

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

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 Jun 17, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@joelsmith: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

@openshift-ci openshift-ci Bot requested review from benluddy and p0lyn0mial June 17, 2026 05:21
@openshift-ci

openshift-ci Bot commented Jun 17, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

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: Enterprise

Run ID: d0a5185e-62c9-42ac-8d38-aff2f8755cd8

📥 Commits

Reviewing files that changed from the base of the PR and between d8d517e and 410f582.

📒 Files selected for processing (3)
  • openshift-kube-apiserver/admission/admissionenablement/register.go
  • openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission.go
  • openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission_test.go

Walkthrough

A new NodeSelectorAdjuster admission mutation plugin is added under openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/. It injects a node-role.kubernetes.io/control-plane node selector into VPA operator pods (k8s-app=vertical-pod-autoscaler-operator in namespace openshift-vertical-pod-autoscaler) at pod creation time. The plugin is gated by IsStandalone() (checks POD_NAMESPACE == openshift-kube-apiserver) and is conditionally registered in RegisterOpenshiftKubeAdmissionPlugins and the openshiftAdmissionPluginsForKubeBeforeMutating list.

Changes

NodeSelectorAdjuster admission plugin

Layer / File(s) Summary
Plugin implementation and admission chain wiring
openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission.go, openshift-kube-apiserver/admission/admissionenablement/register.go
Defines PluginName, IsStandalone(), Register(), Admit(), requiresNodeSelectorAdjustment(), and addControlPlaneNodeSelector(); conditionally registers the plugin and appends it to openshiftAdmissionPluginsForKubeBeforeMutating when IsStandalone() is true.
Unit tests
openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission_test.go
Table-driven tests cover Admit mutation for qualifying and non-qualifying pods (correct label/namespace, wrong namespace, wrong value, pre-existing selector, non-pod resource, subresource), requiresNodeSelectorAdjustment predicate, IsStandalone env var detection, and pod/user test helpers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: introducing a new NodeSelectorAdjuster admission plugin specifically for standalone clusters.
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 The PR adds standard Go tests with table-driven test cases using static, descriptive names. Ginkgo tests are not used in this PR, so the Ginkgo-specific check is not applicable.
Test Structure And Quality ✅ Passed The test file uses standard Go testing (testing.T), not Ginkgo. All quality requirements are met: (1) Single responsibility - each table-driven test case tests one scenario via t.Run(); (2) Setup/c...
Microshift Test Compatibility ✅ Passed No Ginkgo e2e tests were added. The new test file (admission_test.go) uses standard Go testing.T with table-driven unit tests, not Ginkgo patterns. Check is not applicable.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No Ginkgo e2e tests added. PR only includes standard Go unit tests (TestAdmit, TestRequiresNodeSelectorAdjustment, TestIsStandalone) with no Ginkgo patterns (Describe/Context/When/It).
Topology-Aware Scheduling Compatibility ✅ Passed Plugin adds control-plane nodeSelector only on standalone clusters (gated by POD_NAMESPACE=openshift-kube-apiserver), which is topology-safe. On HyperShift, plugin does not activate. On all standal...
Ote Binary Stdout Contract ✅ Passed This PR adds a kube-apiserver admission plugin, not an OTE test extension. No stdout writes detected at process level in any of the three changed files (admission.go, admission_test.go, register.go...
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR adds only standard Go unit tests (admission_test.go with testing.T), not Ginkgo e2e tests. Custom check specifically applies to Ginkgo tests with It(), Describe(), Context(), When() patterns.
No-Weak-Crypto ✅ Passed No weak cryptography (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), custom crypto implementations, or non-constant-time secret comparisons found in the PR code.
Container-Privileges ✅ Passed PR contains only Go source code implementing an admission controller plugin, not Kubernetes manifests. No container privileges, privileged mode, host PID/network/IPC, SYS_ADMIN capability, or allow...
No-Sensitive-Data-In-Logs ✅ Passed No logging that exposes passwords, tokens, API keys, PII, session IDs, internal hostnames, or customer data found. The code uses %T format in error message (type only, not value) and only accesses...

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

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

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.

@joelsmith

Copy link
Copy Markdown
Author

@rphillips could you please take a look at this?

@openshift-ci-robot

openshift-ci-robot commented Jun 17, 2026

Copy link
Copy Markdown

@joelsmith: This pull request references AUTOSCALE-812 which is a valid jira issue.

Details

In response to this:

Add a new kube-apiserver admission plugin that adds the node-role.kubernetes.io/control-plane node selector to qualifying control-plane-adjacent Day 2 operator pods at admission time.

On standalone OpenShift clusters, operators like the VPA operator must run on master nodes for security reasons (e.g., limiting blast radius of privilege escalation from a worker-node compromise). HCP clusters have a different security posture (using the cluster rather than the node as a security boundary) and no master nodes exist in the guest cluster, so the plugin must not activate. The plugin detects standalone clusters by checking POD_NAMESPACE=openshift-kube-apiserver at start-up.

Currently the VPA operator pod (identified by the label k8s-app=vertical-pod-autoscaler-operator) is the only registered workload. The design is extensible to other control-plane-adjacent operators by adding label matchers to requiresNodeSelectorAdjustment().

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

Summary by CodeRabbit

Release Notes

  • New Features

  • Added automatic pod scheduling adjustment for control-plane node placement in standalone deployments.

  • Tests

  • Added comprehensive unit tests for pod scheduling mutation and environment detection logic.

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 commented Jun 17, 2026

Copy link
Copy Markdown

@joelsmith: 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-aws-ovn-serial-1of2 410f582 link true /test e2e-aws-ovn-serial-1of2
ci/prow/e2e-aws-ovn-runc 410f582 link false /test e2e-aws-ovn-runc
ci/prow/e2e-aws-ovn-fips 410f582 link true /test e2e-aws-ovn-fips
ci/prow/e2e-aws-ovn-techpreview 410f582 link false /test e2e-aws-ovn-techpreview
ci/prow/e2e-metal-ipi-ovn-ipv6 410f582 link true /test e2e-metal-ipi-ovn-ipv6
ci/prow/e2e-aws-ovn-crun 410f582 link true /test e2e-aws-ovn-crun
ci/prow/k8s-e2e-gcp-serial 410f582 link true /test k8s-e2e-gcp-serial
ci/prow/e2e-gcp 410f582 link true /test e2e-gcp

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.

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

Labels

backports/unvalidated-commits Indicates that not all commits come to merged upstream PRs. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants