-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Generate mainfests automatically from Helm #4278
Conversation
8a8c717
to
19cc9bb
Compare
Codecov Report
@@ Coverage Diff @@
## main #4278 +/- ##
=======================================
Coverage 51.97% 51.97%
=======================================
Files 59 59
Lines 16962 16962
=======================================
Hits 8816 8816
Misses 7849 7849
Partials 297 297
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
697a111
to
1ff8b7a
Compare
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.
Docs-related changes LGTM!
3a20bd6
to
4fc8248
Compare
cfd90f4
to
1512776
Compare
4d2d3c9
to
5229fc0
Compare
f448169
to
0909984
Compare
charts/nginx-ingress/README.md
Outdated
@@ -100,15 +100,15 @@ CRDs](#upgrading-the-crds). | |||
To upgrade the release `my-release`: | |||
|
|||
```console | |||
helm upgrade my-release oci://ghcr.io/nginxinc/charts/nginx-ingress --version 1.0.1 | |||
helm upgrade my-release -n nginx-ingress oci://ghcr.io/nginxinc/charts/nginx-ingress --version 1.0.1 |
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.
according to old command the installation was in default ns and now user is running upgrade in a different ns? I understand that these command as a guide but most users usually straight-up copy-paste and upgrade will fail due to ownership of clusterroles as well.
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.
How do you propose we change it?
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.
by leaving it to default? and giving user the option to set namespace maybe
6260c58
to
b0fc3f9
Compare
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.
looks good.
This will be great to provide a singe .yaml file for customers to use for helm upgrdes.
4638bc0
to
2e9caa4
Compare
Uses Helm Charts in examples/helm-chart to template single file manifests in deploy/.
2e9caa4
to
e7b5228
Compare
Proposed changes
Uses Helm Charts in examples/helm-chart to template single file manifests in
deploy/
.