From 24247731ab137e86b427b47dad1f6ea0c6e4d6af Mon Sep 17 00:00:00 2001 From: zhangtao <111836083+sophon-zt@users.noreply.github.com> Date: Tue, 14 May 2024 15:16:55 +0800 Subject: [PATCH] fix: configuration mounted in subpath mode do not support dynamic update (#7332) (cherry picked from commit 865d21f3da84faa7394ceb89bd7830fccea62c46) --- .../config_manager/config_handler.go | 12 +- .../config_manager/handler_util.go | 33 ++++- .../config_manager/handler_util_test.go | 114 ++++++++++++++---- .../config_manager/reload_util.go | 15 +++ pkg/controller/configuration/config_utils.go | 2 +- 5 files changed, 149 insertions(+), 27 deletions(-) diff --git a/pkg/configuration/config_manager/config_handler.go b/pkg/configuration/config_manager/config_handler.go index 6d6d7e29a97..52ffcbe74ee 100644 --- a/pkg/configuration/config_manager/config_handler.go +++ b/pkg/configuration/config_manager/config_handler.go @@ -299,7 +299,7 @@ func CreateExecHandler(command []string, mountPoint string, configMeta *ConfigSp var formatterConfig *appsv1alpha1.FormatterConfig if backupPath != "" && configMeta != nil && configMeta.ReloadOptions != nil { - if err := backupConfigFiles([]string{configMeta.MountPoint}, filter, backupPath); err != nil { + if err := checkAndBackup(*configMeta, []string{configMeta.MountPoint}, filter, backupPath); err != nil { return nil, err } formatterConfig = &configMeta.FormatterConfig @@ -323,6 +323,16 @@ func CreateExecHandler(command []string, mountPoint string, configMeta *ConfigSp return shellTrigger, nil } +func checkAndBackup(configMeta ConfigSpecInfo, dirs []string, filter regexFilter, backupPath string) error { + if isSyncReloadAction(configMeta) { + return nil + } + if err := backupConfigFiles(dirs, filter, backupPath); err != nil { + return err + } + return nil +} + func fromConfigSpecInfo(meta *ConfigSpecInfo) string { if meta == nil || len(meta.ConfigSpec.Keys) == 0 { return "" diff --git a/pkg/configuration/config_manager/handler_util.go b/pkg/configuration/config_manager/handler_util.go index 5632423edaa..667fbee3373 100644 --- a/pkg/configuration/config_manager/handler_util.go +++ b/pkg/configuration/config_manager/handler_util.go @@ -185,13 +185,40 @@ func GetSupportReloadConfigSpecs(configSpecs []appsv1alpha1.ComponentConfigSpec, return reloadConfigSpecMeta, nil } -func FilterSubPathVolumeMount(metas []ConfigSpecMeta, volumes []corev1.VolumeMount) []ConfigSpecMeta { +// FilterSupportReloadActionConfigSpecs filters the provided ConfigSpecMeta slices based on the reload action type and volume mount configuration. +// It handles two types of updates to ConfigMaps: +// +// 1. Async mode: KubeBlocks controller is responsible for updating the ConfigMap, while kubelet synchronizes the ConfigMap to volumes. +// The config-manager detects configuration changes using fsnotify and executes the reload action. This requires volume mounting the ConfigMap. +// However, in async mode, if the volume mount is a subpath, kubelet does not synchronize the ConfigMap content to the container (see kubernetes/kubernetes#50345). +// As a result, the config-manager cannot detect configuration changes and does not support dynamic parameter updates for such configurations. +// Therefore, async-type ConfigSpecs with subpath volume mounts need to be removed. +// +// 2. Sync mode: For sync mode (regardless of the reload action type - TPLScriptType trigger or ShellType trigger), the controller directly watches +// the ConfigMap changes and actively invokes the reload action. +// +// Both async and sync types need to pass the ConfigSpecs to the config-manager. +// +// The check logic is an OR condition: either it is the first type (sync mode) or the second type (async) with a non-subpath volume mount configuration. +func FilterSupportReloadActionConfigSpecs(metas []ConfigSpecMeta, volumes []corev1.VolumeMount) []ConfigSpecMeta { var filtered []ConfigSpecMeta for _, meta := range metas { - v := FindVolumeMount(volumes, meta.ConfigSpec.VolumeName) - if v == nil || v.SubPath == "" || meta.ReloadType == appsv1alpha1.TPLScriptType { + if isSyncReloadAction(meta.ConfigSpecInfo) || + !isSubPathMount(FindVolumeMount(volumes, meta.ConfigSpec.VolumeName)) { filtered = append(filtered, meta) } } return filtered } + +func isSubPathMount(v *corev1.VolumeMount) bool { + // Configmap uses subPath case: https://github.com/kubernetes/kubernetes/issues/50345 + // The files are being updated on the host VM, but can't be updated in the container. + return v != nil && v.SubPath != "" +} + +func isSyncReloadAction(meta ConfigSpecInfo) bool { + // If synchronous reloadAction is supported, kubelet limitations can be ignored. + return meta.ReloadType == appsv1alpha1.TPLScriptType && !core.IsWatchModuleForTplTrigger(meta.TPLScriptTrigger) || + meta.ReloadType == appsv1alpha1.ShellType && !core.IsWatchModuleForShellTrigger(meta.ShellTrigger) +} diff --git a/pkg/configuration/config_manager/handler_util_test.go b/pkg/configuration/config_manager/handler_util_test.go index a117d88928f..7257c70935d 100644 --- a/pkg/configuration/config_manager/handler_util_test.go +++ b/pkg/configuration/config_manager/handler_util_test.go @@ -26,14 +26,15 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" - cfgutil "github.com/apecloud/kubeblocks/pkg/configuration/core" + cfgcore "github.com/apecloud/kubeblocks/pkg/configuration/core" + cfgutil "github.com/apecloud/kubeblocks/pkg/configuration/util" testutil "github.com/apecloud/kubeblocks/pkg/testutil/k8s" ) @@ -160,7 +161,7 @@ var _ = Describe("Handler Util Test", func() { Context("TestValidateReloadOptions", func() { It("Should succeed with no error", func() { mockK8sCli.MockGetMethod( - testutil.WithFailed(cfgutil.MakeError("failed to get resource."), testutil.WithTimes(1)), + testutil.WithFailed(cfgcore.MakeError("failed to get resource."), testutil.WithTimes(1)), testutil.WithSucceed(testutil.WithTimes(1)), ) @@ -443,9 +444,10 @@ var _ = Describe("Handler Util Test", func() { }) func TestFilterSubPathVolumeMount(t *testing.T) { - createConfigMeta := func(volumeName string, reloadType appsv1alpha1.CfgReloadType) ConfigSpecMeta { + createConfigMeta := func(volumeName string, reloadType appsv1alpha1.CfgReloadType, reloadAction *appsv1alpha1.ReloadOptions) ConfigSpecMeta { return ConfigSpecMeta{ConfigSpecInfo: ConfigSpecInfo{ - ReloadType: reloadType, + ReloadOptions: reloadAction, + ReloadType: reloadType, ConfigSpec: appsv1alpha1.ComponentConfigSpec{ ComponentTemplateSpec: appsv1alpha1.ComponentTemplateSpec{ VolumeName: volumeName, @@ -464,9 +466,19 @@ func TestFilterSubPathVolumeMount(t *testing.T) { name: "test1", args: args{ metas: []ConfigSpecMeta{ - createConfigMeta("test1", appsv1alpha1.UnixSignalType), - createConfigMeta("test2", appsv1alpha1.ShellType), - createConfigMeta("test3", appsv1alpha1.TPLScriptType), + createConfigMeta("test1", appsv1alpha1.UnixSignalType, &appsv1alpha1.ReloadOptions{ + UnixSignalTrigger: &appsv1alpha1.UnixSignalTrigger{}, + }), + createConfigMeta("test2", appsv1alpha1.ShellType, &appsv1alpha1.ReloadOptions{ + ShellTrigger: &appsv1alpha1.ShellTrigger{ + Sync: cfgutil.ToPointer(true), + }, + }), + createConfigMeta("test3", appsv1alpha1.TPLScriptType, &appsv1alpha1.ReloadOptions{ + TPLScriptTrigger: &appsv1alpha1.TPLScriptTrigger{ + Sync: cfgutil.ToPointer(true), + }, + }), }, volumes: []corev1.VolumeMount{ {Name: "test1", SubPath: "test1"}, @@ -475,15 +487,30 @@ func TestFilterSubPathVolumeMount(t *testing.T) { }, }, want: []ConfigSpecMeta{ - createConfigMeta("test3", appsv1alpha1.TPLScriptType), + createConfigMeta("test2", appsv1alpha1.ShellType, &appsv1alpha1.ReloadOptions{ + ShellTrigger: &appsv1alpha1.ShellTrigger{ + Sync: cfgutil.ToPointer(true), + }, + }), + createConfigMeta("test3", appsv1alpha1.TPLScriptType, &appsv1alpha1.ReloadOptions{ + TPLScriptTrigger: &appsv1alpha1.TPLScriptTrigger{ + Sync: cfgutil.ToPointer(true), + }, + }), }, }, { name: "test2", args: args{ metas: []ConfigSpecMeta{ - createConfigMeta("test1", appsv1alpha1.UnixSignalType), - createConfigMeta("test2", appsv1alpha1.ShellType), - createConfigMeta("test3", appsv1alpha1.TPLScriptType), + createConfigMeta("test1", appsv1alpha1.UnixSignalType, &appsv1alpha1.ReloadOptions{ + UnixSignalTrigger: &appsv1alpha1.UnixSignalTrigger{}, + }), + createConfigMeta("test2", appsv1alpha1.ShellType, &appsv1alpha1.ReloadOptions{ + ShellTrigger: &appsv1alpha1.ShellTrigger{}, + }), + createConfigMeta("test3", appsv1alpha1.TPLScriptType, &appsv1alpha1.ReloadOptions{ + TPLScriptTrigger: &appsv1alpha1.TPLScriptTrigger{}, + }), }, volumes: []corev1.VolumeMount{ {Name: "test1"}, @@ -492,29 +519,72 @@ func TestFilterSubPathVolumeMount(t *testing.T) { }, }, want: []ConfigSpecMeta{ - createConfigMeta("test1", appsv1alpha1.UnixSignalType), - createConfigMeta("test2", appsv1alpha1.ShellType), - createConfigMeta("test3", appsv1alpha1.TPLScriptType), + createConfigMeta("test1", appsv1alpha1.UnixSignalType, &appsv1alpha1.ReloadOptions{ + UnixSignalTrigger: &appsv1alpha1.UnixSignalTrigger{}, + }), + createConfigMeta("test2", appsv1alpha1.ShellType, &appsv1alpha1.ReloadOptions{ + ShellTrigger: &appsv1alpha1.ShellTrigger{}, + }), + createConfigMeta("test3", appsv1alpha1.TPLScriptType, &appsv1alpha1.ReloadOptions{ + TPLScriptTrigger: &appsv1alpha1.TPLScriptTrigger{}, + }), }, }, { name: "test3", args: args{ metas: []ConfigSpecMeta{ - createConfigMeta("test1", appsv1alpha1.UnixSignalType), - createConfigMeta("test2", appsv1alpha1.ShellType), - createConfigMeta("test3", appsv1alpha1.TPLScriptType), + createConfigMeta("test1", appsv1alpha1.UnixSignalType, &appsv1alpha1.ReloadOptions{ + UnixSignalTrigger: &appsv1alpha1.UnixSignalTrigger{}, + }), + createConfigMeta("test2", appsv1alpha1.ShellType, &appsv1alpha1.ReloadOptions{ + ShellTrigger: &appsv1alpha1.ShellTrigger{}, + }), + createConfigMeta("test3", appsv1alpha1.TPLScriptType, &appsv1alpha1.ReloadOptions{ + TPLScriptTrigger: &appsv1alpha1.TPLScriptTrigger{}, + }), }, volumes: []corev1.VolumeMount{}, }, want: []ConfigSpecMeta{ - createConfigMeta("test1", appsv1alpha1.UnixSignalType), - createConfigMeta("test2", appsv1alpha1.ShellType), - createConfigMeta("test3", appsv1alpha1.TPLScriptType), + createConfigMeta("test1", appsv1alpha1.UnixSignalType, &appsv1alpha1.ReloadOptions{ + UnixSignalTrigger: &appsv1alpha1.UnixSignalTrigger{}, + }), + createConfigMeta("test2", appsv1alpha1.ShellType, &appsv1alpha1.ReloadOptions{ + ShellTrigger: &appsv1alpha1.ShellTrigger{}, + }), + createConfigMeta("test3", appsv1alpha1.TPLScriptType, &appsv1alpha1.ReloadOptions{ + TPLScriptTrigger: &appsv1alpha1.TPLScriptTrigger{}, + }), + }, + }, { + name: "test4", + args: args{ + metas: []ConfigSpecMeta{ + createConfigMeta("test1", appsv1alpha1.UnixSignalType, &appsv1alpha1.ReloadOptions{ + UnixSignalTrigger: &appsv1alpha1.UnixSignalTrigger{}, + }), + createConfigMeta("test2", appsv1alpha1.ShellType, &appsv1alpha1.ReloadOptions{ + ShellTrigger: &appsv1alpha1.ShellTrigger{ + Sync: cfgutil.ToPointer(false), + }, + }), + createConfigMeta("test3", appsv1alpha1.TPLScriptType, &appsv1alpha1.ReloadOptions{ + TPLScriptTrigger: &appsv1alpha1.TPLScriptTrigger{ + Sync: cfgutil.ToPointer(false), + }, + }), + }, + volumes: []corev1.VolumeMount{ + {Name: "test1", SubPath: "test1"}, + {Name: "test2", SubPath: "test2"}, + {Name: "test3", SubPath: "test3"}, + }, }, + want: nil, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.Equalf(t, tt.want, FilterSubPathVolumeMount(tt.args.metas, tt.args.volumes), "FilterSubPathVolumeMount(%v, %v)", tt.args.metas, tt.args.volumes) + assert.Equalf(t, tt.want, FilterSupportReloadActionConfigSpecs(tt.args.metas, tt.args.volumes), "FilterSupportReloadActionConfigSpecs(%v, %v)", tt.args.metas, tt.args.volumes) }) } } diff --git a/pkg/configuration/config_manager/reload_util.go b/pkg/configuration/config_manager/reload_util.go index 72824fd38d3..e0f97549c37 100644 --- a/pkg/configuration/config_manager/reload_util.go +++ b/pkg/configuration/config_manager/reload_util.go @@ -232,6 +232,13 @@ func createFileRegex(fileRegex string) (regexFilter, error) { func scanConfigFiles(dirs []string, filter regexFilter) ([]string, error) { configs := make([]string, 0) for _, dir := range dirs { + isDir, err := isDirectory(dir) + if err != nil { + return nil, err + } + if !isDir { + continue + } files, err := os.ReadDir(dir) if err != nil { return nil, err @@ -248,6 +255,14 @@ func scanConfigFiles(dirs []string, filter regexFilter) ([]string, error) { return configs, nil } +func isDirectory(path string) (bool, error) { + fi, err := os.Stat(path) + if err != nil { + return false, err + } + return fi.IsDir(), nil +} + func ScanConfigVolume(mountPoint string) ([]string, error) { filter, _ := createFileRegex("") return scanConfigFiles([]string{mountPoint}, filter) diff --git a/pkg/controller/configuration/config_utils.go b/pkg/controller/configuration/config_utils.go index 4793aef3a8b..18385182aa4 100644 --- a/pkg/controller/configuration/config_utils.go +++ b/pkg/controller/configuration/config_utils.go @@ -76,7 +76,7 @@ func buildConfigManagerWithComponent(podSpec *corev1.PodSpec, configSpecs []apps } // Configmap uses subPath case: https://github.com/kubernetes/kubernetes/issues/50345 // The files are being updated on the host VM, but can't be updated in the container. - configSpecMetas = cfgcm.FilterSubPathVolumeMount(configSpecMetas, volumeDirs) + configSpecMetas = cfgcm.FilterSupportReloadActionConfigSpecs(configSpecMetas, volumeDirs) if len(configSpecMetas) == 0 { return nil }