Skip to content

Commit

Permalink
docs: check updates for the Helm reference
Browse files Browse the repository at this point in the history
Cilium's documentation has a reference for the Helm values supported in
its Charts. The reference is auto-generated, and is supposed to be
updated each time the Charts are modified. To help keep the reference
up-to-date, Cilium's CI should warn when developers forgot to regenerate
and commit the document.

Because the CI reported missing updates in the past, we thought that
this was covered. But it turns out that the CI would only complain when
it _fails_ to update the Helm reference - typically when some words need
to be added to the spelling list. If the update goes fine, there is no
check in place to validate that the regenerated file is identical to the
one currently in the repository. This has led to multiple PRs missing
the update for the Helm reference in the past.

Address the issue by adding a check-helmvalues.sh script to validate
that the current file is identical to the version in Git's HEAD. Run
this script from the Makefile, as part of the "check" target.

We also create a "update-helm-values" target, which looks cleaner to add
as a prerequisite for "check" instead of passing the name of a .rst
file. We also introduce a "FORCE" phony target to explicitly mark that
we want the file regenerated each time.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
  • Loading branch information
qmonnet authored and kkourt committed Oct 22, 2021
1 parent d78e721 commit 6167f8a
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 6 deletions.
1 change: 1 addition & 0 deletions CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ Makefile* @cilium/build
/Documentation/check-cmdref.sh @cilium/docs-structure
/Documentation/check-crd-compat-table.sh @cilium/docs-structure
/Documentation/check-examples.sh @cilium/docs-structure
/Documentation/check-helmvalues.sh @cilium/docs-structure
/Documentation/cilium_operator.rst @cilium/operator
/Documentation/cmdref/ @cilium/nonexistantteam
/Documentation/commit-access.rst @cilium/contributing @cilium/docs-structure
Expand Down
25 changes: 20 additions & 5 deletions Documentation/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
include ../Makefile.defs
include ../Makefile.quiet

HELM_VALUES := helm-values.rst

.PHONY: default clean builder-image cilium-build cmdref epub latex html run-server stop-server

default: html
Expand Down Expand Up @@ -37,9 +39,11 @@ update-cmdref: builder-image cilium-build
-$(QUIET)rm -rf cmdref/cilium*.md
$(QUIET)$(DOCKER_RUN) ./update-cmdref.sh

check: builder-image update-cmdref
check: builder-image update-cmdref update-helm-values
@$(ECHO_CHECK) cmdref
$(QUIET) ./check-cmdref.sh
@$(ECHO_CHECK) $(HELM_VALUES)
$(QUIET) ./check-helmvalues.sh
@$(ECHO_CHECK) examples
$(QUIET)$(DOCKER_RUN) ./check-examples.sh

Expand All @@ -51,7 +55,18 @@ copy-api:
@$(ECHO_GEN)_api
$(QUIET)cp -r ../api _api

HELM_VALUES = helm-values.rst
# $(HELM_DOCS_IMAGE), necessary to update the reference for Helm values,
# attempts to run a Go binary compiled for x86_64. Skip the update on other
# architectures by making update-helm-values an empty target, unless the user
# passes a compatible image.
HELM_VALUES_DEP := $(HELM_VALUES)
ifneq ($(shell uname -m),x86_64)
ifeq ($(origin HELM_DOCS_IMAGE), file)
$(info Documentation: skipping update for the Helm reference (image needs x86_64))
HELM_VALUES_DEP :=
endif
endif
update-helm-values: $(HELM_VALUES_DEP)

HELM_DOCS_ROOT_PATH := $(DOCKER_CTR_ROOT_DIR)
HELM_DOCS_CHARTS_DIR := $(HELM_DOCS_ROOT_PATH)/install/kubernetes
Expand All @@ -63,17 +78,17 @@ M2R_SHA := a3dc2c3fcdc31260dcb7cd42e39763281a6fdce4ca59b4c150fb3bfff5be9eb7
M2R_IMAGE := docker.io/bmcustodio/m2r:$(M2R_VERSION)@sha256:$(M2R_SHA)
M2R := $(DOCKER_CTR) $(M2R_IMAGE) m2r

.PHONY: $(HELM_VALUES)
.PHONY: update-helm-values FORCE
$(HELM_VALUES): TMP_FILE_1 := helm-values.tmp
$(HELM_VALUES): TMP_FILE_2 := helm-values.awk
$(HELM_VALUES):
$(HELM_VALUES): FORCE
$(QUIET)$(HELM_DOCS) -d -c $(HELM_DOCS_CHARTS_DIR) -t $(HELM_DOCS_OUTPUT_DIR)/$(TMP_FILE_1).tmpl > $(TMP_FILE_1)
$(QUIET)awk -F'|' '{print "|"$$2"|"$$5"|"$$3"|"$$4"|"}' $(TMP_FILE_1) > $(TMP_FILE_2)
$(QUIET)$(M2R) --overwrite $(TMP_FILE_2)
$(QUIET)printf '..\n %s\n\n%s\n' "AUTO-GENERATED. Please DO NOT edit manually." "$$(cat $@)" > $@
$(QUIET)$(RM) -- $(TMP_FILE_1) $(TMP_FILE_2)

epub latex html: builder-image copy-api $(HELM_VALUES)
epub latex html: builder-image copy-api update-helm-values
@$(ECHO_GEN)_build/$@
$(QUIET)$(DOCKER_RUN) ./check-build.sh $(@) $(SPHINX_OPTS)

Expand Down
14 changes: 14 additions & 0 deletions Documentation/check-helmvalues.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/usr/bin/env bash

set -o errexit
set -o nounset
set -o pipefail

script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
helm_values="${script_dir}/helm-values.rst"

if ! git diff --quiet -- "${helm_values}" ; then
git --no-pager diff "${helm_values}"
echo "HINT: to fix this, run 'make -C Documentation update-helm-values'"
exit 1
fi
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ install-manpages: ## Install manpages the Cilium CLI.
mandb

postcheck: build ## Run Cilium build postcheck (update-cmdref, build documentation etc.).
$(QUIET)$(MAKE) $(SUBMAKEOPTS) -C Documentation update-cmdref check
$(QUIET)$(MAKE) $(SUBMAKEOPTS) -C Documentation check

licenses-all: ## Generate file with all the License from dependencies.
@$(GO) run ./tools/licensegen > LICENSE.all || ( rm -f LICENSE.all ; false )
Expand Down

0 comments on commit 6167f8a

Please sign in to comment.