Skip to content
This repository was archived by the owner on Apr 20, 2026. It is now read-only.

Create 1.25 kubelet with Lyft patches#44

Merged
abhinavdahiya merged 11 commits into
release-1.25.16-lyft.1from
1-25-custom-patch
Dec 1, 2023
Merged

Create 1.25 kubelet with Lyft patches#44
abhinavdahiya merged 11 commits into
release-1.25.16-lyft.1from
1-25-custom-patch

Conversation

@abhinavdahiya
Copy link
Copy Markdown

@abhinavdahiya abhinavdahiya commented Nov 29, 2023

git cherry-pick origin/release-1.24.16..origin/release-1.24.16-lyft.1

tomwans and others added 11 commits November 29, 2023 11:06
CRI-O properly implements the CRI interface, and therefore it is
capable of returning the container stats if being asked for.

There is no reason to keep CRI-O as a special use case that has to
be run with the legacy mode making kubelet using cadvisor on each
container.

This patch removes the hardcoded assumptions that CRI-O has cannot
handle to return containers stats through CRI.

Fixes kubernetes#73750

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
We're not guaranteed that the pod passed in has the ContainerSpec
we're looking for. With this, we check if the pod has the container
spec, and if it doesn't, we try to recover it one more time.
Based on the api documentation
https://pkg.go.dev/k8s.io/api/core/v1#ContainerState
```
ContainerState holds a possible state of container. Only one of its members may be specified.
If none of them is specified, the default one is ContainerStateWaiting.
```

There is a case where no explicit state is known for a container and it must be
treated as waiting state.

So we must check for nil value (since the State is a value and pointer we have to check
that all possible fields are nil , :( ) and treat it as Waiting state.

Also fixes the `Restart running non-sidecars despite sidecar becoming not ready` test.
A pod if needs to be restarted (i.e it had existed and has run at least one) the state
of the container should be set to last running state.

compare to zero value
…atus is not available (#37)

1. When all container statuses are nil, we should default to application
containers wait untill statuses are available

2. When application container is nil, we should default to application
containers waiting state
…nt (#40)

based on
https://lyft.slack.com/archives/C017X524VC7/p1685639606706689?thread_ts=1685481685.730389&cid=C017X524VC7

when there are no sidecars we should allow all containers to start even
when there is lack of container status.
This fixes a regression introduced in
#37 where we would block starting
containers until we get container status reported. this triggers KillPod
action in 1.16 as no progress can be made.

---------

Co-authored-by: Tom Wanielista <tomwans@users.noreply.github.com>
@abhinavdahiya
Copy link
Copy Markdown
Author

➜  kubernetes git:(1-25-custom-patch) gotest -timeout 30s ./pkg/kubelet/cadvisor/...
?   	k8s.io/kubernetes/pkg/kubelet/cadvisor	[no test files]
?   	k8s.io/kubernetes/pkg/kubelet/cadvisor/testing	[no test files]
➜  kubernetes git:(1-25-custom-patch) gotest -timeout 30s ./pkg/kubelet/kuberuntime/...
ok  	k8s.io/kubernetes/pkg/kubelet/kuberuntime	(cached)
ok  	k8s.io/kubernetes/pkg/kubelet/kuberuntime/logs	(cached)
ok  	k8s.io/kubernetes/pkg/kubelet/kuberuntime/util	(cached)
➜  kubernetes git:(1-25-custom-patch) gotest -timeout 30s ./pkg/kubelet/status/...
?   	k8s.io/kubernetes/pkg/kubelet/status/testing	[no test files]
ok  	k8s.io/kubernetes/pkg/kubelet/status	(cached)

@abhinavdahiya
Copy link
Copy Markdown
Author

Tested the kubelet in boba2 cluster and the kubelet is running as expected https://github.com/lyft/genesis-device/pull/7502

@abhinavdahiya abhinavdahiya changed the base branch from release-1.25.16 to release-1.25.16-lyft.1 December 1, 2023 16:37
@abhinavdahiya abhinavdahiya merged commit a123fcc into release-1.25.16-lyft.1 Dec 1, 2023
@abhinavdahiya abhinavdahiya deleted the 1-25-custom-patch branch December 1, 2023 16:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants