We recently received this bug report: #132966. The issue seems to be
that the new range configured doesn't work for existing pods on the
node, and therefore the kubelet fails to start.
While I'll also improve the documentation to mention you need to drain
the node, let's also improve the error message saying which pod this
error comes from.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
This change completes the contextual logging migration for the memory
manager by updating the Manager interface and all implementations to
accept logr.Logger parameters instead of context.Context.
Key changes:
- Update Manager interface methods to accept logr.Logger:
* AddContainer, RemoveContainer, GetMemoryNUMANodes
* GetAllocatableMemory, GetMemory (removed context entirely)
- Update Policy interface methods to accept logr.Logger instead of context.Context
- Pass logger to NewManager() and policy constructors (NewPolicyStatic, NewPolicyNone, NewPolicyBestEffort)
- Update internal_container_lifecycle to use klog.TODO() when calling memory manager methods
- Update fake manager to accept and use logger parameter
- Update all test code to pass logger instead of context
This aligns with the contextual logging migration pattern where:
- Functions that need a logger accept logr.Logger parameter directly
- Logger is passed from the boundary (e.g., Start()) down to implementation
- klog.TODO() is used temporarily in call sites where logger is not yet available
- Context is only used where truly needed (e.g., Start() method)
This follows the same pattern as recent migrations in:
- pkg/kubelet/cm/topologymanager (#134174)
- pkg/kubelet/cm/devicemanager (#134293)
- pkg/kubelet/cm/cpumanager (#125912)
Related to the initial memory manager contextual logging work in #130727.
Signed-off-by: Swati Sehgal <swsehgal@redhat.com>
Updated allocateContainerResources to skip resources referring
extended resource claims as they are managed by DRA drivers.
These resources shouldn't be processed by device plugins.
migrate the cpumanager code to contextual logging
design musings:
- golang contexts "[...] carries deadlines, cancellation signals, and
other request-scoped values across API boundaries and between
processes." (see: https://pkg.go.dev/context#pkg-overview).
Thus, it generally makes sense to add contexts to most of the APIs
and functions as the resource management is done in the admission flow, when
ultimately we process a request and we have time bounds;
- however, receiving a context from the outside and depending on it
kind also conflicts with the goal of having uniform logging and "
- attach key/value pairs that get included in all log messages
- add names that describe which component or operation triggered a log messages"
(see:
https://github.com/kubernetes/enhancements/tree/master/keps/sig-instrumentation/3077-contextual-logging#summary)
Some of this conflict is due to the transitional nature of the PRs
which we do when updating the codelet codebase. We can envision a
final state on which the top level caller adds identifying information
like pod UID and pod namespace/name pair; but up until all the calling
chain is updated and wired, the results are gonna be inconsistent.
- continuing from the point above, the most realistic transition plan
for the kubelet is outlined in
https://github.com/kubernetes/kubernetes/issues/130069 .
Since we do want to move in parallel and migrate on a
subsystem-by-subsystem basis, each subsystem will be in charge to add
the data they want to their contexts. I think this is the only
reasonnable way forward, and the cost will be overhead and redundancy.
We should do sweeping changes to uniform the flows once we completed
the full transition to ensure uniformity.
- there are some well known packages which want to operate by design
only in in-memory data structure, which arguably can't block.
Shoehorning a context feels not great. Instead, we intentionally pass
only the logger, which still is the core idea of contextual logging
(see:
https://github.com/kubernetes/kubernetes/pull/125912#discussion_r1935919144)
examples are the topology subpackage and the cpu_assignment logic
(takeByTopology and friends)
Signed-off-by: Francesco Romani <fromani@redhat.com>
This replaces functions that wrap another function with no change with
a direct reference to the wrapped function.
Signed-off-by: Stephen Kitt <skitt@redhat.com>
Golang allows to call methods on a nil object, as long as the methods
don't dereference the nil object. This is what we do here.
This makes all the userns configurations (idsPerPod or mapping configs
in /etc/subuid or /etc/subgid) to be ignored if the feature is off.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
The first UID used for userns mappings needs to be at least the
idsPerPod. When idsPerPod is extended to be 65536*2, for example, then
the default UID of 65536 doesn't work.
While this can be configured by the user, let's just improve the default
first UID to be the same as idsPerPod. This makes the default first UID
work out of the box if the user just wants to tune that.
This also simplifies testing, as we don't need to create a system user
and /etc/subuid and /etc/subgid files to test the idsPerPod setting.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
The idsPerPod where completely ignored since they were introduced in PR:
https://github.com/kubernetes/kubernetes/pull/130028
The problem was the following:
1. The userns manager (as well as all managers) is created
before the config is copied to the kubelet object[1]
2. The userns manager on creation is calling the kubelet_getter
GetUserNamespacesIDsPerPod to get the idsPerPod
3. The getter checks the configuration stored in the kubelet
object, which hasn't been set at that point.
4. As the config is nil (unset), it returns the default value.
Therefore, the value was ignored.
To solve this, let's just pass the idsPerPod as a parameter to
MakeUserNsManager(). This is the common pattern already used in the
kubelet initialization.
[1]: 461ba83084/pkg/kubelet/kubelet.go (L1078-L1087)
[2]: 461ba83084/pkg/kubelet/kubelet_getters.go (L145)
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
