Merge pull request #19148 from mansoor17syed/issue_19066

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

server/etcdmain/config.go

@@ -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
 }
 

server/etcdmain/config_test.go

@@ -25,6 +25,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"sigs.k8s.io/yaml"
 
 	"go.etcd.io/etcd/pkg/v3/featuregate"
@@ -653,3 +654,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)
+			}
+		})
+	}
+}