Fixed patching obproxy parameters (#441)

charts/oceanbase-dashboard/Chart.yaml

@@ -15,10 +15,10 @@ type: application
 # This is the chart version. This version number should be incremented each time you make changes
 # to the chart and its templates, including the app version.
 # Versions are expected to follow Semantic Versioning (https://semver.org/)
-version: 0.3.0
+version: 0.2.1
 
 # This is the version number of the application being deployed. This version number should be
 # incremented each time you make changes to the application. Versions are not expected to
 # follow Semantic Versioning. They should reflect the version the application is using.
 # It is recommended to use it with quotes.
-appVersion: "0.3.0"
+appVersion: "0.2.1"

internal/dashboard/business/obproxy/configmap.go

@@ -71,22 +71,33 @@ func updateConfigMap(ctx context.Context, ns, name string, param *obproxy.PatchO
 		}
 		return nil, httpErr.NewInternal("Failed to get obproxy config map, err msg: " + err.Error())
 	}
-	if cm.Data == nil {
-		cm.Data = map[string]string{}
-	}
-	for _, kv := range param.AddedParameters {
+	cm.Data = map[string]string{}
+	for _, kv := range param.Parameters {
 		cm.Data[strings.ToUpper(envPrefix+kv.Key)] = kv.Value
 	}
-	for _, key := range param.DeletedParameters {
-		delete(cm.Data, strings.ToUpper(envPrefix+key))
-	}
 	configMap, err := client.GetClient().ClientSet.CoreV1().ConfigMaps(ns).Update(ctx, cm, metav1.UpdateOptions{})
 	if err != nil {
 		return nil, httpErr.NewInternal("Failed to update obproxy config map, err msg: " + err.Error())
 	}
 	return configMap, nil
 }
 
+func doesParametersChanged(ctx context.Context, ns, name string, param *obproxy.PatchOBProxyParam) (bool, error) {
+	cm, err := client.GetClient().ClientSet.CoreV1().ConfigMaps(ns).Get(ctx, cmPrefix+name, metav1.GetOptions{})
+	if err != nil {
+		if kubeerrors.IsNotFound(err) {
+			return false, httpErr.NewNotFound("ConfigMap not found")
+		}
+		return false, httpErr.NewInternal("Failed to get obproxy config map, err msg: " + err.Error())
+	}
+	for _, kv := range param.Parameters {
+		if val, ok := cm.Data[strings.ToUpper(envPrefix+kv.Key)]; !ok || val != kv.Value {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
 func deleteConfigMap(ctx context.Context, ns, name string) (*corev1.ConfigMap, error) {
 	cm, err := client.GetClient().ClientSet.CoreV1().ConfigMaps(ns).Get(ctx, cmPrefix+name, metav1.GetOptions{})
 	if err != nil {

internal/dashboard/business/obproxy/obproxy.go

@@ -140,19 +140,18 @@ func PatchOBProxy(ctx context.Context, ns, name string, param *obproxy.PatchOBPr
 	}
 
 	parametersUpdated := false
-	if param.AddedParameters != nil || param.DeletedParameters != nil {
-		_, err := updateConfigMap(ctx, ns, name, param)
+	if param.Parameters != nil {
+		changed, err := doesParametersChanged(ctx, ns, name, param)
 		if err != nil {
 			return nil, err
 		}
-		parametersUpdated = true
-	}
-	if updated {
-		deployment, err := client.GetClient().ClientSet.AppsV1().Deployments(ns).Update(ctx, deploy, metav1.UpdateOptions{})
-		if err != nil {
-			return nil, httpErr.NewInternal("Failed to update obproxy, err msg: " + err.Error())
+		if changed {
+			_, err := updateConfigMap(ctx, ns, name, param)
+			if err != nil {
+				return nil, err
+			}
+			parametersUpdated = true
 		}
-		return buildOBProxy(ctx, deployment)
 	}
 	odp, err := buildOBProxy(ctx, deploy)
 	if err != nil {
@@ -168,14 +167,21 @@ func PatchOBProxy(ctx context.Context, ns, name string, param *obproxy.PatchOBPr
 			if err != nil {
 				return nil, httpErr.NewInternal("Failed to get oceanbase connection by host " + pod.PodIP)
 			}
-			for _, param := range param.AddedParameters {
-				err = conn.ExecWithDefaultTimeout(ctx, fmt.Sprintf("ALTER proxyconfig SET %s = %s;", param.Key, param.Value))
+			for _, param := range param.Parameters {
+				err = conn.ExecWithDefaultTimeout(ctx, fmt.Sprintf("ALTER proxyconfig SET %s = ?;", param.Key), param.Value)
 				if err != nil {
 					return nil, httpErr.NewInternal("Failed to update obproxy config, err msg: " + err.Error())
 				}
 			}
 		}
 	}
+	if updated || parametersUpdated {
+		deployment, err := client.GetClient().ClientSet.AppsV1().Deployments(ns).Update(ctx, deploy, metav1.UpdateOptions{})
+		if err != nil {
+			return nil, httpErr.NewInternal("Failed to update obproxy, err msg: " + err.Error())
+		}
+		return buildOBProxy(ctx, deployment)
+	}
 	return odp, nil
 }
 

internal/dashboard/generated/swagger/docs.go

@@ -495,7 +495,7 @@ const docTemplate = `{
                         "in": "body",
                         "required": true,
                         "schema": {
-                            "$ref": "#/definitions/route.Route"
+                            "$ref": "#/definitions/route.RouteParam"
                         }
                     }
                 ],
@@ -5667,21 +5667,15 @@ const docTemplate = `{
         "obproxy.PatchOBProxyParam": {
             "type": "object",
             "properties": {
-                "addedParameters": {
-                    "type": "array",
-                    "items": {
-                        "$ref": "#/definitions/common.KVPair"
-                    }
+                "image": {
+                    "type": "string"
                 },
-                "deletedParameters": {
+                "parameters": {
                     "type": "array",
                     "items": {
-                        "type": "string"
+                        "$ref": "#/definitions/common.KVPair"
                     }
                 },
-                "image": {
-                    "type": "string"
-                },
                 "replicas": {
                     "type": "integer"
                 },
@@ -7867,7 +7861,7 @@ const docTemplate = `{
                 }
             }
         },
-        "route.Route": {
+        "route.RouteParam": {
             "type": "object",
             "required": [
                 "aggregateLabels",
@@ -7890,6 +7884,9 @@ const docTemplate = `{
                 "groupWait": {
                     "type": "integer"
                 },
+                "id": {
+                    "type": "string"
+                },
                 "matchers": {
                     "type": "array",
                     "items": {

internal/dashboard/generated/swagger/swagger.json

@@ -488,7 +488,7 @@
                         "in": "body",
                         "required": true,
                         "schema": {
-                            "$ref": "#/definitions/route.Route"
+                            "$ref": "#/definitions/route.RouteParam"
                         }
                     }
                 ],
@@ -5660,21 +5660,15 @@
         "obproxy.PatchOBProxyParam": {
             "type": "object",
             "properties": {
-                "addedParameters": {
-                    "type": "array",
-                    "items": {
-                        "$ref": "#/definitions/common.KVPair"
-                    }
+                "image": {
+                    "type": "string"
                 },
-                "deletedParameters": {
+                "parameters": {
                     "type": "array",
                     "items": {
-                        "type": "string"
+                        "$ref": "#/definitions/common.KVPair"
                     }
                 },
-                "image": {
-                    "type": "string"
-                },
                 "replicas": {
                     "type": "integer"
                 },
@@ -7860,7 +7854,7 @@
                 }
             }
         },
-        "route.Route": {
+        "route.RouteParam": {
             "type": "object",
             "required": [
                 "aggregateLabels",
@@ -7883,6 +7877,9 @@
                 "groupWait": {
                     "type": "integer"
                 },
+                "id": {
+                    "type": "string"
+                },
                 "matchers": {
                     "type": "array",
                     "items": {

internal/dashboard/generated/swagger/swagger.yaml

@@ -522,16 +522,12 @@ definitions:
     type: object
   obproxy.PatchOBProxyParam:
     properties:
-      addedParameters:
+      image:
+        type: string
+      parameters:
         items:
           $ref: '#/definitions/common.KVPair'
         type: array
-      deletedParameters:
-        items:
-          type: string
-        type: array
-      image:
-        type: string
       replicas:
         type: integer
       resource:
@@ -2076,7 +2072,7 @@ definitions:
     - size
     - storageClass
     type: object
-  route.Route:
+  route.RouteParam:
     properties:
       aggregateLabels:
         items:
@@ -2086,6 +2082,8 @@ definitions:
         type: integer
       groupWait:
         type: integer
+      id:
+        type: string
       matchers:
         items:
           $ref: '#/definitions/alarm.Matcher'
@@ -2675,7 +2673,7 @@ paths:
         name: body
         required: true
         schema:
-          $ref: '#/definitions/route.Route'
+          $ref: '#/definitions/route.RouteParam'
       produces:
       - application/json
       responses:

internal/dashboard/model/obproxy/param.go

@@ -35,10 +35,9 @@ type CreateOBProxyParam struct {
 }
 
 type PatchOBProxyParam struct {
-	Image             *string              `json:"image,omitempty"`
-	ServiceType       *string              `json:"serviceType,omitempty" example:"ClusterIP" enums:"ClusterIP,NodePort,LoadBalancer,ExternalName" default:"ClusterIP"`
-	Replicas          *int32               `json:"replicas,omitempty"`
-	Resource          *common.ResourceSpec `json:"resource,omitempty"`
-	AddedParameters   []common.KVPair      `json:"addedParameters,omitempty"`
-	DeletedParameters []string             `json:"deletedParameters,omitempty"`
+	Image       *string              `json:"image,omitempty"`
+	ServiceType *string              `json:"serviceType,omitempty" example:"ClusterIP" enums:"ClusterIP,NodePort,LoadBalancer,ExternalName" default:"ClusterIP"`
+	Replicas    *int32               `json:"replicas,omitempty"`
+	Resource    *common.ResourceSpec `json:"resource,omitempty"`
+	Parameters  []common.KVPair      `json:"parameters,omitempty"`
 }

scripts/connect.sh

@@ -26,6 +26,7 @@ function print_help {
   echo "  --show-password       Show the password in the output."
   echo "  --proxy               Connect to the obproxy deployment with default name."
   echo "  --proxy-name <Name>   Connect to the obproxy deployment with the specified name. (--deploy-name in setup-obproxy.sh)"
+  echo "  --proxy-ns <Namespace>    Namespace of the obproxy. Default is the same as namespace of the obcluster."
 }
 
 # elif [[ $CONNECT == true ]]; then
@@ -110,6 +111,10 @@ while [[ $# -gt 0 ]]; do
       PROXY_DEPLOY_NAME=$2
       shift
       ;;
+    --proxy-ns)
+      PROXY_NS=$2
+      shift
+      ;;
     --show-password)
       SHOW_PASSWORD=true
       ;;
@@ -143,17 +148,18 @@ CONNECTING_HOST=""
 CONNECTING_PORT=""
 
 if [[ $CONNECT_PROXY == true ]]; then
+  CONNECTING_NS=${PROXY_NS:-$NAMESPACE}
   if [[ -z $PROXY_DEPLOY_NAME ]]; then
     PROXY_DEPLOY_NAME=obproxy-$OB_CLUSTER
   fi
 
-  kubectl get deployment $PROXY_DEPLOY_NAME -n $NAMESPACE &> /dev/null
+  kubectl get deployment $PROXY_DEPLOY_NAME -n $CONNECTING_NS &> /dev/null
   if [[ $? -ne 0 ]]; then
-    echo "Error: The obproxy deployment \"$PROXY_DEPLOY_NAME\" in namespace \"$NAMESPACE\" does not exist."
+    echo "Error: The obproxy deployment \"$PROXY_DEPLOY_NAME\" in namespace \"$CONNECTING_NS\" does not exist."
     exit 1
   fi
 
-  CONNECTING_HOST=$(kubectl get service svc-$PROXY_DEPLOY_NAME -n $NAMESPACE -o jsonpath='{.spec.clusterIP}')
+  CONNECTING_HOST=$(kubectl get service svc-$PROXY_DEPLOY_NAME -n $CONNECTING_NS -o jsonpath='{.spec.clusterIP}')
   CONNECTING_PORT=2883
 else
   POD_IP=$(kubectl get pods -n $NAMESPACE -l ref-obcluster=$OB_CLUSTER -o jsonpath='{.items[0].status.podIP}')