feat(Helm): Update Loki Helm chart for restricted environments #14440

davidham · 2024-10-09T20:01:18Z

What this PR does / why we need it:
We are attempting to deploy this chart, with Enterprise enabled and in distributed mode, to a restricted k8s environment. The two main restrictions driving this PR are:

we can't deploy any cluster-level resources, only namespaced ones
we don't have the ability to delete pods in the live environment
pods have no permissions in the k8s namespace except what is granted to them with a Role

With this in mind, I am proposing the following changes:

Statefulsets for ingester and index-gateway
The change here is to make the spec.updateStrategy.type configurable, and to default to RollingUpdate.

Enterprise tokengen
For this change, if enterprise and tokengen are enabled, and rbac.namespaced is true, the chart will render a Role and RoleBinding. If rbac.namespaced is false, it will render a ClusterRole and ClusterRoleBinding.

Which issue(s) this PR fixes:
n/a

Special notes for your reviewer:

Checklist

Reviewed the CONTRIBUTING.md guide (required)
Documentation added
Tests updated
Title matches the required conventional commits format, see here
- Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

Changed the behavior so that if `.Values.rbac.namespaced` is enabled, it creates a Role instead of a ClusterRole for tokengen.

CLAassistant · 2024-10-09T20:01:26Z

All committers have signed the CLA.

…vironments

DylanGuedes

Thank you for your PR ❤️
Asked a few questions below, my only concern with the current approach is that it makes it difficult to configure other attributes (that not the type) and that we're making a small change to the indexGateway strategy and I'm not sure if that is a breaking change or not.

production/helm/loki/templates/index-gateway/statefulset-index-gateway.yaml

DylanGuedes · 2024-10-09T21:16:13Z

production/helm/loki/templates/ingester/statefulset-ingester-zone-a.yaml

  updateStrategy:
-    type: OnDelete
+    type: {{ .Values.ingester.updateStrategy }}


ditto: could you please make the whole updateStrategy configurable otherwise users won't be able to set the rollingUpdate

DylanGuedes · 2024-10-09T21:16:19Z

production/helm/loki/templates/ingester/statefulset-ingester-zone-b.yaml

  updateStrategy:
-    type: OnDelete
+    type: {{ .Values.ingester.updateStrategy }}


DylanGuedes · 2024-10-09T21:16:24Z

production/helm/loki/templates/ingester/statefulset-ingester-zone-c.yaml

  updateStrategy:
-    type: OnDelete
+    type: {{ .Values.ingester.updateStrategy }}


DylanGuedes · 2024-10-09T21:16:31Z

production/helm/loki/templates/ingester/statefulset-ingester.yaml

@@ -18,8 +18,7 @@ spec:
 {{- end }}
  podManagementPolicy: Parallel
  updateStrategy:
-    rollingUpdate:
-      partition: 0
+    type: {{ .Values.ingester.updateStrategy }}


production/helm/loki/templates/tokengen/clusterrole-tokengen.yaml

production/helm/loki/templates/tokengen/clusterrolebinding-tokengen.yaml

DylanGuedes · 2024-10-09T21:19:49Z

production/helm/loki/values.yaml

@@ -2313,6 +2315,8 @@ indexGateway:
  # -- Set the optional grpc service protocol. Ex: "grpc", "http2" or "https"
  appProtocol:
    grpc: ""
+  # -- UpdateStrategy for the StatefulSet. One of 'OnDelete' or 'RollingUpdate'.
+  updateStrategy: RollingUpdate


ditto: after making the changes to make the updateStrategy configurable, in case setting partition: 0 is different than the default, can you add it to this values.yaml, to not make a breaking change?

production/helm/loki/templates/tokengen/clusterrole-tokengen.yaml

production/helm/loki/templates/tokengen/clusterrolebinding-tokengen.yaml

davidham · 2024-10-09T22:29:55Z

production/helm/loki/values.yaml

+    # -- One of  'OnDelete' or 'RollingUpdate'
+    type: RollingUpdate
+    # -- Optional for updateStrategy.type=RollingUpdate. See [Partitioned rolling updates](https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#partitions) in the StatefulSet docs for details.
+    # rollingUpdate:


If you specify a spec.updateStrategy.rollingUpdate block in values.yaml, you have to null it out if you want to set spec.updateStrategy.type=OnDelete. But I think partition: 0 is a no-op in any case.

github-actions · 2024-10-10T10:24:14Z

Kubernetes Manifest Diff Summary

Scenario: default-single-binary-values (Added: 0, Modified: 4, Removed: 0)

Summary:

Added: 0
Modified: 4
Removed: 0

Added Files

No added files

Modified Files

loki/templates/index-gateway/statefulset-index-gateway.yaml

*** /home/runner/work/loki/loki/output/base/default-single-binary-values/loki/templates/index-gateway/statefulset-index-gateway.yaml	2024-10-10 10:24:09.264117944 +0000
--- /home/runner/work/loki/loki/output/pr/default-single-binary-values/loki/templates/index-gateway/statefulset-index-gateway.yaml	2024-10-10 10:24:12.160095354 +0000
***************
*** 14,21 ****
 spec:
 replicas: 2
 updateStrategy:
! rollingUpdate:
! partition: 0
 serviceName: loki-test-chart-name-index-gateway-headless
 revisionHistoryLimit: 10
 selector:
--- 14,20 ----
 spec:
 replicas: 2
 updateStrategy:
! type: RollingUpdate
 serviceName: loki-test-chart-name-index-gateway-headless
 revisionHistoryLimit: 10
 selector:

loki/templates/ingester/statefulset-ingester-zone-c.yaml

*** /home/runner/work/loki/loki/output/base/default-single-binary-values/loki/templates/ingester/statefulset-ingester-zone-c.yaml	2024-10-10 10:24:09.264117944 +0000
--- /home/runner/work/loki/loki/output/pr/default-single-binary-values/loki/templates/ingester/statefulset-ingester-zone-c.yaml	2024-10-10 10:24:12.160095354 +0000
***************
*** 30,36 ****
 name: ingester-zone-c
 rollout-group: ingester
 updateStrategy:
! type: OnDelete
 template:
 metadata:
 annotations:
--- 30,36 ----
 name: ingester-zone-c
 rollout-group: ingester
 updateStrategy:
! type: RollingUpdate
 template:
 metadata:
 annotations:

loki/templates/ingester/statefulset-ingester-zone-b.yaml

*** /home/runner/work/loki/loki/output/base/default-single-binary-values/loki/templates/ingester/statefulset-ingester-zone-b.yaml	2024-10-10 10:24:09.264117944 +0000
--- /home/runner/work/loki/loki/output/pr/default-single-binary-values/loki/templates/ingester/statefulset-ingester-zone-b.yaml	2024-10-10 10:24:12.160095354 +0000
***************
*** 30,36 ****
 name: ingester-zone-b
 rollout-group: ingester
 updateStrategy:
! type: OnDelete
 template:
 metadata:
 annotations:
--- 30,36 ----
 name: ingester-zone-b
 rollout-group: ingester
 updateStrategy:
! type: RollingUpdate
 template:
 metadata:
 annotations:

loki/templates/ingester/statefulset-ingester-zone-a.yaml

*** /home/runner/work/loki/loki/output/base/default-single-binary-values/loki/templates/ingester/statefulset-ingester-zone-a.yaml	2024-10-10 10:24:09.264117944 +0000
--- /home/runner/work/loki/loki/output/pr/default-single-binary-values/loki/templates/ingester/statefulset-ingester-zone-a.yaml	2024-10-10 10:24:12.160095354 +0000
***************
*** 30,36 ****
 name: ingester-zone-a
 rollout-group: ingester
 updateStrategy:
! type: OnDelete
 template:
 metadata:
 annotations:
--- 30,36 ----
 name: ingester-zone-a
 rollout-group: ingester
 updateStrategy:
! type: RollingUpdate
 template:
 metadata:
 annotations:

Removed Files

No removed files

Scenario: default-values (Added: 0, Modified: 0, Removed: 0)

Summary:

Added: 0
Modified: 0
Removed: 0

Added Files

No added files

Modified Files

No modified files

Removed Files

No removed files

Scenario: ingress-values (Added: 0, Modified: 0, Removed: 0)

Summary:

Added: 0
Modified: 0
Removed: 0

Added Files

No added files

Modified Files

No modified files

Removed Files

No removed files

Scenario: legacy-monitoring-values (Added: 0, Modified: 0, Removed: 0)

Summary:

Added: 0
Modified: 0
Removed: 0

Added Files

No added files

Modified Files

No modified files

Removed Files

No removed files

Scenario: simple-scalable-aws-kube-irsa-values (Added: 0, Modified: 0, Removed: 0)

Summary:

Added: 0
Modified: 0
Removed: 0

Added Files

No added files

Modified Files

No modified files

Removed Files

No removed files

**Statefulsets for ingester and index-gateway** The change here is to make the `spec.updateStrategy.type` configurable, and to default to `RollingUpdate`. **Enterprise tokengen** For this change, if `enterprise` and `tokengen` are enabled, and `rbac.namespaced` is true, the chart will render a `Role` and `RoleBinding`. If `rbac.namespaced` is false, it will render a `ClusterRole` and `ClusterRoleBinding`.

davidham added 3 commits October 9, 2024 11:55

feat(index-gateway): Make updateStrategy configurable

e0a3efc

feat(ingester): Made statefulset updateStrategy configurable

7dbabe0

feat(tokengen): Updated role creation

08842b5

Changed the behavior so that if `.Values.rbac.namespaced` is enabled, it creates a Role instead of a ClusterRole for tokengen.

pull-request-size bot added the size/S label Oct 9, 2024

github-actions bot added the area/helm label Oct 9, 2024

This comment has been minimized.

Sign in to view

davidham added 2 commits October 9, 2024 15:04

docs: Corrected make target

011a4a2

docs: Updated Helm docs

cf96803

pull-request-size bot added size/M and removed size/S labels Oct 9, 2024

github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Oct 9, 2024

This comment has been minimized.

Sign in to view

davidham changed the title ~~feat(Helm): Updates for restricted environments~~ feat(Helm): Update Loki Helm chart for restricted environments Oct 9, 2024

Merge branch 'main' into feature/helm-chart-updates-for-restricted-en…

7e675d8

…vironments

davidham marked this pull request as ready for review October 9, 2024 20:35

davidham requested a review from a team as a code owner October 9, 2024 20:35

This comment has been minimized.

Sign in to view

MasslessParticle approved these changes Oct 9, 2024

View reviewed changes

DylanGuedes reviewed Oct 9, 2024

View reviewed changes

davidham added 2 commits October 9, 2024 16:32

fix: Change the template and not the output

f7223ca

feat: made updateStrategy a table for more flexibility

03b08cd

This comment has been minimized.

Sign in to view

davidham commented Oct 9, 2024

View reviewed changes

DylanGuedes approved these changes Oct 9, 2024

View reviewed changes

update README.md file

cf42730

DylanGuedes merged commit adc7538 into main Oct 10, 2024
64 checks passed

DylanGuedes deleted the feature/helm-chart-updates-for-restricted-environments branch October 10, 2024 10:39

loki-gh-app bot mentioned this pull request Nov 14, 2024

chore(k227): release 3.3.0 #14750

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Helm): Update Loki Helm chart for restricted environments #14440

feat(Helm): Update Loki Helm chart for restricted environments #14440

davidham commented Oct 9, 2024 •

edited

Loading

CLAassistant commented Oct 9, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

DylanGuedes left a comment •

edited

Loading

DylanGuedes Oct 9, 2024

DylanGuedes Oct 9, 2024

DylanGuedes Oct 9, 2024

DylanGuedes Oct 9, 2024

DylanGuedes Oct 9, 2024

This comment has been minimized.

davidham Oct 9, 2024

github-actions bot commented Oct 10, 2024

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

feat(Helm): Update Loki Helm chart for restricted environments #14440

feat(Helm): Update Loki Helm chart for restricted environments #14440

Conversation

davidham commented Oct 9, 2024 • edited Loading

CLAassistant commented Oct 9, 2024 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

DylanGuedes left a comment • edited Loading

Choose a reason for hiding this comment

DylanGuedes Oct 9, 2024

Choose a reason for hiding this comment

DylanGuedes Oct 9, 2024

Choose a reason for hiding this comment

DylanGuedes Oct 9, 2024

Choose a reason for hiding this comment

DylanGuedes Oct 9, 2024

Choose a reason for hiding this comment

DylanGuedes Oct 9, 2024

Choose a reason for hiding this comment

This comment has been minimized.

davidham Oct 9, 2024

Choose a reason for hiding this comment

github-actions bot commented Oct 10, 2024

Kubernetes Manifest Diff Summary

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

Added Files

Modified Files

Removed Files

davidham commented Oct 9, 2024 •

edited

Loading

CLAassistant commented Oct 9, 2024 •

edited

Loading

DylanGuedes left a comment •

edited

Loading