mimirtool config: Add more retained old defaults (#1762)

* mimirtool config: Add more retained old defaults

The following parameters have their old defaults retained even when
`--update-defaults` is used with `mimirtool config covert`:

* `activity_tracker.filepath`
* `alertmanager.data_dir`
* `blocks_storage.filesystem.dir`
* `compactor.data_dir`
* `ruler.rule_path`
* `ruler_storage.filesystem.dir`
* `graphite.querier.schemas.backend` (only in GEM)

These are filepaths for which the new defaults don't make more sense
than the old ones. In fact updating these can lead to subpar migration
experience because components start using directories that don't exist.

Because activity_tracker.filepath changed its name since cortex the
tests needed to allow for differentiating old common options and new
ones. This is something that was already there for GEM and was added
for cortex/mimir too.

Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>

* Update CHANGELOG.md

Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>

CHANGELOG.md

@@ -82,7 +82,7 @@
 
 ### Mimirtool
 
-* [BUGFIX] `config convert`: Retain Cortex defaults for `blocks_storage.backend`, `ruler_storage.backend`, and `alertmanager_storage.backend`. #1626
+* [BUGFIX] `config convert`: Retain Cortex defaults for `blocks_storage.backend`, `ruler_storage.backend`, `alertmanager_storage.backend`, `auth.type`, `activity_tracker.filepath`, `alertmanager.data_dir`, `blocks_storage.filesystem.dir`, `compactor.data_dir`, `ruler.rule_path`, `ruler_storage.filesystem.dir`, and `graphite.querier.schemas.backend`. #1626 #1762
 
 ### Tools
 

docs/sources/operators-guide/tools/mimirtool.md

@@ -660,11 +660,20 @@ It supports converting both CLI flags and [YAML configuration files]({{< relref
 `mimirtool config convert` helps you migrate from Cortex to Grafana Mimir. There are changes to the default values of
 some configuration parameters in Mimir v2.0.0 that you might not want to use as part of this migration:
 
-- `blocks_storage.backend` - Cortex default is `s3`, Mimir default is `filesystem`
-- `ruler_storage.backend` - Cortex default is `s3`, Mimir default is `filesystem`
-- `alertmanager_storage.backend` - Cortex default is `s3`, Mimir default is `filesystem`
-- `server.http_listen_port` - Cortex default is `80`, Mimir default is `8080`
-- (GEM only) `auth.type` - GEM 1.x default is `trust`, 2.x default is `enterprise`
+| Parameter                                     | Cortex/GEM 1.7 default   | Mimir/GEM 2.0 default    |
+| --------------------------------------------- | ------------------------ | ------------------------ |
+| `blocks_storage.backend`                      | `s3`                     | `filesystem`             |
+| `ruler_storage.backend`                       | `s3`                     | `filesystem`             |
+| `alertmanager_storage.backend`                | `s3`                     | `filesystem`             |
+| `server.http_listen_port`                     | `80`                     | `8080`                   |
+| `activity_tracker.filepath`                   | `./active-query-tracker` | `./metrics-activity.log` |
+| `alertmanager.data_dir`                       | `data/`                  | `./data-alertmanager/`   |
+| `blocks_storage.filesystem.dir`               | `<empty>`                | `blocks`                 |
+| `compactor.data_dir`                          | `./data`                 | `./data-compactor/`      |
+| `ruler.rule_path`                             | `/rules`                 | `./data-ruler/`          |
+| `ruler_storage.filesystem.dir`                | `<empty>`                | `ruler`                  |
+| (GEM only) `auth.type`                        | `trust`                  | `enterprise`             |
+| (GEM only) `graphite.querier.schemas.backend` | `s3`                     | `filesystem`             |
 
 For these parameters `mimirtool config convert` will output the Cortex default value even when the configuration parameter is not explicitly set in the input
 configuration. If in the input configuration you explicitly set the Cortex default value, and you have provided

pkg/mimirtool/config/convert_test.go

@@ -21,17 +21,17 @@ type conversionInput struct {
 	dontLoadCommonOpts bool
 }
 
-func (in *conversionInput) loadCommonOpts(t *testing.T, yamlFile, flagsFile string) (commonYAML []byte, commonFlags []string) {
+func (in *conversionInput) loadCommonOpts(t *testing.T, oldYAMLFile, newYAMLFile, oldFlagsFile, newFlagsFile string) (newCommonYAML []byte, newCommonFlags []string) {
 	if in.dontLoadCommonOpts {
 		return nil, nil
 	}
 	if in.yaml == nil {
-		in.yaml = loadFile(t, yamlFile)
-		commonYAML = in.yaml
+		in.yaml = loadFile(t, oldYAMLFile)
+		newCommonYAML = loadFile(t, newYAMLFile)
 	}
 	if in.flags == nil {
-		in.flags = loadFlags(t, flagsFile)
-		commonFlags = in.flags
+		in.flags = loadFlags(t, oldFlagsFile)
+		newCommonFlags = loadFlags(t, newFlagsFile)
 	}
 
 	return
@@ -46,7 +46,7 @@ func testConvertGEM(t *testing.T, tc conversionInput, test func(t *testing.T, ou
 	t.Run("gem170->gem200", func(t *testing.T) {
 		t.Parallel()
 
-		expectedCommonYAML, expectedCommonFlags := tc.loadCommonOpts(t, "testdata/gem/common-options-old.yaml", "testdata/gem/common-flags-old.txt")
+		expectedCommonYAML, expectedCommonFlags := tc.loadCommonOpts(t, "testdata/gem/common-options-old.yaml", "testdata/gem/common-options-new.yaml", "testdata/gem/common-flags-old.txt", "testdata/gem/common-flags-new.txt")
 		outYAML, outFlags, notices, err := Convert(tc.yaml, tc.flags, GEM170ToGEM200Mapper(), DefaultGEM170Config, DefaultGEM200COnfig, tc.useNewDefaults, tc.outputDefaults)
 
 		if expectedCommonYAML != nil {
@@ -66,7 +66,7 @@ func testConvertCortex(t *testing.T, tc conversionInput, test func(t *testing.T,
 	t.Run("cortex->mimir", func(t *testing.T) {
 		t.Parallel()
 
-		expectedCommonYAML, expectedCommonFlags := tc.loadCommonOpts(t, "testdata/common-options.yaml", "testdata/common-flags.txt")
+		expectedCommonYAML, expectedCommonFlags := tc.loadCommonOpts(t, "testdata/common-options-old.yaml", "testdata/common-options-new.yaml", "testdata/common-flags-old.txt", "testdata/common-flags-new.txt")
 		outYAML, outFlags, notices, err := Convert(tc.yaml, tc.flags, CortexToMimirMapper(), DefaultCortexConfig, DefaultMimirConfig, tc.useNewDefaults, tc.outputDefaults)
 
 		if expectedCommonYAML != nil {
@@ -131,7 +131,7 @@ func TestConvert_Cortex(t *testing.T) {
 			inFile:       "testdata/noop-old.yaml",
 			inFlagsFile:  "testdata/flags-precedence-old.flags.txt",
 			outFlagsFile: "testdata/flags-precedence-new.flags.txt",
-			outFile:      "testdata/common-options.yaml",
+			outFile:      "testdata/common-options-new.yaml",
 			skipGEMTest:  true, // no need to test this in GEM too; plus, output for GEM also includes GEM common opts
 		},
 		{
@@ -257,23 +257,23 @@ func TestConvert_Cortex(t *testing.T) {
 			skipGEMTest:       true,
 			dontAddCommonOpts: true, // The common opts are in the outFile. That's the same reason why this test case doesn't work for GEM
 			inFile:            "testdata/empty.yaml",
-			outFile:           "testdata/old-defaults-retained-new.yaml",
+			outFile:           "testdata/common-options-new.yaml",
 		},
 		{
 			name:              "values where the old default should be retained are retained even with useNewDefaults=true",
 			skipGEMTest:       true,
 			dontAddCommonOpts: true, // The common opts are in the outFile. That's the same reason why this test case doesn't work for GEM
 			useNewDefaults:    true,
-			inFile:            "testdata/old-defaults-retained-old.yaml",
-			outFile:           "testdata/old-defaults-retained-new.yaml",
+			inFile:            "testdata/common-options-old.yaml",
+			outFile:           "testdata/common-options-new.yaml",
 		},
 		{
 			name:              "values where the old default should be retained are retained with useNewDefaults=false",
 			skipGEMTest:       true,
 			dontAddCommonOpts: true, // The common opts are in the outFile. That's the same reason why this test case doesn't work for GEM
 			useNewDefaults:    true,
-			inFile:            "testdata/old-defaults-retained-old.yaml",
-			outFile:           "testdata/old-defaults-retained-new.yaml",
+			inFile:            "testdata/common-options-old.yaml",
+			outFile:           "testdata/common-options-new.yaml",
 		},
 		{
 			name:              "values where the old default should be retained but are with random values are retained with useNewDefaults=false",
@@ -353,21 +353,21 @@ func TestConvert_GEM(t *testing.T) {
 			name:              "values where the old default should be retained are printed even when implicitly using the old default",
 			dontAddCommonOpts: true, // The common opts are in the outFile.
 			inFile:            "testdata/empty.yaml",
-			outFile:           "testdata/gem/old-defaults-retained-new.yaml",
+			outFile:           "testdata/gem/common-options-new.yaml",
 		},
 		{
 			name:              "values where the old default should be retained are retained even with useNewDefaults=true",
 			dontAddCommonOpts: true, // The common opts are in the outFile.
 			useNewDefaults:    true,
-			inFile:            "testdata/gem/old-defaults-retained-old.yaml",
-			outFile:           "testdata/gem/old-defaults-retained-new.yaml",
+			inFile:            "testdata/gem/common-options-old.yaml",
+			outFile:           "testdata/gem/common-options-new.yaml",
 		},
 		{
 			name:              "values where the old default should be retained are retained with useNewDefaults=false",
 			dontAddCommonOpts: true, // The common opts are in the outFile.
 			useNewDefaults:    true,
-			inFile:            "testdata/gem/old-defaults-retained-old.yaml",
-			outFile:           "testdata/gem/old-defaults-retained-new.yaml",
+			inFile:            "testdata/gem/common-options-old.yaml",
+			outFile:           "testdata/gem/common-options-new.yaml",
 		},
 		{
 			name:              "values where the old default should be retained but are with random values are retained with useNewDefaults=false",
@@ -500,8 +500,6 @@ func TestReportDeletedFlags(t *testing.T) {
 }
 
 var changedCortexDefaults = []ChangedDefault{
-	{Path: "activity_tracker.filepath", OldDefault: "./active-query-tracker", NewDefault: "./metrics-activity.log"},
-	{Path: "alertmanager.data_dir", OldDefault: "data/", NewDefault: "./data-alertmanager/"},
 	{Path: "alertmanager.enable_api", OldDefault: "false", NewDefault: "true"},
 	{Path: "alertmanager.external_url", OldDefault: "", NewDefault: "http://localhost:8080/alertmanager"},
 	{Path: "alertmanager.sharding_ring.instance_interface_names", OldDefault: "eth0,en0", NewDefault: "<nil>"},
@@ -524,12 +522,10 @@ var changedCortexDefaults = []ChangedDefault{
 	{Path: "blocks_storage.bucket_store.metadata_cache.memcached.max_idle_connections", OldDefault: "16", NewDefault: "100"},
 	{Path: "blocks_storage.bucket_store.metadata_cache.memcached.timeout", OldDefault: "100ms", NewDefault: "200ms"},
 	{Path: "blocks_storage.bucket_store.sync_dir", OldDefault: "tsdb-sync", NewDefault: "./tsdb-sync/"},
-	{Path: "blocks_storage.filesystem.dir", OldDefault: "", NewDefault: "blocks"},
 	{Path: "blocks_storage.tsdb.close_idle_tsdb_timeout", OldDefault: "0s", NewDefault: "13h0m0s"},
 	{Path: "blocks_storage.tsdb.dir", OldDefault: "tsdb", NewDefault: "./tsdb/"},
 	{Path: "blocks_storage.tsdb.retention_period", OldDefault: "6h0m0s", NewDefault: "24h0m0s"},
 	{Path: "compactor.block_sync_concurrency", OldDefault: "20", NewDefault: "8"},
-	{Path: "compactor.data_dir", OldDefault: "./data", NewDefault: "./data-compactor/"},
 	{Path: "compactor.sharding_ring.instance_interface_names", OldDefault: "eth0,en0", NewDefault: "<nil>"},
 	{Path: "compactor.sharding_ring.kvstore.store", OldDefault: "consul", NewDefault: "memberlist"},
 	{Path: "compactor.sharding_ring.wait_stability_min_duration", OldDefault: "1m0s", NewDefault: "0s"},
@@ -570,9 +566,7 @@ var changedCortexDefaults = []ChangedDefault{
 	{Path: "ruler.enable_api", OldDefault: "false", NewDefault: "true"},
 	{Path: "ruler.ring.instance_interface_names", OldDefault: "eth0,en0", NewDefault: "<nil>"},
 	{Path: "ruler.ring.kvstore.store", OldDefault: "consul", NewDefault: "memberlist"},
-	{Path: "ruler.rule_path", OldDefault: "/rules", NewDefault: "./data-ruler/"},
 	{Path: "ruler.ruler_client.max_send_msg_size", OldDefault: "16777216", NewDefault: "104857600"},
-	{Path: "ruler_storage.filesystem.dir", OldDefault: "", NewDefault: "ruler"},
 	{Path: "store_gateway.sharding_ring.instance_interface_names", OldDefault: "eth0,en0", NewDefault: "<nil>"},
 	{Path: "store_gateway.sharding_ring.kvstore.store", OldDefault: "consul", NewDefault: "memberlist"},
 	{Path: "store_gateway.sharding_ring.wait_stability_min_duration", OldDefault: "1m0s", NewDefault: "0s"},
@@ -619,7 +613,6 @@ func TestChangedGEMDefaults(t *testing.T) {
 		{Path: "graphite.querier.metric_name_cache.memcached.max_idle_connections", OldDefault: "16", NewDefault: "100"},
 		{Path: "graphite.querier.metric_name_cache.memcached.max_item_size", OldDefault: "0", NewDefault: "1048576"},
 		{Path: "graphite.querier.metric_name_cache.memcached.timeout", OldDefault: "100ms", NewDefault: "200ms"},
-		{Path: "graphite.querier.schemas.backend", OldDefault: "s3", NewDefault: "filesystem"},
 		{Path: "instrumentation.enabled", OldDefault: "false", NewDefault: "true"},
 		{Path: "limits.compactor_split_groups", OldDefault: "4", NewDefault: "1"},
 		{Path: "limits.compactor_tenant_shard_size", OldDefault: "1", NewDefault: "0"},

pkg/mimirtool/config/cortex.go

@@ -46,6 +46,18 @@ func CortexToMimirMapper() Mapper {
 		setOldDefaultExplicitly("blocks_storage.backend"),
 		setOldDefaultExplicitly("ruler_storage.backend"),
 		setOldDefaultExplicitly("alertmanager_storage.backend"),
+		// Prevent activity_tracker.filepath from being updates with a new default (./metrics-activity.log) implicitly and always set it to the old default (./active-query-tracker)
+		setOldDefaultWithNewPathExplicitly("querier.active_query_tracker_dir", "activity_tracker.filepath"),
+		// Prevent alertmanager.data_dir from being updates with a new default (./data-alertmanager/) implicitly and always set it to the old default (data/)
+		setOldDefaultExplicitly("alertmanager.data_dir"),
+		// Prevent blocks_storage.filesystem.dir from being updates with a new default (blocks) implicitly and always set it to the old default ("")
+		setOldDefaultExplicitly("blocks_storage.filesystem.dir"),
+		// Prevent compactor.data_dir from being updates with a new default (./data-compactor/) implicitly and always set it to the old default (./data)
+		setOldDefaultExplicitly("compactor.data_dir"),
+		// Prevent ruler.rule_path from being updates with a new default (./data-ruler/) implicitly and always set it to the old default (/rules)
+		setOldDefaultExplicitly("ruler.rule_path"),
+		// Prevent ruler_storage.filesystem.dir from being updates with a new default (ruler) implicitly and always set it to the old default ("")
+		setOldDefaultExplicitly("ruler_storage.filesystem.dir"),
 	}
 }
 
@@ -521,18 +533,22 @@ func mapInstanceInterfaceNames(ifaceNames map[string]string) Mapper {
 }
 
 func setOldDefaultExplicitly(path string) Mapper {
+	return setOldDefaultWithNewPathExplicitly(path, path)
+}
+
+func setOldDefaultWithNewPathExplicitly(oldPath, newPath string) Mapper {
 	return MapperFunc(func(source, target Parameters) error {
-		v, err := source.GetValue(path)
+		v, err := source.GetValue(oldPath)
 		if err != nil {
 			return err
 		}
 
-		if v.IsUnset() || !differentFromDefault(source, path) {
-			err = target.SetValue(path, source.MustGetDefaultValue(path))
+		if v.IsUnset() || !differentFromDefault(source, oldPath) {
+			err = target.SetValue(newPath, source.MustGetDefaultValue(oldPath))
 			// We set the default again after the value itself because when prepareSourceDefaults is mapping defaults
 			// `SetValue` actually sets the default value.
 			// Also set the source default to Nil, so that when updating defaults this parameter isn't affected
-			err2 := target.SetDefaultValue(path, Nil)
+			err2 := target.SetDefaultValue(newPath, Nil)
 			return multierror.New(err, err2).Err()
 		}
 

pkg/mimirtool/config/gem.go

@@ -85,6 +85,20 @@ func GEM170ToGEM200Mapper() Mapper {
 		setOldDefaultExplicitly("blocks_storage.backend"),
 		setOldDefaultExplicitly("ruler_storage.backend"),
 		setOldDefaultExplicitly("alertmanager_storage.backend"),
+		// Prevent activity_tracker.filepath from being updates with a new default (./metrics-activity.log) implicitly and always set it to the old default (./active-query-tracker)
+		setOldDefaultWithNewPathExplicitly("querier.active_query_tracker_dir", "activity_tracker.filepath"),
+		// Prevent alertmanager.data_dir from being updates with a new default (./data-alertmanager/) implicitly and always set it to the old default (data/)
+		setOldDefaultExplicitly("alertmanager.data_dir"),
+		// Prevent blocks_storage.filesystem.dir from being updates with a new default (blocks) implicitly and always set it to the old default ("")
+		setOldDefaultExplicitly("blocks_storage.filesystem.dir"),
+		// Prevent compactor.data_dir from being updates with a new default (./data-compactor/) implicitly and always set it to the old default (./data)
+		setOldDefaultExplicitly("compactor.data_dir"),
+		// Prevent ruler.rule_path from being updates with a new default (./data-ruler/) implicitly and always set it to the old default (/rules)
+		setOldDefaultExplicitly("ruler.rule_path"),
+		// Prevent ruler_storage.filesystem.dir from being updates with a new default (ruler) implicitly and always set it to the old default ("")
+		setOldDefaultExplicitly("ruler_storage.filesystem.dir"),
+		// Prevent graphite.querier.schemas.backend from being updated with a new default (filesystem) implicitly and always set it to the old default (s3)
+		setOldDefaultExplicitly("graphite.querier.schemas.backend"),
 	}
 }
 

pkg/mimirtool/config/testdata/common-flags-new.txt

@@ -0,0 +1,10 @@
+-server.http-listen-port=80
+-blocks-storage.backend=s3
+-ruler-storage.backend=s3
+-alertmanager-storage.backend=s3
+-activity-tracker.filepath=./active-query-tracker
+-alertmanager.storage.path=data/
+-blocks-storage.filesystem.dir=
+-compactor.data-dir=./data
+-ruler.rule-path=/rules
+-ruler-storage.filesystem.dir=

pkg/mimirtool/config/testdata/common-flags-old.txt

@@ -0,0 +1,10 @@
+-server.http-listen-port=80
+-blocks-storage.backend=s3
+-ruler-storage.backend=s3
+-alertmanager-storage.backend=s3
+-querier.active-query-tracker-dir=./active-query-tracker
+-alertmanager.storage.path=data/
+-blocks-storage.filesystem.dir=
+-compactor.data-dir=./data
+-ruler.rule-path=/rules
+-ruler-storage.filesystem.dir=

pkg/mimirtool/config/testdata/common-options-new.yaml

@@ -0,0 +1,27 @@
+server:
+  http_listen_port: 80
+
+blocks_storage:
+  backend: s3
+  filesystem:
+    dir: ''
+
+ruler_storage:
+  backend: s3
+  filesystem:
+    dir: ''
+
+alertmanager_storage:
+  backend: s3
+
+activity_tracker:
+  filepath: ./active-query-tracker
+
+alertmanager:
+  data_dir: data/
+
+compactor:
+  data_dir: ./data
+
+ruler:
+  rule_path: /rules

pkg/mimirtool/config/testdata/common-options-old.yaml

@@ -0,0 +1,27 @@
+server:
+  http_listen_port: 80
+
+blocks_storage:
+  backend: s3
+  filesystem:
+    dir: ''
+
+ruler_storage:
+  backend: s3
+  filesystem:
+    dir: ''
+
+alertmanager_storage:
+  backend: s3
+
+querier:
+  active_query_tracker_dir: ./active-query-tracker
+
+alertmanager:
+  data_dir: data/
+
+compactor:
+  data_dir: ./data
+
+ruler:
+  rule_path: /rules

pkg/mimirtool/config/testdata/gem/common-flags-new.txt

@@ -2,4 +2,11 @@
 -auth.type=trust
 -blocks-storage.backend=s3
 -ruler-storage.backend=s3
--alertmanager-storage.backend=s3
+-alertmanager-storage.backend=s3
+-graphite.querier.schemas.backend=s3
+-activity-tracker.filepath=./active-query-tracker
+-alertmanager.storage.path=data/
+-blocks-storage.filesystem.dir=
+-compactor.data-dir=./data
+-ruler.rule-path=/rules
+-ruler-storage.filesystem.dir=

pkg/mimirtool/config/testdata/gem/common-flags-old.txt

@@ -2,4 +2,11 @@
 -auth.type=trust
 -blocks-storage.backend=s3
 -ruler-storage.backend=s3
--alertmanager-storage.backend=s3
+-alertmanager-storage.backend=s3
+-graphite.querier.schemas.backend=s3
+-querier.active-query-tracker-dir=./active-query-tracker
+-alertmanager.storage.path=data/
+-blocks-storage.filesystem.dir=
+-compactor.data-dir=./data
+-ruler.rule-path=/rules
+-ruler-storage.filesystem.dir=