refactor(kubelet/cpumanager): move fake manager to fakemanager subpackage and returning concrete type #135220

priyansh3006 · 2025-11-07T11:17:15Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This PR moves the fake CPU manager implementation to a dedicated fakemanager subpackage,
following the client-go pattern for better code organization and separation of concerns.
Returns a concrete type instead of an interface for better testability

Key changes:

Create pkg/kubelet/cm/cpumanager/fakemanager/ subpackage
Move FakeManager implementation to fakemanager/fake_manager.go
Export RuntimeService interface (was runtimeService) for subpackage usage
Update NewFakeManager() to return concrete type (*FakeManager) instead of Manager interface

Which issue(s) this PR is related to:

Related to #135110

Special notes for your reviewer:
NONE

Does this PR introduce a user-facing change?
NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
No additional documentation needed.

k8s-ci-robot · 2025-11-07T11:17:25Z

Hi @priyansh3006. Thanks for your PR.

I'm waiting for a github.com 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.

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.

SergeyKanzhelev · 2025-11-07T18:10:38Z

/release-note-none
/ok-to-test
/priority backlog

SergeyKanzhelev · 2025-11-07T18:11:53Z

Fixes #135110

This line in description will close the whole bug. I do not think it is accurate. Please change it to "Related to #135110".

/assign @ffromani

priyansh3006 · 2025-11-08T09:58:52Z

/retest

priyansh3006 · 2025-11-08T10:42:22Z

@SergeyKanzhelev @ffromani
About E2E Test Failures :
The RestartAllContainersOnContainerExits feature gate failures in e2e_node tests are pre-existing issues unrelated to this refactoring. These tests fail during initialization before cpumanager even runs. Local testing confirms all cpumanager
tests pass and the refactoring is working correctly.
Is there anything I can do to help resolve the e2e test issue or assist with the review of this PR?

ffromani · 2025-11-09T09:40:37Z

test failures seems unrelated indeed. I'll have a look at the PR ASAP. It's unlikely to make the 1.35 cutoff, but that's fine.

ffromani

thanks for your submission! let's rearrange the commits to make sure each commit is atomic and complete (e.g. not API changes in a commit and fix of the calling sites into another) and let's make sure to record the rationale within the commit message. I can add some suggestions later on

ffromani · 2025-11-10T09:24:58Z

pkg/kubelet/cm/cpumanager/fakemanager/fake_cpu_manager.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2017 The Kubernetes Authors.
+Copyright 2025 The Kubernetes Authors.


no need to bump the copyright year especially for trivial changes

ffromani · 2025-11-10T09:25:44Z

pkg/kubelet/cm/cpumanager/fakemanager/fake_cpu_manager.go

 */

-package cpumanager
+package fakemanager


I'd rename this fake and let the importers do the rename, e.g.

import ( fakecpumanager "k8s.io/kubernetes/pkg/kubelet/cm/cpumanager/fake" )

ffromani · 2025-11-10T09:52:01Z

pkg/kubelet/cm/cpumanager/fake/fake_cpu_manager.go

+type (
+	Manager        = cpumanager.Manager
+	Policy         = cpumanager.Policy
+	ActivePodsFunc = cpumanager.ActivePodsFunc
+	RuntimeService = cpumanager.RuntimeService
+)


aliases are likely a good idea 👍

ffromani · 2025-11-10T09:52:19Z

pkg/kubelet/cm/cpumanager/fakemanager/fake_cpu_manager.go

+	RuntimeService = cpumanager.RuntimeService
+)
+
+type FakeManager struct {


if we rename the package fake, this can just be Manager

ffromani · 2025-11-10T09:52:41Z

pkg/kubelet/cm/cpumanager/fakemanager/fake_cpu_manager.go

-func (m *fakeManager) Start(ctx context.Context, activePods ActivePodsFunc, sourcesReady config.SourcesReady, podStatusProvider status.PodStatusProvider, containerRuntime runtimeService, initialContainers containermap.ContainerMap) error {
-	logger := klog.FromContext(ctx)
-	logger.Info("Start()")
+var _ Manager = &FakeManager{}


this should be private to avoid confusion about exported names

Renamed the struct from FakeManager to manager to make it private

ffromani · 2025-11-10T09:53:29Z

pkg/kubelet/cm/cpumanager/fakemanager/fake_cpu_manager.go

+func NewFakeManager(logger logr.Logger) *FakeManager {
 	logger = klog.LoggerWithName(logger, "cpu.fake")
-	return &fakeManager{
+	return &FakeManager{
 		logger: logger,
 		state:  state.NewMemoryState(logger),
 	}


this change should sit in its own separate commit

ffromani · 2025-11-10T09:54:11Z

pkg/kubelet/cm/cpumanager/cpu_manager.go

+type RuntimeService interface {
 	UpdateContainerResources(ctx context.Context, id string, resources *runtimeapi.ContainerResources) error


this is a bit unexpected but indeed necessary and should be harmless

ffromani · 2025-11-10T09:55:57Z

/triage accepted

k8s-ci-robot · 2025-11-10T14:57:02Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

pkg/kubelet/cm/OWNERS

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

priyansh3006 · 2025-11-10T16:40:43Z

@ffromani Thanks for looking into this and I Appreciate the quick response
I've reorganized the commits into 3 complete commits as requested:

Commit 1: Return concrete type from NewFakeManager

Changed return type to *fakeManager for better testability

Commit 2: Move fake manager to dedicated subpackage

Created fake/ subpackage following client-go pattern
Renamed struct to manager (private) and made interface check explicit

Commit 3: Update imports for fake subpackage

Updated all calling sites to use new subpackage
Changed imports to fakecpumanager "...cpumanager/fake"

k8s-ci-robot · 2025-11-10T17:03:20Z

@priyansh3006: 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
pull-kubernetes-node-e2e-restartallcontainers	`bce5d6c`	link	false	`/test pull-kubernetes-node-e2e-restartallcontainers`
pull-kubernetes-unit-windows-master	`3d6c4e2`	link	false	`/test pull-kubernetes-unit-windows-master`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Nov 7, 2025

k8s-ci-robot requested review from klueska and mrunalp November 7, 2025 11:17

k8s-ci-robot added area/kubelet sig/node Categorizes an issue or PR as relevant to SIG Node. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Nov 7, 2025

github-project-automation bot added this to SIG Node: code and documentation PRs Nov 7, 2025

github-project-automation bot moved this to Triage in SIG Node: code and documentation PRs Nov 7, 2025

k8s-ci-robot assigned ffromani Nov 7, 2025

ffromani reviewed Nov 10, 2025

View reviewed changes

priyansh3006 added 3 commits November 10, 2025 21:57

move fake manager to dedicated subpackage

fcccfec

cpumanager fakeManager return a concrete type

d321298

update imports for fake cpumanager subpackage

3d6c4e2

priyansh3006 force-pushed the cpu_fakemanager branch from 774739d to 3d6c4e2 Compare November 10, 2025 16:31

		type RuntimeService interface {
		UpdateContainerResources(ctx context.Context, id string, resources *runtimeapi.ContainerResources) error

refactor(kubelet/cpumanager): move fake manager to fakemanager subpackage and returning concrete type #135220

Are you sure you want to change the base?

refactor(kubelet/cpumanager): move fake manager to fakemanager subpackage and returning concrete type #135220

Conversation

priyansh3006 commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Nov 7, 2025

Uh oh!

SergeyKanzhelev commented Nov 7, 2025

Uh oh!

SergeyKanzhelev commented Nov 7, 2025

Uh oh!

priyansh3006 commented Nov 8, 2025

Uh oh!

priyansh3006 commented Nov 8, 2025

Uh oh!

ffromani commented Nov 9, 2025

Uh oh!

ffromani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ffromani commented Nov 10, 2025

Uh oh!

k8s-ci-robot commented Nov 10, 2025

Uh oh!

priyansh3006 commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Nov 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

priyansh3006 commented Nov 7, 2025 •

edited

Loading

priyansh3006 commented Nov 10, 2025 •

edited

Loading