chore(ksonnet): Simplify configuration of ingester deployment #10542

chaudum · 2023-09-11T15:09:03Z

What this PR does / why we need it:

Simplify deployment with ksonnet:

Remove stateful_ingesters flag, because ingesters should always be deployed as StatefulSet with WAL (write ahead log) enabled.

Checklist

Reviewed the CONTRIBUTING.md guide (required)
Documentation added
Tests updated
CHANGELOG.md updated
- If the change is worth mentioning in the release notes, add add-to-release-notes label
Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR

salvacorts · 2023-09-28T08:09:40Z

production/ksonnet/loki/config.libsonnet

-    // flags for running ingesters/queriers as a statefulset instead of deployment type.
-    // WAL enabled configurations automatically use statefulsets.
-    stateful_ingesters: false,
+    // flags for running queriers as a statefulset instead of deployment type.


I think this comment should go in line 30 next to stateful_queriers: false,

Removed the comment completely as it does not make sense any more.

salvacorts · 2023-09-28T08:18:37Z

production/ksonnet/loki/ingester.libsonnet

@@ -47,42 +46,19 @@ local k = import 'ksonnet-util/kausal.libsonnet';
    container.mixin.readinessProbe.httpGet.withPort($._config.http_listen_port) +
    container.mixin.readinessProbe.withInitialDelaySeconds(15) +
    container.mixin.readinessProbe.withTimeoutSeconds(1) +
-    k.util.resourcesRequests('1', '5Gi') +
-    k.util.resourcesLimits('2', '10Gi') +
+    k.util.resourcesRequests('1', '7Gi') +


I wonder if we should make this configurable.

Same here. These are just default values

salvacorts · 2023-09-28T08:18:44Z

production/ksonnet/loki/config.libsonnet

    ingester_pvc_size: '10Gi',
    ingester_pvc_class: 'fast',

+    ingester_data_disk_size: self.ingester_pvc_size,  // keep backwards compatibility


Looks like this is the variable that is actually used:

loki/production/ksonnet/loki/ingester.libsonnet

Line 15 in 4eddc0e

pvc.mixin.spec.resources.withRequests({ storage: $._config.ingester_data_disk_size }) +

loki/production/ksonnet/loki/multi-zone.libsonnet

Line 68 in 4eddc0e

pvc.mixin.spec.resources.withRequests({ storage: $._config.ingester_data_disk_size }) +

Provided we are deprecating stuff, should we remove ingester_data_disk_size and only have ingester_pvc_size to keep things simple?

Since ingester_data_disk_size is referenced, why do you want to replace it with the other variable?

I was proposing to remove ingester_data_disk_size and only use ingester_pvc_size.

salvacorts · 2023-09-28T08:22:27Z

production/ksonnet/loki/ingester.libsonnet


-    $.newLokiStatefulSet(name, 3, container, ingester_data_pvc) +
+  newIngesterStatefulSet(name, container, with_anti_affinity=true)::
+    $.newLokiStatefulSet(name, 3, container, [ingester_data_pvc, ingester_wal_pvc]) +


IIUC, the 3 here is the number of replicas. Shouldn't this be configurable?

The replica value is usually overwritten in a scaling.libsonnet, so the initial value does not matter too much - as long as it fulfils the replication factor.

salvacorts · 2023-09-28T08:28:23Z

production/ksonnet/loki/multi-zone.libsonnet

-    local zone_name = 'zone-%s' % zone;
+  // The ingesters should persist TSDB blocks and WAL on a persistent
+  // volume in order to be crash resilient.
+  local ingester_data_pvc =


Looks like we are repeating a bunch of code from ingester.libsonnet. I wonder if we should reuse some of the things we define on that file here.

salvacorts

LGTM

salvacorts · 2023-10-20T13:15:01Z

production/ksonnet/loki/config.libsonnet

    ingester_pvc_size: '10Gi',
    ingester_pvc_class: 'fast',

+    ingester_data_disk_size: self.ingester_pvc_size,  // keep backwards compatibility


I was proposing to remove ingester_data_disk_size and only use ingester_pvc_size.

This PR removes the option for deploying ingesters as Kubernetes Deployment. Instead, always run ingesters as StatefulSet. The configuration setting `stateful_ingesters` is removed. As a follow up PR, the WAL configuration will be removed as well, because stateful ingesters should always be run with a write ahead log. Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

Ingesters should always be deployed as StatefulSet with WAL (write ahead log) enabled. Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

These changes for the multi-zone ingester deployment are taken from Grafana Cloud downstream and have been in production for some time. Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

…a#10542) Simplify deployment with ksonnet: Remove `stateful_ingesters` flag, because ingesters should always be deployed as StatefulSet with WAL (write ahead log) enabled. Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

pull-request-size bot added the size/L label Sep 11, 2023

chaudum changed the title ~~Chaudum/ksonnet remove stateless ingesters~~ chore(ksonnet): Simplify configuration of ingester deployment Sep 11, 2023

chaudum force-pushed the chaudum/ksonnet-remove-stateless-ingesters branch from 40b09b8 to d2b2202 Compare September 11, 2023 15:22

chaudum marked this pull request as ready for review September 22, 2023 08:19

chaudum requested a review from a team as a code owner September 22, 2023 08:19

salvacorts reviewed Sep 28, 2023

View reviewed changes

chaudum force-pushed the chaudum/ksonnet-remove-stateless-ingesters branch from 175d5eb to 3ea39b7 Compare October 20, 2023 12:43

salvacorts approved these changes Oct 20, 2023

View reviewed changes

chaudum added 9 commits October 22, 2023 21:12

Fixup: remove duplicate fields

Verified

This commit was signed with the committer’s verified signature.

chaudum Christian Haudum

GPG key ID: 92E55CD2AD9DA62F

Verified
Learn about vigilant mode

9653148

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

Add changelog entry

Verified

This commit was signed with the committer’s verified signature.

chaudum Christian Haudum

GPG key ID: 92E55CD2AD9DA62F

Verified
Learn about vigilant mode

780057d

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

Remove irrelevant comment

Verified

This commit was signed with the committer’s verified signature.

chaudum Christian Haudum

GPG key ID: 92E55CD2AD9DA62F

Verified
Learn about vigilant mode

8921d7d

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

Remove duplicate ingester PVC definition

Verified

This commit was signed with the committer’s verified signature.

chaudum Christian Haudum

GPG key ID: 92E55CD2AD9DA62F

Verified
Learn about vigilant mode

74f2ae3

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

Remove max_transfer_retries

Verified

This commit was signed with the committer’s verified signature.

chaudum Christian Haudum

GPG key ID: 92E55CD2AD9DA62F

Verified
Learn about vigilant mode

21bf854

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

Fix changelog

Verified

This commit was signed with the committer’s verified signature.

chaudum Christian Haudum

GPG key ID: 92E55CD2AD9DA62F

Verified
Learn about vigilant mode

2150946

Signed-off-by: Christian Haudum <christian.haudum@gmail.com>

chaudum force-pushed the chaudum/ksonnet-remove-stateless-ingesters branch from 059dae2 to 2150946 Compare October 22, 2023 19:14

chaudum enabled auto-merge (squash) October 22, 2023 19:15

chaudum merged commit b43e253 into main Oct 22, 2023

chaudum deleted the chaudum/ksonnet-remove-stateless-ingesters branch October 22, 2023 19:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(ksonnet): Simplify configuration of ingester deployment #10542

chore(ksonnet): Simplify configuration of ingester deployment #10542

chaudum commented Sep 11, 2023 •

edited

Loading

salvacorts Sep 28, 2023

chaudum Oct 20, 2023

salvacorts Sep 28, 2023

chaudum Oct 20, 2023

salvacorts Sep 28, 2023

chaudum Oct 20, 2023

salvacorts Oct 20, 2023

salvacorts Sep 28, 2023

chaudum Oct 20, 2023

salvacorts Sep 28, 2023

salvacorts left a comment

salvacorts Oct 20, 2023

chore(ksonnet): Simplify configuration of ingester deployment #10542

chore(ksonnet): Simplify configuration of ingester deployment #10542

Conversation

chaudum commented Sep 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

salvacorts left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chaudum commented Sep 11, 2023 •

edited

Loading