feat: Automate helm docs

[souleb]

This PR is about automating the helm docs generation for a given helm chart version. It relies on helm-docs.

A template is provided in order to generate heml.rst, the reStructuredText formatted doc. This template has access to all the metadata in the chart's requirements.yaml, and Chart.yaml as well as the chart values. The advantage here is that any changes to those special files in the chart or in the template itself, can automatically generate an up to date documentation.

Note As we want to avoid any manual action i.e. copy for the helm chart values, the values.yaml is the source of truth and has to be update to contain all needed information. One critical missing part is the fields descriptions. We can also set markers in it in order to hide some fields.

TO-DO

• Create a Makefile to install helm-docs, fetch the right chart version and generate helm.rst
• Provide a helm.rst.gotmpl that can generate the right helm.rst
• Update the github workflows in order to automatically regenerate the helm.rst before publishing a new version
• Update the helm chart to hold the fields descriptions

Edit: Depends on Mellanox/network-operator#1011

[adrianchiris]

some feedback:

we should generate helm rst doc but should not blindly publish it on merge in workflow, it can easily go south when autogenerating from comments added by devs, without any review in the middle this will most likely get missed (or caught too late)

1. having a generate-helm-doc makefile target to generate doc from helm chart + template
2. this should be integrated into network operator CD pipeline (after the chart was generated and pushed, or provided via path to local chart). ending up with a PR to this repo which updates helm rst doc
3. helm update PR is reviewed and merged like any other doc change
4. eventually a release is tagged in networo-operator-docs proj.

[souleb]

Thanks @adrianchiris for the feedback.

we should generate helm rst doc but should not blindly publish it on merge in workflow, it can easily go south when autogenerating from comments added by devs, without any review in the middle this will most likely get missed (or caught too late)

I agree with your concern here. But I think we should always regenerate the helm.rst before publishing to make sure we have an up to date version. Otherwise, editing manually after merging a PR that update the helm.rst doc can still leave it out of sync. Also, I think that the helm.rst should live in the same repo that generates it to have a simpler workflow.

So what about this update to your list? :

1. having a generate-helm-doc makefile target to generate doc from helm chart + template
2. A makefile target that runs on every PR to regenerate the doc(helm.rst). This makes sure that the doc is part of the PR review, and it should only change if the chart itself is updated.
3. This should be integrated into network operator repository. So helm.rst will be part of the network operator release tarball. We avoid creating PRs to the network-operator-docs repo, and have only a single file to track for this.
4. eventually a release is tagged in network-operator-docs repo. As part of the release pipeline workflow, the helm.rst is pulled from the corresponding network operator release

[adrianchiris]

Thanks @adrianchiris for the feedback.

we should generate helm rst doc but should not blindly publish it on merge in workflow, it can easily go south when autogenerating from comments added by devs, without any review in the middle this will most likely get missed (or caught too late)

I agree with your concern here. But I think we should always regenerate the helm.rst before publishing to make sure we have an up to date version. Otherwise, editing manually after merging a PR that update the helm.rst doc can still leave it out of sync. Also, I think that the helm.rst should live in the same repo that generates it to have a simpler workflow.

So what about this update to your list? :

1. having a generate-helm-doc makefile target to generate doc from helm chart + template
2. A makefile target that runs on every PR to regenerate the doc(helm.rst). This makes sure that the doc is part of the PR review, and it should only change if the chart itself is updated.
3. This should be integrated into network operator repository. So helm.rst will be part of the network operator release tarball. We avoid creating PRs to the network-operator-docs repo, and have only a single file to track for this.
4. eventually a release is tagged in network-operator-docs repo. As part of the release pipeline workflow, the helm.rst is pulled from the corresponding network operator release

Hey @souleb ! Here are some point i had in mind (which i guess we disagree on :) ):

• i see network-operator project as a source of "metadata" used for generating docs in this repo (e.g component version, helm chart comments etc...). doc artefacts remain here.
• i see our release (CD) pipeline updating documentation for helm chart and versions when releasing new version of network operator (beta, rc , GA...) by: running some makefile target (e.g make generate-helm-doc) then submitting PR to this project to be reviewed and merged. (manual fixes may be needed both here and in the chart until we dial it in)
• once we see its all working well, CD pipeline can push directly (or just merge it own PR)
• as a developer writing docs for a specific feature i dont want my PR to contain distracting information (e.g updates to helm.rst)

some of my comments reviewing this PR are related to the points above

[souleb]

• i see our release (CD) pipeline updating documentation for helm chart and versions when releasing new version of network operator (beta, rc , GA...) by: running some makefile target (e.g make generate-helm-doc) then submitting PR to this project to be reviewed and merged. (manual fixes may be needed both here and in the chart until we dial it in)
• once we see its all working well, CD pipeline can push directly (or just merge it own PR)

So every time a developer makes change to the api and the helm chart in network-operator, he will have to deal with 2 PRs, the fist in network-operator and a second in network-operator-docs? If I understand, the second PR will be generated once the first is merged. So if there is an issue, the developer will have to resubmit a PR in network-operator to fix it.

• as a developer writing docs for a specific feature i dont want my PR to contain distracting information (e.g updates to helm.rst)

This is not distracting, because this will be part of the PR only if there are changes in the helm chart or doc template? And this way we have the fastest feedback loop and can avoid back and forth between 2 repositories.

See: Mellanox/network-operator#1011 for latest state

[souleb]

Do not merge before Mellanox/network-operator#1011

[adrianchiris]

just making sure:

if i run
HELM_CHART_DEP_ROOT=/path/to/my/local/network-operator/deployment/network-operator make helm-docs

it will work right ?

[souleb]

yes, but there are two assumptions here:

• HELM_CHART_DEP_ROOT should contain your chart
• .helmdocsignore targets $(BUILDDIR)/helmcharts, so if you target another path, it won't be taken into account.

[adrianchiris]

can we add support for that ? does it make sense ?

what i was thinking:

1. dev works on some feature in network operator, eventually updates chart and submits PR
2. dev generates updated helm.rst from his local network operator branch and submits PR to network-operator-docs

[souleb]

the content of the .helmdocsignore as to be relative it seems. I wanted to use realpath to make it works, but this isn't POSIX and have different behaviour between linux and mac... I don't have a solution so far for this.

On second thoughts, if the user provides his Branch and Fork url, he can use make branch-helm-doc just fine.

[rollandf]

Maybe have another env PARAM like 'LOCAL_HELM_SOURCE_DIR`, and if it exists, copy content to build/_output/helmcharts/network-operator/charts ?

[souleb]

let's take some time to think about this one. For now what I think is that a single repo would enable that easily :)

[adrianchiris]

let me followup with PR on generating from local sources. then each can pick an choose. thx for working on this one @souleb !

[adrianchiris]

left a small question otherwise LGTM

once Mellanox/network-operator#1011 is merged, can you followup with a PR to update helm.rst ?

[souleb]

once Mellanox/network-operator#1011 is merged, can you followup with a PR to update helm.rst ?

This PR depends on it. Once it is merged, I'll update helm.rst here by running make branch-helm-docs