Skip to content

Commit

Permalink
Fix: generate tiflash config hash by combining tiflash config and pro…
Browse files Browse the repository at this point in the history 
…xy config (#6027)
  • Loading branch information
@fgksgf
fgksgf authored Jan 10, 2025
1 parent 286680b commit 33926aa
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 54 deletions.
4 changes: 2 additions & 2 deletions pkg/controllers/tiflash/tasks/cm.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ func TaskConfigMap(state *ReconcileContext, c client.Client) task.Task {
return task.Fail().With("tiflash proxy config cannot be encoded: %w", err)
}

state.ConfigHash, 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)
}
expected := newConfigMap(state.TiFlash(), flashData, proxyData, state.ConfigHash)
if e := c.Apply(ctx, expected); e != nil {
Expand Down
4 changes: 3 additions & 1 deletion pkg/controllers/tiflash/tasks/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@

package tasks

import "fmt"
import (
"fmt"
)

func ConfigMapName(tiflashName string) string {
return tiflashName
Expand Down
21 changes: 13 additions & 8 deletions pkg/utils/hasher/hasher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
164 changes: 121 additions & 43 deletions pkg/utils/hasher/hasher_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,116 +25,194 @@ 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
[[products]]
name = "Nail"
sku = 284758393
color = "gray"`),
semanticallyEquivalentStr: v1alpha1.ConfigFile(`[[products]]
color = "gray"`)},
comparedStrings: []v1alpha1.ConfigFile{
v1alpha1.ConfigFile(`[[products]]
sku = 738594937
name = "Hammer"
[[products]]
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)
}
}
}
})
Expand Down

0 comments on commit 33926aa

Please sign in to comment.