Jsonnet: add experimental ingest storage support #8028
Conversation
| @@ -336,72 +332,4 @@ | |||
| if !$._config.is_microservices_deployment_mode || ($._config.multi_zone_store_gateway_enabled && !$._config.multi_zone_store_gateway_migration_enabled) | |||
| then null | |||
| else super.store_gateway_pdb, | |||
|
|
|||
| // | |||
| // Rollout operator. | |||
There was a problem hiding this comment.
Note to reviewers: I've simply moved it to a dedicated file.
| $._config.multi_zone_ingester_enabled || | ||
| $._config.multi_zone_store_gateway_enabled || | ||
| $._config.cortex_compactor_concurrent_rollout_enabled || | ||
| $._config.ingest_storage_ingester_autoscaling_enabled, |
There was a problem hiding this comment.
Note to reviewers: added $._config.ingest_storage_ingester_autoscaling_enabled. This is the only logic change compared to the version in operations/mimir/multi-zone.libsonnet.
pracucci
force-pushed
the
jsonnet-add-ingest-storage-support
branch
from
0cf0618 to
1b24186
Compare
| @@ -8,7 +8,7 @@ should_be_namespaced(contents) { | |||
| } | |||
|
|
|||
| should_be_namespaced(contents) { | |||
| not contents.kind in ["PodSecurityPolicy", "Namespace"] | |||
| not contents.kind in ["PodSecurityPolicy", "Namespace", "CustomResourceDefinition"] | |||
There was a problem hiding this comment.
Note to reviewers: CustomResourceDefinition doesn't have namespace because it's a global resource.
| namespace: $._config.namespace, | ||
| }, | ||
| spec: { | ||
| replicas:: null, // We don't want this field to be managed by Flux. |
There was a problem hiding this comment.
| replicas:: null, // We don't want this field to be managed by Flux. | |
| replicas:: null, // Hide replicas field. |
| ingest_storage_migration_classic_ingesters_scale_down: false, | ||
| ingest_storage_migration_classic_ingesters_zone_a_decommission: true, | ||
| ingest_storage_migration_classic_ingesters_zone_b_decommission: true, | ||
| ingest_storage_migration_classic_ingesters_zone_c_decommission: true, |
There was a problem hiding this comment.
There is only one field for scaling down all zones, but 3 fields for decommission. If these statefulset sets are scaled down already, why not decomission all of them at once?
There was a problem hiding this comment.
OK, I see that this variable is also used in later steps.
There was a problem hiding this comment.
We'll decommission them all together, but then restore 1 zone at a time when we migrate ingester-zone-[abc]-partition back to ingester-zone-[abc].
| $.ingest_storage_ingester_args + | ||
| $.ingest_storage_ingester_ring_client_args + { | ||
| // Run partition ingesters on a dedicated hash ring, so that they don't clash with classic ingesters. | ||
| 'ingester.ring.prefix': 'partition-ingesters/', |
There was a problem hiding this comment.
nit: Would be nice to use common local var for partition-ingesters/.
There was a problem hiding this comment.
Done in b02e79d. I've used a config option instead of a local var, because it's scattered around 2 files.
| @@ -0,0 +1,12 @@ | |||
| // Migration step 8: | |||
There was a problem hiding this comment.
Looking at diff for generated YAML files between step 7 and 8, I think we're missing setup of ingester-zone-[abc], before we scale down ingester-zone-[abc]-partition. Ingesters without the suffix were removed in step 6, but they still don't exist at this point. Is that OK? If we scale down -partition ingesters now, we have no ingesters running and read-path is broken. WDYT?
There was a problem hiding this comment.
I think we're missing setup of ingester-zone-[abc], before we scale down ingester-zone-[abc]-partition
We don't setup ingester-zone-[abc] before scaling down ingester-zone-[abc]-partition but after that because we'll reuse PVCs (since migrating PVCs is a migration step not done through jsonnet it's not visibile in the codified steps). ingester-zone-[abc] is setup in step 9. To keep the steps easier here, I've codified all zones at once, but in practice the migration will be 1 zone at a time to have no downtime.
There was a problem hiding this comment.
If we scale down -partition ingesters now, we have no ingesters running and read-path is broken
From step 8:
// Migration step 8:
// - Begin migrating ingester-zone-[abc]-partition StatefulSets to ingester-zone-[abc]. This should be done zone-by-zone,
// but to keep the jsonnet test smaller we're going to do all zones at once. In this step we're going to scale down
// ingester-zone-[abc]-partition replicas to 0.
In practice will be done 1 zone at a time.
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
pracucci
force-pushed
the
jsonnet-add-ingest-storage-support
branch
from
85ec995 to
b02e79d
Compare
What this PR does
In this PR I'm adding the jsonnet support for the experimental ingest storage. This PR is upstreaming the jsonnet we're using at Grafana Labs to test the ingest storage, and it migration from the Mimir classic architecture.
I've codified the migration steps in the jsonnet "tests". The diff of each step matched with the actual diff applied at Grafana Labs when we tested the migration process:
Which issue(s) this PR fixes or relates to
N/A
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.