Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/add helm docs #67

Open
wants to merge 142 commits into
base: feat/add-docusaurus
Choose a base branch
from
Open

Feat/add helm docs #67

wants to merge 142 commits into from

Conversation

JTaeuber
Copy link
Member

@JTaeuber JTaeuber commented Nov 4, 2024

Motivation

Adding documentation for the Helm Chart

Part of #38

Blocked by #80 and #69

Changes

  • fixed pre-commit email
  • refactored values.yaml
  • added documentation pages for the helm chart

Tests done

Ran the site locally

TODO

Make links to arguments and environment variable pages.

@JTaeuber JTaeuber added the documentation Improvements or additions to documentation label Nov 4, 2024
@jonathan-mayer jonathan-mayer removed the blocked A PR which is blocked by other Issues/PRs label Feb 24, 2025
@JTaeuber
Copy link
Member Author

JTaeuber commented Feb 25, 2025

  • Still missing Basic Night Time Saving
  • Refactor of installation to be in sync with helm chart readme
  • Add documentation for leader election
  • Add values.yaml changes to Migrating docs

@jonathan-mayer feel free to look over the rest if you have the time

@jonathan-mayer
Copy link
Member

we should consider updating these packages:

Package                           Current  Wanted  Latest  Location                                       Depended by
@eslint/compat                      1.2.6   1.2.7   1.2.7  node_modules/@eslint/compat                    website
@tailwindcss/postcss                4.0.6   4.0.9   4.0.9  node_modules/@tailwindcss/postcss              website
@typescript-eslint/eslint-plugin   8.24.0  8.25.0  8.25.0  node_modules/@typescript-eslint/eslint-plugin  website
@typescript-eslint/parser          8.24.0  8.25.0  8.25.0  node_modules/@typescript-eslint/parser         website
eslint                             9.20.1  9.21.0  9.21.0  node_modules/eslint                            website
postcss                             8.5.2   8.5.3   8.5.3  node_modules/postcss                           website
tailwindcss                         4.0.6   4.0.9   4.0.9  node_modules/tailwindcss                       website

@jonathan-mayer
Copy link
Member

jonathan-mayer commented Feb 28, 2025

We dont need this right? the crds permission is unnessessary since we dont need to do anything with the definitions themselves. We also currently dont need the admission controller ones and taking the recent comments in the issues topic into account it seems as though we won't need them.

{{- if not .Values.constrainedDownscaler }}
- apiGroups:
  - constraints.gatekeeper.sh
  resources:
  - kubedownscalerjobsconstraint
  verbs:
  - get
  - create
  - watch
  - list
  - update
  - patch
  - delete
- apiGroups:
  - kyverno.io
  resources:
  - policies
  resourceNames:
  - kube-downscaler-jobs-policy
  verbs:
  - get
  - create
  - watch
  - list
  - update
  - patch
  - delete
- apiGroups:
  - kyverno.io
  resources:
  - policies
  verbs:
  - get
  - create
  - watch
  - list
- apiGroups:
  - templates.gatekeeper.sh
  resources:
  - constrainttemplate
  verbs:
  - create
  - get
  - list
  - watch
- apiGroups:
  - apiextensions.k8s.io
  resources:
  - customresourcedefinitions
  verbs:
  - create
  - get
  - list
  - watch
{{- end }}

also we only ever do get, list and update on the resources so i would change that too

@jonathan-mayer
Copy link
Member

please change every admonition to have empty lines around its contents. see https://docusaurus.io/docs/markdown-features/admonitions#usage-with-prettier for why we (at least sometimes) need it and we should be consistent across the whole docs

Copy link
Member

@jonathan-mayer jonathan-mayer left a comment

Choose a reason for hiding this comment

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

LLYHSWCOTM

In order to get started installing the GoKubeDownscaler using our Helm Chart you need:

- [Helm installed][helm-intro] on a system
- have access to a Kubernetes cluster in some kind of way.
Copy link
Member

Choose a reason for hiding this comment

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

this makes the sentence strucutre better: "... to use our Helm Chart you need access to a Kubernetes ...".

Suggested change
- have access to a Kubernetes cluster in some kind of way.
- access to a Kubernetes cluster

If left empty like it is by default the GoKubeDownscaler will use the
[appVersion](repo:deployments/chart/Chart.yaml#L10) of the currently used Helm Chart.

You can find all versions of the GoKubeDownscaler [on the github releases page](repo/releases).
Copy link
Member

Choose a reason for hiding this comment

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

"/home/installadm/dev/gokubedownscaler2/website/content/docs/1 - helm-chart/2 - values/1 - image.mdx:32:51": No repository path specified

Suggested change
You can find all versions of the GoKubeDownscaler [on the github releases page](repo/releases).
You can find all versions of the GoKubeDownscaler [on the github releases page](https://github.com/caas-team/GoKubeDownscaler/releases).

Copy link
Member

Choose a reason for hiding this comment

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

the order right now is not only wierd, since its "Downcsaler", "Workflows", "Helm Chart", "Docker Image" but also has duplicate indexes. I would put the Workflow docs after the Helm Chart. Update the index of the Docker Image docs along with this change.

## Serviceaccount

The [serviceaccount.yaml](repo:deployments/chart/templates/serviceaccount.yaml) file creates a ServiceAccount
that will be used by the go-kube-downscaler to interact with Kubernetes.
Copy link
Member

Choose a reason for hiding this comment

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

we should only use one way to write GoKubeDownscaler in the docs.

Suggested change
that will be used by the go-kube-downscaler to interact with Kubernetes.
that will be used by the GoKubeDownscaler to interact with Kubernetes.


## Workload Permissions

The Helm Chart assigns get, watch, list, update and patch permissions for the workloads defined in [`includedResources`](ref:docs-helm-included-resources).
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
The Helm Chart assigns get, watch, list, update and patch permissions for the workloads defined in [`includedResources`](ref:docs-helm-included-resources).
The Helm Chart assigns `get`, `watch`, `list`, `update` and `patch` permissions for the workloads defined in [`includedResources`](ref:docs-helm-included-resources).

The `configMap` value contains the two fields `name` and `extraConfig`:

- `name` defines the name of the configmap for the GoKubeDownscaler.
- `extraConfig` adds additional specified environment variables to the ConfigMap.
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
- `extraConfig` adds additional specified environment variables to the ConfigMap.
- `extraConfig` adds additional specified [environment variables](ref:docs-environment-variables) to the ConfigMap.

:::

By default the `extraConfig` field is empty.
Appending a line will add the corresponding environment variable to the configMap.
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
Appending a line will add the corresponding environment variable to the configMap.
Appending a line will add the corresponding [environment variable](ref:docs-environment-variables) to the configMap.


This then changes how the GoKubeDownscaler behaves.

:::tip For example:
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
:::tip For example:
:::tip Example


If you want the GoKubeDownscaler to be scheduled on a specific type of node you can specify the determining labels of the node here.

:::tip For example:
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
:::tip For example:
:::tip Example


This will add the `EXCLUDE_DEPLOYMENTS` environment variable to the configMap.

The given configuration will exclude the deployments with the name deployment1 and deployment2
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
The given configuration will exclude the deployments with the name deployment1 and deployment2
The given configuration will exclude the deployments with the name `deployment1` and `deployment2`

@JTaeuber JTaeuber linked an issue Mar 2, 2025 that may be closed by this pull request
* fix: correction for leader election flag assignment (#123)

* refactor: added release namespace to k8s resources in the chart

* Update configmap fields

* chore: automatically push pre-commit changes

---------

Co-authored-by: Jan <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LeaderElection helm value is not parsed properly Implement GitBook-like documentation
4 participants