-
Notifications
You must be signed in to change notification settings - Fork 34
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
Distribute mountpoint binary instead of package #48
Conversation
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.
for some reason CI is failing among different PRs, seems that one of the recent commits to main broke it, I'll try adding more diagnostics output to CI to understand whats happening
mkdir -p /mountpoint-s3 && \ | ||
tar -xvzf mount-s3-${MOUNTPOINT_VERSION}-$MP_ARCH.tar.gz -C /mountpoint-s3 && \ | ||
# strip debugging information to reduce binary size | ||
strip --strip-debug /mountpoint-s3/bin/mount-s3 && \ |
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.
how many MBs we're saving with that? is it a good trade-off? as I understand this will make MPs traces unreadable
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.
also taking into the account that MP operational runbooks may rely on this info
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.
The binary goes from 55 MB to 12, so it's not insignificant. I will double check with the mountpoint team though.
@@ -2,75 +2,5 @@ | |||
|
|||
set -euox pipefail | |||
|
|||
NSENTER_HOST="nsenter --target 1 --mount --net" |
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.
🚀
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.
Yep glad to see this go. It was gross. And now we don't have to run with hostPID: true
Cutting down on image size and overall complexity by using the standalone tarball of mountpoint instead of installing a deb/rpm. Mountpoint links dynamically, so the container also packages a couple .so files.
Cutting down on image size and overall complexity by using the standalone tarball of mountpoint instead of installing a deb/rpm. Mountpoint links dynamically, so the container also packages a couple .so files.