-
Notifications
You must be signed in to change notification settings - Fork 20
(feat): Add support for auto-mounting extra configs #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1d84ac2
2f430de
8fca96b
cc4e7ed
212a500
c589a56
c9e77dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ rules: | |
| - "" | ||
| resources: | ||
| - configmaps | ||
| - secrets | ||
| - services | ||
| verbs: | ||
| - create | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| ## Append samples of your project ## | ||
| resources: | ||
| - sample_config.yaml | ||
| - v1alpha1_valkeycluster.yaml | ||
| # +kubebuilder:scaffold:manifestskustomizesamples |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| --- | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: valkeycluster-sample-conf | ||
| data: | ||
| extra.conf: | | ||
| maxmemory 50mb | ||
| maxmemory-policy allkeys-lfu | ||
| aof.conf: | | ||
| appendonly yes | ||
| appendfilename "orbit.aof" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| /* | ||
| Copyright 2025 Valkey Contributors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package controller | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| corev1 "k8s.io/api/core/v1" | ||
| apierrors "k8s.io/apimachinery/pkg/api/errors" | ||
| "k8s.io/apimachinery/pkg/types" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| logf "sigs.k8s.io/controller-runtime/pkg/log" | ||
| valkeyiov1alpha1 "valkey.io/valkey-operator/api/v1alpha1" | ||
| ) | ||
|
|
||
| func getConfigVolumes(ctx context.Context, c client.Client, cluster *valkeyiov1alpha1.ValkeyCluster) ([]corev1.Volume, error) { | ||
|
|
||
| // Volume containing health, and liveliness scripts | ||
| scriptsVolume := corev1.Volume{ | ||
| Name: "scripts", | ||
| VolumeSource: corev1.VolumeSource{ | ||
| ConfigMap: &corev1.ConfigMapVolumeSource{ | ||
| LocalObjectReference: corev1.LocalObjectReference{ | ||
| Name: cluster.Name, | ||
| }, | ||
| DefaultMode: func(i int32) *int32 { return &i }(0755), | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| // Volume containing default Valkey configuration | ||
| defaultConfigVolume := corev1.Volume{ | ||
| Name: "valkey-conf", | ||
| VolumeSource: corev1.VolumeSource{ | ||
| ConfigMap: &corev1.ConfigMapVolumeSource{ | ||
| LocalObjectReference: corev1.LocalObjectReference{ | ||
| Name: cluster.Name, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| // User config volume | ||
| userConfigVolume, err := getUserConfigVolume(ctx, c, cluster) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return []corev1.Volume{ | ||
| scriptsVolume, | ||
| defaultConfigVolume, | ||
| userConfigVolume, | ||
| }, nil | ||
| } | ||
|
|
||
| // Discover user-created Valkey configuration. This can be either a Secret, or ConfigMap, created by the user | ||
| func getUserConfigVolume(ctx context.Context, c client.Client, cluster *valkeyiov1alpha1.ValkeyCluster) (corev1.Volume, error) { | ||
| log := logf.FromContext(ctx) | ||
|
|
||
| configMapName := cluster.Name + "-conf" | ||
| userConfigFilter := types.NamespacedName{ | ||
| Namespace: cluster.Namespace, | ||
| Name: configMapName, | ||
| } | ||
|
|
||
| // First, look for a Secret named "$clusterName-conf" | ||
| err := c.Get(ctx, userConfigFilter, &corev1.Secret{}) | ||
| if err == nil { | ||
| log.V(1).Info("found user-created secret config") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should pass "$clusterName-conf" from the input spec so users can optionally provide any existing ConfigMap if needed. Also, getUserConfigVolume / userConfig should only be used when this new custom field is set in the spec; otherwise, the default behavior should remain not to use any existing resource. we need to agree on spec changes with others. Please update spec design or feel free to start discussion. not sure if this feature is priority for others. |
||
| return getSecretConfigVolume("user-conf", configMapName, false), nil | ||
| } | ||
| if !apierrors.IsNotFound(err) { | ||
| log.Error(err, "failed to search for user config Secret") | ||
| return corev1.Volume{}, err | ||
| } | ||
|
|
||
| // Next, look for a ConfigMap named "$clusterName-conf" | ||
| err = c.Get(ctx, userConfigFilter, &corev1.ConfigMap{}) | ||
| if err == nil { | ||
| log.V(1).Info("found user-created configMap") | ||
| return getConfigVolume("user-conf", configMapName, false), nil | ||
| } | ||
| if !apierrors.IsNotFound(err) { | ||
| log.Error(err, "failed to search for user config ConfigMap") | ||
| return corev1.Volume{}, err | ||
| } | ||
|
|
||
| // If neither found, create empty (optional) config-volume | ||
| log.V(1).Info("using empty configMap") | ||
| return getConfigVolume("user-conf", configMapName, true), nil | ||
| } | ||
|
|
||
| func getConfigVolume(configVolumeName, configMapName string, optional bool) corev1.Volume { | ||
| return corev1.Volume{ | ||
| Name: configVolumeName, | ||
| VolumeSource: corev1.VolumeSource{ | ||
| ConfigMap: &corev1.ConfigMapVolumeSource{ | ||
| LocalObjectReference: corev1.LocalObjectReference{ | ||
| Name: configMapName, | ||
| }, | ||
| Optional: func(b bool) *bool { return &b }(optional), | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| func getSecretConfigVolume(configVolumeName, configMapName string, optional bool) corev1.Volume { | ||
| return corev1.Volume{ | ||
| Name: configVolumeName, | ||
| VolumeSource: corev1.VolumeSource{ | ||
| Secret: &corev1.SecretVolumeSource{ | ||
| SecretName: configMapName, | ||
| Optional: func(b bool) *bool { return &b }(optional), | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| /* | ||
| Copyright 2025 Valkey Contributors. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package controller | ||
|
|
||
| import ( | ||
| "reflect" | ||
| "testing" | ||
|
|
||
| corev1 "k8s.io/api/core/v1" | ||
| ) | ||
|
|
||
| func TestGetConfigVolume(t *testing.T) { | ||
|
|
||
| volumeName := "test1" | ||
| mapName := "alice-conf" | ||
| optional := false | ||
|
|
||
| vol := getConfigVolume(volumeName, mapName, optional) | ||
|
|
||
| if vol.Name != volumeName { | ||
| t.Errorf("Expected volume name to be '%v', got %v", volumeName, vol.Name) | ||
| } | ||
| if reflect.TypeOf(vol.VolumeSource) != reflect.TypeOf(corev1.VolumeSource{}) { | ||
| t.Errorf("Expected VolumeSource to be native k8s type") | ||
| } | ||
| if reflect.TypeOf(vol.VolumeSource.ConfigMap) != reflect.TypeOf(&corev1.ConfigMapVolumeSource{}) { | ||
| t.Errorf("Expected VolumeSource.ConfigMap to be native k8s type") | ||
| } | ||
| if vol.VolumeSource.ConfigMap.Name != mapName { | ||
| t.Errorf("Expected ConfigMap name to be '%v', got '%v'", mapName, vol.VolumeSource.ConfigMap.Name) | ||
| } | ||
| if *vol.VolumeSource.ConfigMap.Optional != optional { | ||
| t.Errorf("Expection ConfigMap.Optional to be %v, got %v", optional, vol.VolumeSource.ConfigMap.Optional) | ||
| } | ||
| } | ||
|
|
||
| func TestGetSecretConfigVolume(t *testing.T) { | ||
|
|
||
| volumeName := "test2" | ||
| secretName := "bob-conf" | ||
| optional := true | ||
|
|
||
| vol := getSecretConfigVolume(volumeName, secretName, optional) | ||
|
|
||
| if vol.Name != volumeName { | ||
| t.Errorf("Expected volume name to be '%v', got %v", volumeName, vol.Name) | ||
| } | ||
| if reflect.TypeOf(vol.VolumeSource) != reflect.TypeOf(corev1.VolumeSource{}) { | ||
| t.Errorf("Expected VolumeSource to be native k8s type") | ||
| } | ||
| if reflect.TypeOf(vol.VolumeSource.Secret) != reflect.TypeOf(&corev1.SecretVolumeSource{}) { | ||
| t.Errorf("Expected VolumeSource.Secret to be native k8s type") | ||
| } | ||
| if vol.VolumeSource.Secret.SecretName != secretName { | ||
| t.Errorf("Expected Secret name to be '%v', got '%v'", secretName, vol.VolumeSource.Secret.SecretName) | ||
| } | ||
| if *vol.VolumeSource.Secret.Optional != optional { | ||
| t.Errorf("Expection Secret.Optional to be %v, got %v", optional, vol.VolumeSource.Secret.Optional) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we move this getConfigVolumes call to inside createDeployment. this way we dont need to pass it as param?