-
Notifications
You must be signed in to change notification settings - Fork 25
Fix runner container image #287
Fix runner container image #287
Conversation
d0405e4
to
6a8da97
Compare
also remove appVersion from chart infos because it doesn't bring much value and might cause confusion and cognitive overhead
6a8da97
to
0ca6208
Compare
pullPolicy: Always | ||
tag: "rolling" |
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.
Could we avoid this? ._.
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.
hmm, what do you mean?
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.
I guess @dottorblaster means to ask if we could avoid the pullPolicy: Always
? If so, I understood this is a side effect of using tag: rolling
(to ensure the image gets pulled even when the tag is the same).
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.
I mean, for now I think it's useful to have an Always
pullPolicy and point to the rolling tag but keep in mind that this will end up in the hands of customers, and I don't know if this is the best strategy to handle the distribution of the containers 🙂
@rtorrero anticipated my answer, Github was down while I was posting this comment :D
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.
Maybe I'm missing something, but I don't see anything particularly problematic about Always
: the container engine will not waste bandwidth if the image hash is identical, but by using Always
we at least enforce to check if there is a new image available, otherwise it will keep using whatever it has already in cache under that repo:tag.
TL;DR: I'm not aware of good reasons not to use it, even for non-rolling tags, and it will surely avoid upgrading headaches.
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.
LGTM
This PR introduces the usage of multi-target Dockerfile builds, to produce to separate containers for the web and the runner components.
The problem we originally had is that the image didn't include
ssh
, so the runner woudn't work properly, causing all the checks to be reported as passing, while actually they would not be executed at all.It also does the following:
appVersion
is gone, the only place where we need to update the pinned version are the bash installers.