From 6d84abcd0fc714b77fe80702462519397766241b Mon Sep 17 00:00:00 2001 From: wangyelei Date: Thu, 5 Dec 2024 14:14:06 +0800 Subject: [PATCH] feat: support update label/annotations for underlying pods and improve restart ops (#8571) (cherry picked from commit d23e2e8f06eec8ff8aaaf861d941facccf579725) --- controllers/apps/cluster_controller_test.go | 134 +++++++++++++++- .../apps/transformer_component_account.go | 7 +- .../apps/transformer_component_service.go | 32 ++-- .../apps/transformer_component_workload.go | 28 +--- pkg/controller/factory/builder.go | 26 ++-- pkg/controller/plan/tls.go | 7 +- pkg/controllerutil/util.go | 42 ++--- pkg/operations/restart.go | 143 ++---------------- pkg/operations/restart_test.go | 73 +-------- 9 files changed, 203 insertions(+), 289 deletions(-) diff --git a/controllers/apps/cluster_controller_test.go b/controllers/apps/cluster_controller_test.go index 7e99498b4c1..1bace46152d 100644 --- a/controllers/apps/cluster_controller_test.go +++ b/controllers/apps/cluster_controller_test.go @@ -36,6 +36,7 @@ import ( appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1" dpv1alpha1 "github.com/apecloud/kubeblocks/apis/dataprotection/v1alpha1" + workloadsv1 "github.com/apecloud/kubeblocks/apis/workloads/v1" "github.com/apecloud/kubeblocks/pkg/constant" "github.com/apecloud/kubeblocks/pkg/controller/builder" "github.com/apecloud/kubeblocks/pkg/generics" @@ -1180,7 +1181,7 @@ var _ = Describe("Cluster Controller", func() { By("update cluster to upgrade component definition") Expect(testapps.GetAndChangeObj(&testCtx, clusterKey, func(cluster *appsv1.Cluster) { - cluster.Spec.ComponentSpecs[0].ComponentDef = "" + cluster.Spec.ComponentSpecs[0].ComponentDef = newCompDefObj.Name })()).ShouldNot(HaveOccurred()) By("check cluster and component objects been upgraded") @@ -1193,5 +1194,136 @@ var _ = Describe("Cluster Controller", func() { g.Expect(comp.Spec.ServiceVersion).Should(Equal(defaultServiceVersion)) })).Should(Succeed()) }) + + Context("cluster component annotations and labels", func() { + BeforeEach(func() { + cleanEnv() + createAllDefinitionObjects() + }) + + AfterEach(func() { + cleanEnv() + }) + + addMetaMap := func(metaMap *map[string]string, key string, value string) { + if *metaMap == nil { + *metaMap = make(map[string]string) + } + (*metaMap)[key] = value + } + + checkRelatedObject := func(compName string, checkFunc func(g Gomega, obj client.Object)) { + // check related services of the component + defaultSvcName := constant.GenerateComponentServiceName(clusterObj.Name, compName, "") + Eventually(testapps.CheckObj(&testCtx, client.ObjectKey{Name: defaultSvcName, + Namespace: testCtx.DefaultNamespace}, func(g Gomega, svc *corev1.Service) { + checkFunc(g, svc) + })).Should(Succeed()) + + // check related account secret of the component + rootAccountSecretName := constant.GenerateAccountSecretName(clusterObj.Name, compName, "root") + Eventually(testapps.CheckObj(&testCtx, client.ObjectKey{Name: rootAccountSecretName, + Namespace: testCtx.DefaultNamespace}, func(g Gomega, secret *corev1.Secret) { + checkFunc(g, secret) + })).Should(Succeed()) + } + + testUpdateAnnoAndLabels := func(compName string, + changeCluster func(cluster *appsv1.Cluster), + checkWorkloadFunc func(g Gomega, labels, annotations map[string]string), + checkRelatedObjFunc func(g Gomega, obj client.Object)) { + Expect(testapps.ChangeObj(&testCtx, clusterObj, func(obj *appsv1.Cluster) { + changeCluster(obj) + })).Should(Succeed()) + + By("check component has updated") + workloadName := constant.GenerateWorkloadNamePattern(clusterObj.Name, defaultCompName) + Eventually(testapps.CheckObj(&testCtx, client.ObjectKey{Name: workloadName, + Namespace: testCtx.DefaultNamespace}, func(g Gomega, compObj *appsv1.Component) { + checkWorkloadFunc(g, compObj.Spec.Labels, compObj.Spec.Annotations) + })).Should(Succeed()) + + By("check related objects annotations and labels") + checkRelatedObject(defaultCompName, func(g Gomega, obj client.Object) { + checkRelatedObjFunc(g, obj) + }) + + By("InstanceSet.spec.template.annotations/labels need to be consistent with component") + // The labels and annotations of the Pod will be kept consistent with those of the InstanceSet + Eventually(testapps.CheckObj(&testCtx, client.ObjectKey{Name: workloadName, Namespace: testCtx.DefaultNamespace}, + func(g Gomega, instanceSet *workloadsv1.InstanceSet) { + checkWorkloadFunc(g, instanceSet.Spec.Template.GetLabels(), instanceSet.Spec.Template.GetAnnotations()) + })).Should(Succeed()) + } + + It("test add/override annotations and labels", func() { + By("creating a cluster") + clusterObj = testapps.NewClusterFactory(testCtx.DefaultNamespace, clusterName, ""). + WithRandomName(). + AddComponent(defaultCompName, compDefObj.Name). + SetServiceVersion(defaultServiceVersion). + SetReplicas(3). + Create(&testCtx). + GetObject() + + By("add annotations and labels") + key1 := "key1" + value1 := "value1" + testUpdateAnnoAndLabels(defaultCompName, + func(cluster *appsv1.Cluster) { + addMetaMap(&cluster.Spec.ComponentSpecs[0].Annotations, key1, value1) + addMetaMap(&cluster.Spec.ComponentSpecs[0].Labels, key1, value1) + }, + func(g Gomega, labels, annotations map[string]string) { + g.Expect(labels[key1]).Should(Equal(value1)) + g.Expect(annotations[key1]).Should(Equal(value1)) + }, + func(g Gomega, obj client.Object) { + g.Expect(obj.GetLabels()[key1]).Should(Equal(value1)) + g.Expect(obj.GetAnnotations()[key1]).Should(Equal(value1)) + }) + + By("override annotations and labels") + value2 := "value2" + testUpdateAnnoAndLabels(defaultCompName, + func(cluster *appsv1.Cluster) { + addMetaMap(&cluster.Spec.ComponentSpecs[0].Annotations, key1, value2) + addMetaMap(&cluster.Spec.ComponentSpecs[0].Labels, key1, value2) + }, + func(g Gomega, labels, annotations map[string]string) { + g.Expect(labels[key1]).Should(Equal(value2)) + g.Expect(annotations[key1]).Should(Equal(value2)) + }, + func(g Gomega, obj client.Object) { + g.Expect(obj.GetLabels()[key1]).Should(Equal(value2)) + g.Expect(obj.GetAnnotations()[key1]).Should(Equal(value2)) + }) + + By("delete the annotations and labels, but retain the deleted annotations and labels for related objects") + key2 := "key2" + testUpdateAnnoAndLabels(defaultCompName, + func(cluster *appsv1.Cluster) { + cluster.Spec.ComponentSpecs[0].Annotations = map[string]string{ + key2: value2, + } + cluster.Spec.ComponentSpecs[0].Labels = map[string]string{ + key2: value2, + } + }, + func(g Gomega, labels, annotations map[string]string) { + g.Expect(labels).ShouldNot(HaveKey(key1)) + g.Expect(annotations).ShouldNot(HaveKey(key1)) + g.Expect(labels[key2]).Should(Equal(value2)) + g.Expect(annotations[key2]).Should(Equal(value2)) + }, + func(g Gomega, obj client.Object) { + g.Expect(obj.GetLabels()[key1]).Should(Equal(value2)) + g.Expect(obj.GetAnnotations()[key1]).Should(Equal(value2)) + g.Expect(obj.GetLabels()[key2]).Should(Equal(value2)) + g.Expect(obj.GetAnnotations()[key2]).Should(Equal(value2)) + }) + + }) + }) }) }) diff --git a/controllers/apps/transformer_component_account.go b/controllers/apps/transformer_component_account.go index 8579ef39639..71412527443 100644 --- a/controllers/apps/transformer_component_account.go +++ b/controllers/apps/transformer_component_account.go @@ -163,11 +163,12 @@ func (t *componentAccountTransformer) buildAccountSecretWithPassword(ctx *compon synthesizeComp *component.SynthesizedComponent, account appsv1.SystemAccount, password []byte) (*corev1.Secret, error) { secretName := constant.GenerateAccountSecretName(synthesizeComp.ClusterName, synthesizeComp.Name, account.Name) secret := builder.NewSecretBuilder(synthesizeComp.Namespace, secretName). - AddLabelsInMap(constant.GetCompLabels(synthesizeComp.ClusterName, synthesizeComp.Name)). - AddLabelsInMap(synthesizeComp.DynamicLabels). + // Priority: static < dynamic < built-in AddLabelsInMap(synthesizeComp.StaticLabels). - AddAnnotationsInMap(synthesizeComp.DynamicAnnotations). + AddLabelsInMap(synthesizeComp.DynamicLabels). + AddLabelsInMap(constant.GetCompLabels(synthesizeComp.ClusterName, synthesizeComp.Name)). AddAnnotationsInMap(synthesizeComp.StaticAnnotations). + AddAnnotationsInMap(synthesizeComp.DynamicAnnotations). PutData(constant.AccountNameForSecret, []byte(account.Name)). PutData(constant.AccountPasswdForSecret, password). SetImmutable(true). diff --git a/controllers/apps/transformer_component_service.go b/controllers/apps/transformer_component_service.go index c0594494d3a..9f104f4d0aa 100644 --- a/controllers/apps/transformer_component_service.go +++ b/controllers/apps/transformer_component_service.go @@ -43,6 +43,7 @@ import ( "github.com/apecloud/kubeblocks/pkg/controller/instanceset" "github.com/apecloud/kubeblocks/pkg/controller/model" "github.com/apecloud/kubeblocks/pkg/controller/multicluster" + intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil" ) var ( @@ -201,12 +202,13 @@ func (t *componentServiceTransformer) buildService(comp *appsv1.Component, serviceFullName := constant.GenerateComponentServiceName(synthesizeComp.ClusterName, synthesizeComp.Name, service.ServiceName) builder := builder.NewServiceBuilder(namespace, serviceFullName). - AddLabelsInMap(constant.GetCompLabels(clusterName, compName)). - AddLabelsInMap(synthesizeComp.DynamicLabels). + // Priority: static < dynamic < built-in AddLabelsInMap(synthesizeComp.StaticLabels). - AddAnnotationsInMap(service.Annotations). - AddAnnotationsInMap(synthesizeComp.DynamicAnnotations). + AddLabelsInMap(synthesizeComp.DynamicLabels). + AddLabelsInMap(constant.GetCompLabels(clusterName, compName)). AddAnnotationsInMap(synthesizeComp.StaticAnnotations). + AddAnnotationsInMap(synthesizeComp.DynamicAnnotations). + AddAnnotationsInMap(service.Annotations). SetSpec(&service.Spec). AddSelectorsInMap(t.builtinSelector(comp)). Optimize4ExternalTraffic() @@ -307,10 +309,12 @@ func (t *componentServiceTransformer) createOrUpdateService(ctx graph.TransformC } newSvc := originSvc.DeepCopy() - newSvc.Spec = service.Spec + intctrlutil.MergeMetadataMapInplace(service.Labels, &newSvc.Labels) + intctrlutil.MergeMetadataMapInplace(service.Annotations, &newSvc.Annotations) // if skip immutable check, update the service directly if skipImmutableCheckForComponentService(originSvc) { + newSvc.Spec = service.Spec resolveServiceDefaultFields(&originSvc.Spec, &newSvc.Spec) if !reflect.DeepEqual(originSvc, newSvc) { graphCli.Update(dag, originSvc, newSvc, inDataContext4G()) @@ -318,22 +322,12 @@ func (t *componentServiceTransformer) createOrUpdateService(ctx graph.TransformC return nil } // otherwise only support to update the override params defined in cluster.spec.componentSpec[].services - - overrideMutableParams := func(originSvc, newSvc *corev1.Service) { - newSvc.Spec.Type = originSvc.Spec.Type - newSvc.Name = originSvc.Name - newSvc.Spec.Selector = originSvc.Spec.Selector - newSvc.Annotations = originSvc.Annotations - } - - // modify mutable field of newSvc to check if it is overridable - overrideMutableParams(originSvc, newSvc) - if !reflect.DeepEqual(originSvc, newSvc) { + overrideMutableParams := func() { // other fields are immutable, we can't update the service - return nil + newSvc.Spec.Type = service.Spec.Type + newSvc.Spec.Selector = service.Spec.Selector } - - overrideMutableParams(service, newSvc) + overrideMutableParams() if !reflect.DeepEqual(originSvc, newSvc) { graphCli.Update(dag, originSvc, newSvc, inDataContext4G()) } diff --git a/controllers/apps/transformer_component_workload.go b/controllers/apps/transformer_component_workload.go index ea995677519..9f4280bc42a 100644 --- a/controllers/apps/transformer_component_workload.go +++ b/controllers/apps/transformer_component_workload.go @@ -151,11 +151,6 @@ func (t *componentWorkloadTransformer) runningInstanceSetObject(ctx graph.Transf func (t *componentWorkloadTransformer) reconcileWorkload(ctx context.Context, cli client.Reader, synthesizedComp *component.SynthesizedComponent, comp *appsv1.Component, runningITS, protoITS *workloads.InstanceSet) error { - if runningITS != nil { - *protoITS.Spec.Selector = *runningITS.Spec.Selector - protoITS.Spec.Template.Labels = intctrlutil.MergeMetadataMaps(runningITS.Spec.Template.Labels, synthesizedComp.DynamicLabels) - } - buildInstanceSetPlacementAnnotation(comp, protoITS) if err := t.reconcileReplicasStatus(ctx, cli, synthesizedComp, runningITS, protoITS); err != nil { @@ -406,21 +401,6 @@ func buildPodSpecVolumeMounts(synthesizeComp *component.SynthesizedComponent) { // 1. new an object targetObj by copying from oldObj // 2. merge all fields can be updated from newObj into targetObj func copyAndMergeITS(oldITS, newITS *workloads.InstanceSet) *workloads.InstanceSet { - // mergeAnnotations keeps the original annotations. - mergeMetadataMap := func(originalMap map[string]string, targetMap *map[string]string) { - if targetMap == nil || originalMap == nil { - return - } - if *targetMap == nil { - *targetMap = map[string]string{} - } - for k, v := range originalMap { - // if the annotation not exist in targetAnnotations, copy it from original. - if _, ok := (*targetMap)[k]; !ok { - (*targetMap)[k] = v - } - } - } updateUpdateStrategy := func(itsObj, itsProto *workloads.InstanceSet) { var objMaxUnavailable *intstr.IntOrString @@ -450,12 +430,8 @@ func copyAndMergeITS(oldITS, newITS *workloads.InstanceSet) *workloads.InstanceS return strings.HasPrefix(k, "monitor.kubeblocks.io") }) } - mergeMetadataMap(itsObjCopy.Annotations, &itsProto.Annotations) - itsObjCopy.Annotations = itsProto.Annotations - - // keep the original template annotations. - // if annotations exist and are replaced, the its will be updated. - mergeMetadataMap(itsObjCopy.Spec.Template.Annotations, &itsProto.Spec.Template.Annotations) + intctrlutil.MergeMetadataMapInplace(itsProto.Annotations, &itsObjCopy.Annotations) + intctrlutil.MergeMetadataMapInplace(itsProto.Labels, &itsObjCopy.Labels) itsObjCopy.Spec.Template = *itsProto.Spec.Template.DeepCopy() itsObjCopy.Spec.Replicas = itsProto.Spec.Replicas itsObjCopy.Spec.Roles = itsProto.Spec.Roles diff --git a/pkg/controller/factory/builder.go b/pkg/controller/factory/builder.go index 61b4d4fea46..428e4d9e41f 100644 --- a/pkg/controller/factory/builder.go +++ b/pkg/controller/factory/builder.go @@ -51,11 +51,12 @@ func BuildInstanceSet(synthesizedComp *component.SynthesizedComponent, component ) podBuilder := builder.NewPodBuilder("", ""). - AddLabelsInMap(constant.GetCompLabels(clusterName, compName, synthesizedComp.Labels)). - AddLabelsInMap(synthesizedComp.DynamicLabels). + // Priority: static < dynamic < built-in AddLabelsInMap(synthesizedComp.StaticLabels). - AddAnnotationsInMap(synthesizedComp.DynamicAnnotations). - AddAnnotationsInMap(synthesizedComp.StaticAnnotations) + AddLabelsInMap(synthesizedComp.DynamicLabels). + AddLabelsInMap(constant.GetCompLabels(clusterName, compName, synthesizedComp.Labels)). + AddAnnotationsInMap(synthesizedComp.StaticAnnotations). + AddAnnotationsInMap(synthesizedComp.DynamicAnnotations) template := corev1.PodTemplateSpec{ ObjectMeta: podBuilder.GetObject().ObjectMeta, Spec: *synthesizedComp.PodSpec.DeepCopy(), @@ -63,16 +64,16 @@ func BuildInstanceSet(synthesizedComp *component.SynthesizedComponent, component itsName := constant.GenerateWorkloadNamePattern(clusterName, compName) itsBuilder := builder.NewInstanceSetBuilder(namespace, itsName). - AddLabelsInMap(constant.GetCompLabels(clusterName, compName)). AddLabelsInMap(synthesizedComp.StaticLabels). + AddLabelsInMap(constant.GetCompLabels(clusterName, compName)). AddAnnotations(constant.KubeBlocksGenerationKey, synthesizedComp.Generation). AddAnnotations(constant.CRDAPIVersionAnnotationKey, workloads.GroupVersion.String()). AddAnnotationsInMap(map[string]string{ constant.AppComponentLabelKey: compDefName, constant.KBAppServiceVersionKey: synthesizedComp.ServiceVersion, }). - AddAnnotationsInMap(getMonitorAnnotations(synthesizedComp, componentDef)). AddAnnotationsInMap(synthesizedComp.StaticAnnotations). + AddAnnotationsInMap(getMonitorAnnotations(synthesizedComp, componentDef)). SetTemplate(template). AddMatchLabelsInMap(constant.GetCompLabels(clusterName, compName)). SetReplicas(synthesizedComp.Replicas). @@ -80,6 +81,9 @@ func BuildInstanceSet(synthesizedComp *component.SynthesizedComponent, component var vcts []corev1.PersistentVolumeClaim for _, vct := range synthesizedComp.VolumeClaimTemplates { + // Priority: static < dynamic < built-in + intctrlutil.MergeMetadataMapInplace(synthesizedComp.StaticLabels, &vct.ObjectMeta.Labels) + intctrlutil.MergeMetadataMapInplace(synthesizedComp.StaticAnnotations, &vct.ObjectMeta.Annotations) intctrlutil.MergeMetadataMapInplace(synthesizedComp.DynamicLabels, &vct.ObjectMeta.Labels) intctrlutil.MergeMetadataMapInplace(synthesizedComp.DynamicAnnotations, &vct.ObjectMeta.Annotations) vcts = append(vcts, vctToPVC(vct)) @@ -229,18 +233,20 @@ func GetRestoreSystemAccountPassword(annotations map[string]string, componentNam return password } +// TODO: add dynamicLabels and dynamicAnnotations by @zhangtao + func BuildConfigMapWithTemplate(cluster *appsv1.Cluster, synthesizedComp *component.SynthesizedComponent, configs map[string]string, cmName string, configTemplateSpec appsv1.ComponentTemplateSpec) *corev1.ConfigMap { return builder.NewConfigMapBuilder(cluster.Namespace, cmName). + AddLabelsInMap(synthesizedComp.StaticLabels). AddLabelsInMap(constant.GetCompLabels(cluster.Name, synthesizedComp.Name)). AddLabels(constant.CMConfigurationTypeLabelKey, constant.ConfigInstanceType). AddLabels(constant.CMTemplateNameLabelKey, configTemplateSpec.TemplateRef). - AddLabelsInMap(synthesizedComp.StaticLabels). - AddAnnotations(constant.DisableUpgradeInsConfigurationAnnotationKey, strconv.FormatBool(false)). AddAnnotationsInMap(synthesizedComp.StaticAnnotations). + AddAnnotations(constant.DisableUpgradeInsConfigurationAnnotationKey, strconv.FormatBool(false)). SetData(configs). GetObject() } @@ -316,8 +322,8 @@ func setToolsScriptsPath(container *corev1.Container, meta cfgcm.ConfigSpecMeta) func BuildServiceAccount(synthesizedComp *component.SynthesizedComponent, saName string) *corev1.ServiceAccount { return builder.NewServiceAccountBuilder(synthesizedComp.Namespace, saName). - AddLabelsInMap(constant.GetCompLabels(synthesizedComp.ClusterName, synthesizedComp.Name)). AddLabelsInMap(synthesizedComp.StaticLabels). + AddLabelsInMap(constant.GetCompLabels(synthesizedComp.ClusterName, synthesizedComp.Name)). AddAnnotationsInMap(synthesizedComp.StaticAnnotations). SetImagePullSecrets(intctrlutil.BuildImagePullSecrets()). GetObject() @@ -325,8 +331,8 @@ func BuildServiceAccount(synthesizedComp *component.SynthesizedComponent, saName func BuildRoleBinding(synthesizedComp *component.SynthesizedComponent, saName string) *rbacv1.RoleBinding { return builder.NewRoleBindingBuilder(synthesizedComp.Namespace, saName). - AddLabelsInMap(constant.GetCompLabels(synthesizedComp.ClusterName, synthesizedComp.Name)). AddLabelsInMap(synthesizedComp.StaticLabels). + AddLabelsInMap(constant.GetCompLabels(synthesizedComp.ClusterName, synthesizedComp.Name)). AddAnnotationsInMap(synthesizedComp.StaticAnnotations). SetRoleRef(rbacv1.RoleRef{ APIGroup: rbacv1.GroupName, diff --git a/pkg/controller/plan/tls.go b/pkg/controller/plan/tls.go index ad1c3a9c8b0..0730327d972 100644 --- a/pkg/controller/plan/tls.go +++ b/pkg/controller/plan/tls.go @@ -45,11 +45,12 @@ func GenerateTLSSecretName(clusterName, componentName string) string { func BuildTLSSecret(synthesizedComp component.SynthesizedComponent) *v1.Secret { name := GenerateTLSSecretName(synthesizedComp.ClusterName, synthesizedComp.Name) return builder.NewSecretBuilder(synthesizedComp.Namespace, name). - AddLabelsInMap(constant.GetCompLabels(synthesizedComp.ClusterName, synthesizedComp.Name)). - AddLabelsInMap(synthesizedComp.DynamicLabels). + // Priority: static < dynamic < built-in AddLabelsInMap(synthesizedComp.StaticLabels). - AddAnnotationsInMap(synthesizedComp.DynamicAnnotations). + AddLabelsInMap(synthesizedComp.DynamicLabels). + AddLabelsInMap(constant.GetCompLabels(synthesizedComp.ClusterName, synthesizedComp.Name)). AddAnnotationsInMap(synthesizedComp.StaticAnnotations). + AddAnnotationsInMap(synthesizedComp.DynamicAnnotations). SetStringData(map[string]string{}). GetObject() } diff --git a/pkg/controllerutil/util.go b/pkg/controllerutil/util.go index 6d1f0cd361d..ebeb8717320 100644 --- a/pkg/controllerutil/util.go +++ b/pkg/controllerutil/util.go @@ -112,34 +112,6 @@ func (r *RequestCtx) WithValue(key, val any) context.Context { return context.WithValue(r.Ctx, key, val) } -func IsNil(i interface{}) bool { - if i == nil { - return true - } - switch reflect.TypeOf(i).Kind() { - case reflect.Ptr, reflect.Map, reflect.Array, reflect.Chan, reflect.Slice: - return reflect.ValueOf(i).IsNil() - } - return false -} - -// MergeMetadataMapInplace merges two map[string]string, the targetMap will be updated. -func MergeMetadataMapInplace(originalMap map[string]string, targetMap *map[string]string) { - if targetMap == nil || originalMap == nil { - return - } - if *targetMap == nil { - *targetMap = map[string]string{} - } - for k, v := range originalMap { - // if the annotation not exist in targetAnnotations, copy it from original. - if _, ok := (*targetMap)[k]; !ok { - (*targetMap)[k] = v - } - } -} - -// MergeMetadataMaps merges targetMaps into originalMap if item not exist in originalMap and return the merged map. func MergeMetadataMaps(originalMap map[string]string, targetMaps ...map[string]string) map[string]string { mergeMap := map[string]string{} for k, v := range originalMap { @@ -155,6 +127,20 @@ func MergeMetadataMaps(originalMap map[string]string, targetMaps ...map[string]s return mergeMap } +// MergeMetadataMapInplace merges two map[string]string, the targetMap will be updated. +func MergeMetadataMapInplace(originalMap map[string]string, targetMap *map[string]string) { + if originalMap == nil { + return + } + if *targetMap == nil { + *targetMap = map[string]string{} + } + for k, v := range originalMap { + // add or override the target map with values from the original map + (*targetMap)[k] = v + } +} + func SetOwnerReference(owner, object metav1.Object) error { return controllerutil.SetOwnerReference(owner, object, innerScheme) } diff --git a/pkg/operations/restart.go b/pkg/operations/restart.go index ef7277a48ec..4289936bccb 100644 --- a/pkg/operations/restart.go +++ b/pkg/operations/restart.go @@ -29,9 +29,7 @@ import ( appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1" opsv1alpha1 "github.com/apecloud/kubeblocks/apis/operations/v1alpha1" - workloads "github.com/apecloud/kubeblocks/apis/workloads/v1" "github.com/apecloud/kubeblocks/pkg/constant" - "github.com/apecloud/kubeblocks/pkg/controller/component" intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil" ) @@ -72,15 +70,15 @@ func (r restartOpsHandler) Action(reqCtx intctrlutil.RequestCtx, cli client.Clie }); err != nil { return err } - orderedComps, err := r.getComponentOrders(reqCtx, cli, opsRes) - if err != nil { - return err + for i := range opsRes.Cluster.Spec.ComponentSpecs { + componentSpec := &opsRes.Cluster.Spec.ComponentSpecs[i] + r.doRestart(opsRes, componentSpec, componentSpec.Name) } - if len(orderedComps) > 0 { - // will restart components in "ReconcileAction" - return nil + for i := range opsRes.Cluster.Spec.Shardings { + shardingSpec := &opsRes.Cluster.Spec.Shardings[i] + r.doRestart(opsRes, &shardingSpec.Template, shardingSpec.Name) } - return r.restartComponents(reqCtx, cli, opsRes, opsRes.OpsRequest.Spec.RestartList, false) + return cli.Update(reqCtx.Ctx, opsRes.Cluster) } // ReconcileAction will be performed when action is done and loops till OpsRequest.status.phase is Succeed/Failed. @@ -94,15 +92,6 @@ func (r restartOpsHandler) ReconcileAction(reqCtx intctrlutil.RequestCtx, cli cl compStatus *opsv1alpha1.OpsRequestComponentStatus) (expectProgressCount int32, completedCount int32, err error) { return handleComponentStatusProgress(reqCtx, cli, opsRes, pgRes, compStatus, r.podApplyCompOps) } - orderedComps, err := r.getComponentOrders(reqCtx, cli, opsRes) - if err != nil { - return "", 0, err - } - if len(orderedComps) > 0 { - if err = r.restartComponents(reqCtx, cli, opsRes, orderedComps, true); err != nil { - return "", 0, err - } - } return r.compOpsHelper.reconcileActionWithComponentOps(reqCtx, cli, opsRes, "restart", handleRestartProgress) } @@ -120,120 +109,16 @@ func (r restartOpsHandler) podApplyCompOps( return !pod.CreationTimestamp.Before(&ops.Status.StartTimestamp) } -func (r restartOpsHandler) getComponentOrders(reqCtx intctrlutil.RequestCtx, cli client.Client, opsRes *OpsResource) ([]opsv1alpha1.ComponentOps, error) { - cd := &appsv1.ClusterDefinition{} - if opsRes.Cluster.Spec.ClusterDef == "" || opsRes.Cluster.Spec.Topology == "" { - return nil, nil - } - if err := cli.Get(reqCtx.Ctx, client.ObjectKey{Name: opsRes.Cluster.Spec.ClusterDef}, cd); err != nil { - return nil, err - } - // components that require sequential restart - var orderedComps []opsv1alpha1.ComponentOps - for _, topology := range cd.Spec.Topologies { - if topology.Name != opsRes.Cluster.Spec.Topology { - continue - } - if topology.Orders != nil && len(topology.Orders.Update) > 0 { - // when using clusterDef and topology, "update orders" includes all components - for _, compName := range topology.Orders.Update { - // get the ordered components to restart - if compOps, ok := r.compOpsHelper.componentOpsSet[compName]; ok { - orderedComps = append(orderedComps, compOps.(opsv1alpha1.ComponentOps)) - } - } - } - break - } - return orderedComps, nil -} - -func (r restartOpsHandler) restartComponents(reqCtx intctrlutil.RequestCtx, cli client.Client, opsRes *OpsResource, comOpsList []opsv1alpha1.ComponentOps, inOrder bool) error { - for index, compOps := range comOpsList { - if !r.matchToRestart(opsRes, comOpsList, index, inOrder) { - continue - } - compNameLabelKey := component.GetComponentNameLabelKey(opsRes.Cluster, compOps.ComponentName) - matchingLabels := client.MatchingLabels{constant.AppInstanceLabelKey: opsRes.Cluster.Name, compNameLabelKey: compOps.ComponentName} - instanceSetList := &workloads.InstanceSetList{} - if err := cli.List(reqCtx.Ctx, instanceSetList, - client.InNamespace(opsRes.Cluster.Namespace), matchingLabels); err != nil { - return err - } - if len(instanceSetList.Items) == 0 { - return fmt.Errorf(`the instanceSet workloads are not exists for the component "%s"`, compOps.ComponentName) - } - for i := range instanceSetList.Items { - instanceSet := &instanceSetList.Items[i] - if r.isRestarted(opsRes, instanceSet, &instanceSet.Spec.Template) { - continue - } - if err := cli.Update(reqCtx.Ctx, instanceSet); err != nil { - return err - } - } - if inOrder { - // if a component has been restarted in order, break - break - } - } - return nil -} - -func (r restartOpsHandler) matchToRestart(opsRes *OpsResource, comOpsList []opsv1alpha1.ComponentOps, index int, inOrder bool) bool { - if !inOrder { - return true - } - compHasRestartCompleted := func(compName string) bool { - if r.getCompReplicas(opsRes.Cluster, compName) == 0 { - return true - } - progressDetails := opsRes.OpsRequest.Status.Components[compName].ProgressDetails - if len(progressDetails) == 0 { - return false - } - for _, v := range progressDetails { - if !isCompletedProgressStatus(v.Status) { - return false - } - } - return true - } - if index > 0 { - if !compHasRestartCompleted(comOpsList[index-1].ComponentName) { - return false - } - } - return !compHasRestartCompleted(comOpsList[index].ComponentName) -} - -func (r restartOpsHandler) getCompReplicas(cluster *appsv1.Cluster, compName string) int32 { - compSpec := cluster.Spec.GetComponentByName(compName) - if compSpec != nil { - return compSpec.Replicas - } - sharding := cluster.Spec.GetShardingByName(compName) - if sharding != nil { - return sharding.Template.Replicas - } - return 0 -} - -// isRestarted checks whether the component has been restarted -func (r restartOpsHandler) isRestarted(opsRes *OpsResource, object client.Object, podTemplate *corev1.PodTemplateSpec) bool { - compName := component.GetComponentNameFromObj(object) - if _, ok := r.compOpsHelper.componentOpsSet[compName]; !ok { - return true +func (r restartOpsHandler) doRestart(opsRes *OpsResource, compSpec *appsv1.ClusterComponentSpec, componentName string) { + if _, ok := r.compOpsHelper.componentOpsSet[componentName]; !ok { + return } - if podTemplate.Annotations == nil { - podTemplate.Annotations = map[string]string{} + if compSpec.Annotations == nil { + compSpec.Annotations = map[string]string{} } - hasRestarted := true startTimestamp := opsRes.OpsRequest.Status.StartTimestamp - workloadRestartTimeStamp := podTemplate.Annotations[constant.RestartAnnotationKey] + workloadRestartTimeStamp := compSpec.Annotations[constant.RestartAnnotationKey] if res, _ := time.Parse(time.RFC3339, workloadRestartTimeStamp); startTimestamp.After(res) { - podTemplate.Annotations[constant.RestartAnnotationKey] = startTimestamp.Format(time.RFC3339) - hasRestarted = false + compSpec.Annotations[constant.RestartAnnotationKey] = startTimestamp.Format(time.RFC3339) } - return hasRestarted } diff --git a/pkg/operations/restart_test.go b/pkg/operations/restart_test.go index 5c19e1ccaee..43f7ddedef7 100644 --- a/pkg/operations/restart_test.go +++ b/pkg/operations/restart_test.go @@ -20,8 +20,6 @@ along with this program. If not, see . package operations import ( - "time" - . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -30,8 +28,6 @@ import ( appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1" opsv1alpha1 "github.com/apecloud/kubeblocks/apis/operations/v1alpha1" - workloads "github.com/apecloud/kubeblocks/apis/workloads/v1" - "github.com/apecloud/kubeblocks/pkg/constant" intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil" "github.com/apecloud/kubeblocks/pkg/generics" testapps "github.com/apecloud/kubeblocks/pkg/testutil/apps" @@ -41,10 +37,9 @@ import ( var _ = Describe("Restart OpsRequest", func() { var ( - randomStr = testCtx.GetRandomStr() - compDefName = "test-compdef-" + randomStr - clusterName = "test-cluster-" + randomStr - clusterDefName = "test-clusterdef-" + randomStr + randomStr = testCtx.GetRandomStr() + compDefName = "test-compdef-" + randomStr + clusterName = "test-cluster-" + randomStr ) cleanEnv := func() { @@ -100,68 +95,6 @@ var _ = Describe("Restart OpsRequest", func() { Expect(err).ShouldNot(HaveOccurred()) }) - ExpectCompRestarted := func(opsRequest *opsv1alpha1.OpsRequest, compName string, expectRestarted bool) { - instanceSetName := constant.GenerateWorkloadNamePattern(clusterName, compName) - Eventually(testapps.CheckObj(&testCtx, client.ObjectKey{Name: instanceSetName, Namespace: testCtx.DefaultNamespace}, - func(g Gomega, pobj *workloads.InstanceSet) { - startTimestamp := opsRes.OpsRequest.Status.StartTimestamp - workloadRestartTimeStamp := pobj.Spec.Template.Annotations[constant.RestartAnnotationKey] - res, _ := time.Parse(time.RFC3339, workloadRestartTimeStamp) - g.Expect(!startTimestamp.After(res)).Should(Equal(expectRestarted)) - })).Should(Succeed()) - } - - It("Test restart OpsRequest with existing update orders", func() { - By("init operations resources") - opsRes, _, cluster = initOperationsResourcesWithTopology(clusterDefName, compDefName, clusterName) - - By("create Restart opsRequest") - opsRes.OpsRequest = createRestartOpsObj(clusterName, "restart-ops-"+randomStr, - defaultCompName, secondaryCompName, thirdCompName) - mockComponentIsOperating(opsRes.Cluster, appsv1.UpdatingComponentPhase, - defaultCompName, secondaryCompName, thirdCompName) - - By("mock restart OpsRequest to Creating") - _, err := GetOpsManager().Do(reqCtx, k8sClient, opsRes) - Expect(err).ShouldNot(HaveOccurred()) - Eventually(testops.GetOpsRequestPhase(&testCtx, client.ObjectKeyFromObject(opsRes.OpsRequest))).Should(Equal(opsv1alpha1.OpsCreatingPhase)) - - By("test restart Action") - rHandler := restartOpsHandler{} - _ = rHandler.Action(reqCtx, k8sClient, opsRes) - ExpectCompRestarted(opsRes.OpsRequest, defaultCompName, false) - ExpectCompRestarted(opsRes.OpsRequest, secondaryCompName, false) - ExpectCompRestarted(opsRes.OpsRequest, thirdCompName, false) - - By("test reconcile Action") - _, err = GetOpsManager().Reconcile(reqCtx, k8sClient, opsRes) - Expect(err).ShouldNot(HaveOccurred()) - ExpectCompRestarted(opsRes.OpsRequest, defaultCompName, true) - ExpectCompRestarted(opsRes.OpsRequest, secondaryCompName, false) - ExpectCompRestarted(opsRes.OpsRequest, thirdCompName, false) - - By("mock restart secondary component completed") - setCompProgress := func(compName string, status opsv1alpha1.ProgressStatus) { - workloadName := constant.GenerateWorkloadNamePattern(clusterName, compName) - opsRes.OpsRequest.Status.Components[compName] = opsv1alpha1.OpsRequestComponentStatus{ - ProgressDetails: []opsv1alpha1.ProgressStatusDetail{ - {ObjectKey: getProgressObjectKey(constant.PodKind, workloadName+"-0"), Status: status}, - {ObjectKey: getProgressObjectKey(constant.PodKind, workloadName+"-1"), Status: status}, - {ObjectKey: getProgressObjectKey(constant.PodKind, workloadName+"-2"), Status: status}, - }, - } - } - setCompProgress(defaultCompName, opsv1alpha1.SucceedProgressStatus) - setCompProgress(secondaryCompName, opsv1alpha1.PendingProgressStatus) - - By("test reconcile Action and expect to restart third component") - _, _ = GetOpsManager().Reconcile(reqCtx, k8sClient, opsRes) - Expect(err == nil).Should(BeTrue()) - ExpectCompRestarted(opsRes.OpsRequest, defaultCompName, true) - ExpectCompRestarted(opsRes.OpsRequest, secondaryCompName, true) - ExpectCompRestarted(opsRes.OpsRequest, thirdCompName, false) - }) - It("expect failed when cluster is stopped", func() { By("init operations resources ") opsRes, _, cluster = initOperationsResources(compDefName, clusterName)