Skip to content

Commit

Permalink
fix: configuration mounted in subpath mode do not support dynamic upd…
Browse files Browse the repository at this point in the history
…ate (#7332)

(cherry picked from commit 865d21f3da84faa7394ceb89bd7830fccea62c46)
  • Loading branch information
sophon-zt authored May 14, 2024
1 parent 560702f commit 2424773
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 27 deletions.
12 changes: 11 additions & 1 deletion pkg/configuration/config_manager/config_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 ""
Expand Down
33 changes: 30 additions & 3 deletions pkg/configuration/config_manager/handler_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
114 changes: 92 additions & 22 deletions pkg/configuration/config_manager/handler_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)),
)

Expand Down Expand Up @@ -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,
Expand All @@ -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"},
Expand All @@ -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"},
Expand All @@ -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)
})
}
}
15 changes: 15 additions & 0 deletions pkg/configuration/config_manager/reload_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/configuration/config_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 2424773

Please sign in to comment.