[installer]: move the kots install script into a bash file in Installer #12202
Conversation
mrsimonemms
force-pushed
the
sje/move-kots-bash-script
branch
4 times, most recently
from
5f58b57 to
584f17e
Compare
|
/werft run publish-to-kots 👍 started the job as gitpod-build-sje-move-kots-bash-script.8 |
mrsimonemms
force-pushed
the
sje/move-kots-bash-script
branch
8 times, most recently
from
3374e1e to
f0b1714
Compare
github-actions
bot
added
the
team: delivery
mrsimonemms
force-pushed
the
sje/move-kots-bash-script
branch
from
f0b1714 to
a6a3605
Compare
|
/hold this will be release in the September |
mrsimonemms
force-pushed
the
sje/move-kots-bash-script
branch
2 times, most recently
from
43fec40 to
4fc43ec
Compare
mrsimonemms
force-pushed
the
sje/move-kots-bash-script
branch
2 times, most recently
from
4fc43ec to
c2e494d
Compare
| component: gitpod-installer | ||
| data: | ||
| # General settings | ||
| CURSOR: repl{{ Cursor | quote }} |
There was a problem hiding this comment.
💡 (idea; non-blocking) Asking Replicated for a generic feature for this
Like this approach a lot.
I wonder if we should ask Replicated whether there is a built-in way to get the whole KOTS config as config map or if not, create a feature request. A template function that returns the whole KOTS config as YAML would probably be good enough.
I think, that creating a generic solution in KOTS should not be that hard for Replicated. It would make our lives much easier to maintain this.
What do you think?
However, that does not block this pull request at all!
There was a problem hiding this comment.
It is possible to get this as a configmap, which we indeed do in the support bundle.
The problem with this as an approach is the KOTS config will have many sensitive pieces of data (passwords for DB/Registry/Storage etc) which we don't actually need for the Installer, as these are sent into secrets that the Installer then references.
We could of course store this as a secret rather than a configmap, but this would still contain information that we don't actually need for the Installer. I do see the benefit to not having to manage this though, but this then has the danger of thinking we're exposing something that we're not and errors being caused.
I'd propose that, for this PR, we keep it as-is and continue to discuss this subject when doing the general refactoring that we're doing.
There was a problem hiding this comment.
I'd propose that, for this PR, we keep it as-is and continue to discuss this subject when doing the general refactoring that we're doing.
100 % agree.
mrsimonemms
force-pushed
the
sje/move-kots-bash-script
branch
3 times, most recently
from
48c764e to
7214ed1
Compare
|
/werft run publish-to-kots 👍 started the job as gitpod-build-sje-move-kots-bash-script.26 |
|
This is ready for test/review. I will leave the "hold" status as think this would benefit from multiple testers/approvers |
| yq e -i ".workspace.runtime.containerdSocket = \"${CONTAINERD_SOCKET_AL}\"" "${CONFIG_FILE}" | ||
| fi | ||
|
|
||
| echo "Gitpod: Inject the Replicated variables into the config" |
There was a problem hiding this comment.
Definitely not asking in this PR, but a next iteration of this change would be to move all the config settings into a separate script so that we can validate the end config as a pre-flight check right? Should be simple enough and hence solving #12015 🤔
There was a problem hiding this comment.
That's exactly the next step - good thought
| yq e -i ".containerRegistry.privateBaseImageAllowList += \"docker.io\"" "${CONFIG_FILE}" | ||
| fi | ||
|
|
||
| if [ "${REG_CUSTOM_DOCKER_ENABLED}" = "1" ]; |
There was a problem hiding this comment.
nit: Should it be called REG_CUSTOM_DOCKER_CONFIG_ENABLED? Would prefer to have CONFIG to be more specific.
There was a problem hiding this comment.
I've renamed it to REG_DOCKER_CONFIG_ENABLED to that keeps consistency with what I've done elsewhere (take the KOTS config name and captialise it). I've not changed the underlying KOTS config name (it's missing a d) in the expectation that someone could have started using it
| echo "Gitpod: configuring external container registry" | ||
|
|
||
| # Get the external-container-registry secret so we can merge the external registry and KOTS registry keys | ||
| kubectl get secret external-container-registry \ |
There was a problem hiding this comment.
:thumsup: Like that this is now routed through a secret, instead of building it here!
| fi | ||
|
|
||
| echo "Gitpod: Create a Helm template directory" | ||
| rm -Rf "${GITPOD_OBJECTS}" |
There was a problem hiding this comment.
nit: Maybe document somewhere in the file about GITPOD_OBJECTS? 🤔 While its straightforward with other env's as they are input it could be confusing on what kots-install.sh emits.
There was a problem hiding this comment.
If it's ok with you, I won't fix in here - this won't be sticking around long as I've already started the next refactor and this probably won't survive. If it does, I'll fix it then
mrsimonemms
force-pushed
the
sje/move-kots-bash-script
branch
from
7214ed1 to
52bf75f
Compare
mrsimonemms
force-pushed
the
sje/move-kots-bash-script
branch
from
52bf75f to
2dc3af7
Compare
|
/unhold |
roboquat
added
the
deployed: workspace
Description
Moves the bash script in the installer job into the installer image. This is done to make the application more reliable, testable and readable. This makes no fundamental changes to how the job works except for fixing ShellCheck errors and changing all the
{{repl }}references to environment variables.This also improves upon the wording and gives troubleshooting steps in an effort to reduce the (small, but not insignificant) number of "help, my installation-status pod is in an errored state" issues we see raised.
Related Issue(s)
Fixes #12197
Fixes #12259
How to test
Install via KOTS.
I've checked this against all permutations that I can work out (k3s, AKS, EKS, GKE) and works on all.
Release Notes
Documentation
Werft options: