Automatic merge from submit-queue (batch tested with PRs 52442, 52247, 46542, 52363, 51781)
Make CPU manager release CPUs when Pod enters completed phase.
**What this PR does / why we need it**: When CPU manager is enabled, this PR releases allocated CPUs when container is not running and is non-restartable.
**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes#52351
**Special notes for your reviewer**:
This bug is only reproduced for pods with `restartPolicy` = `Never` or `OnFailure`. The following output is from a 4 CPU node. This bug can be reproduced as long >= half the cores are requested.
pod1.yaml:
```
apiVersion: v1
kind: Pod
metadata:
name: test-pod1
spec:
containers:
- image: ubuntu
command: ["/bin/bash"]
args: ["-c", "sleep 5"]
name: test-container1
resources:
requests:
cpu: 2
memory: 100Mi
limits:
cpu: 2
memory: 100Mi
restartPolicy: "Never"
```
pod2.yaml:
```
apiVersion: v1
kind: Pod
metadata:
name: test-pod2
spec:
containers:
- image: ubuntu
command: ["/bin/bash"]
args: ["-c", "sleep 5"]
name: test-container1
resources:
requests:
cpu: 2
memory: 100Mi
limits:
cpu: 2
memory: 100Mi
restartPolicy: "Never"
```
Run a local Kubernetes cluster with CPU manager enabled.
```sh
KUBELET_FLAGS='--feature-gates=CPUManager=true --cpu-manager-policy=static --cpu-manager-reconcile-period=1s --kube-reserved=cpu=500m' ./hack/local-up-cluster.sh
```
_Before:_
Create `test-pod1` using pod1.yaml.
```
./cluster/kubectl.sh create -f pod1.yaml
```
Wait for the pod to complete and wait another 90 seconds (give enough time for GC to kick-in).
Create `test-pod2` using pod2.yaml.
```
./cluster/kubectl.sh create -f pod2.yaml
```
Get all pods in the cluster.
```
./cluster/kubectl.sh get pods -a
NAME READY STATUS RESTARTS AGE
test-pod1 0/1 Completed 0 1m
test-pod2 0/1 not enough cpus available to satisfy request 0 9s
```
_After:_
Create `test-pod1` using pod1.yaml.
```
./cluster/kubectl.sh create -f pod1.yaml
```
Wait for the pod to complete and wait another 90 seconds (give enough time for GC to kick-in).
Create `test-pod2` using pod2.yaml.
```
./cluster/kubectl.sh create -f pod2.yaml
```
Get all pods in the cluster.
```
./cluster/kubectl.sh get pods -a
NAME READY STATUS RESTARTS AGE
test-pod1 0/1 Completed 0 1m
test-pod2 0/1 Completed 0 9s
```
Automatic merge from submit-queue (batch tested with PRs 51186, 50350, 51751, 51645, 51837)
Wait for container cleanup before deletion
We should wait to delete pod API objects until the pod's containers have been cleaned up. See issue: #50268 for background.
This changes the kubelet container gc, which deletes containers belonging to pods considered "deleted".
It adds two conditions under which a pod is considered "deleted", allowing containers to be deleted:
Pods where deletionTimestamp is set, and containers are not running
Pods that are evicted
This PR also changes the function PodResourcesAreReclaimed by making it return false if containers still exist.
The eviction manager will wait for containers of previous evicted pod to be deleted before evicting another pod.
The status manager will wait for containers to be deleted before removing the pod API object.
/assign @vishh
Automatic merge from submit-queue (batch tested with PRs 45724, 48051, 46444, 51056, 51605)
Mount propagation in kubelet
Together with #45724 it implements mount propagation as proposed in https://github.com/kubernetes/community/pull/659
There is:
- New alpha annotation that allows user to explicitly set propagation mode for each `VolumeMount` in pod containers (to be replaced with real `VolumeMount.Propagation` field during beta) + validation + tests. "Private" is the default one (= no change to existing pods).
I know about proposal for real API fields for alpha feature in https://docs.google.com/document/d/1wuoSqHkeT51mQQ7dIFhUKrdi3-1wbKrNWeIL4cKb9zU/edit, but it seems it's not implemented yet. It would save me quite lot of code and ugly annotation.
- Updated CRI API to transport chosen propagation to Docker.
- New `kubelet --experimental-mount-propagation` option to enable the previous bullet without modifying types.go (worked around with changing `KubeletDeps`... not nice, but it's better than adding a parameter to `NewMainKubelet` and removing it in the next release...)
```release-note
kubelet has alpha support for mount propagation. It is disabled by default and it is there for testing only. This feature may be redesigned or even removed in a future release.
```
@derekwaynecarr @dchen1107 @kubernetes/sig-node-pr-reviews
Automatic merge from submit-queue (batch tested with PRs 50381, 51307, 49645, 50995, 51523)
Bugfix: Use local JSON log buffer in parseDockerJSONLog.
**What this PR does / why we need it**:
The issue described in #47800 is due to a race condition in `ReadLogs`: Because the JSON log buffer (`dockerJSONLog`) is package-scoped, any two goroutines modifying the buffer could race and overwrite the other's changes. In particular, one goroutine could unmarshal a JSON log line into the buffer, then another goroutine could `Reset()` the buffer, and the resulting `Stream` would be empty (`""`). This empty `Stream` is caught in a `case` block and raises an `unexpected stream type` error.
This PR creates a new buffer for each execution of `parseDockerJSONLog`, so each goroutine is guaranteed to have a local instance of the buffer.
**Which issue this PR fixes**: fixes#47800
**Release note**:
```release-note
Fixed an issue (#47800) where `kubectl logs -f` failed with `unexpected stream type ""`.
```
Automatic merge from submit-queue
When faild create pod sandbox record event.
I created pods because of the failure to create a sandbox, but there was no clear message telling me what was the failure, so I wanted to record an event when the sandbox was created.
**Release note**:
```release-note
NONE
```
Automatic merge from submit-queue (batch tested with PRs 51054, 51101, 50031, 51296, 51173)
Refactor kuberuntime test case with sets.String
**What this PR does / why we need it**:
change to make got and want use sets.String instead, since that is both safe and more clearly shows the intent.
ref: https://github.com/kubernetes/kubernetes/pull/50554
**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes https://github.com/kubernetes/kubernetes/issues/51396
**Special notes for your reviewer**:
**Release note**:
```release-note
NONE
```
Automatic merge from submit-queue (batch tested with PRs 50536, 50809, 50220, 50399, 50176)
Set ExecSync timeout in liveness prober.
Although Dockershim doesn't actually support `ExecSync` timeout (see [here](https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/dockershim/exec.go#L137)), we should set the timeout, so that the other runtime which supports the timeout could work properly.
Fixes#50389.
/cc @yujuhong @timstclair @feiskyer
Whenever pod sandbox needs to be recreated, all containers associated
with it will be killed by kubelet. This change ensures that the init
containers will be rerun in such cases.
The change also refactors the compute logic so that the control flow of
init containers act is more aligned with the regular containers. Unit
tests are added to verify the logic.
Automatic merge from submit-queue (batch tested with PRs 49488, 50407, 46105, 50456, 50258)
Add UpdateContainerResources method to CRI
This is first step toward support for opinionated cpu pinning for certain guaranteed pods.
In order to do this, the kubelet needs to be able to dynamically update the cpuset at the container level, which is managed by the container runtime. Thus the kubelet needs a method to communicate over the CRI so the runtime can then modify the container cgroup.
This is used in the situation where a core is added or removed from the shared pool to become a exclusive core for a new G pod. The cpuset for all containers in the shared pool will need to be updated to add or remove that core.
Opening this up now so we can start discussion. The need for a change to the CRI might be unexpected.
@derekwaynecarr @vishh @ConnorDoyle
```release-note
NONE
```
Automatic merge from submit-queue (batch tested with PRs 50103, 49677, 49449, 43586, 48969)
Do not try to run preStopHook when the gracePeriod is 0
**What this PR does / why we need it**:
1. Sometimes when the user force deletes a POD with no gracePeriod, its possible that kubelet attempts to execute the preStopHook which will certainly fail. This PR prevents this inavitable PreStopHook failure.
```
kubectl delete --force --grace-period=0 po/<pod-name>
```
2. This also adds UT for LifeCycle Hooks
```
time go test --cover -v --run "Hook" ./pkg/kubelet/kuberuntime/
.
.
.
--- PASS: TestLifeCycleHook (0.00s)
--- PASS: TestLifeCycleHook/PreStop-CMDExec (0.00s)
--- PASS: TestLifeCycleHook/PreStop-HTTPGet (0.00s)
--- PASS: TestLifeCycleHook/PreStop-NoTimeToRun (0.00s)
--- PASS: TestLifeCycleHook/PostStart-CmdExe (0.00s)
PASS
coverage: 15.3% of statements
ok k8s.io/kubernetes/pkg/kubelet/kuberuntime 0.429s
```
**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #
**Special notes for your reviewer**:
**Release note**:
```
Do not try to run preStopHook when the gracePeriod is 0
```
Automatic merge from submit-queue (batch tested with PRs 49651, 49707, 49662, 47019, 49747)
Add support for `no_new_privs` via AllowPrivilegeEscalation
**What this PR does / why we need it**:
Implements kubernetes/community#639
Fixes#38417
Adds `AllowPrivilegeEscalation` and `DefaultAllowPrivilegeEscalation` to `PodSecurityPolicy`.
Adds `AllowPrivilegeEscalation` to container `SecurityContext`.
Adds the proposed behavior to `kuberuntime`, `dockershim`, and `rkt`. Adds a bunch of unit tests to ensure the desired default behavior and that when `DefaultAllowPrivilegeEscalation` is explicitly set.
Tests pass locally with docker and rkt runtimes. There are also a few integration tests with a `setuid` binary for sanity.
**Release note**:
```release-note
Adds AllowPrivilegeEscalation to control whether a process can gain more privileges than it's parent process
```
Automatic merge from submit-queue (batch tested with PRs 47357, 49514, 49271, 49572, 49476)
Using only the exit code to decide when to fall back on logs
We expect the exit code to be non-zero if the the container process was
OOM killed. Remove the check that uses the "Reason" field.
Automatic merge from submit-queue (batch tested with PRs 49444, 47864, 48584, 49395, 49118)
Move event type
Change SandboxChanged to a constant and move to the event package below.
**Release note**:
```release-note
NONE
```
Automatic merge from submit-queue
Revert workaround in PR 46246 as APIs have been consistent
**What this PR does / why we need it**:
No need to convert v1.ObjectReference as APIs have been consistent in `k8s.io/api/core/v1`.
**Which issue this PR fixes** : fixes#48668
**Special notes for your reviewer**:
/assign @derekwaynecarr @caesarxuchao
**Release note**:
```release-note
NONE
```
Automatic merge from submit-queue (batch tested with PRs 47327, 48194)
Checked container spec when killing container.
**What this PR does / why we need it**:
Checked container spec when getting container, return error if failed.
**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes#48173
**Release note**:
```release-note-none
```
Automatic merge from submit-queue (batch tested with PRs 44058, 48085, 48077, 48076, 47823)
don't pass CRI error through to waiting state reason
Raw gRPC errors are getting into the `Reason` field of the container status `State`, causing it to be output inline on a `kubectl get pod`
xref https://bugzilla.redhat.com/show_bug.cgi?id=1449820
Basically the issue is that the err and msg are reversed in `startContainer()`. The msg is short and the err is long. It should be the other way around.
This PR changes `startContainer()` to return a short error that becomes the Reason and the extracted gPRC error description that becomes the Message.
@derekwaynecarr @smarterclayton @eparis
The verification function is fixed to check the value of RunAsNonRoot,
not just the existence of it. Also adds unit tests to verify the correct
behavior.
Automatic merge from submit-queue
Delete all dead containers and sandboxes when under disk pressure.
This PR modifies the eviction manager to add dead container and sandbox garbage collection as a resource reclaim function for disk. It also modifies the container GC logic to allow pods that are terminated, but not deleted to be removed.
It still does not delete containers that are less than the minGcAge. This should prevent nodes from entering a permanently bad state if the entire disk is occupied by pods that are terminated (in the state failed, or succeeded), but not deleted.
There are two improvements we should consider making in the future:
- Track the disk space and inodes reclaimed by deleting containers. We currently do not track this, and it prevents us from determining if deleting containers resolves disk pressure. So we may still evict a pod even if we are able to free disk space by deleting dead containers.
- Once we can track disk space and inodes reclaimed, we should consider only deleting the containers we need to in order to relieve disk pressure. This should help avoid a scenario where we try and delete a massive number of containers all at once, and overwhelm the runtime.
/assign @vishh
cc @derekwaynecarr
```release-note
Disk Pressure triggers the deletion of terminated containers on the node.
```
Automatic merge from submit-queue (batch tested with PRs 45860, 45119, 44525, 45625, 44403)
Make a log line more clear in kuberuntime_manager.go.
Make a log in `podSandboxChanged` more clear.
@yujuhong @feiskyer
Automatic merge from submit-queue
Fix AssertCalls usage for kubelet fake runtimes unit tests
Despite its name, AssertCalls() does not assert anything. It returns an error that should be checked. This was causing false negatives for a handful of unit tests, which are also fixed here.
Tests for the image manager needed to be rearranged in order to accommodate a potentially different sequence of calls each tick because the image puller changes behavior based on prior errors.
**What this PR does / why we need it**: Fixes broken unit tests
**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*:
**Special notes for your reviewer**:
**Release note**:
```release-note
NONE
```
Automatic merge from submit-queue (batch tested with PRs 45453, 45307, 44987)
Migrate the docker client code from dockertools to dockershim
Move docker client code from dockertools to dockershim/libdocker. This includes
DockerInterface (renamed to Interface), FakeDockerClient, etc.
This is part of #43234
Despite its name, AssertCalls() does not assert anything. It returns an
error that must be checked. This was causing false negatives for
a handful of unit tests.
Automatic merge from submit-queue (batch tested with PRs 45314, 45250, 41733)
CRI: add ImageFsInfo API
**What this PR does / why we need it**:
kubelet currently relies on cadvisor to get the ImageFS info for supported runtimes, i.e., docker and rkt. This PR adds ImageFsInfo API to CRI so kubelet could get the ImageFS correctly for all runtimes.
**Which issue this PR fixes**
First step for #33048 ~~also reverts temporary ImageStats in #33870~~.
**Special notes for your reviewer**:
**Release note**:
```release-note
NONE
```
Automatic merge from submit-queue
Fix container hostPid settings
**What this PR does / why we need it**:
HostPid is not set correctly for containers.
**Which issue this PR fixes**
Fixes#44041.
**Special notes for your reviewer**:
Should be cherry-picked into v1.6 branch.
**Release note**:
```release-note
Fix container hostPid settings.
```
cc @yujuhong @derekwaynecarr @unclejack @kubernetes/sig-node-pr-reviews
Automatic merge from submit-queue (batch tested with PRs 43653, 43654, 43652)
CRI: Check nil pointer to avoid kubelet panic.
When working on the containerd kubernetes integration, I casually returns an empty `sandboxStatus.Linux{}`, but it cause kubelet to panic.
This won't happen when runtime returns valid data, but we should not make the assumption here.
/cc @yujuhong @feiskyer
PR #29378 introduces ClusterFirstWithHostNet policy but only dockertools
was updated to support the feature. This PR updates kuberuntime to
support it for all runtimes.
Also fixes#43352.
Introduced chages:
1. Re-writing of the resolv.conf file generated by docker.
Cluster dns settings aren't passed anymore to docker api in all cases, not only for pods with host network:
the resolver conf will be overwritten after infra-container creation to override docker's behaviour.
2. Added new one dnsPolicy - 'ClusterFirstWithHostNet', so now there are:
- ClusterFirstWithHostNet - use dns settings in all cases, i.e. with hostNet=true as well
- ClusterFirst - use dns settings unless hostNetwork is true
- Default
Fixes#17406
Automatic merge from submit-queue (batch tested with PRs 41921, 41695, 42139, 42090, 41949)
Unify fake runtime helper in kuberuntime, rkt and dockertools.
Addresses https://github.com/kubernetes/kubernetes/pull/42081#issuecomment-282429775.
Add `pkg/kubelet/container/testing/fake_runtime_helper.go`, and change `kuberuntime`, `rkt` and `dockertools` to use it.
@yujuhong This is a small unit test refactoring PR. Could you help me review it?
