Optimize addon docker image #28

felipecrs · 2022-02-05T15:15:27Z

Comparison (before and after):

❯ docker images
REPOSITORY                              TAG       IMAGE ID       CREATED          SIZE
ghcr.io/allenporter/amd64-rtsp-to-web   latest    83ebfeef827a   11 days ago      954MB
ghcr.io/felipecrs/amd64-rtsp-to-web     latest    1253f6cf9ad8   13 minutes ago   60.3MB

Which means that the addon is much quicker to install now, and takes less space.

allenporter · 2022-02-05T15:37:25Z

Nice thanks for the improvements. I will do some testing on this on a couple platforms.

felipecrs · 2022-02-05T15:38:39Z

Me too, I'm outside home now.

allenporter · 2022-02-05T20:39:11Z

There appear to be some build failures with the images.

felipecrs · 2022-02-06T03:12:22Z

@allenporter now it should be working normally, I tested. This is the final size:

$ docker images
REPOSITORY                                             TAG       IMAGE ID       CREATED          SIZE
new                                                    latest    de37fea230f1   10 seconds ago   60.3MB

rtsp-to-web/Dockerfile

allenporter · 2022-02-07T06:40:54Z

rtsp-to-web/Dockerfile

-RUN chmod +x /app/rtsp-to-web
+# Build
+ENV GO111MODULE="on"
+ENV GIN_MODE="release"


My impression is that this is a runtime variable and not a build time variable, hence why it was at the end and not at the start. move back?

From my past experience with Golang, I really don't think so. But moving them back does not hurt much anyway.

allenporter · 2022-02-08T06:33:44Z

rtsp-to-web/build.yaml

@@ -1,14 +1,13 @@
-# https://developers.home-assistant.io/docs/add-ons/configuration#add-on-dockerfile
+# https://developers.home-assistant.io/docs/add-ons/configuration/#add-on-extended-build


Can you help me understand more the motivation behind changing the base image from the example? My impression is this adds extra build labels for the community addons, but this isn't a community add-on.

Oh, the new base image has a newer Alpine. That's all I cared about.

Alpine on the regular base image is 3.14, while in this one is 3.15.

I can revert if you prefer, but I thought it would be an enhancement. Image size is very small anyway. Actually I didn't compare both, and I would be surprised if they differ much.

Ops, I think now I have a better reasoning to prefer the new one:

❯ docker images REPOSITORY TAG IMAGE ID CREATED SIZE ghcr.io/home-assistant/amd64-base 3.14 e6325b527bd3 6 hours ago 68.4MB ghcr.io/hassio-addons/base/amd64 11.0.1 a0e49bd90483 4 weeks ago 25.5MB

OK, well, i don't think it is appropriate for to have these community add-on labels since this is not a community add-on project, so I think there needs to be a different solution for that.

I still don't want these labels, maintainer, etc.

allenporter · 2022-02-09T04:00:51Z

I'll give this a try soon.

In order to fix https://github.com/allenporter/stream-addons/issues/29 i'm moving the configuration around to /data so that it can sit in the add-on persistent volume in #31 and pulling in a couple changes from here (e.g. removing redundant lines while i'm updating them). If you have time to review, i'll take it.

felipecrs · 2022-02-14T21:29:40Z

@allenporter I still didn't get my hands on a Raspberry Pi for testing in ARM, but I'm confident that my last change should have fixed it there.

Also, I'm using this version of the addon ever since, and it's working very well.

allenporter · 2022-02-15T16:22:52Z

Thanks for the reminder, i'll give this a try soon.

allenporter · 2022-02-17T16:59:40Z

Thanks for the reminder, i'll give this a try soon.

Yes, this is working, great!

$ docker pull ghcr.io/felipecrs/armv7-rtsp-to-web:latest
$ docker run -p 8083:8083 ghcr.io/felipecrs/armv7-rtsp-to-web:latest
...
[16:44:29] INFO: Starting discovery service
[16:44:29] INFO: Starting RSTPtoWeb

allenporter

Thanks overall for the improvements here. I still have some nitpicky things -- some arguably style things -- that i'd like to still talk through/address.

allenporter · 2022-02-17T16:47:38Z

rtsp-to-web/Dockerfile

-ARG TEMPIO_VERSION=2021.09.0
-ARG BUILD_ARCH
+ARG BUILD_ARCH="amd64"
+ARG BUILD_FROM="ghcr.io/hassio-addons/base/${BUILD_ARCH}:11.0.1"


I'd either leave this out or make it latest so we don't have something else to keep up to date.

Newer versions can potentially break. Every other community addon also pins the version. This is a very good practice for build reproducibility and traceability.

Example:

Upgrade s6-overlay from 2.2.0.3 to 3.1.0.1 hassio-addons/addon-base#176

This is going to break every addon once it gets released.

Yes, i'm for pinning. However, my impression is that we're already pinning this when building the image from automation. This ARG default value is only used when run manually. I almost like it better without a default value so its less clear that its specified somewhere else and this value is never used for the actual published image.

Oh yeah, the default value here is only meant for manual build of the image.

Are you sure you want me to remove it? You will have to type in the whole docker build call with two long --build-args every time you just want to build the image. Most of other community addons also does this:

https://github.com/hassio-addons/addon-vscode/blob/main/vscode/Dockerfile#L1

Yeah, but the author probably has scripts to update the base image versions. That was also why I suggested latest here as a compromise since this is just for development. I just don't want to have to maintain this. (The image version in the build script is automatically maintained, but this isn't)

rtsp-to-web/Dockerfile

felipecrs · 2022-02-17T17:55:40Z

Please let me know what you think. I did some research, and I realized that GIN_MODE is indeed required in runtime, but GO111MODULE is not (it's only required during go mod download and go build).

allenporter

I think we're close

rtsp-to-web/Dockerfile

allenporter

I'd like to make these two changes the merge this

rtsp-to-web/build.yaml

rtsp-to-web/Dockerfile

allenporter · 2022-02-20T00:50:53Z

I'd like to do a partial revert of these two changes and merge your PR. Perhaps we can keep discussing after, as i'd like to make bigger changes, but don't want to force you to have to do all the merging yourself.

allenporter · 2022-02-20T00:53:55Z

rtsp-to-web/Dockerfile


-ARG REPO=github.com/deepch/RTSPtoWeb
-WORKDIR /build/$REPO
+RUN apk add --no-cache git go=~1.17.4-r0


This didn't work, though I didn't look closely as to why exactly:
https://github.com/allenporter/stream-addons/runs/5261769429?check_suite_focus=true

ERROR: unable to select packages: go-1.16.10-r0: breaks: world[go~1.17.4-r0] The command '/bin/ash -o pipefail -c apk add --no-cache git go=~1.17.4-r0' returned a non-zero code: 1

I'd like to revert this.

rtsp-to-web/Dockerfile

Optimize docker image

358ddf7

allenporter reviewed Feb 7, 2022

View reviewed changes

felipecrs added 3 commits February 7, 2022 20:27

Address comment and optimize build again

58f4bb2

Restore env vars in final image

070f859

Restore changes on build.yamkl

5f2acf6

allenporter reviewed Feb 8, 2022

View reviewed changes

felipecrs added 2 commits February 8, 2022 11:21

Try to fix ARM build

4242562

Correct versions

df7652c

allenporter mentioned this pull request Feb 9, 2022

Move config to /data for persistent storage #31

Merged

allenporter reviewed Feb 17, 2022

View reviewed changes

Simplify args and unpin git version

f51eec3

Set GIN_MODE only in final stage

ae11f82

allenporter reviewed Feb 18, 2022

View reviewed changes

rtsp-to-web/Dockerfile Show resolved Hide resolved

allenporter reviewed Feb 20, 2022

View reviewed changes

rtsp-to-web/build.yaml Outdated Show resolved Hide resolved

rtsp-to-web/Dockerfile Outdated Show resolved Hide resolved

allenporter added 2 commits February 19, 2022 16:50

Update rtsp-to-web/Dockerfile

bf1d743

Update rtsp-to-web/build.yaml

ebb6302

allenporter reviewed Feb 20, 2022

View reviewed changes

rtsp-to-web/Dockerfile Outdated Show resolved Hide resolved

Update rtsp-to-web/Dockerfile

ea4397b

allenporter merged commit 16fcaea into allenporter:main Feb 20, 2022

felipecrs deleted the optimize-addon branch February 20, 2022 03:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize addon docker image #28

Optimize addon docker image #28

felipecrs commented Feb 5, 2022 •

edited

Loading

allenporter commented Feb 5, 2022

felipecrs commented Feb 5, 2022

allenporter commented Feb 5, 2022

felipecrs commented Feb 6, 2022 •

edited

Loading

allenporter Feb 7, 2022

felipecrs Feb 7, 2022

allenporter Feb 8, 2022

felipecrs Feb 8, 2022 •

edited

Loading

felipecrs Feb 8, 2022

allenporter Feb 17, 2022 •

edited

Loading

allenporter Feb 18, 2022

allenporter commented Feb 9, 2022

felipecrs commented Feb 14, 2022 •

edited

Loading

allenporter commented Feb 15, 2022

allenporter commented Feb 17, 2022

allenporter left a comment •

edited

Loading

allenporter Feb 17, 2022

felipecrs Feb 17, 2022

felipecrs Feb 17, 2022

allenporter Feb 18, 2022

felipecrs Feb 18, 2022 •

edited

Loading

felipecrs Feb 18, 2022 •

edited

Loading

allenporter Feb 18, 2022 •

edited

Loading

felipecrs commented Feb 17, 2022

allenporter left a comment

allenporter left a comment

allenporter commented Feb 20, 2022

allenporter Feb 20, 2022 •

edited

Loading

		@@ -1,14 +1,13 @@
		# https://developers.home-assistant.io/docs/add-ons/configuration#add-on-dockerfile
		# https://developers.home-assistant.io/docs/add-ons/configuration/#add-on-extended-build

Optimize addon docker image #28

Optimize addon docker image #28

Conversation

felipecrs commented Feb 5, 2022 • edited Loading

allenporter commented Feb 5, 2022

felipecrs commented Feb 5, 2022

allenporter commented Feb 5, 2022

felipecrs commented Feb 6, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felipecrs Feb 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allenporter Feb 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

allenporter commented Feb 9, 2022

felipecrs commented Feb 14, 2022 • edited Loading

allenporter commented Feb 15, 2022

allenporter commented Feb 17, 2022

allenporter left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felipecrs Feb 18, 2022 • edited Loading

Choose a reason for hiding this comment

felipecrs Feb 18, 2022 • edited Loading

Choose a reason for hiding this comment

allenporter Feb 18, 2022 • edited Loading

Choose a reason for hiding this comment

felipecrs commented Feb 17, 2022

allenporter left a comment

Choose a reason for hiding this comment

allenporter left a comment

Choose a reason for hiding this comment

allenporter commented Feb 20, 2022

allenporter Feb 20, 2022 • edited Loading

Choose a reason for hiding this comment

felipecrs commented Feb 5, 2022 •

edited

Loading

felipecrs commented Feb 6, 2022 •

edited

Loading

felipecrs Feb 8, 2022 •

edited

Loading

allenporter Feb 17, 2022 •

edited

Loading

felipecrs commented Feb 14, 2022 •

edited

Loading

allenporter left a comment •

edited

Loading

felipecrs Feb 18, 2022 •

edited

Loading

felipecrs Feb 18, 2022 •

edited

Loading

allenporter Feb 18, 2022 •

edited

Loading

allenporter Feb 20, 2022 •

edited

Loading