Update contributor guide and some other improvements (#804)

* Update contributor guide and some other improvements

Signed-off-by: Pete Wall <pete.wall@grafana.com>

* Fix lint. Also, re-add shellspec because we need to be able to run Helm from within it

Signed-off-by: Pete Wall <pete.wall@grafana.com>

---------

Signed-off-by: Pete Wall <pete.wall@grafana.com>

.configs/lintconf.yaml

@@ -43,3 +43,5 @@ rules:
     #     configmap.yaml
   truthy:
     level: warning
+check-version-increment: false
+helm-dependency-extra-args: --skip-refresh

.github/workflows/integration-test.yml

@@ -70,4 +70,4 @@ jobs:
         env:
           CREATE_CLUSTER: "false"
           DEPLOY_GRAFANA: "false"
-        run: ./scripts/integration-test.sh "charts/k8s-monitoring/tests/integration/${{ matrix.test }}"
+        run: ./scripts/run-integration-test.sh "charts/k8s-monitoring/tests/integration/${{ matrix.test }}"

CONTRIBUTING.md

@@ -2,51 +2,116 @@
 
 We welcome contributions and improvements! Feel free to submit PRs.
 
-The [examples](charts/k8s-monitoring-v1/docs/examples) directory contains examples of using this Helm chart, and are built from the current state
-of the chart. When you make changes, they will likely involve changes to some of those examples.
+The process for making a successful contribution is:
 
-Required tools:
+1.  Make your change
+2.  Build any generated files: `make build`
+3.  Run unit tests: `make test`
+4.  Create your PR!
+5.  Wait for checks to pass
+    -   Lint checks
+    -   Unit tests
+    -   Integration tests
+
+## Tools
+
+This repository heavily makes use of automation and tooling to generate files and run tests. The following tools are
+required to work on this repository:
 
--   [chart-testing](https://github.com/helm/chart-testing)
 -   [Helm](https://helm.sh/docs/intro/install/)
--   [helm-docs](https://github.com/norwoodj/helm-docs)
--   [kind](https://kind.sigs.k8s.io/)
--   [Grafana Alloy](https://github.com/grafana/alloy) (used for linting the generated config files)
--   [shellspec](https://github.com/shellspec/shellspec)
--   [yamllint](https://yamllint.readthedocs.io/en/stable/index.html)
--   [yq](https://github.com/mikefarah/yq/)
+-   [Docker](https://docs.docker.com/get-docker/) - Used for running some tools in containers and required for `kind`.
+-   [chart-testing](https://github.com/helm/chart-testing) - Used for linting Helm charts.
+-   [kind](https://kind.sigs.k8s.io/) - Used for creating local Kubernetes clusters for integration testing.
+-   [Grafana Alloy](https://github.com/grafana/alloy) - Used for linting the generated config files.
+-   [yamllint](https://yamllint.readthedocs.io/en/stable/index.html) - Used for linting YAML files.
+
+Some tools are optional. If they are not present on your system, the equivalent Docker image will be run. Consider
+installing them for a better experience:
+
+-   [helm-docs](https://github.com/norwoodj/helm-docs) - Used for generating Helm chart README.md files.
+-   [helm unittest](https://github.com/helm-unittest/helm-unittest) - Used for executing Helm chart unit tests.
+-   [shellspec](https://github.com/shellspec/shellspec) - Used for executing some unit tests.
+
+Each chart has a Makefile with targets to automate much of the process.
+
+## Building
+
+Lots of files in this repository are generated in order to reduce duplication and ensure consistency. To build these
+files, run `make build`.
 
-Run `make install-deps` to install all requirements (Mac supported only at the moment using Brew).
+You can also run `make build` from inside the specific Chart directory to build only the files for that Chart.
 
-## Building generated files
+These files are generated and any changes should be included in your PR:
 
-After you have made your changes, ensure that you build the automatically generated files in this chart, by doing the
-following:
+-   `README.md` - The README file for the chart, this is generated from the values.yaml file.
+-   `CHART.lock` - The lock file for the chart dependencies, this is generated from the Chart.yaml file.
+-   `values.schema.json` - The JSON schema for the values.yaml file, this is generated from the values.yaml file and any
+  modification files found in the `schema-mods` directories.
+-   `docs/examples` - The examples directory contains usage examples that are rendered from the chart. This works in
+  the `k8s-monitoring` and `k8s-monitoring-v1` charts.
 
--   Use [Helm Docs](https://github.com/norwoodj/helm-docs) to check for updates to the chart documentation
-    -   `helm-docs` OR `cd charts/k8s-monitoring-v1 make README.md`
--   Check for updates to the example outputs
-    -   `make test`
-    -   If changes are acceptable, regenerate the outputs and re-test:
-    -   `make regenerate-example-outputs test`
+Some charts will also generate additional templates and docs based on other files. For example, the
+`feature-integrations` chart creates docs and templates based on each supported integration's own values.yaml file.
 
-## Bumping Dependent Chart Versions
+### Updating chart dependencies
 
-Chart dependencies are automatically updated via PRs, but if you want to manually set a chart dependency version:
+If your chart is dependent on external charts, you can update the dependencies by:
+
+1.  Set the dependency's version in Chart.yaml.
+2.  Update the Chart.lock file by running `make -C charts/<the chart you are modifying> build`.
+
+### Updating feature dependencies
+
+If you made a change to a feature Helm chart, you'll also need to update the main `k8s-monitoring` Helm chart to include
+your changes. To do this, run the following command:
+
+```bash
+rm charts/k8s-monitoring/Chart.lock
+make -C charts/k8s-monitoring build
+```
 
--   Set the dependency's version in [Chart.yaml](charts/k8s-monitoring-v1/Chart.yaml).
--   Update the Chart.lock file by running `cd charts/k8s-monitoring-v1 helm dependency update`
--   Follow the steps above in [Building generated files](#building-generated-files) to update the examples and docs.
--   Finally, take a moment to inspect the generated output for anything that might cause trouble.
+Include the modified chart bundles and Chart.lock file in your PR.
 
 ## Testing
 
-You can test your changes with a similar platform to what is used in the CI/CD pipelines.
+Each chart contains unit tests and some contain integration tests. Unit tests run quickly and test parts of the chart
+that don't require a Kubernetes cluster. Integration tests require a Kubernetes cluster and test the chart in a real
+environment with dependencies.
+
+### Unit tests
+
+Unit tests can be run with `make test`. You can also run `make test` from inside the specific Chart directory to run
+only the tests for that Chart.
+
+### Integration tests
+
+Integration tests run with a Kubernetes Cluster. The process for running the test is different for the v1 and v2 charts.
+
+### `k8s-monitoring-v1` integration tests
+
+To run the integration tests for the `k8s-monitoring-v1` chart, use the `setup-local-test-cluster.sh` script to build a
+Kubernetes cluster using kind and deploy the telemetry data sources, data stores and Grafana. If you provide a values
+file as an argument to that script (i.e. `setup-local-test-cluster.sh values.yaml`), it installs the k8s-monitoring Helm'
+chart and runs helm test as well.
+
+### `k8s-monitoring` integration tests
+
+To run the integration tests for the `k8s-monitoring` chart, use the following commands:
+
+```bash
+./scripts/run-integration-test.sh charts/k8s-monitoring/tests/integration/<test dir>
+```
+
+This will create a new Kubernetes cluster using kind, deploy any required dependencies, deploy the `k8s-monitoring` Helm
+chart, and deploy the `k8s-monitoring-test` chart which contains the tests.
+
+You can modify the behavior of the test by setting environment variables:
 
-To build the cluster, use the [setup-local-test-cluster.sh](charts/k8s-monitoring-v1/test/setup-local-test-cluster.sh) script to build a
-Kubernetes cluster using [kind](https://kind.sigs.k8s.io/) and deploy the telemetry data sources, data stores and
-Grafana. If you provide a values file as an argument to that script (i.e. `setup-local-test-cluster.sh values.yaml`), it
-installs the k8s-monitoring Helm chart and runs `helm test` as well.
+-   `DELETE_CLUSTER` - If set to `false`, the cluster will not be deleted after the test completes.
+-   `CREATE_CLUSTER` - If set to `false`, the script will not create a new cluster, but will use whatever cluster your
+  kubectl is configured to use.
+-   `DEPLOY_GRAFANA` - If set to `false`, the script will not deploy Grafana. Grafana is only deployed to make debugging
+  easier, because it will be pre-configured to connect to the database instances.
 
 ## Creating new releases
 
@@ -56,28 +121,28 @@ The process for creating a new release is:
 
 1.  Update the chart version and push to main
 2.  Ensure the CI tests on main are successful
-3.  Run [release.sh](./scripts/release.sh) to start the Release workflow.
+3.  Run the appropriate release GitHub Action workflow.
 
 ### Updating the chart version
 
-Use the [set-version.sh](./scripts/set-version.sh) script to update the version and regenerate the files that contain
-version numbers. For example:
+Use the [set-version.sh](./charts/k8s-monitoring-v1/scripts/set-version.sh) script to update the version and regenerate
+the files that contain version numbers. For example:
 
 ```bash
 % ./scripts/set-version.sh 1.2.3
 ```
 
 This will also set the App Version to the latest release of
 the [Kubernetes Monitoring](https://grafana.com/solutions/kubernetes/) in Grafana Cloud. If you do not have access to
-the GitHub repository, you can set the App version in the second argument to the set-version.sh script. For example:
+that GitHub repository, you can set the App version in the second argument to the set-version.sh script. For example:
 
 ```bash
 % ./scripts/set-version.sh 1.2.3 3.1.2
 ```
 
 ### Starting the release workflow
 
-The [helm-release.yml](./.github/workflows/helm-release-v1.yml) GitHub workflow handles the details of packaging the Chart,
-creating the release on this repository, creating a release on
+The [helm-release-v1.yml](./.github/workflows/helm-release-v1.yml) GitHub workflow handles the details of packaging the
+v1 Chart, creating the release on this repository, creating a release on
 the [grafana/helm-charts](https://github.com/grafana/helm-charts) repository, and finally updating the Helm chart
 repository.

charts/feature-annotation-autodiscovery/Makefile

@@ -30,7 +30,7 @@ build: README.md Chart.lock values.schema.json $(UPDATECLI_FILES)
 .PHONY: test
 test: build
 	helm lint .
-	ct lint --lint-conf ../../.configs/lintconf.yaml --check-version-increment=false --helm-dependency-extra-args=--skip-refresh --charts .
+	ct lint --lint-conf ../../.configs/lintconf.yaml --helm-dependency-extra-args=--skip-refresh --charts .
 ifdef HAS_HELM_UNITTEST
 	helm unittest .
 else

charts/feature-application-observability/Makefile

@@ -30,7 +30,7 @@ build: README.md Chart.lock values.schema.json $(UPDATECLI_FILES)
 .PHONY: test
 test: build
 	helm lint .
-	ct lint --lint-conf ../../.configs/lintconf.yaml --check-version-increment=false --helm-dependency-extra-args=--skip-refresh --charts .
+	ct lint --lint-conf ../../.configs/lintconf.yaml --helm-dependency-extra-args=--skip-refresh --charts .
 ifdef HAS_HELM_UNITTEST
 	helm unittest .
 else

charts/feature-cluster-events/Makefile

@@ -30,7 +30,7 @@ build: README.md Chart.lock values.schema.json $(UPDATECLI_FILES)
 .PHONY: test
 test: build
 	helm lint .
-	ct lint --lint-conf ../../.configs/lintconf.yaml --check-version-increment=false --helm-dependency-extra-args=--skip-refresh --charts .
+	ct lint --lint-conf ../../.configs/lintconf.yaml --helm-dependency-extra-args=--skip-refresh --charts .
 ifdef HAS_HELM_UNITTEST
 	helm unittest .
 else

charts/feature-cluster-metrics/Makefile

@@ -33,7 +33,7 @@ test: build
 	helm repo add kepler https://sustainable-computing-io.github.io/kepler-helm-chart
 
 	helm lint .
-	ct lint --lint-conf ../../.configs/lintconf.yaml --check-version-increment=false --helm-dependency-extra-args=--skip-refresh --charts .
+	ct lint --lint-conf ../../.configs/lintconf.yaml --helm-dependency-extra-args=--skip-refresh --charts .
 ifdef HAS_HELM_UNITTEST
 	helm unittest .
 else

charts/feature-cluster-metrics/default-allow-lists/kubelet_resource.yaml

@@ -1,4 +1,4 @@
 ---
 # The minimal set of resource metrics from the Kubelet required for Kubernetes Monitoring
 - node_cpu_usage_seconds_total
-- node_memory_working_set_bytes
+- node_memory_working_set_bytes

charts/feature-frontend-observability/Makefile

@@ -30,7 +30,7 @@ build: README.md Chart.lock values.schema.json $(UPDATECLI_FILES)
 .PHONY: test
 test: build
 	helm lint .
-	ct lint --lint-conf ../../.configs/lintconf.yaml --check-version-increment=false --helm-dependency-extra-args=--skip-refresh --charts .
+	ct lint --lint-conf ../../.configs/lintconf.yaml --helm-dependency-extra-args=--skip-refresh --charts .
 ifdef HAS_HELM_UNITTEST
 	helm unittest .
 else

charts/feature-integrations/Makefile

@@ -78,7 +78,7 @@ build: README.md $(INTEGRATION_DOCS_FILES) Chart.lock values.schema.json templat
 .PHONY: test
 test: build
 	helm lint .
-	ct lint --lint-conf ../../.configs/lintconf.yaml --check-version-increment=false --helm-dependency-extra-args=--skip-refresh --charts .
+	ct lint --lint-conf ../../.configs/lintconf.yaml --helm-dependency-extra-args=--skip-refresh --charts .
 ifdef HAS_HELM_UNITTEST
 	helm unittest .
 else

charts/feature-pod-logs/Makefile

@@ -30,7 +30,7 @@ build: README.md Chart.lock values.schema.json $(UPDATECLI_FILES)
 .PHONY: test
 test: build
 	helm lint .
-	ct lint --lint-conf ../../.configs/lintconf.yaml --check-version-increment=false --helm-dependency-extra-args=--skip-refresh --charts .
+	ct lint --lint-conf ../../.configs/lintconf.yaml --helm-dependency-extra-args=--skip-refresh --charts .
 ifdef HAS_HELM_UNITTEST
 	helm unittest .
 else

charts/feature-profiling/Makefile

@@ -30,7 +30,7 @@ build: README.md Chart.lock values.schema.json $(UPDATECLI_FILES)
 .PHONY: test
 test: build
 	helm lint .
-	ct lint --lint-conf ../../.configs/lintconf.yaml --check-version-increment=false --helm-dependency-extra-args=--skip-refresh --charts .
+	ct lint --lint-conf ../../.configs/lintconf.yaml --helm-dependency-extra-args=--skip-refresh --charts .
 ifdef HAS_HELM_UNITTEST
 	helm unittest .
 else

charts/feature-prometheus-operator-objects/Makefile

@@ -31,7 +31,7 @@ build: README.md Chart.lock values.schema.json $(UPDATECLI_FILES)
 test: build
 	helm repo add prometheus-community https://prometheus-community.github.io/helm-charts
 	helm lint .
-	ct lint --lint-conf ../../.configs/lintconf.yaml --check-version-increment=false --helm-dependency-extra-args=--skip-refresh --charts .
+	ct lint --lint-conf ../../.configs/lintconf.yaml --helm-dependency-extra-args=--skip-refresh --charts .
 ifdef HAS_HELM_UNITTEST
 	helm unittest .
 else

charts/k8s-monitoring-test/Makefile

@@ -31,7 +31,7 @@ build: README.md Chart.lock values.schema.json $(UPDATECLI_FILES)
 test: build
 	helm repo add prometheus-community https://prometheus-community.github.io/helm-charts
 	helm lint .
-	ct lint --lint-conf ../../.configs/lintconf.yaml --check-version-increment=false --helm-dependency-extra-args=--skip-refresh --charts .
+	ct lint --lint-conf ../../.configs/lintconf.yaml --helm-dependency-extra-args=--skip-refresh --charts .
 ifdef HAS_HELM_UNITTEST
 	helm unittest .
 else

charts/k8s-monitoring-v1/Makefile

@@ -1,5 +1,6 @@
 SHELL := /bin/bash
 HAS_HELM_DOCS := $(shell command -v helm-docs;)
+HAS_SHELLSPEC := $(shell command -v shellspec;)
 
 CHART_TEMPLATE_FILES = $(shell find templates -name "*.tpl")
 CHART_YAML_FILES = $(shell find templates -name "*.yaml")
@@ -85,7 +86,11 @@ push-image:
 .PHONY: test
 test: test/test-runner.sh
 	./test/test-runner.sh --show-diffs
-	cd test; shellspec
+ifdef HAS_SHELLSPEC
+	shellspec -c test
+else
+	docker run --platform linux/amd64 --rm --volume $(shell pwd):/src shellspec/shellspec -c /src/test -s /bin/sh
+endif
 
 #
 # Example targets

charts/k8s-monitoring-v1/default_allow_lists/kubelet_resource.yaml

@@ -1,4 +1,4 @@
 ---
 # The minimal set of resource metrics from the Kubelet required for Kubernetes Monitoring
 - node_cpu_usage_seconds_total
-- node_memory_working_set_bytes
+- node_memory_working_set_bytes

charts/k8s-monitoring/Chart.lock

@@ -42,4 +42,4 @@ dependencies:
   repository: https://grafana.github.io/helm-charts
   version: 0.9.1
 digest: sha256:734d4c8f6076481eb580378daa65fe163c78e9e07a1a214cb4b2fed16441b4c9
-generated: "2024-10-15T16:29:11.853045-05:00"
+generated: "2024-10-16T09:29:39.056825-05:00"

scripts/lint-yaml.sh

@@ -8,7 +8,7 @@ heading "Kubernetes Monitoring Helm" "Onboarding - Performing YAML Linting using
 
 # make sure yamllint exists
 if [[ "$(command -v yamllint || true)" = "" ]]; then
-  echo >&2 "pipeyamllintnv command is required, see (https://pypi.org/project/yamllint/) or run: brew install yamllint";
+  emergency "yamllint is required if running lint locally, see (https://pypi.org/project/yamllint/) or run: brew install yamllint";
   exit 1;
 fi
 

scripts/integration-test.sh → scripts/run-integration-test.sh

@@ -7,7 +7,7 @@ source "${PARENT_DIR}/scripts/includes/logging.sh"
 heading "Kubernetes Monitoring Helm" "Integration Tester"
 
 usage() {
-  echo "USAGE: integration-test.sh <test-dir>"
+  echo "USAGE: run-integration-test.sh <test-dir>"
   echo ""
   echo "Runs an integration test"
   echo ""

scripts/setup.sh

@@ -52,16 +52,8 @@ else
   success "kind is installed"
 fi
 
-# make sure shellspec exists
-info "Checking to see if shellspec is installed"
-if [[ "$(command -v shellspec)" = "" ]]; then
-  warning "shellspec is required if running locally, see: (https://shellspec.info/) or run: brew install shellspec";
-else
-  success "shellspec is installed"
-fi
-
 # make sure yamllint exists
-info "Checking to see if shellspec is installed"
+info "Checking to see if yamllint is installed"
 if [[ "$(command -v yamllint)" = "" ]]; then
   warning "yamllint is required if running locally, see: (https://github.com/adrienverge/yamllint) or run: brew install yamllint";
 else