Skip to content

Conversation

@miz060
Copy link
Member

@miz060 miz060 commented Jul 15, 2024

Merge Checklist
  • Followed patch format from upstream recommendation: https://github.com/kata-containers/community/blob/main/CONTRIBUTING.md#patch-format
    • Included a single commit in a given PR - at least unless there are related commits and each makes sense as a change on its own.
  • Aware about the PR to be merged using "create a merge commit" rather than "squash and merge" (or similar)
  • genPolicy only: Ensured the tool still builds on Windows
  • The upstream/missing label (or upstream/not-needed) has been set on the PR.
Summary

At the moment, we have circular dependencies between tardev-snapshotter.service and containerd.service. Specifically, containerd.service needs tardev-snapshotter.service to run any CC pods, while tardev-snapshotter.service needs containerd.service to download image layers. This dependency will be eliminated once we switch to using remote-snapshotter.

Currently, tardev-snapshotter.service's binding to containerd.service gets delayed, and we won't be able to run any CC pods until the boot process is completed. It doesn't matter which service starts first.

Based on the current logic, it makes more sense to use WantedBy=kubelet.service in tardev-snapshotter.service, as we won't be able to start any CC pods without kubelet. This is also what we have been using by having sed -i -e 's/containerd.service/kubelet.service/g' tardev-snapshotter.service in kata-containers-cc.spec.

In the future, once tardev-snapshotter becomes a remote snapshotter again, it will make more sense to use WantedBy=containerd.service.

@miz060 miz060 requested review from a team as code owners July 15, 2024 21:28
@danmihai1
Copy link

Please add to your commit description the reason why kubelet.service is better than containerd.service.

@miz060 miz060 force-pushed the mitchzhu/tardev-service branch from a32f35f to 724b1ae Compare July 15, 2024 21:39
At the moment, we have circular dependencies between
tardev-snapshotter.service and containerd.service. Specifically,
containerd.service needs tardev-snapshotter.service to run any CC pods,
while tardev-snapshotter.service needs containerd.service to download
image layers. This dependency will be eliminated once we switch to
using remote-snapshotter. Currently, tardev-snapshotter.service's
binding to containerd.service gets delayed, and we won't be able to
run any CC pods until the boot process is completed. It doesn't matter
which service starts first. Based on the current logic, it makes more
sense to use WantedBy=kubelet.service in tardev-snapshotter.service, as
we won't be able to start any CC pods without kubelet. In the future,
once tardev-snapshotter becomes a remote snapshotter again, it will
make more sense to use WantedBy=containerd.service.

Signed-off-by: Mitch Zhu <[email protected]>
@miz060 miz060 force-pushed the mitchzhu/tardev-service branch from 724b1ae to e6e6d34 Compare July 16, 2024 00:51
@miz060 miz060 added the upstream/missing PRs that are yet to be upstreamed label Jul 16, 2024
@miz060 miz060 merged commit 1276d40 into msft-main Jul 16, 2024
@miz060 miz060 deleted the mitchzhu/tardev-service branch January 14, 2025 00:44
sprt pushed a commit that referenced this pull request Feb 10, 2025
tardev: update tardev-snapshotter.service
sprt pushed a commit that referenced this pull request Feb 27, 2025
tardev: update tardev-snapshotter.service
sprt pushed a commit that referenced this pull request Mar 3, 2025
tardev: update tardev-snapshotter.service
sprt pushed a commit that referenced this pull request Mar 4, 2025
tardev: update tardev-snapshotter.service
sprt pushed a commit that referenced this pull request Mar 4, 2025
tardev: update tardev-snapshotter.service
sprt pushed a commit that referenced this pull request Mar 4, 2025
tardev: update tardev-snapshotter.service
sprt pushed a commit that referenced this pull request Mar 4, 2025
tardev: update tardev-snapshotter.service
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

upstream/missing PRs that are yet to be upstreamed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants