Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for #19066 Print warnings when deprecated options are configured in config file #19148

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions server/etcdmain/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,21 +182,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
88 changes: 88 additions & 0 deletions server/etcdmain/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ import (
"errors"
"flag"
"fmt"
"maps"
"net/url"
"os"
"reflect"
"slices"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -653,3 +655,89 @@ 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
if !reflect.DeepEqual(foundFlags, tc.expectedFlags) {
t.Errorf("deprecated flags mismatch:\ngot: %v\nwant: %v",
slices.Sorted(maps.Keys(foundFlags)),
slices.Sorted(maps.Keys(tc.expectedFlags)))
}

// Special check for experimental-warning-unary-request-duration
if tc.configFileYAML.ExperimentalWarningUnaryRequestDuration != 0 {
t.Log("Note: experimental-warning-unary-request-duration deprecation is handled separately")
}
})
}
}
Loading