Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

docs: complete overhaul and switch to mkdocs #321

Merged
merged 39 commits into from
Mar 25, 2020
Merged

docs: complete overhaul and switch to mkdocs #321

merged 39 commits into from
Mar 25, 2020

Conversation

hiddeco
Copy link
Member

@hiddeco hiddeco commented Mar 1, 2020

This PR (or rather mega-PR) catches up with all changes made to the Helm Operator that were still undocumented.

It introduces new get started guides including a quickstart guide, a refined HelmRelease guide including explanations and guides on more advanced topics, updated FAQ questions, embedded Helm chart documentation generated from the chart's README.md, auto-generated API / CRD reference, and much more.

It also switches from sphinx to mkdocs due to various issues we had with the Markdown support of sphinx.

Rendered documentation preview: https://test-helm-op.readthedocs.io
Addresses: #222, #293, #316, #327, #94

@hiddeco hiddeco added the docs Issue or PR related to documentation label Mar 1, 2020
@hiddeco hiddeco force-pushed the docs/revamp branch 9 times, most recently from 5a7f1ce to 1d08578 Compare March 5, 2020 11:21
@hiddeco hiddeco force-pushed the docs/revamp branch 4 times, most recently from 7f4cbf0 to 90031fb Compare March 23, 2020 22:47
@hiddeco hiddeco force-pushed the docs/revamp branch 2 times, most recently from 7d9284d to 898aef4 Compare March 24, 2020 10:24
delete/recreate if needed. Defaults to `false` when omitted.
* `recreate` _(Optional)_: When set to `true`, performs pods restart for the
resource if applicable. Defaults to `false` when omitted.
* `timeout` _(Optional)_: Time to wait for any individual Kubernetes operation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not mistaken, this is the timeout for the rollback operation, not the duration after which the operator triggers a rollback. Can you please make it clear people should use .spec.timeout for that purpose?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under the impression that this would be clear by it being a nested key under rollback.

Would

Time to wait for any individual Kubernetes operation that is part of the rollback

be more clear to you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, clearer. My point is that it should not be mistaken for .spec.timeout, which is mostly what users care about I believe.

Copy link
Member Author

@hiddeco hiddeco Mar 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing that comes to mind now is that people will likely want to increase the terminationGracePeriod on the Helm Operator pod when they set the timeout to a high value to prevent releases from getting stuck in a faulty state.

Thanks for the suggestion!

the value of the annotation equals to `<namespace>:helmrelease/<name>`.

The purpose of this annotation is to indicate that the cause of that resource
is a `HelmRelease`. It is also functions as a safe guard during reconciliation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
is a `HelmRelease`. It is also functions as a safe guard during reconciliation
is a `HelmRelease`. It also functions as a safe guard during reconciliation

- item2
```

## Values from sources
Copy link
Contributor

@sa-spag sa-spag Mar 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please mention the downside of this feature is that changes made on these values take more time to be propagated (as per #151)? Or perhaps mention it under "Reconciliation".

docs/faq.md Outdated
1. Increase the [various polling intervals](references/operator.md) based on
what your environment really needs:

- **`--status-update-interval`:** The interval at which Helm is polled for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- **`--status-update-interval`:** The interval at which Helm is polled for
- `--status-update-interval`: The interval at which Helm is polled for

is detected the Helm Operator will collect all `HelmRelease` resources making
use of the mirror and inspect if the change updates the chart at the `path`
given, when this is true it will schedule a new release and an upgrade will
follow.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add a note to suggest users to use Git tags to version their charts.

@sa-spag
Copy link
Contributor

sa-spag commented Mar 24, 2020

Prometheus metrics are no longer documented. Is that intended?

@hiddeco
Copy link
Member Author

hiddeco commented Mar 24, 2020

@sa-spag I removed them during the initial rewrite / clean up as I was working in parallel on #334 which will extend those metrics. I will re-add a placeholder page under references.

@sa-spag
Copy link
Contributor

sa-spag commented Mar 24, 2020

What about /api/v1/sync-git endpoint? Do we want to document it?

@hiddeco
Copy link
Member Author

hiddeco commented Mar 24, 2020

Copy link
Member

@squaremo squaremo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughtful and thorough documentation is a pleasure to read, so reviewing this was ❤️

90% of my comments are tidying up punctuation and could go in one batched commit. There's just a handful that might need typing rather than a button click -- pretty good for 4000-odd lines!

README.md Outdated

The FluxCD projects adheres to the [CNCF Code of
Flux and its umbrella projects adhere to the [CNCF Code of
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"its umbrella projects" is tricky to interpret. Does it refer to other projects under fluxcd/? If so, I think it might be better to say "Flux and other projects in the FluxCD umbrella" or name them explicitly (or just name this project explicitly).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading this now I agree, I have opted for The Helm Operator and other projects in [...].

| `git.ssh.configMapKey` | `config` | The name of the key in the kubernetes config map specified above
| `chartsSyncInterval` | `3m` | Period on which to reconcile the Helm releases with `HelmRelease` resources
| `statusUpdateInterval` | `None` | Period on which to update the Helm release status in `HelmRelease` resources
| `workers` | `None` | Amount of workers processing releases
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Number" rather than "Amount" (number is for things that can be counted e.g., workers, amount is for stuff that cannot be counted, e.g., water)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️

| `logReleaseDiffs` | `false` | Helm Operator should log the diff when a chart release diverges (possibly insecure)
| `allowNamespace` | `None` | If set, this limits the scope to a single namespace. If not specified, all namespaces will be watched
| `helm.versions` | `v2,v3` | Helm versions supported by this operator instance, if v2 is specified then Tiller is required
| `tillerNamespace` | `kube-system` | Namespace in which the Tiller server can be found
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth noting that this and many following only matter for v2

docs/faq.md Outdated
[`--status-update-interval`](references/operator.md#reconciliation-configuration)).

The message is the error reported during the latest release failure. When this
failure was for example due to a validation error, the upgrade has never been
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
failure was for example due to a validation error, the upgrade has never been
failure was for example due to a validation error, the upgrade has not been
completed, and the status of the release will thus still be `deployed`.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(can't be applied as is, since it changes the next line too)

docs/faq.md Outdated
### It takes a long time before the operator processes an update to a resource. How can I speed up the processing of releases?

The operator watches for changes of Custom Resources of kind `HelmRelease`. It
receives Kubernetes Events and queues these to be processed, all resources will
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
receives Kubernetes Events and queues these to be processed, all resources will
receives Kubernetes Events and queues these to be processed. All resources will

release until `maxRetries` is reached. Defaults to `false` when omitted.
* `maxRetries` _(Optional)_: The maximum amount of retries that should be
attempted for a rolled back release. Defaults to `5` when omitted, use `0`
for an unlimited amount of retries.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for an unlimited amount of retries.
for an unlimited number of retries.

It is possible to define a list of config maps, secrets (in the same namespace
as the `HelmRelease` by default, or in a configured namespace) or external
sources (URLs) from which to take values. For charts from a Git
repository, there is an additional option available to refer to a file in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a file in ...?


The Helm Operator is a [Kubernetes operator](https://kubernetes.io/docs/concepts/extend-kubernetes/operator/),
allowing one to declaratively manage Helm chart releases. Combined with
[Flux](https://github.com/fluxcd/flux) this can be utilized to automate
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[Flux](https://github.com/fluxcd/flux) this can be utilized to automate
[Flux](https://github.com/fluxcd/flux) it can automate

The Helm Operator is a [Kubernetes operator](https://kubernetes.io/docs/concepts/extend-kubernetes/operator/),
allowing one to declaratively manage Helm chart releases. Combined with
[Flux](https://github.com/fluxcd/flux) this can be utilized to automate
releases in a GitOps manner, but the usage of Flux is not a strict
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
releases in a GitOps manner, but the usage of Flux is not a strict
releases in a GitOps manner, but the use of Flux is not a strict

releases in a GitOps manner, but the usage of Flux is not a strict
requirement.

The desired state of a Helm release is described through a Kubernetes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a concise and lucid explanation of the operator 💯

Copy link
Member

@squaremo squaremo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughtful and thorough documentation is a pleasure to read, so reviewing this was ❤️

90% of my comments are tidying up punctuation and could go in one batched commit. There's just a handful that might need typing rather than a button click -- pretty good for 4000-odd lines!

@hiddeco hiddeco merged commit cc1fbe2 into master Mar 25, 2020
@hiddeco hiddeco deleted the docs/revamp branch March 25, 2020 14:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
docs Issue or PR related to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants