Skip to content

Commit

Permalink
test: fix TestConfigFileDeprecatedOptions
Browse files Browse the repository at this point in the history
Use minimal config struct for YAML marshaling.
Replace custom mapToSortedSlice with standard library functions.
Fix flag verification for deprecated experimental options.

Fixes #19066

Signed-off-by: mansoora <[email protected]>
  • Loading branch information
mansoor17syed authored and gangli113 committed Jan 13, 2025
1 parent 47c4b9a commit 0c18f4d
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 8 deletions.
18 changes: 10 additions & 8 deletions server/etcdmain/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,21 +187,23 @@ func (cfg *config) parse(arguments []string) error {
return perr
}

var warningsForDeprecatedFlags []string
cfg.cf.flagSet.Visit(func(f *flag.Flag) {
if msg, ok := deprecatedFlags[f.Name]; ok {
warningsForDeprecatedFlags = append(warningsForDeprecatedFlags, msg)
// Check for deprecated options from both command line and config file
var warningsForDeprecatedOpts []string
for flagName := range cfg.ec.FlagsExplicitlySet {
if msg, ok := deprecatedFlags[flagName]; ok {
warningsForDeprecatedOpts = append(warningsForDeprecatedOpts, msg)
}
})
if len(warningsForDeprecatedFlags) > 0 {
}

// Log warnings if any deprecated options were found
if len(warningsForDeprecatedOpts) > 0 {
if lg := cfg.ec.GetLogger(); lg != nil {
for _, msg := range warningsForDeprecatedFlags {
for _, msg := range warningsForDeprecatedOpts {
lg.Warn(msg)
}
}
}

// now logger is set up
return err
}

Expand Down
89 changes: 89 additions & 0 deletions server/etcdmain/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"sigs.k8s.io/yaml"

"go.etcd.io/etcd/pkg/v3/featuregate"
Expand Down Expand Up @@ -742,3 +743,91 @@ func validateClusteringFlags(t *testing.T, cfg *config) {
t.Errorf("advertise-client-urls = %v, want %v", cfg.ec.AdvertiseClientUrls, wcfg.ec.AdvertiseClientUrls)
}
}

func TestConfigFileDeprecatedOptions(t *testing.T) {
// Define a minimal config struct with only the fields we need
type configFileYAML struct {
SnapshotCount uint64 `json:"snapshot-count,omitempty"`
MaxSnapFiles uint `json:"max-snapshots,omitempty"`
ExperimentalCompactHashCheckEnabled bool `json:"experimental-compact-hash-check-enabled,omitempty"`
ExperimentalCompactHashCheckTime time.Duration `json:"experimental-compact-hash-check-time,omitempty"`
ExperimentalWarningUnaryRequestDuration time.Duration `json:"experimental-warning-unary-request-duration,omitempty"`
}

testCases := []struct {
name string
configFileYAML configFileYAML
expectedFlags map[string]struct{}
}{
{
name: "no deprecated options",
configFileYAML: configFileYAML{},
expectedFlags: map[string]struct{}{},
},
{
name: "deprecated experimental options",
configFileYAML: configFileYAML{
ExperimentalCompactHashCheckEnabled: true,
ExperimentalCompactHashCheckTime: 2 * time.Minute,
ExperimentalWarningUnaryRequestDuration: time.Second,
},
expectedFlags: map[string]struct{}{
"experimental-compact-hash-check-enabled": {},
"experimental-compact-hash-check-time": {},
},
},
{
name: "deprecated snapshot options",
configFileYAML: configFileYAML{
SnapshotCount: 10000,
MaxSnapFiles: 5,
},
expectedFlags: map[string]struct{}{
"snapshot-count": {},
"max-snapshots": {},
},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
// Create config file
b, err := yaml.Marshal(&tc.configFileYAML)
if err != nil {
t.Fatal(err)
}

tmpfile := mustCreateCfgFile(t, b)
defer os.Remove(tmpfile.Name())

// Parse config
cfg := newConfig()
err = cfg.parse([]string{fmt.Sprintf("--config-file=%s", tmpfile.Name())})
if err != nil {
t.Fatal(err)
}

// Check which flags were set and marked as deprecated
foundFlags := make(map[string]struct{})
for flagName := range cfg.ec.FlagsExplicitlySet {
if _, ok := deprecatedFlags[flagName]; ok {
foundFlags[flagName] = struct{}{}
}
}

// Compare sets of flags
assert.Equalf(t, tc.expectedFlags, foundFlags, "deprecated flags mismatch - expected: %v, got: %v",
tc.expectedFlags, foundFlags)

// Note: experimental-warning-unary-request-duration deprecation is handled
// through a separate mechanism in embed.Config
if tc.configFileYAML.ExperimentalWarningUnaryRequestDuration != 0 {
assert.Equalf(t, cfg.ec.WarningUnaryRequestDuration,
tc.configFileYAML.ExperimentalWarningUnaryRequestDuration,
"experimental warning duration mismatch - expected: %v, got: %v",
tc.configFileYAML.ExperimentalWarningUnaryRequestDuration,
cfg.ec.WarningUnaryRequestDuration)
}
})
}
}

0 comments on commit 0c18f4d

Please sign in to comment.