Skip to content

Commit

Permalink
all configmap should be updated when not in manual mode
Browse files Browse the repository at this point in the history 
Signed-off-by: haorenfsa <[email protected]>
  • Loading branch information
@haorenfsa
haorenfsa committed Feb 8, 2025
1 parent 149985a commit 3707210
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 40 deletions.
1 change: 1 addition & 0 deletions apis/milvus.io/v1beta1/components_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ type MilvusComponents struct {
// note: the active configmap won't switch automatically
// because we may want to change configmap for existing pods
// so it's hard to determine when to switch. you need to switch it manually.
// if EnableManualMode is set to false, changes will be applied to all configmaps
ActiveConfigMap string `json:"activeConfigMap,omitempty"`

// RunAsNonRoot whether to run milvus as non-root user
Expand Down
38 changes: 37 additions & 1 deletion pkg/controllers/configmaps.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ import (
corev1 "k8s.io/api/core/v1"
kerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/yaml"

"github.com/milvus-io/milvus-operator/apis/milvus.io/v1beta1"
"github.com/milvus-io/milvus-operator/pkg/config"
"github.com/milvus-io/milvus-operator/pkg/util"
"github.com/pkg/errors"
)

func (r *MilvusReconciler) getMinioAccessInfo(ctx context.Context, mc v1beta1.Milvus) (string, string) {
Expand Down Expand Up @@ -116,7 +119,40 @@ func (r *MilvusReconciler) updateConfigMap(ctx context.Context, mc v1beta1.Milvu
}

func (r *MilvusReconciler) ReconcileConfigMaps(ctx context.Context, mc v1beta1.Milvus) error {
namespacedName := NamespacedName(mc.Namespace, mc.GetActiveConfigMap())
if mc.Spec.Com.EnableManualMode {
namespacedName := NamespacedName(mc.Namespace, mc.GetActiveConfigMap())
return reconcileOneConfigMap(r, ctx, mc, namespacedName)
}
// reconcile all configmaps
cmLabels := NewAppLabels(mc.Name)
cmList := &corev1.ConfigMapList{}
err := r.List(ctx, cmList, &client.ListOptions{
Namespace: mc.Namespace,
LabelSelector: labels.SelectorFromSet(cmLabels),
})
if err != nil {
return errors.Wrap(err, "list configmaps")
}
if len(cmList.Items) == 0 {
// when no configmap found, create one
cmList.Items = append(cmList.Items, corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: mc.GetActiveConfigMap(),
Namespace: mc.Namespace,
},
})
}

for _, cm := range cmList.Items {
err = reconcileOneConfigMap(r, ctx, mc, client.ObjectKeyFromObject(&cm))
if err != nil {
return errors.Wrapf(err, "reconcile configmap[%s]", &cm.ObjectMeta)
}
}
return nil
}

var reconcileOneConfigMap = func(r *MilvusReconciler, ctx context.Context, mc v1beta1.Milvus, namespacedName types.NamespacedName) error {
old := &corev1.ConfigMap{}
err := r.Get(ctx, namespacedName, old)
if kerrors.IsNotFound(err) {
Expand Down
110 changes: 71 additions & 39 deletions pkg/controllers/configmaps_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package controllers
import (
"context"
"errors"
"fmt"
"testing"

"github.com/milvus-io/milvus-operator/apis/milvus.io/v1beta1"
Expand All @@ -12,18 +11,75 @@ import (
corev1 "k8s.io/api/core/v1"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/yaml"
)

func TestReconcileConfigMaps_CreateIfNotfound(t *testing.T) {
func TestReconcileConfigMaps(t *testing.T) {
env := newTestEnv(t)
defer env.checkMocks()
r := env.Reconciler
mockClient := env.MockClient
r := env.Reconciler
ctx := env.ctx
mc := env.Inst

// mock reconcileOneConfigMap
bak := reconcileOneConfigMap
defer func() {
reconcileOneConfigMap = bak
}()
var reconcileCMCount int
reconcileOneConfigMap = func(r *MilvusReconciler, ctx context.Context, mc v1beta1.Milvus, namespacedName types.NamespacedName) error {
reconcileCMCount++
return nil
}
t.Run("reconcile only active cm when manual mode enabled", func(t *testing.T) {
reconcileCMCount = 0
mc.Spec.Com.EnableManualMode = true
err := r.ReconcileConfigMaps(ctx, mc)
assert.NoError(t, err)
assert.Equal(t, 1, reconcileCMCount)
})

t.Run("create if none", func(t *testing.T) {
reconcileCMCount = 0
mc.Spec.Com.EnableManualMode = false
mockClient.EXPECT().
List(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
err := r.ReconcileConfigMaps(ctx, mc)
assert.NoError(t, err)
assert.Equal(t, 1, reconcileCMCount)
})

t.Run("reconcile all cm when manual mode disabled", func(t *testing.T) {
reconcileCMCount = 0
mc.Spec.Com.EnableManualMode = false
mockClient.EXPECT().
List(gomock.Any(), gomock.Any(), gomock.Any()).
DoAndReturn(func(ctx, obj any, opts ...any) error {
list := obj.(*corev1.ConfigMapList)
list.Items = []corev1.ConfigMap{
{}, {}, {},
}
return nil
})
err := r.ReconcileConfigMaps(ctx, mc)
assert.NoError(t, err)
assert.Equal(t, 3, reconcileCMCount)
})
}

func TestReconcileOneConfigMap_CreateIfNotfound(t *testing.T) {
env := newTestEnv(t)
defer env.checkMocks()
r := env.Reconciler
mockClient := env.MockClient
ctx := env.ctx
mc := env.Inst
nn := types.NamespacedName{
Namespace: mc.Namespace,
Name: mc.GetActiveConfigMap(),
}
// all ok
gomock.InOrder(
mockClient.EXPECT().
Expand All @@ -36,32 +92,35 @@ func TestReconcileConfigMaps_CreateIfNotfound(t *testing.T) {
mockClient.EXPECT().
Create(gomock.Any(), gomock.Any()).Return(nil),
)
err := r.ReconcileConfigMaps(ctx, mc)
err := reconcileOneConfigMap(r, ctx, mc, nn)
assert.NoError(t, err)

// get failed
mockClient.EXPECT().
Get(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&corev1.ConfigMap{})).
Return(errors.New("some network issue"))
err = r.ReconcileConfigMaps(ctx, mc)
err = reconcileOneConfigMap(r, ctx, mc, nn)
assert.Error(t, err)

// get failed
mockClient.EXPECT().
Get(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&corev1.ConfigMap{})).
Return(errors.New("some network issue"))
err = r.ReconcileConfigMaps(ctx, mc)
err = reconcileOneConfigMap(r, ctx, mc, nn)
assert.Error(t, err)
}

func TestReconcileConfigMaps_Existed(t *testing.T) {
func TestReconcileOneConfigMap_Existed(t *testing.T) {
env := newTestEnv(t)
defer env.checkMocks()
r := env.Reconciler
mockClient := env.MockClient
ctx := env.ctx
mc := env.Inst

nn := types.NamespacedName{
Namespace: mc.Namespace,
Name: mc.GetActiveConfigMap(),
}
t.Run("call client.Update if changed configmap", func(t *testing.T) {
mc.Spec.HookConf.Data = map[string]interface{}{
"x": "y",
Expand All @@ -83,35 +142,8 @@ func TestReconcileConfigMaps_Existed(t *testing.T) {
Update(gomock.Any(), gomock.Any()).Return(nil),
)

err := r.ReconcileConfigMaps(ctx, mc)
err := reconcileOneConfigMap(r, ctx, mc, nn)
assert.NoError(t, err)
mockClient.EXPECT().
Get(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&corev1.ConfigMap{})).
DoAndReturn(func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...any) error {
cm := obj.(*corev1.ConfigMap)
cm.Name = "cm1"
cm.Namespace = "ns"
err := mockClient.Get(ctx, client.ObjectKeyFromObject(cm), cm)
if err != nil {
return err
}
if len(cm.Data) == 0 {
return errors.New("expect data in configmap")
}
if _, ok := cm.Data["hook.yaml"]; !ok {
return errors.New("expect hook.yaml as key in data")
}
expected, err := yaml.Marshal(map[string]string{
"x": "y",
})
if err != nil {
return err
}
if cm.Data["hook.yaml"] != string(expected) {
return fmt.Errorf("content not match, expected: %s, got: %s", cm.Data["hook.yaml"], string(expected))
}
return nil
})
})

t.Run("not call client.Update if configmap not changed", func(t *testing.T) {
Expand All @@ -130,7 +162,7 @@ func TestReconcileConfigMaps_Existed(t *testing.T) {
Get(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&corev1.Secret{})).
Return(k8sErrors.NewNotFound(schema.GroupResource{}, "mockErr")).Times(2),
)
err := r.ReconcileConfigMaps(ctx, mc)
err := reconcileOneConfigMap(r, ctx, mc, nn)
assert.NoError(t, err)
})

Expand All @@ -150,7 +182,7 @@ func TestReconcileConfigMaps_Existed(t *testing.T) {
Get(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&corev1.Secret{})).
Return(k8sErrors.NewNotFound(schema.GroupResource{}, "mockErr")).Times(2),
)
err := r.ReconcileConfigMaps(ctx, mc)
err := reconcileOneConfigMap(r, ctx, mc, nn)
assert.NoError(t, err)
})

Expand Down

0 comments on commit 3707210

Please sign in to comment.