From ef0a27dc0a7b17628b13be1fca11df3268d1d8c9 Mon Sep 17 00:00:00 2001 From: powerfool Date: Thu, 19 Dec 2024 16:23:42 +0800 Subject: [PATCH] Fixed the API for patching OBCluster (#682) --- internal/clients/obtenant.go | 3 +- .../dashboard/business/oceanbase/obcluster.go | 65 ++++++++++++++----- .../dashboard/model/param/obcluster_param.go | 13 ++-- .../dashboard/model/response/obcluster.go | 4 ++ internal/dashboard/utils/unique.go | 31 +++++++++ 5 files changed, 94 insertions(+), 22 deletions(-) create mode 100644 internal/dashboard/utils/unique.go diff --git a/internal/clients/obtenant.go b/internal/clients/obtenant.go index 09e0a597e..bbc350659 100644 --- a/internal/clients/obtenant.go +++ b/internal/clients/obtenant.go @@ -20,6 +20,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/rand" "github.com/oceanbase/ob-operator/api/v1alpha1" "github.com/oceanbase/ob-operator/internal/clients/schema" @@ -111,7 +112,7 @@ func ListAllTenantBackupPolicies(ctx context.Context, ns string, listOptions met func ForceDeleteTenantBackupPolicy(ctx context.Context, nn types.NamespacedName) error { _, err := RescueClient.Create(ctx, &v1alpha1.OBResourceRescue{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "force-delete-", + Name: "force-delete-" + rand.String(8), }, Spec: v1alpha1.OBResourceRescueSpec{ TargetKind: schema.OBTenantBackupPolicyKind, diff --git a/internal/dashboard/business/oceanbase/obcluster.go b/internal/dashboard/business/oceanbase/obcluster.go index 6c0251f67..354fa18b5 100644 --- a/internal/dashboard/business/oceanbase/obcluster.go +++ b/internal/dashboard/business/oceanbase/obcluster.go @@ -76,6 +76,7 @@ func buildOBClusterOverview(ctx context.Context, obcluster *v1alpha1.OBCluster) } clusterMode := modelcommon.ClusterModeNormal annotations := obcluster.GetAnnotations() + deletionProtection := false if annotations != nil { if mode, ok := annotations[oceanbaseconst.AnnotationsMode]; ok { switch mode { @@ -86,15 +87,18 @@ func buildOBClusterOverview(ctx context.Context, obcluster *v1alpha1.OBCluster) default: } } + deletionProtection = annotations[oceanbaseconst.AnnotationsIgnoreDeletion] == "true" } return &response.OBClusterOverview{ OBClusterMeta: response.OBClusterMeta{ - UID: string(obcluster.UID), - Namespace: obcluster.Namespace, - Name: obcluster.Name, - ClusterName: obcluster.Spec.ClusterName, - ClusterId: obcluster.Spec.ClusterId, - Mode: clusterMode, + UID: string(obcluster.UID), + Namespace: obcluster.Namespace, + Name: obcluster.Name, + ClusterName: obcluster.Spec.ClusterName, + ClusterId: obcluster.Spec.ClusterId, + Mode: clusterMode, + SupportStaticIP: obcluster.SupportStaticIP(), + DeletionProtection: deletionProtection, }, Status: getStatisticStatus(obcluster), StatusDetail: obcluster.Status.Status, @@ -510,6 +514,31 @@ func buildOBClusterParameters(parameters []modelcommon.KVPair) []apitypes.Parame return obparameters } +func modifyParametersIncrementally(obcluster *v1alpha1.OBCluster, adding []modelcommon.KVPair, deleting []string) { + deletingMap := make(map[string]struct{}, len(deleting)) + for _, key := range deleting { + deletingMap[key] = struct{}{} + } + exsitingMap := make(map[string]string, len(obcluster.Spec.Parameters)) + for _, param := range obcluster.Spec.Parameters { + exsitingMap[param.Name] = param.Value + } + for _, key := range deleting { + delete(exsitingMap, key) + } + for _, addingParam := range adding { + exsitingMap[addingParam.Key] = addingParam.Value + } + newParameters := make([]apitypes.Parameter, 0, len(exsitingMap)) + for key, value := range exsitingMap { + newParameters = append(newParameters, apitypes.Parameter{ + Name: key, + Value: value, + }) + } + obcluster.Spec.Parameters = newParameters +} + func generateUUID() string { parts := strings.Split(uuid.New().String(), "-") return parts[len(parts)-1] @@ -600,7 +629,7 @@ func ScaleOBServer(ctx context.Context, obzoneIdentity *param.OBZoneIdentity, sc return nil, errors.Wrapf(err, "Get obcluster %s %s", obzoneIdentity.Namespace, obzoneIdentity.Name) } if obcluster.Status.Status != clusterstatus.Running { - return nil, errors.Errorf("OBCluster status is invalid %s", obcluster.Status.Status) + return nil, errors.Errorf("OBCluster status is invalid: %s, expected to be running", obcluster.Status.Status) } found := false replicaChanged := false @@ -633,7 +662,7 @@ func DeleteOBZone(ctx context.Context, obzoneIdentity *param.OBZoneIdentity) (*r return nil, errors.Wrapf(err, "Get obcluster %s %s", obzoneIdentity.Namespace, obzoneIdentity.Name) } if obcluster.Status.Status != clusterstatus.Running { - return nil, errors.Errorf("OBCluster status is invalid %s", obcluster.Status.Status) + return nil, errors.Errorf("OBCluster status is invalid: %s, expected to be running", obcluster.Status.Status) } if len(obcluster.Spec.Topology) <= 2 { return nil, oberr.NewBadRequest("Forbid to delete zone when the number of zone <= 2") @@ -664,7 +693,7 @@ func AddOBZone(ctx context.Context, obclusterIdentity *param.K8sObjectIdentity, return nil, errors.Wrapf(err, "Get obcluster %s %s", obclusterIdentity.Namespace, obclusterIdentity.Name) } if obcluster.Status.Status != clusterstatus.Running { - return nil, errors.Errorf("OBCluster status is invalid %s", obcluster.Status.Status) + return nil, errors.Errorf("OBCluster status is invalid: %s, expected to be running", obcluster.Status.Status) } for _, obzone := range obcluster.Spec.Topology { if obzone.Zone == zone.Zone { @@ -745,11 +774,11 @@ func PatchOBCluster(ctx context.Context, nn *param.K8sObjectIdentity, param *par return nil, errors.Wrapf(err, "Get obcluster %s %s", nn.Namespace, nn.Name) } if obcluster.Status.Status != clusterstatus.Running { - return nil, errors.Errorf("OBCluster status is invalid %s", obcluster.Status.Status) + return nil, errors.Errorf("OBCluster status is invalid: %s, expected to be running", obcluster.Status.Status) } alreadyIgnoredDeletion := obcluster.Annotations[oceanbaseconst.AnnotationsIgnoreDeletion] == "true" - if obcluster.Spec.OBServerTemplate != nil { + if param.Resource != nil { // Update resource if specified obcluster.Spec.OBServerTemplate.Resource = &apitypes.ResourceSpec{ Cpu: *apiresource.NewQuantity(param.Resource.Cpu, apiresource.DecimalSI), @@ -803,6 +832,8 @@ func PatchOBCluster(ctx context.Context, nn *param.K8sObjectIdentity, param *par } else if len(param.Parameters) > 0 { // Update parameters if specified obcluster.Spec.Parameters = buildOBClusterParameters(param.Parameters) + } else if len(param.ModifiedParameters) > 0 || len(param.DeletedParameters) > 0 { + modifyParametersIncrementally(obcluster, param.ModifiedParameters, param.DeletedParameters) } if param.AddDeletionProtection && !alreadyIgnoredDeletion { @@ -825,14 +856,14 @@ func RestartOBServers(ctx context.Context, nn *param.K8sObjectIdentity, param *p return nil, errors.Wrapf(err, "Get obcluster %s %s", nn.Namespace, nn.Name) } if obcluster.Status.Status != clusterstatus.Running { - return nil, errors.Errorf("OBCluster status is invalid %s", obcluster.Status.Status) + return nil, errors.Errorf("OBCluster status is invalid: %s, expected to be running", obcluster.Status.Status) } // Create OBClusterOperation for restarting observers operation := &v1alpha1.OBClusterOperation{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "restart-observers-", - Namespace: nn.Namespace, + Name: utils.GenerateName("restart-observers"), + Namespace: nn.Namespace, }, Spec: v1alpha1.OBClusterOperationSpec{ Type: constants.ClusterOpTypeRestartOBServers, @@ -859,14 +890,14 @@ func DeleteOBServers(ctx context.Context, nn *param.K8sObjectIdentity, param *pa return nil, errors.Wrapf(err, "Get obcluster %s %s", nn.Namespace, nn.Name) } if obcluster.Status.Status != clusterstatus.Running { - return nil, errors.Errorf("OBCluster status is invalid %s", obcluster.Status.Status) + return nil, errors.Errorf("OBCluster status is invalid: %s, expected to be running", obcluster.Status.Status) } // Create OBClusterOperation for deleting observers operation := &v1alpha1.OBClusterOperation{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "delete-observers-", - Namespace: nn.Namespace, + Name: utils.GenerateName("delete-observers"), + Namespace: nn.Namespace, }, Spec: v1alpha1.OBClusterOperationSpec{ Type: constants.ClusterOpTypeDeleteOBServers, diff --git a/internal/dashboard/model/param/obcluster_param.go b/internal/dashboard/model/param/obcluster_param.go index c5dfdd63b..ee92ef954 100644 --- a/internal/dashboard/model/param/obcluster_param.go +++ b/internal/dashboard/model/param/obcluster_param.go @@ -90,16 +90,21 @@ type OBZoneIdentity struct { } type PatchOBClusterParam struct { - Resource common.ResourceSpec `json:"resource"` + Resource *common.ResourceSpec `json:"resource"` Storage *OBServerStorageSpec `json:"storage"` Monitor *MonitorSpec `json:"monitor"` RemoveMonitor bool `json:"removeMonitor"` BackupVolume *NFSVolumeSpec `json:"backupVolume"` RemoveBackupVolume bool `json:"removeBackupVolume"` - Parameters []common.KVPair `json:"parameters,omitempty"` - AddDeletionProtection bool `json:"addDeletionProtection"` - RemoveDeletionProtection bool `json:"removeDeletionProtection"` + // Replace all parameters + Parameters []common.KVPair `json:"parameters,omitempty"` + // Add or modify some parameters + ModifiedParameters []common.KVPair `json:"modifiedParameters,omitempty"` + // Delete some parameters + DeletedParameters []string `json:"deletedParameters,omitempty"` + AddDeletionProtection bool `json:"addDeletionProtection"` + RemoveDeletionProtection bool `json:"removeDeletionProtection"` } type RestartOBServersParam v1alpha1.RestartOBServersConfig diff --git a/internal/dashboard/model/response/obcluster.go b/internal/dashboard/model/response/obcluster.go index 36b3d6e0d..5e62b3560 100644 --- a/internal/dashboard/model/response/obcluster.go +++ b/internal/dashboard/model/response/obcluster.go @@ -58,7 +58,11 @@ type OBClusterMeta struct { ClusterName string `json:"clusterName" binding:"required"` ClusterId int64 `json:"clusterId" binding:"required"` Mode common.ClusterMode `json:"mode" binding:"required"` + + SupportStaticIP bool `json:"supportStaticIP" binding:"required"` + DeletionProtection bool `json:"deletionProtection" binding:"required"` } + type OBClusterOverview struct { OBClusterMeta `json:",inline"` Status string `json:"status" binding:"required"` diff --git a/internal/dashboard/utils/unique.go b/internal/dashboard/utils/unique.go new file mode 100644 index 000000000..ffb341599 --- /dev/null +++ b/internal/dashboard/utils/unique.go @@ -0,0 +1,31 @@ +/* +Copyright (c) 2024 OceanBase +ob-operator is licensed under Mulan PSL v2. +You can use this software according to the terms and conditions of the Mulan PSL v2. +You may obtain a copy of Mulan PSL v2 at: + http://license.coscl.org.cn/MulanPSL2 +THIS SOFTWARE IS PROVIDED ON AN "AS IS" BASIS, WITHOUT WARRANTIES OF ANY KIND, +EITHER EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO NON-INFRINGEMENT, +MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. +See the Mulan PSL v2 for more details. +*/ + +package utils + +import ( + "fmt" + "strings" + "time" + + "k8s.io/apimachinery/pkg/util/rand" +) + +// GenerateName generates unique name based on the base name +// Usually used for generating unique name for one-time resources +func GenerateName(base string) string { + current := time.Now().Unix() + if strings.HasSuffix(base, "-") { + return fmt.Sprintf("%s%d-%s", base, current, rand.String(5)) + } + return fmt.Sprintf("%s-%d-%s", base, current, rand.String(5)) +}