From 54daf163f3c8ebcde6df0a1cee02f1bb1cb197c7 Mon Sep 17 00:00:00 2001 From: sophon Date: Tue, 20 Jun 2023 16:55:45 +0800 Subject: [PATCH] fix: configmap mounted with subPath do not update when changed (#3870) --- .../config_manager/handler_util.go | 15 +++- .../config_manager/handler_util_test.go | 78 +++++++++++++++++++ internal/configuration/config_util.go | 3 +- internal/configuration/config_util_test.go | 3 +- internal/controller/plan/prepare.go | 15 ++-- internal/gotemplate/tpl_engine.go | 3 +- 6 files changed, 103 insertions(+), 14 deletions(-) diff --git a/internal/configuration/config_manager/handler_util.go b/internal/configuration/config_manager/handler_util.go index a3d1a369c69..52531809081 100644 --- a/internal/configuration/config_manager/handler_util.go +++ b/internal/configuration/config_manager/handler_util.go @@ -154,6 +154,8 @@ func GetSupportReloadConfigSpecs(configSpecs []appsv1alpha1.ComponentConfigSpec, continue } reloadConfigSpecMeta = append(reloadConfigSpecMeta, ConfigSpecMeta{ + ToolsImageSpec: cc.Spec.ToolsImageSpec, + ScriptConfig: cc.Spec.ScriptConfigs, ConfigSpecInfo: ConfigSpecInfo{ ReloadOptions: cc.Spec.ReloadOptions, ConfigSpec: configSpec, @@ -161,9 +163,18 @@ func GetSupportReloadConfigSpecs(configSpecs []appsv1alpha1.ComponentConfigSpec, DownwardAPIOptions: cc.Spec.DownwardAPIOptions, FormatterConfig: *cc.Spec.FormatterConfig, }, - ToolsImageSpec: cc.Spec.ToolsImageSpec, - ScriptConfig: cc.Spec.ScriptConfigs, }) } return reloadConfigSpecMeta, nil } + +func FilterSubPathVolumeMount(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 { + filtered = append(filtered, meta) + } + } + return filtered +} diff --git a/internal/configuration/config_manager/handler_util_test.go b/internal/configuration/config_manager/handler_util_test.go index ca674aa8e11..bd5f1fa811e 100644 --- a/internal/configuration/config_manager/handler_util_test.go +++ b/internal/configuration/config_manager/handler_util_test.go @@ -26,6 +26,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -372,3 +373,80 @@ var _ = Describe("Handler Util Test", func() { }) }) }) + +func TestFilterSubPathVolumeMount(t *testing.T) { + createConfigMeta := func(volumeName string, reloadType appsv1alpha1.CfgReloadType) ConfigSpecMeta { + return ConfigSpecMeta{ConfigSpecInfo: ConfigSpecInfo{ + ReloadType: reloadType, + ConfigSpec: appsv1alpha1.ComponentConfigSpec{ + ComponentTemplateSpec: appsv1alpha1.ComponentTemplateSpec{ + VolumeName: volumeName, + }}}} + } + + type args struct { + metas []ConfigSpecMeta + volumes []corev1.VolumeMount + } + tests := []struct { + name string + args args + want []ConfigSpecMeta + }{{ + name: "test1", + args: args{ + metas: []ConfigSpecMeta{ + createConfigMeta("test1", appsv1alpha1.UnixSignalType), + createConfigMeta("test2", appsv1alpha1.ShellType), + createConfigMeta("test3", appsv1alpha1.TPLScriptType), + }, + volumes: []corev1.VolumeMount{ + {Name: "test1", SubPath: "test1"}, + {Name: "test2", SubPath: "test2"}, + {Name: "test3", SubPath: "test3"}, + }, + }, + want: []ConfigSpecMeta{ + createConfigMeta("test3", appsv1alpha1.TPLScriptType), + }, + }, { + name: "test2", + args: args{ + metas: []ConfigSpecMeta{ + createConfigMeta("test1", appsv1alpha1.UnixSignalType), + createConfigMeta("test2", appsv1alpha1.ShellType), + createConfigMeta("test3", appsv1alpha1.TPLScriptType), + }, + volumes: []corev1.VolumeMount{ + {Name: "test1"}, + {Name: "test2"}, + {Name: "test3"}, + }, + }, + want: []ConfigSpecMeta{ + createConfigMeta("test1", appsv1alpha1.UnixSignalType), + createConfigMeta("test2", appsv1alpha1.ShellType), + createConfigMeta("test3", appsv1alpha1.TPLScriptType), + }, + }, { + name: "test3", + args: args{ + metas: []ConfigSpecMeta{ + createConfigMeta("test1", appsv1alpha1.UnixSignalType), + createConfigMeta("test2", appsv1alpha1.ShellType), + createConfigMeta("test3", appsv1alpha1.TPLScriptType), + }, + volumes: []corev1.VolumeMount{}, + }, + want: []ConfigSpecMeta{ + createConfigMeta("test1", appsv1alpha1.UnixSignalType), + createConfigMeta("test2", appsv1alpha1.ShellType), + createConfigMeta("test3", appsv1alpha1.TPLScriptType), + }, + }} + 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) + }) + } +} diff --git a/internal/configuration/config_util.go b/internal/configuration/config_util.go index ca30e038540..ef09d7578a1 100644 --- a/internal/configuration/config_util.go +++ b/internal/configuration/config_util.go @@ -27,6 +27,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" appsv1alpha1 "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" + cfgutil "github.com/apecloud/kubeblocks/internal/configuration/util" ) type ParamPairs struct { @@ -42,7 +43,7 @@ func MergeAndValidateConfigs(configConstraint appsv1alpha1.ConfigConstraintSpec, newCfg map[string]string configOperator ConfigOperator - updatedKeys = set.NewLinkedHashSetString() + updatedKeys = cfgutil.NewSet() ) cmKeySet := FromCMKeysSelector(cmKey) diff --git a/internal/configuration/config_util_test.go b/internal/configuration/config_util_test.go index 702ee880bc3..4856ee44d5c 100644 --- a/internal/configuration/config_util_test.go +++ b/internal/configuration/config_util_test.go @@ -32,6 +32,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/apecloud/kubeblocks/apis/apps/v1alpha1" + cfgutil "github.com/apecloud/kubeblocks/internal/configuration/util" testapps "github.com/apecloud/kubeblocks/internal/testutil/apps" testutil "github.com/apecloud/kubeblocks/internal/testutil/k8s" "github.com/apecloud/kubeblocks/test/testdata" @@ -266,7 +267,7 @@ func TestFromUpdatedConfig(t *testing.T) { "key2": "config context2", "key3": "config context2", }, - sets: set.NewLinkedHashSetString(), + sets: cfgutil.NewSet(), }, want: map[string]string{}, }} diff --git a/internal/controller/plan/prepare.go b/internal/controller/plan/prepare.go index 0d6e1d4d09e..f0d8092f436 100644 --- a/internal/controller/plan/prepare.go +++ b/internal/controller/plan/prepare.go @@ -24,7 +24,6 @@ import ( "fmt" "strings" - "github.com/StudioSol/set" "github.com/spf13/viper" corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -118,13 +117,8 @@ func updateResourceAnnotationsWithTemplate(obj client.Object, allTemplateAnnotat // into PodSpec if configuration reload option is on func buildConfigManagerWithComponent(podSpec *corev1.PodSpec, configSpecs []appsv1alpha1.ComponentConfigSpec, ctx context.Context, cli client.Client, cluster *appsv1alpha1.Cluster, component *component.SynthesizedComponent) error { - var ( - err error - - buildParams *cfgcm.CfgManagerBuildParams - // volumeDirs []corev1.VolumeMount - // usingConfigSpecs []appsv1alpha1.ComponentConfigSpec - ) + var err error + var buildParams *cfgcm.CfgManagerBuildParams volumeDirs, usingConfigSpecs := getUsingVolumesByConfigSpecs(podSpec, configSpecs) if len(volumeDirs) == 0 { @@ -134,6 +128,9 @@ func buildConfigManagerWithComponent(podSpec *corev1.PodSpec, configSpecs []apps if err != nil { return err } + // 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) if len(configSpecMetas) == 0 { return nil } @@ -240,7 +237,7 @@ func getUsingVolumesByConfigSpecs(podSpec *corev1.PodSpec, configSpecs []appsv1a if !cfgcore.NeedReloadVolume(configSpec) { continue } - sets := set.NewLinkedHashSetString() + sets := util.NewSet() for _, container := range config2Containers[configSpec.Name] { volume := intctrlutil.GetVolumeMountByVolume(container, configSpec.VolumeName) if volume != nil && !sets.InArray(volume.Name) { diff --git a/internal/gotemplate/tpl_engine.go b/internal/gotemplate/tpl_engine.go index daa588a061b..889e649e56d 100644 --- a/internal/gotemplate/tpl_engine.go +++ b/internal/gotemplate/tpl_engine.go @@ -31,6 +31,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" cfgcore "github.com/apecloud/kubeblocks/internal/configuration" + cfgutil "github.com/apecloud/kubeblocks/internal/configuration/util" types2 "github.com/apecloud/kubeblocks/internal/controller/client" ) @@ -194,7 +195,7 @@ func NewTplEngine(values *TplValues, funcs *BuiltInObjectsFunc, tplName string, tplValues: values, ctx: ctx, cli: cli, - importModules: set.NewLinkedHashSetString(), + importModules: cfgutil.NewSet(), importFuncs: make(map[string]functional), }