-
Notifications
You must be signed in to change notification settings - Fork 560
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cidata: automatically upgrade existing containerd/nerdctl ; nerdctl: update to v0.17.1 #694
Conversation
When the timestamp of `/mnt/lima-cidata/nerdctl-full.tgz:bin/nerdctl` is newer than `/usr/local/bin/nerdctl`, containerd/nerdctl is automatically upgraded. Drawback: increases boot time by 4-5 seconds. Fix issue 303 Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@@ -8,14 +8,31 @@ fi | |||
# This script does not work unless systemd is available | |||
command -v systemctl >/dev/null 2>&1 || exit 0 | |||
|
|||
if [ ! -x /usr/local/bin/nerdctl ]; then | |||
# Extract bin/nerdctl and compare whether it is newer than the current /usr/local/bin/nerdctl (if already exists). | |||
# Takes 4-5 seconds. (FIXME: optimize) |
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.
Idea for optimization:
root@lima-default:/mnt/lima-cidata# gunzip -c nerdctl-full.tgz | head -c 512 | tar tv | awk '/ bin\/$/ {print $4}'
2022-03-03
root@lima-default:/mnt/lima-cidata# date -r /usr/local/bin/nerdctl "+%F"
2022-03-03
Then do simple string comparison on the dates.
This relies on a tar header block being 512 bytes (so tar
will not throw an error), and bin/
being the first entry in the tarball. I think these are reasonable assumptions, but am not totally sure. If the first line doesn't return a non-empty string, fall back on the expensive check by extracting the tarball.
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 think these are reasonable assumptions, but am not totally sure.
Not guaranteed. https://github.com/containerd/nerdctl/blob/20233c26d26f11ca73a9a775fba87ca884ab14d2/Makefile#L67
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.
Yeah, not guaranteed, but seems likely to be true, and with the fallback it should still work.
My only other idea would be to extract the nerdctl version from the download URL and then do a version comparison (pass it in as LIMA_CIDATA_NERDCTL_VERSION
).
Still not guaranteed if somebody uses a custom download location that doesn't include the version, or worse, a different version, but you can argue that they will get what they deserve if they do this. 😄
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, but I'm not merging in case you want to include the suggested optimization in this PR.
Let me merge and open an issue ticket for optimization in future: |
When the timestamp of
/mnt/lima-cidata/nerdctl-full.tgz:bin/nerdctl
is newer than/usr/local/bin/nerdctl
, containerd/nerdctl is automatically upgraded.Drawback: increases boot time by 4-5 seconds.
Fix #303
Also bumps up the default nerdctl version to 0.17.1: https://github.com/containerd/nerdctl/releases/tag/v0.17.1