Improve logging, doc and test for kubelet config-dir file extension

cmd/kubelet/app/options/options.go

@@ -86,8 +86,9 @@ type KubeletFlags struct {
 	// Omit this flag to use the combination of built-in default configuration values and flags.
 	KubeletConfigFile string
 
-	// kubeletDropinConfigDirectory is a path to a directory to specify dropins allows the user to optionally specify
-	// additional configs to overwrite what is provided by default and in the KubeletConfigFile flag
+	// KubeletDropinConfigDirectory is the path to a directory containing drop-in configuration files that override settings from defaults and the --config file.
+	// The path may be absolute or relative; relative paths are under the Kubelet's current working directory.
+	// Drop-in files must have a '.conf' suffix and are processed in lexical order. Only '.conf' files are loaded; all other files are ignored.
 	KubeletDropinConfigDirectory string
 
 	// WindowsService should be set to true if kubelet is running as a service on Windows.
@@ -277,7 +278,7 @@ func (f *KubeletFlags) AddFlags(mainfs *pflag.FlagSet) {
 	f.addOSFlags(fs)
 
 	fs.StringVar(&f.KubeletConfigFile, "config", f.KubeletConfigFile, "The Kubelet will load its initial configuration from this file. The path may be absolute or relative; relative paths start at the Kubelet's current working directory. Omit this flag to use the built-in default configuration values. Command-line flags override configuration from this file.")
-	fs.StringVar(&f.KubeletDropinConfigDirectory, "config-dir", "", "Path to a directory to specify drop-ins, allows the user to optionally specify additional configs to overwrite what is provided by default and in the KubeletConfigFile flag. [default='']")
+	fs.StringVar(&f.KubeletDropinConfigDirectory, "config-dir", "", "Path to a directory containing drop-in configuration files that override settings from defaults and the --config file. The path may be absolute or relative; relative paths start at the Kubelet's current working directory. Drop-in files must have a '.conf' suffix (e.g., '99-kubelet-address.conf') and are processed in lexical order. Only '.conf' files are loaded; all other files are ignored. [default='']")
 	fs.StringVar(&f.KubeConfig, "kubeconfig", f.KubeConfig, "Path to a kubeconfig file, specifying how to connect to the API server. Providing --kubeconfig enables API server mode, omitting --kubeconfig enables standalone mode.")
 
 	fs.StringVar(&f.BootstrapKubeconfig, "bootstrap-kubeconfig", f.BootstrapKubeconfig, "Path to a kubeconfig file that will be used to get client certificate for kubelet. "+

cmd/kubelet/app/server.go

@@ -218,8 +218,10 @@ is checked every 20 seconds (also configurable with a flag).`,
 				}
 			}
 			// Merge the kubelet configurations if --config-dir is set
+			var skippedDropinFiles []string
 			if len(kubeletFlags.KubeletDropinConfigDirectory) > 0 {
-				if err := mergeKubeletConfigurations(kubeletConfig, kubeletFlags.KubeletDropinConfigDirectory); err != nil {
+				skippedDropinFiles, err = mergeKubeletConfigurations(kubeletConfig, kubeletFlags.KubeletDropinConfigDirectory)
+				if err != nil {
 					return fmt.Errorf("failed to merge kubelet configs: %w", err)
 				}
 			}
@@ -284,6 +286,14 @@ is checked every 20 seconds (also configurable with a flag).`,
 			for k := range config.StaticPodURLHeader {
 				config.StaticPodURLHeader[k] = []string{"<masked>"}
 			}
+
+			// Log skipped drop-in files if any were encountered during configuration merge
+			if len(skippedDropinFiles) > 0 {
+				for _, skippedFile := range skippedDropinFiles {
+					logger.V(4).Info("Skipped file in drop-in directory (does not have .conf extension)", "file", skippedFile)
+				}
+			}
+
 			// log the kubelet's config for inspection
 			logger.V(5).Info("KubeletConfiguration", "configuration", klog.Format(config))
 
@@ -321,62 +331,69 @@ is checked every 20 seconds (also configurable with a flag).`,
 // For example, if the drop-in directory contains files named "10-config.conf" and "20-config.conf",
 // the configurations in "10-config.conf" will be applied first, and then the configurations in "20-config.conf" will be applied,
 // potentially overriding the previous values.
-func mergeKubeletConfigurations(kubeletConfig *kubeletconfiginternal.KubeletConfiguration, kubeletDropInConfigDir string) error {
+// Returns a list of skipped files and an error.
+func mergeKubeletConfigurations(kubeletConfig *kubeletconfiginternal.KubeletConfiguration, kubeletDropInConfigDir string) ([]string, error) {
 	const dropinFileExtension = ".conf"
+	var skippedFiles []string
 
 	// TODO: move the "internal --> versioned --> merge --> default --> internal" operation inside the loop,
 	// and use the version of each drop-in file as the versioned target once config files can be in versions other than just v1beta1
 	versionedConfig := &kubeletconfigv1beta1.KubeletConfiguration{}
 	if err := kubeletconfigv1beta1conversion.Convert_config_KubeletConfiguration_To_v1beta1_KubeletConfiguration(kubeletConfig, versionedConfig, nil); err != nil {
-		return err
+		return nil, err
 	}
 
 	baseKubeletConfigJSON, err := json.Marshal(versionedConfig)
 	if err != nil {
-		return fmt.Errorf("failed to marshal base config: %w", err)
+		return nil, fmt.Errorf("failed to marshal base config: %w", err)
 	}
 	// Walk through the drop-in directory and update the configuration for each file
 	if err := filepath.WalkDir(kubeletDropInConfigDir, func(path string, info fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
-		if !info.IsDir() && filepath.Ext(info.Name()) == dropinFileExtension {
-			dropinConfigJSON, gvk, err := loadDropinConfigFileIntoJSON(path)
-			if err != nil {
-				return fmt.Errorf("failed to load kubelet dropin file, path: %s, error: %w", path, err)
-			}
-			if gvk == nil || gvk.Empty() {
-				return fmt.Errorf("failed to load kubelet dropin file, path: %s, no apiVersion/kind", path)
-			}
-			// TODO: expand this once more than a single kubelet config version exists
-			if *gvk != kubeletconfigv1beta1.SchemeGroupVersion.WithKind("KubeletConfiguration") {
-				return fmt.Errorf("failed to load kubelet dropin file, path: %s, unknown apiVersion/kind: %v", path, gvk.String())
-			}
-			mergedConfigJSON, err := jsonpatch.MergePatch(baseKubeletConfigJSON, dropinConfigJSON)
-			if err != nil {
-				return fmt.Errorf("failed to merge drop-in and current config: %w", err)
-			}
-			baseKubeletConfigJSON = mergedConfigJSON
+		if info.IsDir() {
+			return nil
 		}
+		if filepath.Ext(info.Name()) != dropinFileExtension {
+			skippedFiles = append(skippedFiles, path)
+			return nil
+		}
+		dropinConfigJSON, gvk, err := loadDropinConfigFileIntoJSON(path)
+		if err != nil {
+			return fmt.Errorf("failed to load kubelet dropin file, path: %s, error: %w", path, err)
+		}
+		if gvk == nil || gvk.Empty() {
+			return fmt.Errorf("failed to load kubelet dropin file, path: %s, no apiVersion/kind", path)
+		}
+		// TODO: expand this once more than a single kubelet config version exists
+		if *gvk != kubeletconfigv1beta1.SchemeGroupVersion.WithKind("KubeletConfiguration") {
+			return fmt.Errorf("failed to load kubelet dropin file, path: %s, unknown apiVersion/kind: %v", path, gvk.String())
+		}
+		mergedConfigJSON, err := jsonpatch.MergePatch(baseKubeletConfigJSON, dropinConfigJSON)
+		if err != nil {
+			return fmt.Errorf("failed to merge drop-in and current config: %w", err)
+		}
+		baseKubeletConfigJSON = mergedConfigJSON
 		return nil
 	}); err != nil {
-		return fmt.Errorf("failed to walk through kubelet dropin directory %q: %w", kubeletDropInConfigDir, err)
+		return nil, fmt.Errorf("failed to walk through kubelet dropin directory %q: %w", kubeletDropInConfigDir, err)
 	}
 
 	// reset versioned config and decode
 	versionedConfig = &kubeletconfigv1beta1.KubeletConfiguration{}
 	if err := json.Unmarshal(baseKubeletConfigJSON, versionedConfig); err != nil {
-		return fmt.Errorf("failed to unmarshal merged JSON into kubelet configuration: %w", err)
+		return nil, fmt.Errorf("failed to unmarshal merged JSON into kubelet configuration: %w", err)
 	}
 	// apply defaulting after decoding
 	kubeletconfigv1beta1conversion.SetDefaults_KubeletConfiguration(versionedConfig)
 
 	// convert back to internal config
 	if err := kubeletconfigv1beta1conversion.Convert_v1beta1_KubeletConfiguration_To_config_KubeletConfiguration(versionedConfig, kubeletConfig, nil); err != nil {
-		return fmt.Errorf("failed to convert merged config to internal kubelet configuration: %w", err)
+		return nil, fmt.Errorf("failed to convert merged config to internal kubelet configuration: %w", err)
 	}
 
-	return nil
+	return skippedFiles, nil
 }
 
 // newFlagSetWithGlobals constructs a new pflag.FlagSet with global flags registered

cmd/kubelet/app/server_test.go

@@ -78,10 +78,12 @@ func TestMergeKubeletConfigurations(t *testing.T) {
 		kubeletConfig           *kubeletconfiginternal.KubeletConfiguration
 		dropin1                 string
 		dropin2                 string
+		customDropins           map[string]string
 		overwrittenConfigFields map[string]interface{}
 		cliArgs                 []string
 		name                    string
 		expectMergeError        string
+		expectedSkippedFiles    []string
 	}{
 		{
 			kubeletConfig: &kubeletconfiginternal.KubeletConfiguration{
@@ -291,6 +293,54 @@ readOnlyPort: 10255
 `,
 			expectMergeError: "",
 		},
+		{
+			name: "yaml files are ignored, only .conf files are loaded",
+			kubeletConfig: &kubeletconfiginternal.KubeletConfiguration{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "KubeletConfiguration",
+					APIVersion: "kubelet.config.k8s.io/v1beta1",
+				},
+				Port: int32(9090),
+			},
+			customDropins: map[string]string{
+				"10-should-be-ignored.yaml": `
+apiVersion: kubelet.config.k8s.io/v1beta1
+kind: KubeletConfiguration
+port: 7777
+`,
+				"20-should-be-ignored.txt": "readme text",
+				"30-valid.conf": `
+apiVersion: kubelet.config.k8s.io/v1beta1
+kind: KubeletConfiguration
+readOnlyPort: 10255
+`,
+			},
+			overwrittenConfigFields: map[string]interface{}{
+				"Port":         int32(9090),  // Port should remain unchanged as .yaml file should be ignored
+				"ReadOnlyPort": int32(10255), // ReadOnlyPort should be set from .conf file
+			},
+			expectedSkippedFiles: []string{"10-should-be-ignored.yaml", "20-should-be-ignored.txt"},
+		},
+		{
+			name: "invalid YAML syntax in drop-in file causes merge to fail",
+			kubeletConfig: &kubeletconfiginternal.KubeletConfiguration{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "KubeletConfiguration",
+					APIVersion: "kubelet.config.k8s.io/v1beta1",
+				},
+				Port: int32(9090),
+			},
+			customDropins: map[string]string{
+				"10-malformed.conf": `
+apiVersion: kubelet.config.k8s.io/v1beta1
+kind: KubeletConfiguration
+port: [this is not valid
+  indentation is wrong
+    more indentation
+`,
+			},
+			expectMergeError: "10-malformed.conf",
+		},
 	}
 
 	for _, test := range testCases {
@@ -311,7 +361,7 @@ readOnlyPort: 10255
 				kubeletFlags.KubeletConfigFile = kubeletConfFile
 				kubeletConfig = test.kubeletConfig
 			}
-			if len(test.dropin1) > 0 || len(test.dropin2) > 0 {
+			if len(test.dropin1) > 0 || len(test.dropin2) > 0 || len(test.customDropins) > 0 {
 				// Create kubelet.conf.d directory and drop-in configuration files
 				kubeletConfDir := filepath.Join(tempDir, "kubelet.conf.d")
 				err := os.Mkdir(kubeletConfDir, 0755)
@@ -327,14 +377,30 @@ readOnlyPort: 10255
 					require.NoError(t, err, "failed to create config from a yaml file")
 				}
 
+				for filename, content := range test.customDropins {
+					err = os.WriteFile(filepath.Join(kubeletConfDir, filename), []byte(content), 0644)
+					require.NoError(t, err, "failed to create custom dropin file")
+				}
+
 				// Merge the kubelet configurations
-				err = mergeKubeletConfigurations(kubeletConfig, kubeletConfDir)
+				skippedFiles, err := mergeKubeletConfigurations(kubeletConfig, kubeletConfDir)
 				if test.expectMergeError == "" {
 					require.NoError(t, err, "failed to merge kubelet drop-in configs")
 				} else {
 					require.Error(t, err)
 					require.ErrorContains(t, err, test.expectMergeError)
 				}
+
+				// Verify skipped files if expected
+				if len(test.expectedSkippedFiles) > 0 {
+					// Extract just the filenames from the full paths
+					var skippedFilenames []string
+					for _, path := range skippedFiles {
+						skippedFilenames = append(skippedFilenames, filepath.Base(path))
+					}
+					require.ElementsMatch(t, test.expectedSkippedFiles, skippedFilenames,
+						"skipped files do not match expected")
+				}
 			}
 
 			// Use kubelet config flag precedence