Use debian-base image from kubernetes repository as base for NPD. #160
Conversation
This image is based on Debian Stretch (9) which has a recent version of systemd libraries that includes all necessary compression algorithms. I propose using that image unconditionally (instead of Alpine when journald is disabled) since the image size with debian-base is not that much of a concern anymore.
k8s-ci-robot
added
cncf-cla: yes
|
Thanks! /lgtm |
| ifeq ($(ENABLE_JOURNALD), 1) | ||
| # Enable journald build tag. | ||
| BUILD_TAGS:=-tags journald | ||
| # Use fedora because it has newer systemd version (229) and support +LZ4. +LZ4 is needed | ||
| # on some os distros such as GCI. | ||
| BASEIMAGE:=fedora |
There was a problem hiding this comment.
Can we only replace this one?
alpine:3.4 is ~4mb.
gcr.io/google-containers/debian-base-amd64:0.3 is ~40mb.
I feel like we should use smaller image if we don't need journald support.
There was a problem hiding this comment.
I feel like we should use smaller image if we don't need journald support.
A few points I have against that:
-
Every relevant Linux distribution uses systemd nowadays, so the point of supporting no journal is kind of moot to me...
-
We already use debian-base-amd:0.3 as base layer for other images (such as fluentd-gcp, etc.) so reusing it might actually reduce overall disk usage since they're all sharing from the same base.
-
Supporting two builds means this needs more testing, it's likely we might end up breaking one but not the other, which might go unnoticed as one of them is seldom or never used... Let's try to reduce overall complexity.
Having said that, if you really insist, I think it's OK to keep using Alpine for no journald... Let me know if you really want to keep Alpine for the non-journal case.
There was a problem hiding this comment.
I agree with all @filbranden 's points.
There was a problem hiding this comment.
Every relevant Linux distribution uses systemd nowadays, so the point of supporting no journal is kind of moot to me...
We have the no-journal support because we were still using containerVM at that time.
I still think we should support no journal, because journal support:
- need
lib-systemd-devto build; - will make the binary non-static.
I still prefer making it possible to have a simple statically linked binary for users who don't use journal, e.g. people may use this in LinuxKit.
As for the base image, as long as BASEIMAGE is over-writable, I'm fine with always using gcr.io/google-containers/debian-base-amd64:0.3 as the default base image to reduce the maintenance overhead.
There was a problem hiding this comment.
I think this PR still does what you suggested... It still keeps the ENABLE_JOURNALD variable and sets build tags appropriately in those cases. So, good to merge?
|
/lgtm |
k8s-ci-robot
added
the
lgtm
This image is based on Debian Stretch (9) which has a recent version of systemd libraries that includes all necessary compression algorithms.
I propose using that image unconditionally (instead of Alpine when journald is disabled) since the image size with debian-base is not that much of a concern anymore.
FTR, I didn't really test this...
This is analogous to a change we're pushing to fluentd-gcp container in GoogleCloudPlatform/k8s-stackdriver#101
@Random-Liu
Cheers,
Filipe
This change is