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

Don't use apk cache to reduce image size #945

Merged
merged 2 commits into from
Jun 29, 2018
Merged

Don't use apk cache to reduce image size #945

merged 2 commits into from
Jun 29, 2018

Conversation

t3easy
Copy link
Contributor

@t3easy t3easy commented Jun 27, 2018

The Problem/Issue/Bug:

Apk cache remains in image and makes it larger as it must be

How this PR Solves The Problem:

Use apk --no-cache parameter

Manual Testing Instructions:

Automated Testing Overview:

Related Issue Link(s):

Release/Deployment notes:

@CLAassistant
Copy link

CLAassistant commented Jun 27, 2018

CLA assistant check
All committers have signed the CLA.

@rickmanelius
Copy link
Contributor

Added @rfay to review because I wasn't sure if there was a particular reason for this.

@rfay
Copy link
Member

rfay commented Jun 27, 2018

This looks great to me, thanks. I pushed this to drud/mariadb-local:20180627_mariadb_no_apk_cache - @t3easy if you could please add a commit setting that container in pkg/version/version.go it would be fantastic. Otherwise ask us to do it and tell us you don't mind us pushing into your fork.

@rfay
Copy link
Member

rfay commented Jun 27, 2018

I should note that this seems to reduce the image size by less than 1MB, so might not be as effective as you hoped. https://hub.docker.com/r/drud/mariadb-local/tags/

Copy link
Contributor

@andrewfrench andrewfrench 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 to me, there's no reason to be pushing around a cache with the container image. Once @rfay's version.go request is in, this is good to pull.

@andrewfrench
Copy link
Contributor

andrewfrench commented Jun 27, 2018

Just to confirm @rfay's point about the image size, here's a comparison of this version and the v0.20.0 image:

CONTAINER ID        IMAGE                         SIZE
340e94a2a9b6        drud/mariadb-local:v0.20.0    151MB (virtual 362MB)
d6918a2aceca        drud/mariadb-local:945-test   151MB (virtual 361MB)

I'd say the change is still valid either way, especially since I'm seeing --no-cache as a recommended flag when building an image. The compressed image size decreases from 64MB to 63MB.

@t3easy
Copy link
Contributor Author

t3easy commented Jun 28, 2018

It's not dramatically but best practice.

Regarding

if you could please add a commit setting that container in pkg/version/version.go it would be fantastic

Just rais var DBTag = "v0.20.0" to v0.20.1?

@rfay
Copy link
Member

rfay commented Jun 28, 2018

var DBTag = 20180627_mariadb_no_apk_cache - the tag I pushed.

We use temporary tags between releases, when we release ddev v1.0.0 (the next one) we'll retag the latest version of the mariadb-local container to v1.0.0 to match the ddev version.

@rfay rfay added this to the v1.0.0 milestone Jun 29, 2018
Copy link
Member

@rfay rfay 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 to me, thanks!

@rfay rfay merged commit 18299a3 into ddev:master Jun 29, 2018
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.

None yet

5 participants