-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Conversation
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.
Works for me, as a juggle. I agree that the "old" Helm docs need a thorough rewrite, too.
I think Stefan will have thoughts on moving content out of the Helm chart README, so let's wait for his review.
Maybe we can explain the Helm integration in another prominent space where we detail the inner workings of Flux?
Yes, this is definitely needed. I have held off doing it because I'm not sure we have the right design yet. I'm still not sure, but enough people ask about it, that's it's worth doing anyway.
Maybe we can just document how to write the CRD in the flux-helm-test repo? That would slim down the docs even further.
I think we should document it here. The example repo (ideally not flux-helm-test, which was intended just for testing that it works, is a mishmash) can be used as an example, of course.
README.md
Outdated
- Operating Flux | ||
- [Using Flux](/site/using.md) | ||
- [the Helm Operator](/site/helm-operator.md) |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
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've added some suggestions
chart/flux/README.md
Outdated
- Charts are colocated under another path ("charts" by default; can be | ||
overriden). Charts are subdirectories under the charts path. | ||
- Custom Resource namespace reflects where the release should be done. | ||
Both the Helm application and its corresponding Custom Resource will |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
site/helm-integration.md
Outdated
metadata: | ||
name: mongodb | ||
namespace: myNamespace | ||
labels: |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
site/helm-integration.md
Outdated
|
||
- name | ||
- namespace | ||
- label.chart ... the same as chartgitpath, with slash replaced with an underscore |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
site/helm-integration.md
Outdated
- chartGitPath ... path (from repo root) to a Chart subdirectory | ||
|
||
|
||
## Optional fields |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
Thanks @stefanprodan Closes #1357 Signed-off-by: Daniel Holbach <[email protected]>
@@ -211,191 +236,4 @@ Update Weave Flux version with: | |||
helm upgrade --reuse-values flux \ | |||
--set image.tag=1.3.2 \ | |||
weaveworks/flux | |||
``` | |||
|
|||
### Installing Weave Flux helm-operator and Helm with TLS enabled |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
@squaremo Do you want me to file an issue for this?
@squaremo An issue for that too? I addressed the other remarks so far and wait for Stefan's go-ahead wrt moving some docs out of the chart README. |
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'm ok with the TLS setup being moved out of the Helm readme. Looks good, thanks Daniel
Thanks for the review. I'll file separate issues to track the other bits which are still missing. |
As discussed in #1205 (and #1178), the Helm docs are not as clear as they should be. We have
Almost all in different places.
This PR is a draft of how I'd like to change things:
As I don't know all the details, it'd be good if we would review the content as well. I just juggled things around to be more condensed.
Thoughts:
flux-helm-test
repo? That would slim down the docs even further.