update test

Author: leon-ape

apis/parameters/v1alpha1/types.go

@@ -167,7 +167,7 @@ type ParametersInFile struct {
 
 // ReloadPolicy defines the policy of reconfiguring.
 // +enum
-// +kubebuilder:validation:Enum={none,restart,rolling,asyncReload,syncReload,dynamicReloadBeginRestart}
+// +kubebuilder:validation:Enum={none,restart,asyncReload,syncReload,dynamicReloadBeginRestart}
 type ReloadPolicy string
 
 const (

controllers/parameters/parameter_util.go …/parameters/parameter_controller_util.gocontrollers/parameters/parameter_util.go renamed to controllers/parameters/parameter_controller_util.go controllers/parameters/parameter_util.go renamed to controllers/parameters/parameter_controller_util.go

@@ -214,7 +214,7 @@ func (r *ReconfigureReconciler) sync(reqCtx intctrlutil.RequestCtx, configMap *c
 			configPatch.UpdateConfig))
 	}
 
-	tasks, err := genReconfigureActionTasks(configSpec, rctx, configPatch, forceRestart)
+	tasks, err := buildReconfigureTasks(configSpec, rctx, configPatch, forceRestart)
 	if err != nil {
 		return intctrlutil.RequeueWithErrorAndRecordEvent(configMap, r.Recorder, err, reqCtx.Log)
 	}
@@ -234,15 +234,15 @@ func (r *ReconfigureReconciler) updateConfigCMStatus(reqCtx intctrlutil.RequestC
 	return intctrlutil.Reconciled()
 }
 
-func (r *ReconfigureReconciler) performUpgrade(rctx *ReconcileContext, reloadTasks []ReloadAction) (ctrl.Result, error) {
+func (r *ReconfigureReconciler) performUpgrade(rctx *ReconcileContext, tasks []reconfigureTask) (ctrl.Result, error) {
 	var (
 		err        error
 		status     returnedStatus
 		reloadType string
 	)
-	for _, task := range reloadTasks {
-		reloadType = task.ReloadType()
-		status, err = task.ExecReload()
+	for _, task := range tasks {
+		reloadType = string(task.policy)
+		status, err = task.reconfigure()
 		if err != nil || status.Status != ESNone {
 			break
 		}

controllers/parameters/reconfigure_controller.go

@@ -28,29 +28,19 @@ import (
 	"github.com/apecloud/kubeblocks/pkg/parameters/core"
 )
 
-type ReloadAction interface {
-	ExecReload() (returnedStatus, error)
-	ReloadType() string
-}
-
 type reconfigureTask struct {
-	parametersv1alpha1.ReloadPolicy
+	policy  parametersv1alpha1.ReloadPolicy
 	taskCtx reconfigureContext
 }
 
-func (r reconfigureTask) ReloadType() string {
-	return string(r.ReloadPolicy)
-}
-
-func (r reconfigureTask) ExecReload() (returnedStatus, error) {
-	if executor, ok := upgradePolicyMap[r.ReloadPolicy]; ok {
+func (r reconfigureTask) reconfigure() (returnedStatus, error) {
+	if executor, ok := upgradePolicyMap[r.policy]; ok {
 		return executor.Upgrade(r.taskCtx)
 	}
-	return returnedStatus{}, fmt.Errorf("not support reload action[%s]", r.ReloadPolicy)
+	return returnedStatus{}, fmt.Errorf("unknown reconfigure policy: %s", r.policy)
 }
 
-func resolveReloadActionPolicy(jsonPatch string,
-	format *parametersv1alpha1.FileFormatConfig,
+func resolveReconfigurePolicy(jsonPatch string, format *parametersv1alpha1.FileFormatConfig,
 	pd *parametersv1alpha1.ParametersDefinitionSpec) (parametersv1alpha1.ReloadPolicy, error) {
 	var policy = parametersv1alpha1.NonePolicy
 	dynamicUpdate, err := core.CheckUpdateDynamicParameters(format, pd, jsonPatch)
@@ -74,14 +64,12 @@ func resolveReloadActionPolicy(jsonPatch string,
 	return policy, nil
 }
 
-// genReconfigureActionTasks generates a list of reconfiguration tasks based on the provided templateSpec,
-// reconfiguration context, configuration patch, and a restart flag.
-func genReconfigureActionTasks(templateSpec *appsv1.ComponentFileTemplate, rctx *ReconcileContext, patch *core.ConfigPatchInfo, restart bool) ([]ReloadAction, error) {
-	var tasks []ReloadAction
+func buildReconfigureTasks(templateSpec *appsv1.ComponentFileTemplate, rctx *ReconcileContext, patch *core.ConfigPatchInfo, restart bool) ([]reconfigureTask, error) {
+	var tasks []reconfigureTask
 
 	// If the patch or ConfigRender is nil, return a single restart task.
 	if patch == nil || rctx.ConfigRender == nil {
-		return []ReloadAction{buildRestartTask(templateSpec, rctx)}, nil
+		return []reconfigureTask{buildRestartTask(templateSpec, rctx)}, nil
 	}
 
 	// needReloadAction determines if a reload action is needed based on the ParametersDefinition and ReloadPolicy.
@@ -100,27 +88,32 @@ func genReconfigureActionTasks(templateSpec *appsv1.ComponentFileTemplate, rctx
 			continue
 		}
 		// Determine the appropriate ReloadPolicy.
-		policy, err := resolveReloadActionPolicy(string(jsonPatch), configFormat.FileFormatConfig, &pd.Spec)
+		policy, err := resolveReconfigurePolicy(string(jsonPatch), configFormat.FileFormatConfig, &pd.Spec)
 		if err != nil {
 			return nil, err
 		}
 		// If a reload action is needed, append a new reload action task to the tasks slice.
 		if needReloadAction(pd, policy) {
-			tasks = append(tasks, buildReloadActionTask(policy, templateSpec, rctx, pd, configFormat, patch))
+			tasks = append(tasks, buildReloadTask(policy, templateSpec, rctx, pd, configFormat, patch))
 		}
 	}
 
 	// If no tasks were added, return a single restart task.
 	if len(tasks) == 0 {
-		return []ReloadAction{buildRestartTask(templateSpec, rctx)}, nil
+		return []reconfigureTask{buildRestartTask(templateSpec, rctx)}, nil
 	}
 
 	return tasks, nil
 }
 
-func buildReloadActionTask(reloadPolicy parametersv1alpha1.ReloadPolicy, templateSpec *appsv1.ComponentFileTemplate, rctx *ReconcileContext, pd *parametersv1alpha1.ParametersDefinition, configDescription *parametersv1alpha1.ComponentConfigDescription, patch *core.ConfigPatchInfo) reconfigureTask {
+func buildReloadTask(policy parametersv1alpha1.ReloadPolicy,
+	templateSpec *appsv1.ComponentFileTemplate,
+	rctx *ReconcileContext,
+	pd *parametersv1alpha1.ParametersDefinition,
+	configDescription *parametersv1alpha1.ComponentConfigDescription,
+	patch *core.ConfigPatchInfo) reconfigureTask {
 	return reconfigureTask{
-		ReloadPolicy: reloadPolicy,
+		policy: policy,
 		taskCtx: reconfigureContext{
 			RequestCtx:           rctx.RequestCtx,
 			Client:               rctx.Client,
@@ -139,7 +132,7 @@ func buildReloadActionTask(reloadPolicy parametersv1alpha1.ReloadPolicy, templat
 
 func buildRestartTask(configTemplate *appsv1.ComponentFileTemplate, rctx *ReconcileContext) reconfigureTask {
 	return reconfigureTask{
-		ReloadPolicy: parametersv1alpha1.RestartPolicy,
+		policy: parametersv1alpha1.RestartPolicy,
 		taskCtx: reconfigureContext{
 			RequestCtx:           rctx.RequestCtx,
 			Client:               rctx.Client,

controllers/parameters/policy_util.go …arameters/reconfigure_controller_util.gocontrollers/parameters/policy_util.go renamed to controllers/parameters/reconfigure_controller_util.go controllers/parameters/policy_util.go renamed to controllers/parameters/reconfigure_controller_util.go

@@ -20,13 +20,11 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 package parameters
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -45,14 +43,6 @@ var (
 	// itsSchemaKind    = workloads.GroupVersion.WithKind(workloads.InstanceSetKind)
 )
 
-func newMockRunningComponent() *appsv1.Component {
-	return &appsv1.Component{
-		Status: appsv1.ComponentStatus{
-			Phase: appsv1.RunningComponentPhase,
-		},
-	}
-}
-
 type paramsOps func(params *reconfigureContext)
 
 func withClusterComponent(replicas int) paramsOps {
@@ -163,62 +153,6 @@ func newMockReconfigureParams(testName string, cli client.Client, paramOps ...pa
 	return params
 }
 
-func newMockPodsWitheContext(ctx reconfigureContext, replicas int, options ...podOptions) []corev1.Pod {
-	pods := make([]corev1.Pod, replicas)
-	for i := 0; i < replicas; i++ {
-		pods[i] = newMockPod(ctx.SynthesizedComponent.Name+"-"+fmt.Sprint(i), ctx.SynthesizedComponent.PodSpec)
-		// pods[i].OwnerReferences = []metav1.OwnerReference{newControllerRef(its, itsSchemaKind)}
-		pods[i].Status.PodIP = "1.1.1.1"
-	}
-	for _, customFn := range options {
-		for i := range pods {
-			pod := &pods[i]
-			customFn(pod, i)
-		}
-	}
-	return pods
-}
-
-func withReadyPod(rMin, rMax int) podOptions {
-	return func(pod *corev1.Pod, index int) {
-		if index < rMin || index >= rMax {
-			return
-		}
-
-		if pod.Status.Conditions == nil {
-			pod.Status.Conditions = make([]corev1.PodCondition, 0)
-		}
-
-		pod.Status.Conditions = append(pod.Status.Conditions, corev1.PodCondition{
-			Type:   corev1.PodReady,
-			Status: corev1.ConditionTrue,
-		})
-
-		pod.Status.Phase = corev1.PodRunning
-	}
-}
-
-func fromPodObjectList(pods []corev1.Pod) []runtime.Object {
-	objs := make([]runtime.Object, len(pods))
-	for i := 0; i < len(pods); i++ {
-		objs[i] = &pods[i]
-	}
-	return objs
-}
-
-type podOptions func(pod *corev1.Pod, index int)
-
-func newMockPod(podName string, podSpec *corev1.PodSpec) corev1.Pod {
-	pod := corev1.Pod{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      podName,
-			Namespace: defaultNamespace,
-		},
-	}
-	pod.Spec = *podSpec.DeepCopy()
-	return pod
-}
-
 func Test_resolveReloadActionPolicy(t *testing.T) {
 	type args struct {
 		jsonPatch string
@@ -334,7 +268,7 @@ func Test_resolveReloadActionPolicy(t *testing.T) {
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := resolveReloadActionPolicy(tt.args.jsonPatch, tt.args.format, tt.args.pd)
+			got, err := resolveReconfigurePolicy(tt.args.jsonPatch, tt.args.format, tt.args.pd)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("resolveReloadActionPolicy(%v, %v, %v)", tt.args.jsonPatch, tt.args.format, tt.args.pd)
 			}

…ntrollers/parameters/policy_util_test.go …ters/reconfigure_controller_util_test.gocontrollers/parameters/policy_util_test.go renamed to controllers/parameters/reconfigure_controller_util_test.go controllers/parameters/policy_util_test.go renamed to controllers/parameters/reconfigure_controller_util_test.go

@@ -20,14 +20,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 package parameters
 
 import (
-	"fmt"
-
-	"sigs.k8s.io/controller-runtime/pkg/client"
-
-	appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1"
 	parametersv1alpha1 "github.com/apecloud/kubeblocks/apis/parameters/v1alpha1"
 	"github.com/apecloud/kubeblocks/pkg/constant"
-	intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil"
 	"github.com/apecloud/kubeblocks/pkg/parameters/core"
 )
 
@@ -42,63 +36,21 @@ type restartPolicy struct{}
 func (s *restartPolicy) Upgrade(rctx reconfigureContext) (returnedStatus, error) {
 	rctx.Log.V(1).Info("simple policy begin....")
 
-	var (
-		newVersion = rctx.getTargetVersionHash()
-		configKey  = rctx.generateConfigIdentifier()
-	)
+	s.restart(rctx)
 
-	if err := s.restart(rctx.Client, rctx.RequestCtx, configKey, newVersion, rctx.Cluster, rctx.ClusterComponent.Name); err != nil {
-		return makeReturnedStatus(ESFailedAndRetry), err
-	}
 	return syncLatestConfigStatus(rctx), nil
 }
 
-func (s *restartPolicy) restart(cli client.Client, ctx intctrlutil.RequestCtx, configKey string, newVersion string, cluster *appsv1.Cluster, compName string) error {
-	cfgAnnotationKey := core.GenerateUniqKeyWithConfig(constant.UpgradeRestartAnnotationKey, configKey)
-
-	compSpec, err := s.getComponentSpecPtrByName(cli, ctx, cluster, compName)
-	if err != nil {
-		return err
-	}
-
-	if compSpec.Annotations == nil {
-		compSpec.Annotations = map[string]string{}
-	}
-
-	if compSpec.Annotations[cfgAnnotationKey] == newVersion {
-		return nil
-	}
-
-	compSpec.Annotations[cfgAnnotationKey] = newVersion
-
-	return cli.Update(ctx.Ctx, cluster)
-}
-
-func (s *restartPolicy) getComponentSpecPtrByName(cli client.Client, ctx intctrlutil.RequestCtx, cluster *appsv1.Cluster, compName string) (*appsv1.ClusterComponentSpec, error) {
-	for i := range cluster.Spec.ComponentSpecs {
-		componentSpec := &cluster.Spec.ComponentSpecs[i]
-		if componentSpec.Name == compName {
-			return componentSpec, nil
-		}
-	}
-	// check if the component is a sharding component
-	compObjList := &appsv1.ComponentList{}
-	if err := cli.List(ctx.Ctx, compObjList, client.MatchingLabels{
-		constant.AppInstanceLabelKey:    cluster.Name,
-		constant.KBAppComponentLabelKey: compName,
-	}); err != nil {
-		return nil, err
+func (s *restartPolicy) restart(rctx reconfigureContext) {
+	var (
+		configKey        = rctx.generateConfigIdentifier()
+		newVersion       = rctx.getTargetVersionHash()
+		cfgAnnotationKey = core.GenerateUniqKeyWithConfig(constant.UpgradeRestartAnnotationKey, configKey)
+	)
+	if rctx.ClusterComponent.Annotations == nil {
+		rctx.ClusterComponent.Annotations = map[string]string{}
 	}
-	if len(compObjList.Items) > 0 {
-		shardingName := compObjList.Items[0].Labels[constant.KBAppShardingNameLabelKey]
-		if shardingName != "" {
-			for i := range cluster.Spec.Shardings {
-				shardSpec := &cluster.Spec.Shardings[i]
-				if shardSpec.Name == shardingName {
-					return &shardSpec.Template, nil
-				}
-			}
-		}
+	if rctx.ClusterComponent.Annotations[cfgAnnotationKey] != newVersion {
+		rctx.ClusterComponent.Annotations[cfgAnnotationKey] = newVersion
 	}
-	return nil, fmt.Errorf("component %s not found", compName)
 }

controllers/parameters/restart_policy.go

@@ -20,51 +20,30 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 package parameters
 
 import (
-	"fmt"
-
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
 	parametersv1alpha1 "github.com/apecloud/kubeblocks/apis/parameters/v1alpha1"
 	workloads "github.com/apecloud/kubeblocks/apis/workloads/v1"
-	testutil "github.com/apecloud/kubeblocks/pkg/testutil/k8s"
 )
 
 var _ = Describe("Reconfigure restartPolicy", func() {
 	var (
-		k8sMockClient *testutil.K8sClientMockHelper
-		policy        = upgradePolicyMap[parametersv1alpha1.RestartPolicy]
+		policy = upgradePolicyMap[parametersv1alpha1.RestartPolicy]
 	)
 
-	BeforeEach(func() {
-		k8sMockClient = testutil.NewK8sMockClient()
-	})
-
-	AfterEach(func() {
-		k8sMockClient.Finish()
-	})
-
 	Context("restart policy test", func() {
 		It("should success without error", func() {
-			mockParam := newMockReconfigureParams("restartPolicy", k8sMockClient.Client(),
+			mockParam := newMockReconfigureParams("restartPolicy", k8sClient,
 				withConfigSpec("test", map[string]string{
 					"key": "value",
 				}),
 				withClusterComponent(2),
 				withWorkload())
 
-			updateErr := fmt.Errorf("mock error")
-			k8sMockClient.MockUpdateMethod(
-				testutil.WithFailed(updateErr, testutil.WithTimes(1)),
-				testutil.WithSucceed(testutil.WithAnyTimes()))
-
-			status, err := policy.Upgrade(mockParam)
-			Expect(err).Should(BeEquivalentTo(updateErr))
-			Expect(status.Status).Should(BeEquivalentTo(ESFailedAndRetry))
-
 			// first upgrade, not pod is ready
 			mockParam.its.Status.InstanceStatus = []workloads.InstanceStatus{}
-			status, err = policy.Upgrade(mockParam)
+			status, err := policy.Upgrade(mockParam)
 			Expect(err).Should(Succeed())
 			Expect(status.Status).Should(BeEquivalentTo(ESRetry))
 			Expect(status.SucceedCount).Should(BeEquivalentTo(int32(0)))