Simplify fluxyaml reference #2634
Conversation
squaremo
force-pushed
the
docs-elaborate-on-fluxyaml
branch
from
1789ab2 to
d16d0e5
Compare
|
Really nice! By now it has become clear that this document was really confusing for users. Part of the problem is that it was extracted from a design document, meant to be precise and not necessarily user-friendly. I will take a deep look tomorrow |
I think it's fine to write it in a friendlier way. We already have the design document if we ever want to look at (which I doubt). |
|
|
||
| * with minimal replication of resource definitions | ||
| * while keeping Flux neutral about the factorization technology used | ||
| - if no `.flux.yaml` file is found, the usual behaviour of looking |
There was a problem hiding this comment.
It may be worth briefly describing the usual behavior
There was a problem hiding this comment.
"looking for YAML files" is already a brief explanation of the usual behaviour, isn't it?
There was a problem hiding this comment.
How about "the usual behaviour of recursively reading all YAML files under the target path."
| - command: kustomize build . | ||
| patchFile: flux-patch.yaml | ||
| ``` | ||
| In addition, `updaters` are given arguments via environment variables: |
There was a problem hiding this comment.
Nitpick:
| In addition, `updaters` are given arguments via environment variables: | |
| In addition, the `updaters` of the `commandUpdated` configuration are given arguments via environment variables: |
There was a problem hiding this comment.
I would include an example of kustomize edit set image
There was a problem hiding this comment.
I did not attempt to give a working example of commandUpdated updaters, because the thought of trying to formulate one terrified me. kustomize edit set imagetag sets the image for all things that use the image, last time I looked.
There was a problem hiding this comment.
This looks really good. I agree with @2opremio suggestions with those in I think the docs are way better.
| - command: kustomize build . | ||
| patchFile: flux-patch.yaml | ||
| ``` | ||
| In addition, `updaters` are given arguments via environment variables: |
There was a problem hiding this comment.
I would include an example of kustomize edit set image
There was a problem hiding this comment.
As a formerly confused user I heartily endorse the new document and submit unsolicited Approval.
In addition to general awesomeness, I like that patch-updated is described first, as the easier and more common option. I also like that the useless helm example was removed.
I was initially on the fence about the use of an example (line 74) which does not produce a usable configuration. But it does demonstrate a number of important points about .flux.yaml placement so I came around to it.
squaremo
force-pushed
the
docs-elaborate-on-fluxyaml
branch
from
0d14550 to
10341ce
Compare
🙏 Thank you Eric, as a formerly confused user, your opinion is very welcome here! |
| * while keeping Flux neutral about the factorization technology used | ||
| The command-line flag `--git-path` (which can be given multiple | ||
| values) marks a "target path" within the git repository in which to | ||
| find manifests. If `--git-path` is not supplied, the top of the git |
There was a problem hiding this comment.
Maybe it's worth indicating that multiple --git-paths can be provided?
There was a problem hiding this comment.
You mean like saying "which can be given multiple values"?
There was a problem hiding this comment.
Yes. Note that it's also valid to provide multiple --git-path arguments (pflag will coalesce them)
There was a problem hiding this comment.
Yup, that's what I mean by "provide multiple values" -- use the flag to provide multiple values. I don't want to get into how the flag parsing works in this doc though.
| take effect for `--git-path=staging`. However: | ||
|
|
||
| Using `--git-path=production` would **not produce a usable | ||
| configuration**, because without an applicable `.flux.yaml`, the files |
There was a problem hiding this comment.
Even if redundant, I think it's worth indicating that a .flux.yaml file cannot be found in that case because there is non in /production or its parent directories (/)
This rewrites the first half of the document describing manifest generation, in pursuit of these goals: - be clear, up front, about how --git-path and .flux.yaml interact, since people tend to misunderstand that; - write more in terms of how to operate the feature, rather than how it works; - explain patchUpdated, which should be preferred in most situations as less complicated, first.
squaremo
force-pushed
the
docs-elaborate-on-fluxyaml
branch
from
10341ce to
44f20f1
Compare
This rewrites the first half of the document describing manifest
generation, in pursuit of these goals:
be clear, up front, about how --git-path and .flux.yaml interact,
since people tend to misunderstand that;
write more in terms of how to operate the feature, rather than how
it works;
explain patchUpdated, which should be preferred in most situations
as less complicated, first.
I set out to include the same information as the original. Although it's a reference, I wrote fairly conversationally. This might mean it's more appropriate to put this elsewhere, and restore the material here as a reference. I'm happy to take advice.
This is intended to address, in part, #2550