This is the first step to implement checkpointing and restoring of
container and containers starting from the lowest layer in the kubelet.
Signed-off-by: Adrian Reber <areber@redhat.com>
The utils found in pkg/kubelet/cri/remote/utils are the same as the
ones in pkg/kubelet/utils, with the difference that the latter have
had a few improvements recently.
This commit removes the duplicated code.
When adding functionality to the kubelet package and a test file, is
kind of painful to run unit tests today locally.
We usually can't run specifying the test file, as if xx_test.go and
xx.go use the same package, we need to specify all the dependencies. As
soon as xx.go uses the Kuebelet type (we need to do that to fake a
kubelet in the unit tests), this is completely impossible to do in
practice.
So the other option is to run the unit tests for the whole package or
run only a specific funtion. Running a single function can work in some
cases, but it is painful when we want to test all the functions we
wrote. On the other hand, running the test for the whole package is very
slow.
Today some unit tests try to connect to the API server (with retries)
create and list lot of pods/volumes, etc. This makes running the unit
test for the kubelet package slow.
This patch tries to make running the unit test for the whole package
more palatable. This patch adds a skip if the short version was
requested (go test -short ...), so we don't try to connect
to the API server or skip other slow tests.
Before this patch running the unit tests took in my computer (I've run
it several times so the compilation is already done):
$ time go test -v
real 0m21.303s
user 0m9.033s
sys 0m2.052s
With this patch it takes ~1/3 of the time:
$ time go test -short -v
real 0m7.825s
user 0m9.588s
sys 0m1.723s
Around 8 seconds is something I can wait to run the tests :)
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
The code as it stands now works, but it is still complicated and previous
versions had race
conditions (https://github.com/kubernetes/kubernetes/issues/108040). Now the
test works without modifying global state. The individual test cases could run
in parallel, this just isn't done because they complete quickly already (2
seconds).
It is useful to have the ability to control whether alpha or beta features are
enabled. We can group features under LoggingAlphaOptions and LoggingBetaOptions
because the configuration is designed so that each feature individually must be
enabled via its own option.
Currently, the JSON format itself is beta (graduated in 1.23) but additional
options for it were only added in 1.23 and thus are still alpha:
$ go run ./staging/src/k8s.io/component-base/logs/example/cmd/logger.go --logging-format=json --log-json-split-stream --log-json-info-buffer-size 1M --feature-gates LoggingBetaOptions=false
[format: Forbidden: Log format json is BETA and disabled, see LoggingBetaOptions feature, options.json.splitStream: Forbidden: Feature LoggingAlphaOptions is disabled, options.json.infoBufferSize: Forbidden: Feature LoggingAlphaOptions is disabled]
$ go run ./staging/src/k8s.io/component-base/logs/example/cmd/logger.go --logging-format=json --log-json-split-stream --log-json-info-buffer-size 1M
[options.json.splitStream: Forbidden: Feature LoggingAlphaOptions is disabled, options.json.infoBufferSize: Forbidden: Feature LoggingAlphaOptions is disabled]
This is the same approach that was taken for CPUManagerPolicyAlphaOptions and
CPUManagerPolicyBetaOptions.
In order to test this without modifying the global feature gate in a test file,
ValidateKubeletConfiguration must take a feature gate as argument.
Making the LoggingConfiguration part of the versioned component-base/config API
had the theoretic advantage that components could have offered different
configuration APIs with experimental features limited to alpha versions (for
example, sanitization offered only in a v1alpha1.KubeletConfiguration). Some
components could have decided to only use stable logging options.
In practice, this wasn't done. Furthermore, we don't want different components
to make different choices regarding which logging features they offer to
users. It should always be the same everywhere, for the sake of consistency.
This can be achieved with a saner Go API by dropping the distinction between
internal and external LoggingConfiguration types. Different stability levels of
indidividual fields have to be covered by documentation (done) and potentially
feature gates (not currently done).
Advantages:
- everything related to logging is under component-base/logs;
previously this was scattered across different packages and
different files under "logs" (why some code was in logs/config.go
vs. logs/options.go vs. logs/logs.go always confused me again
and again when coming back to the code):
- long-term config and command line API are clearly separated
into the "api" package underneath that
- logs/logs.go itself only deals with legacy global flags and
logging configuration
- removal of separate Go APIs like logs.BindLoggingFlags and
logs.Options
- LogRegistry becomes an implementation detail, with less code
and less exported functionality (only registration needs to
be exported, querying is internal)
Terminal pods may continue to report a ready condition of true because
there is a delay in reconciling the ready condition of the containers
from the runtime with the pod status. It should be invalid for kubelet
to report a terminal phase with a true ready condition. To fix the
issue, explicitly override the ready condition to false for terminal
pods during status updates.
Signed-off-by: David Porter <david@porter.me>
The pod worker is the owner of when a container is running or not,
and the start and stop of the probes for a given pod should be
handled during the pod sync loop. This ensures that probes do not
continue running even after eviction.
Because the pod semantics allow lifecycle probes to shorten grace
period, the probe is removed after the containers in a pod are
terminated successfully. As an optimization, if the pod will have
a very short grace period (0 or 1 seconds) we stop the probes
immediately to reduce resource usage during eviction slightly.
After this change, the probe manager is only called by the pod
worker or by the reconcile loop.
where pod sandbox won't have HostProcess bit set if pod does not have a
security context but containers specify HostProcess.
Signed-off-by: Mark Rossetti <marosset@microsoft.com>
This resolves a couple of issues for CSI volume reconstruction.
1. IsLikelyNotMountPoint is known not to work for bind mounts and was
causing problems for subpaths and hostpath volumes.
2. Inline volumes were failing reconstruction due to calling
GetVolumeName, which only works when there is a PV spec.
v1.43.0 marked grpc.WithInsecure() deprecated so this commit moves to use
what is the recommended replacement:
grpc.WithTransportCredentials(insecure.NewCredentials())
Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>
The means by which we extract and parse the version of an API object is
not specific to etcd3. In order to allow for a generic suite of tests
against any storage.Interface imlpementation, we need this logic to live
outside of the etcd3 package, or import cycles will exist.
Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
This is the first step towards being able to support a new plugin API version
in parallel with the existing one.
Signed-off-by: Kevin Klues <kklues@nvidia.com>
When parsing a resolv.conf file that has "search .", parseResolvConf should
accept the "." entry verbatim. Before this commit, parseResolvConf
unconditionally trimmed the "." suffix, which in the case of "." resulted
in a "" entry (that is, the empty string). This empty entry could lead
parseResolvConf to produce a resolv.conf file with "search ". Resolvers
could fail to parse such a resolv.conf file from parseResolvConf, thus
breaking DNS resolution in pods. After this commit, parseResolvConf
accepts a resolv.conf file with "search ." and passes the "." entry through
verbatim to produce a valid resolv.conf file. The "." suffix is still
trimmed for any entry that does not solely comprise ".".
Follow-up to commit a215a88d91.
* pkg/kubelet/network/dns/dns.go (parseResolvConf): Handle a "." entry in
the search path by copying it verbatim.
* pkg/kubelet/network/dns/dns_test.go (TestParseResolvConf): Add a test
case for "search .".
* Add FeatureGate PodHostIPs
* Add HostIPs field and update PodIPs field
* Types conversion
* Add dropDisabledStatusFields
* Add HostIPs for kubelet
* Add fuzzer for PodStatus
* Add status.hostIPs in ConvertDownwardAPIFieldLabel
* Add status.hostIPs in validEnvDownwardAPIFieldPathExpressions
* Downward API support for status.hostIPs
* Add DownwardAPI validation for status.hostIPs
* Add e2e to check that hostIPs works
* Add e2e to check that Downward API works
* Regenerate
The changes (mostly in pkg/kubelet/cm) are there to adopt changed
runc 1.1 API, and simplify things a bit. In particular:
1. simplify cgroup manager instantiation, using a new, easier way of
libcontainers/cgroups/manager.New;
2. replace libcontainerAdapter with a boolean variable (all it did
was passing on whether systemd manager should be used);
3. trivial change due to removed cgroupfs.HugePageSizes and added
cgroups.HugePageSizes();
4. do not calculate cgroup paths in update / destroy, since libcontainer
cgroup managers now calculate the paths upon creation (previously,
they were doing that only in Apply, so using e.g. Set or Destroy right
after creation was impossible without specifying paths).
We currently still calculate cgroup paths in Exists -- this is to be
addressed separately.
Co-Authored-By: Elana Hashman <ehashman@redhat.com>
Components that run in a container but modify the host network
namespace iptables rules need to know whether the system is using
iptables-legacy or iptables-nft. Given that kubelet will run before
any container-based components, it is well-positioned to help them
figure this out. So create a chain with a well-known name that they
can look for.
Some of these changes are cosmetic (repeatedly calling klog.V instead of
reusing the result), others address real issues:
- Logging a message only above a certain verbosity threshold without
recording that verbosity level (if klog.V().Enabled() { klog.Info... }):
this matters when using a logging backend which records the verbosity
level.
- Passing a format string with parameters to a logging function that
doesn't do string formatting.
All of these locations where found by the enhanced logcheck tool from
https://github.com/kubernetes/klog/pull/297.
In some cases it reports false positives, but those can be suppressed with
source code comments.
When using a legacy cloud provider, if kubelet is passed a node address
in --node-ip it will use this address in preference out the the
addresses by the cloud provider.
When using an external cloud provider, kubelet will annotate the Node
with the first --node-ip for use by the cloud provider. The cloud
provider validates this annotation but does not otherwise use it,
meaning that --node-ip has no effect.
This change moves the node address filtering code from kubelet to
component-helpers and updates both kubelet and cloud-provider to use it.
There is no functional change to kubelet, but cloud-provider now honours
kubelet's --node-ip.
Create an E2E test that creates a job that spawns a pod that should
succeed. The job reserves a fixed amount of CPU and has a large number
of completions and parallelism. Use to repro github.com/kubernetes/kubernetes/issues/106884
Signed-off-by: David Porter <david@porter.me>
Other components must know when the Kubelet has released critical
resources for terminal pods. Do not set the phase in the apiserver
to terminal until all containers are stopped and cannot restart.
As a consequence of this change, the Kubelet must explicitly transition
a terminal pod to the terminating state in the pod worker which is
handled by returning a new isTerminal boolean from syncPod.
Finally, if a pod with init containers hasn't been initialized yet,
don't default container statuses or not yet attempted init containers
to the unknown failure state.
Previously, callers of `Exists()` would not know why the cGroup was or
was not existing. In one call-site in particular, the `kubelet` would
entirely fail to start if the cGroup validation did not succeed. In
these cases we MUST explain what went wrong and pass that information
clearly to the caller. Previously, some but not all of the reasons for
invalidation were logged at a low log-level instead. This led to poor
UX.
The original method was retained on the interface so as to make this
diff small.
Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
Instead of doing (almost) the same thing from the three different
methods (Create, Update, Destroy), move the functionality to
libctCgroupConfig, replacing updateSystemdCgroupInfo.
The needResources bool is needed because we do not need resources
during Destroy, so we skip the unneeded resource conversion.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 79be8be10e made hugetlb settings optional if cgroup v2 is used and
hugetlb is not available, fixing issue 92933. Note at that time this was only
needed for v2, because for v1 the resources were set one-by-one, and only for
supported resources.
Commit d312ef7eb6 switched the code to using Set from runc/libcontainer
cgroups manager, and expanded the check to cgroup v1 as well.
Move this check earlier, to inside m.toResources, so instead of
converting all hugetlb resources from ResourceConfig to libcontainers's
Resources.HugetlbLimit, and then setting it to nil, we can skip the
conversion entirely if hugetlb is not supported, thus not doing the work
that is not needed.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit ecd6361f added setting PidsLimit to Create and Update.
Commit bce9d5f2 added setting PidsLimit to m.toResources.
Now, PidsLimit is assigned twice.
Remove the duplicate.
Fixes: bce9d5f2
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
There's no need to call m.Update (which will create another instance of
libcontainer cgroup manager, convert all the resources and then set
them). All this is already done here, except for Set().
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
For the 'single-numa' and 'restricted' TopologyManager policies, pods are only
admitted if all of their containers have perfect alignment across the set of
resources they are requesting. The best-effort policy, on the other hand, will
prefer allocations that have perfect alignment, but fall back to a non-preferred
alignment if perfect alignment can't be achieved.
The existing algorithm of how to choose the best hint from the set of
"non-preferred" hints is fairly naive and often results in choosing a
sub-optimal hint. It works fine in cases where all resources would end up
coming from a single NUMA node (even if its not the same NUMA nodes), but
breaks down as soon as multiple NUMA nodes are required for the "best"
alignment. We will never be able to achieve perfect alignment with these
non-preferred hints, but we should try and do something more intelligent than
simply choosing the hint with the narrowest mask.
In an ideal world, we would have the TopologyManager return a set of
"resources-relative" hints (as opposed to a common hint for all resources as is
done today). Each resource-relative hint would indicate how many other
resources could be aligned to it on a given NUMA node, and a hint provider
would use this information to allocate its resources in the most aligned way
possible. There are likely some edge cases to consider here, but such an
algorithm would allow us to do partial-perfect-alignment of "some" resources,
even if all resources could not be perfectly aligned.
Unfortunately, supporting something like this would require a major redesign to
how the TopologyManager interacts with its hint providers (as well as how those
hint providers make decisions based on the hints they get back).
That said, we can still do better than the naive algorithm we have today, and
this patch provides a mechanism to do so.
We start by looking at the set of hints passed into the TopologyManager for
each resource and generate a list of the minimum number of NUMA nodes required
to satisfy an allocation for a given resource. Each entry in this list then
contains the 'minNUMAAffinity.Count()' for a given resources. Once we have this
list, we find the *maximum* 'minNUMAAffinity.Count()' from the list and mark
that as the 'bestNonPreferredAffinityCount' that we would like to have
associated with whatever "bestHint" we ultimately generate. The intuition being
that we would like to (at the very least) get alignment for those resources
that *require* multiple NUMA nodes to satisfy their allocation. If we can't
quite get there, then we should try to come as close to it as possible.
Once we have this 'bestNonPreferredAffinityCount', the algorithm proceeds as
follows:
If the mergedHint and bestHint are both non-preferred, then try and find a hint
whose affinity count is as close to (but not higher than) the
bestNonPreferredAffinityCount as possible. To do this we need to consider the
following cases and react accordingly:
1. bestHint.NUMANodeAffinity.Count() > bestNonPreferredAffinityCount
2. bestHint.NUMANodeAffinity.Count() == bestNonPreferredAffinityCount
3. bestHint.NUMANodeAffinity.Count() < bestNonPreferredAffinityCount
For case (1), the current bestHint is larger than the
bestNonPreferredAffinityCount, so updating to any narrower mergeHint is
preferred over staying where we are.
For case (2), the current bestHint is equal to the
bestNonPreferredAffinityCount, so we would like to stick with what we have
*unless* the current mergedHint is also equal to bestNonPreferredAffinityCount
and it is narrower.
For case (3), the current bestHint is less than bestNonPreferredAffinityCount,
so we would like to creep back up to bestNonPreferredAffinityCount as close as
we can. There are three cases to consider here:
3a. mergedHint.NUMANodeAffinity.Count() > bestNonPreferredAffinityCount
3b. mergedHint.NUMANodeAffinity.Count() == bestNonPreferredAffinityCount
3c. mergedHint.NUMANodeAffinity.Count() < bestNonPreferredAffinityCount
For case (3a), we just want to stick with the current bestHint because choosing
a new hint that is greater than bestNonPreferredAffinityCount would be
counter-productive.
For case (3b), we want to immediately update bestHint to the current
mergedHint, making it now equal to bestNonPreferredAffinityCount.
For case (3c), we know that *both* the current bestHint and the current
mergedHint are less than bestNonPreferredAffinityCount, so we want to choose
one that brings us back up as close to bestNonPreferredAffinityCount as
possible. There are three cases to consider here:
3ca. mergedHint.NUMANodeAffinity.Count() > bestHint.NUMANodeAffinity.Count()
3cb. mergedHint.NUMANodeAffinity.Count() < bestHint.NUMANodeAffinity.Count()
3cc. mergedHint.NUMANodeAffinity.Count() == bestHint.NUMANodeAffinity.Count()
For case (3ca), we want to immediately update bestHint to mergedHint because
that will bring us closer to the (higher) value of
bestNonPreferredAffinityCount.
For case (3cb), we want to stick with the current bestHint because choosing the
current mergedHint would strictly move us further away from the
bestNonPreferredAffinityCount.
Finally, for case (3cc), we know that the current bestHint and the current
mergedHint are equal, so we simply choose the narrower of the 2.
This patch implements this algorithm for the case where we must choose from a
set of non-preferred hints and provides a set of unit-tests to verify its
correctness.
Signed-off-by: Kevin Klues <kklues@nvidia.com>
