Skip to content
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

Build: Make distroless image default and publish mimir-alpine #8204

Merged
merged 4 commits into from
May 30, 2024

Conversation

ying-jeanne
Copy link
Contributor

@ying-jeanne ying-jeanne commented May 28, 2024

What this PR does

Which issue(s) this PR fixes or relates to

Make distroless the default base image for grafana/mimir. To minimize the change, i just switched dockerfiles and changed makefile, pipeline stayed mostly untouched.

Checklist

  • n/a Tests updated.
  • n/a Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • n/a about-versioning.md updated with experimental features.

@ying-jeanne ying-jeanne changed the title Security: Make distroless image default and publish mimir-alpine Build: Make distroless image default and publish mimir-alpine May 28, 2024
@ying-jeanne
Copy link
Contributor Author

ying-jeanne commented May 28, 2024

wait for support ticket to be implemented to create 3 repo in dockerhub

@ying-jeanne ying-jeanne force-pushed the make-distroless-default branch 2 times, most recently from dee79e4 to 7f76fbc Compare May 28, 2024 19:43
@ying-jeanne
Copy link
Contributor Author

ying-jeanne commented May 28, 2024

the run for testing also alpine during deploy https://github.com/grafana/mimir/actions/runs/9290241970/job/25566956834

@ying-jeanne ying-jeanne force-pushed the make-distroless-default branch 2 times, most recently from a2c7154 to fca8c2c Compare May 29, 2024 17:46
@ying-jeanne ying-jeanne marked this pull request as ready for review May 29, 2024 17:48
Copy link
Member

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, left minor comments. Thank you.

CHANGELOG.md Outdated Show resolved Hide resolved
Makefile Outdated
fi;
@echo
# We need gcompat -- compatibility layer with glibc, as race-detector currently requires glibc, but Alpine uses musl libc instead.
$(SUDO) docker build --build-arg=revision=$(GIT_REVISION) --build-arg=goproxyValue=$(GOPROXY_VALUE) --build-arg=USE_BINARY_SUFFIX=true --build-arg=BINARY_SUFFIX=_race --build-arg=EXTRA_PACKAGES="gcompat" -t $(IMAGE_PREFIX)$(shell basename $(@D)):$(IMAGE_TAG_RACE) $(@D)/
# for grafana/mimir which is based on distroless image, the EXTRA_PACKAGES is just ignored.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we even pass EXTRA_PACKAGES to the build then? We can remove it.

Copy link
Contributor Author

@ying-jeanne ying-jeanne May 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a search in project, this is used in other dockerfiles still, not only for cmd/mimir, we still need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean now, it is UPTODATE_RACE target which is only for cmd/mimir, removing it.

ying-jeanne and others added 2 commits May 30, 2024 10:00
@ying-jeanne ying-jeanne merged commit 26c1c9a into main May 30, 2024
29 checks passed
@ying-jeanne ying-jeanne deleted the make-distroless-default branch May 30, 2024 16:23
--build-arg=BASEIMG="gcr.io/distroless/base-nossl-debian12" \
-t $(IMAGE_PREFIX)$(shell basename $(@D))-distroless:$(IMAGE_TAG_RACE) $(@D)/; \
--build-arg=EXTRA_PACKAGES="gcompat" \
-t $(IMAGE_PREFIX)$(shell basename $(@D))-alpine:$(IMAGE_TAG_RACE) $(@D)/; \
fi;
@echo
# We need gcompat -- compatibility layer with glibc, as race-detector currently requires glibc, but Alpine uses musl libc instead.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment should be moved to "alpine" version. For distroless image, we should restore comment about using distroless/base-nossl-debian12 as base image.

narqo pushed a commit to narqo/grafana-mimir that referenced this pull request Jun 6, 2024
…a#8204)

* Security: Make distroless image default and publish mimir-alpine

* Update CHANGELOG.md

* Update CHANGELOG.md

Co-authored-by: Peter Štibraný <pstibrany@gmail.com>

* remove extra_package for distroless uptodate_race target

---------

Co-authored-by: Peter Štibraný <pstibrany@gmail.com>
simonswine added a commit to simonswine/pyroscope that referenced this pull request Jul 22, 2024
Following a trend by at other database teams:

* grafana/loki#13325
* grafana/mimir#8204
simonswine added a commit to simonswine/pyroscope that referenced this pull request Jul 22, 2024
Following a trend by at other database teams:

* grafana/loki#13325
* grafana/mimir#8204
simonswine added a commit to grafana/pyroscope that referenced this pull request Jul 23, 2024
* chore: Replace alpine with distroless

Following a trend by at other database teams:

* grafana/loki#13325
* grafana/mimir#8204

* Also build debug image for main
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants