-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[kots]: add the KOTS installation manifests #8395
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
Conversation
a52c201
to
6ab3fd6
Compare
Codecov Report
@@ Coverage Diff @@
## main #8395 +/- ##
==========================================
- Coverage 15.06% 11.17% -3.89%
==========================================
Files 51 18 -33
Lines 4899 993 -3906
==========================================
- Hits 738 111 -627
+ Misses 4089 880 -3209
+ Partials 72 2 -70
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
f18f93b
to
23d39ac
Compare
9ae983f
to
f87fe10
Compare
helm: | ||
@echo "Installing Helm dependencies" | ||
@rm -f manifests/*.tgz | ||
@for f in $(shell ls -d charts/*/); do cd $${f} && helm dep up && helm package . --destination ../../manifests && cd -; done |
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.
Instead of this helm magic, couldn't we just download the cert-manager package like this?
$ curl -sSLO https://charts.jetstack.io/charts/cert-manager-v1.7.0.tgz
Or wouldn't this work for some reason?
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.
That would work, but I've done it as a general function in case we ever put a second Helm chart in there - we use external-dns
in all the guides which might be worth offering in Kots in future
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.
It's awesome that we have a generic solution for this problem at hand. To be honest, I would prefer the simple curl
command, for now, to keep it simple. With that, we wouldn't need to add (and maintain) helm
to the workspace image right now.
I know it's not easy to part with such an elegant, generic solution once you've poured it into code. However, this still exists in the archived gitpod-io/replicated
repo and we can release it from there at any time as soon as we need it. You're not forgotten you beautiful code. We remember you! Promise! 👋
What do you think?
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.
Oh @corneliusludmann, you'll make me blush 😊
Again, as I said in the .gitpod.yml
comment - I treat these as a proposition and then we all talk about it. I'm happy with switching to a simpler solution.
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.
Right, I think that my original approach is the correct one. Using the .tgz
file provided by cert-manager has everything nested in a cert-manager
folder. For KOTS to work, it needs to be in the root of the .tgz
file - try pulling in the curl
command you used and then run make lint
to see the error.
So, that makes there two ways of achieving this:
- create a noddy
Chart.yaml
file and use thehelm package
function - download the
.tgz
file, extract it, re-compress it with thecert-manager
and use that new.tgz
file
Whilst both have their limitations, I think approach 1
is the simpler and more maintainable one. We could just put the correct .tgz
file in there, but I think that this might be forgotten when we come to do an update in future
Thoughts?
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.
Agree. When a simple curl
does not work, using helm seems to be the better approach.
f87fe10
to
a4db25c
Compare
96fdaa9
to
fd35036
Compare
fd35036
to
98f75d9
Compare
|
helm: | ||
@echo "Installing Helm dependencies" | ||
@rm -f manifests/*.tgz | ||
@for f in $(shell ls -d charts/*/); do cd $${f} && helm dep up && helm package . --destination ../../manifests && cd -; done |
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.
It's awesome that we have a generic solution for this problem at hand. To be honest, I would prefer the simple curl
command, for now, to keep it simple. With that, we wouldn't need to add (and maintain) helm
to the workspace image right now.
I know it's not easy to part with such an elegant, generic solution once you've poured it into code. However, this still exists in the archived gitpod-io/replicated
repo and we can release it from there at any time as soon as we need it. You're not forgotten you beautiful code. We remember you! Promise! 👋
What do you think?
71f1942
to
c62a6e2
Compare
c62a6e2
to
f994cc3
Compare
f994cc3
to
cdedf00
Compare
Description
This incorporates the Replicated/KOTS installation into the installation section of the repo. KOTS is a way of allowing us to manage how enterprises install our software
The change to
.gitpod.yml
is to install the Replicated CLI (to publish the manifests) and the KOTS (to install Gitpod with KOTS) pluginTested on:
AWS in-clusterAWS externalNB this does not install the external dependencies, but merely that it provisions the application in those environments
I'm proposing avoiding the AWS testing at this stage until we establish whether the errors with the
2022.02.0
release are caused by this release or (more likely) an error in the AWS setupRelated Issue(s)
Fixes #
How to test
Release Notes
Documentation