Exclude Helm manifests in get-started guide #1902
Conversation
|
I think we should make it more clear to the user that installation of the Helm operator is optional (and only required because the If the user now follows these steps, but for example, has not installed Tiller on the cluster (because we did not tell them to), the apply of the |
|
That's true. Then, we either change the example repository to one without HelmReleases or we add instructions to install Tiller and indicate that helm is optional. Thoughts? |
|
BTW, how could the guide have possiblly ever worked? I'll do some digging |
|
From the Head was right: #1527 |
I think the latter would be good, with a note saying that for demo purposes we install the operator too (and thus Tiller must be installed) but it is fine to only make use of the Flux daemon. |
|
Hum. Maybe I'm missing something, but AFAICS the get started tutorial is about "flux standalone" (cf this note), whereas the helm get started tutorial is about helm: the need to install Tiller is discussed there. |
Correct, the problem is that the repository linked in this doc (and used as an example) contains |
|
We can also change the repository.
…On Fri, Apr 5, 2019, 12:06 Hidde Beydals ***@***.***> wrote:
AFAICS the get started tutorial is about "flux standalone"
Correct, the problem is that the repository linked in this doc (and used
as an example) contains HelmRelease resources, confusing users (see
linked issue).
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1902 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ACQOJCzeAzdeO7gHxTEkKgFp-mHEgYtzks5vdyA0gaJpZM4cdEfK>
.
|
|
This is unfortunate. It'd be great if we could
It looks like we can't easily have it all this time. My inclination would be to merge this PR, but make a note that we at least document why this is necessary in the future, and even better: make the use of the example repo here more "obvious"? |
We can also restrict the repo to directories not including |
2opremio
force-pushed
the
helm-op-get-started
branch
from
ac8051d
to
f451677
Compare
2opremio
changed the title
2opremio
force-pushed
the
helm-op-get-started
branch
from
f451677
to
0ca2796
Compare
|
Thanks for the fix - this looks clean. (Even if it adds another step to the "get started" experience - I guess I'll file a separate issue about this.) Have you tried this locally? In my case the new podinfo gets deployed, but for some reason the color does not change. |
It does work for me. Do the logs show anything odd? I don't see why any of the changes I made would cause flux not to update the deployment correctly. |
|
Just started with a fresh minikube, same thing. The correct revision is deployed. Colour stays green. |
|
I will take a deeper look. However I doubt that's caused by this PR. Does it work when deploying helm and the helm op? |
|
The tutorials follow different paths:
|
|
Maybe we can just copy https://github.com/weaveworks/flux/blame/master/site/helm-get-started.md#L147 (147 and following) over to |
|
/cc @stefanprodan on the above |
|
Or on a second thought, it should. https://github.com/stefanprodan/k8s-podinfo/blob/7d5200a78a6206c69e761be4c8791afe9427895d/cmd/podinfo/main.go#L33 |
|
Can me merge this and create a separate issue for the color? |
I retried and it didn't, I would had sworn it did. ¯_(ツ)_/¯ Sorry @dholbach |
|
Let's copy over the Helm Get Started bits into the other tutorial in a separate issues. |
|
Is this ready to merge or is it blocked on something -- I'm not clear after reading the comments 😅 |
|
Yes, let's do it, and fix the second part of it in a separate PR. |
The get started guide uses repo github.com/weaveworks/flux-get-started
which contains HelmRelease manifests, but the guide didn't instruct
the reader to deploy the Helm operator.
Fixes #1898