-
Notifications
You must be signed in to change notification settings - Fork 519
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2649 +/- ##
=======================================
Coverage 72.35% 72.35%
=======================================
Files 137 137
Lines 25341 25341
=======================================
Hits 18335 18335
Misses 5949 5949
Partials 1057 1057 |
20b94f3
to
ec6b2c8
Compare
tar -xzf "$CONTAINERD_DOWNLOADS_DIR/$CONTAINERD_TGZ_TMP" -C / | ||
sed -i '/\[Service\]/a ExecStartPost=\/sbin\/iptables -P FORWARD ACCEPT -w' /etc/systemd/system/containerd.service | ||
echo "Successfully installed cri-containerd..." | ||
removeContainerd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall I infer that this is a "best practice, just-in-case" type operation? In other words, the most deterministic way to install a particular version of the moby-containerd
is to ensure no pre-existing moby-containerd
and moby-runc
packages aren't present on the OS already?
Also, is moby-runc
installed as a dependency of moby-containerd
? Or is it just a conflicting package and so we need to ensure it's removed before we install moby-containerd
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied this pattern from installMoby
is all.
moby-runc
is a dependency for moby-containerd
and is automatically installed with moby-containerd
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it.
Should we invoke removeMoby
at the beginning of the containerd installation process to ensure we don't ship both packages?
Likewise we should probably invoke removeContainerd
during moby installation.
@cpuguy83 Are the |
|
ec6b2c8
to
9bbbc85
Compare
4a69ac4
to
61a70e6
Compare
61a70e6
to
987f2d3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cpuguy83, jackfrancis The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Reason for Change:
Issue Fixed:
Fixes #2370
Requirements:
Notes:
This is a POC.
Looking for feedback on the implementation.
Also need tests to run, currently having issues deploying things.