diff --git a/pkg/controllers/tiflash/tasks/cm.go b/pkg/controllers/tiflash/tasks/cm.go index e32d17cf45..a2c2df25bc 100644 --- a/pkg/controllers/tiflash/tasks/cm.go +++ b/pkg/controllers/tiflash/tasks/cm.go @@ -57,15 +57,10 @@ func TaskConfigMap(state *ReconcileContext, c client.Client) task.Task { return task.Fail().With("tiflash proxy config cannot be encoded: %w", err) } - cfgHash1, err := hasher.GenerateHash(state.TiFlash().Spec.Config) + state.ConfigHash, err = hasher.GenerateHash(state.TiFlash().Spec.Config, state.TiFlash().Spec.ProxyConfig) if err != nil { - return task.Fail().With("failed to generate hash for `tiflash.spec.config`: %w", err) + return task.Fail().With("failed to generate hash for tiflash's config: %w", err) } - cfgHash2, err := hasher.GenerateHash(state.TiFlash().Spec.ProxyConfig) - if err != nil { - return task.Fail().With("failed to generate hash for `tiflash.spec.proxyConfig`: %w", err) - } - state.ConfigHash = cfgHash1 + cfgHash2 expected := newConfigMap(state.TiFlash(), flashData, proxyData, state.ConfigHash) if e := c.Apply(ctx, expected); e != nil { return task.Fail().With("can't create/update cm of tiflash: %w", e) diff --git a/pkg/utils/hasher/hasher.go b/pkg/utils/hasher/hasher.go index 71e5cc08ad..0ba6d63bec 100644 --- a/pkg/utils/hasher/hasher.go +++ b/pkg/utils/hasher/hasher.go @@ -26,16 +26,21 @@ import ( hashutil "github.com/pingcap/tidb-operator/third_party/kubernetes/pkg/util/hash" ) -// GenerateHash takes a TOML string as input, unmarshals it into a map, -// and generates a hash of the resulting configuration. The hash is then -// encoded into a safe string format and returned. +// GenerateHash takes multiple TOML strings as input, unmarshals each TOML string into a map, +// then append the map to an array, generates a hash of the resulting configuration. +// The hash is then encoded into a safe string format and returned. // If the order of keys in the TOML string is different, the hash will be the same. -func GenerateHash(tomlStr v1alpha1.ConfigFile) (string, error) { - var config map[string]any - if err := toml.NewDecoder(bytes.NewReader([]byte(tomlStr))).Decode(&config); err != nil { - return "", fmt.Errorf("failed to unmarshal toml string %s: %w", tomlStr, err) +// If the order of input TOML strings is different, the hash will be different. +func GenerateHash(tomlStrings ...v1alpha1.ConfigFile) (string, error) { + configs := make([]map[string]any, 0, len(tomlStrings)) + for _, tomlStr := range tomlStrings { + var config map[string]any + if err := toml.NewDecoder(bytes.NewReader([]byte(tomlStr))).Decode(&config); err != nil { + return "", fmt.Errorf("failed to unmarshal toml string %s: %w", tomlStr, err) + } + configs = append(configs, config) } hasher := fnv.New32a() - hashutil.DeepHashObject(hasher, config) + hashutil.DeepHashObject(hasher, configs) return rand.SafeEncodeString(fmt.Sprint(hasher.Sum32())), nil } diff --git a/pkg/utils/hasher/hasher_test.go b/pkg/utils/hasher/hasher_test.go index f54c3ca2be..5935a09e8f 100644 --- a/pkg/utils/hasher/hasher_test.go +++ b/pkg/utils/hasher/hasher_test.go @@ -25,81 +25,96 @@ import ( func TestGenerateHash(t *testing.T) { tests := []struct { - name string - tomlStr v1alpha1.ConfigFile - semanticallyEquivalentStr v1alpha1.ConfigFile - wantHash string - wantError bool + name string + tomlStrings []v1alpha1.ConfigFile + comparedStrings []v1alpha1.ConfigFile + shouldGetSameHash bool + wantHash string + wantError bool }{ + // Single input { name: "Valid TOML string", - tomlStr: v1alpha1.ConfigFile(`foo = 'bar' + tomlStrings: []v1alpha1.ConfigFile{ + v1alpha1.ConfigFile(`foo = 'bar' [log] k1 = 'v1' -k2 = 'v2'`), - semanticallyEquivalentStr: v1alpha1.ConfigFile(`foo = 'bar' +k2 = 'v2'`)}, + comparedStrings: []v1alpha1.ConfigFile{ + v1alpha1.ConfigFile(`foo = 'bar' [log] k2 = 'v2' -k1 = 'v1'`), - wantHash: "5dbbcf4574", - wantError: false, +k1 = 'v1'`)}, + wantHash: "6cc5875ff8", + shouldGetSameHash: true, + wantError: false, }, { name: "Different config value", - tomlStr: v1alpha1.ConfigFile(`foo = 'foo' + tomlStrings: []v1alpha1.ConfigFile{ + v1alpha1.ConfigFile(`foo = 'foo' [log] k2 = 'v2' -k1 = 'v1'`), - wantHash: "f5bc46cb9", +k1 = 'v1'`)}, + wantHash: "7dd86884c", wantError: false, }, { name: "multiple sections with blank line", - tomlStr: v1alpha1.ConfigFile(`[a] + tomlStrings: []v1alpha1.ConfigFile{ + v1alpha1.ConfigFile(`[a] k1 = 'v1' [b] -k2 = 'v2'`), - semanticallyEquivalentStr: v1alpha1.ConfigFile(`[a] +k2 = 'v2'`)}, + comparedStrings: []v1alpha1.ConfigFile{ + v1alpha1.ConfigFile(`[a] k1 = 'v1' [b] -k2 = 'v2'`), - wantHash: "79598d5977", - wantError: false, +k2 = 'v2'`)}, + shouldGetSameHash: true, + wantHash: "6577c5d475", + wantError: false, }, { - name: "Empty TOML string", - tomlStr: v1alpha1.ConfigFile(``), - wantHash: "7d6fc488b7", - wantError: false, + name: "Empty TOML string", + tomlStrings: []v1alpha1.ConfigFile{v1alpha1.ConfigFile(``)}, + wantHash: "76b94dc7f9", + wantError: false, }, { name: "Invalid TOML string", - tomlStr: v1alpha1.ConfigFile(`key1 = "value1" - key2 = value2`), // Missing quotes around value2 + tomlStrings: []v1alpha1.ConfigFile{ + v1alpha1.ConfigFile(`key1 = "value1" +key2 = value2`), // Missing quotes around value2 + }, wantHash: "", wantError: true, }, { name: "Nested tables", - tomlStr: v1alpha1.ConfigFile(`[parent] + tomlStrings: []v1alpha1.ConfigFile{ + v1alpha1.ConfigFile(`[parent] child1 = "value1" child2 = "value2" [parent.child] grandchild1 = "value3" -grandchild2 = "value4"`), - semanticallyEquivalentStr: v1alpha1.ConfigFile(`[parent] +grandchild2 = "value4"`)}, + comparedStrings: []v1alpha1.ConfigFile{ + v1alpha1.ConfigFile(`[parent] child2 = "value2" child1 = "value1" [parent.child] grandchild2 = "value4" -grandchild1 = "value3"`), - wantHash: "7bf645ccb4", - wantError: false, +grandchild1 = "value3"`)}, + shouldGetSameHash: true, + wantHash: "794796f9d", + wantError: false, }, { name: "Array of tables", - tomlStr: v1alpha1.ConfigFile(`[[products]] + tomlStrings: []v1alpha1.ConfigFile{ + v1alpha1.ConfigFile(`[[products]] name = "Hammer" sku = 738594937 @@ -107,8 +122,9 @@ sku = 738594937 name = "Nail" sku = 284758393 -color = "gray"`), - semanticallyEquivalentStr: v1alpha1.ConfigFile(`[[products]] +color = "gray"`)}, + comparedStrings: []v1alpha1.ConfigFile{ + v1alpha1.ConfigFile(`[[products]] sku = 738594937 name = "Hammer" @@ -116,25 +132,87 @@ name = "Hammer" sku = 284758393 name = "Nail" -color = "gray"`), - wantHash: "7549cf87f4", - wantError: false, +color = "gray"`)}, + shouldGetSameHash: true, + wantHash: "7cb6dd5946", + wantError: false, + }, + // Multiple inputs + { + name: "both of the inputs change the order of keys", + tomlStrings: []v1alpha1.ConfigFile{ + v1alpha1.ConfigFile(`[parent] +child1 = "value1" +child2 = "value2" +[parent.child] +grandchild1 = "value3" +grandchild2 = "value4"`), + v1alpha1.ConfigFile(`[parent] +child1 = "value1" +child2 = "value2" +[parent.child] +grandchild1 = "value3" +grandchild2 = "value4"`)}, + comparedStrings: []v1alpha1.ConfigFile{ + v1alpha1.ConfigFile(`[parent] +child2 = "value2" +child1 = "value1" +[parent.child] +grandchild2 = "value4" +grandchild1 = "value3"`), + v1alpha1.ConfigFile(`[parent] +child2 = "value2" +child1 = "value1" +[parent.child] +grandchild2 = "value4" +grandchild1 = "value3"`)}, + shouldGetSameHash: true, + wantHash: "7c946f9fd6", + wantError: false, + }, + { + name: "change the order of inputs", + tomlStrings: []v1alpha1.ConfigFile{ + v1alpha1.ConfigFile(`[parent] +child1 = "value1" +child2 = "value2" +[parent.child] +grandchild1 = "value3" +grandchild2 = "value4"`), + v1alpha1.ConfigFile(`[foo] +k1 = "value1"`)}, + comparedStrings: []v1alpha1.ConfigFile{ + v1alpha1.ConfigFile(`[foo] +k1 = "value1"`), + v1alpha1.ConfigFile(`[parent] +child1 = "value1" +child2 = "value2" +[parent.child] +grandchild1 = "value3" +grandchild2 = "value4"`)}, + shouldGetSameHash: false, + wantHash: "5c8f696db7", + wantError: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotHash, err := GenerateHash(tt.tomlStr) + hash1, err := GenerateHash(tt.tomlStrings...) if tt.wantError { assert.Error(t, err) } else { require.NoError(t, err) - assert.Equal(t, tt.wantHash, gotHash) + assert.Equal(t, tt.wantHash, hash1) - if string(tt.semanticallyEquivalentStr) != "" { - reorderedHash, err := GenerateHash(tt.semanticallyEquivalentStr) + if len(tt.comparedStrings) > 0 { + hash2, err := GenerateHash(tt.comparedStrings...) require.NoError(t, err) - assert.Equal(t, tt.wantHash, reorderedHash) + if tt.shouldGetSameHash { + assert.Equal(t, hash1, hash2) + } else { + assert.NotEqual(t, hash1, hash2) + } } } })