This is an automated email from the ASF dual-hosted git repository.

pingsutw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/submarine.git


The following commit(s) were added to refs/heads/master by this push:
     new 600c4c27 SUBMARINE-1165. Change Submarine-database from deployment to 
statefulset
600c4c27 is described below

commit 600c4c2760a44ce48702707647587fd5704c126d
Author: cdmikechen <cdmikec...@hotmail.com>
AuthorDate: Sun Apr 3 11:59:42 2022 +0800

    SUBMARINE-1165. Change Submarine-database from deployment to statefulset
    
    ### What is this PR for?
    Change the database from deployment to statefulset, so we can avoid some 
locking table/database problems when upgrading or restarting database server.
    
    ### What type of PR is it?
    Refactoring
    
    ### Todos
    * [x] - Remove database.replicas in CRD. The current database configuration 
does not support HA or cluster mode. Delete this configuration to avoid 
unnecessary database problems. There are already relevant JIRA issues: 
https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-1225
    * [x] - Replace database resource definition from deployment to statefulset 
in submarine-operator
    * [x] - we may get nil exception, and we may need to merge PR 
https://github.com/apache/submarine/pull/911 before passing git test flow.
    
    ### What is the Jira issue?
    https://issues.apache.org/jira/projects/SUBMARINE/issues/SUBMARINE-1165
    
    ### How should this be tested?
    Should be tested by exists cases.
    
    ### Screenshots (if appropriate)
    No
    
    ### Questions:
    * Do the license files need updating? No
    * Are there breaking changes for older versions? Yes
    * Does this need new documentation? No
    
    Author: cdmikechen <cdmikec...@hotmail.com>
    
    Signed-off-by: Kevin <pings...@apache.org>
    
    Closes #913 from cdmikechen/SUBMARINE-1165 and squashes the following 
commits:
    
    c55ac548 [cdmikechen] Rename statefulset
    b51af621 [cdmikechen] change database deployment check to statefulset check
    d34cfd66 [cdmikechen] SUBMARINE-1165. Change Submarine-database from 
deployment to statefulset
---
 helm-charts/submarine/crds/crd.yaml                | 13 +++-----
 helm-charts/submarine/templates/rbac.yaml          |  1 +
 .../artifacts/examples/example-submarine.yaml      |  1 -
 .../artifacts/submarine/submarine-database.yaml    |  6 ++--
 submarine-cloud-v2/main.go                         |  1 +
 .../pkg/apis/submarine/v1alpha1/types.go           |  1 -
 .../submarine/v1alpha1/zz_generated.deepcopy.go    |  7 +---
 submarine-cloud-v2/pkg/controller/controller.go    | 31 +++++++++++++++---
 .../pkg/controller/controller_builder.go           |  1 +
 .../pkg/controller/controller_builder_config.go    |  8 +++++
 submarine-cloud-v2/pkg/controller/parser.go        | 11 +++++++
 .../pkg/controller/submarine_database.go           | 37 ++++++++++++++--------
 12 files changed, 82 insertions(+), 36 deletions(-)

diff --git a/helm-charts/submarine/crds/crd.yaml 
b/helm-charts/submarine/crds/crd.yaml
index dbd90db1..a54ce183 100644
--- a/helm-charts/submarine/crds/crd.yaml
+++ b/helm-charts/submarine/crds/crd.yaml
@@ -23,7 +23,7 @@ metadata:
     app.kubernetes.io/name: submarine-operator
     app.kubernetes.io/version: "0.7.0"
     app.kubernetes.io/component: submarine-operator
-    app.kubernetes.io/managed-by: helm
+    app.kubernetes.io/managed-by: Helm
 spec:
   group: submarine.apache.org
   names:
@@ -48,13 +48,13 @@ spec:
             required:
               - version
             properties:
-              version: 
+              version:
                 description: submarine docker image version
                 type: string
               server:
                 type: object
                 properties:
-                  image: 
+                  image:
                     description: Use this to overwrite the image when 
development
                     type: string
                   replicas:
@@ -63,12 +63,9 @@ spec:
               database:
                 type: object
                 properties:
-                  image: 
+                  image:
                     description: Use this to overwrite the image when 
development
                     type: string
-                  replicas:
-                    type: integer
-                    minimum: 1
                   storageSize:
                     type: string
                   mysqlRootPasswordSecret:
@@ -111,4 +108,4 @@ spec:
     served: true
     storage: true
     subresources:
-      status: {}
\ No newline at end of file
+      status: {}
diff --git a/helm-charts/submarine/templates/rbac.yaml 
b/helm-charts/submarine/templates/rbac.yaml
index f54a60da..02dec67c 100644
--- a/helm-charts/submarine/templates/rbac.yaml
+++ b/helm-charts/submarine/templates/rbac.yaml
@@ -66,6 +66,7 @@ rules:
       - "apps"
     resources:
       - deployments
+      - statefulsets
       - replicasets
     verbs:
       - "*"
diff --git a/submarine-cloud-v2/artifacts/examples/example-submarine.yaml 
b/submarine-cloud-v2/artifacts/examples/example-submarine.yaml
index c8a92c42..d23b04a4 100644
--- a/submarine-cloud-v2/artifacts/examples/example-submarine.yaml
+++ b/submarine-cloud-v2/artifacts/examples/example-submarine.yaml
@@ -26,7 +26,6 @@ spec:
     replicas: 1
   database:
     # image: "apache/submarine:database-0.7.0" # overwrite the image when 
development
-    replicas: 1
     storageSize: "1Gi"
     mysqlRootPasswordSecret: "root-pass-secret"
   tensorboard:
diff --git a/submarine-cloud-v2/artifacts/submarine/submarine-database.yaml 
b/submarine-cloud-v2/artifacts/submarine/submarine-database.yaml
index afe12d4a..a6efafe7 100644
--- a/submarine-cloud-v2/artifacts/submarine/submarine-database.yaml
+++ b/submarine-cloud-v2/artifacts/submarine/submarine-database.yaml
@@ -39,15 +39,18 @@ spec:
     - name: "submarine-database"
       port: 3306
       targetPort: 3306
+  clusterIP: None
+  type: ClusterIP
   selector:
     app: "submarine-database"
 ---
 # Source: submarine/templates/submarine-database.yaml
 apiVersion: apps/v1
-kind: Deployment
+kind: StatefulSet
 metadata:
   name: "submarine-database"
 spec:
+  serviceName: submarine-database
   replicas: 1
   selector:
     matchLabels:
@@ -56,7 +59,6 @@ spec:
     metadata:
       labels:
         app: "submarine-database"
-
     spec:
       containers:
         - name: "submarine-database"
diff --git a/submarine-cloud-v2/main.go b/submarine-cloud-v2/main.go
index e7d3a002..607a12f5 100644
--- a/submarine-cloud-v2/main.go
+++ b/submarine-cloud-v2/main.go
@@ -125,6 +125,7 @@ func NewSubmarineController(
                WithTraefikClientset(traefikClient).
                
WithSubmarineInformer(submarineInformerFactory.Submarine().V1alpha1().Submarines()).
                
WithDeploymentInformer(kubeInformerFactory.Apps().V1().Deployments()).
+               
WithStatefulSetInformer(kubeInformerFactory.Apps().V1().StatefulSets()).
                
WithNamespaceInformer(kubeInformerFactory.Core().V1().Namespaces()).
                WithServiceInformer(kubeInformerFactory.Core().V1().Services()).
                
WithServiceAccountInformer(kubeInformerFactory.Core().V1().ServiceAccounts()).
diff --git a/submarine-cloud-v2/pkg/apis/submarine/v1alpha1/types.go 
b/submarine-cloud-v2/pkg/apis/submarine/v1alpha1/types.go
index a1cc4891..6f5bb3f6 100644
--- a/submarine-cloud-v2/pkg/apis/submarine/v1alpha1/types.go
+++ b/submarine-cloud-v2/pkg/apis/submarine/v1alpha1/types.go
@@ -39,7 +39,6 @@ type SubmarineServerSpec struct {
 
 type SubmarineDatabaseSpec struct {
        Image                   string `json:"image"`
-       Replicas                *int32 `json:"replicas"`
        StorageSize             string `json:"storageSize"`
        MysqlRootPasswordSecret string `json:"mysqlRootPasswordSecret"`
 }
diff --git 
a/submarine-cloud-v2/pkg/apis/submarine/v1alpha1/zz_generated.deepcopy.go 
b/submarine-cloud-v2/pkg/apis/submarine/v1alpha1/zz_generated.deepcopy.go
index fe813e3c..9d5736db 100644
--- a/submarine-cloud-v2/pkg/apis/submarine/v1alpha1/zz_generated.deepcopy.go
+++ b/submarine-cloud-v2/pkg/apis/submarine/v1alpha1/zz_generated.deepcopy.go
@@ -56,11 +56,6 @@ func (in *Submarine) DeepCopyObject() runtime.Object {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, 
writing into out. in must be non-nil.
 func (in *SubmarineDatabaseSpec) DeepCopyInto(out *SubmarineDatabaseSpec) {
        *out = *in
-       if in.Replicas != nil {
-               in, out := &in.Replicas, &out.Replicas
-               *out = new(int32)
-               **out = **in
-       }
        return
 }
 
@@ -181,7 +176,7 @@ func (in *SubmarineSpec) DeepCopyInto(out *SubmarineSpec) {
        if in.Database != nil {
                in, out := &in.Database, &out.Database
                *out = new(SubmarineDatabaseSpec)
-               (*in).DeepCopyInto(*out)
+               **out = **in
        }
        if in.Tensorboard != nil {
                in, out := &in.Tensorboard, &out.Tensorboard
diff --git a/submarine-cloud-v2/pkg/controller/controller.go 
b/submarine-cloud-v2/pkg/controller/controller.go
index e2ee1cd4..bbb965ed 100644
--- a/submarine-cloud-v2/pkg/controller/controller.go
+++ b/submarine-cloud-v2/pkg/controller/controller.go
@@ -82,7 +82,7 @@ const (
        observerRbacYamlPath        = artifactPath + 
"submarine-observer-rbac.yaml"
 )
 
-var dependents = []string{serverName, databaseName, tensorboardName, 
mlflowName, minioName}
+var dependents = []string{serverName, tensorboardName, mlflowName, minioName}
 
 const (
        // SuccessSynced is used as part of the Event 'reason' when a Submarine 
is synced
@@ -112,6 +112,7 @@ type Controller struct {
 
        namespaceLister             corelisters.NamespaceLister
        deploymentLister            appslisters.DeploymentLister
+       statefulsetLister           appslisters.StatefulSetLister
        serviceaccountLister        corelisters.ServiceAccountLister
        serviceLister               corelisters.ServiceLister
        persistentvolumeclaimLister corelisters.PersistentVolumeClaimLister
@@ -311,12 +312,12 @@ func (c *Controller) updateSubmarineStatus(submarine, 
submarineCopy *v1alpha1.Su
        }
 
        // Update database replicas
-       databaseDeployment, err := c.getDeployment(submarine.Namespace, 
databaseName)
+       statefulset, err := c.getStatefulSet(submarine.Namespace, databaseName)
        if err != nil {
                return err
        }
-       if databaseDeployment != nil {
-               submarineCopy.Status.AvailableDatabaseReplicas = 
databaseDeployment.Status.AvailableReplicas
+       if statefulset != nil {
+               submarineCopy.Status.AvailableDatabaseReplicas = 
statefulset.Status.ReadyReplicas
        }
 
        // Skip update if nothing changed.
@@ -410,6 +411,17 @@ func (c *Controller) getDeployment(namespace, name string) 
(*appsv1.Deployment,
        return deployment, nil
 }
 
+func (c *Controller) getStatefulSet(namespace, name string) 
(*appsv1.StatefulSet, error) {
+       statefulset, err := 
c.statefulsetLister.StatefulSets(namespace).Get(name)
+       if err != nil {
+               if errors.IsNotFound(err) {
+                       return nil, nil
+               }
+               return nil, err
+       }
+       return statefulset, nil
+}
+
 func (c *Controller) validateSubmarine(submarine *v1alpha1.Submarine) error {
 
        // Print out the spec of the Submarine resource
@@ -469,6 +481,7 @@ func (c *Controller) createSubmarine(submarine 
*v1alpha1.Submarine) error {
 }
 
 func (c *Controller) checkSubmarineDependentsReady(submarine 
*v1alpha1.Submarine) (bool, error) {
+       // deployment dependents check
        for _, name := range dependents {
                deployment, err := c.getDeployment(submarine.Namespace, name)
                if err != nil {
@@ -487,6 +500,16 @@ func (c *Controller) 
checkSubmarineDependentsReady(submarine *v1alpha1.Submarine
                        return false, nil
                }
        }
+       // database check
+       // statefulset.Status.Conditions does not have the specified type enum 
like
+       // deployment.Status.Conditions => DeploymentConditionType ,
+       // so we will ignore the verification status for the time being
+       statefulset, err := c.getStatefulSet(submarine.Namespace, databaseName)
+       if err != nil {
+               return false, err
+       } else if statefulset == nil || statefulset.Status.Replicas != 
statefulset.Status.ReadyReplicas {
+               return false, nil
+       }
 
        return true, nil
 }
diff --git a/submarine-cloud-v2/pkg/controller/controller_builder.go 
b/submarine-cloud-v2/pkg/controller/controller_builder.go
index 808f5436..8d3ab63f 100644
--- a/submarine-cloud-v2/pkg/controller/controller_builder.go
+++ b/submarine-cloud-v2/pkg/controller/controller_builder.go
@@ -82,6 +82,7 @@ func (cb *ControllerBuilder) addListers() *ControllerBuilder {
        cb.controller.submarinesSynced = 
cb.config.submarineInformer.Informer().HasSynced
 
        cb.controller.deploymentLister = cb.config.deploymentInformer.Lister()
+       cb.controller.statefulsetLister = cb.config.statefulsetInformer.Lister()
        cb.controller.namespaceLister = cb.config.namespaceInformer.Lister()
        cb.controller.serviceLister = cb.config.serviceInformer.Lister()
        cb.controller.serviceaccountLister = 
cb.config.serviceaccountInformer.Lister()
diff --git a/submarine-cloud-v2/pkg/controller/controller_builder_config.go 
b/submarine-cloud-v2/pkg/controller/controller_builder_config.go
index 1c48d276..2212fb5f 100644
--- a/submarine-cloud-v2/pkg/controller/controller_builder_config.go
+++ b/submarine-cloud-v2/pkg/controller/controller_builder_config.go
@@ -36,6 +36,7 @@ type BuilderConfig struct {
        traefikclientset              traefik.Interface
        namespaceInformer             coreinformers.NamespaceInformer
        deploymentInformer            appsinformers.DeploymentInformer
+       statefulsetInformer           appsinformers.StatefulSetInformer
        serviceInformer               coreinformers.ServiceInformer
        serviceaccountInformer        coreinformers.ServiceAccountInformer
        persistentvolumeclaimInformer 
coreinformers.PersistentVolumeClaimInformer
@@ -99,6 +100,13 @@ func (bc *BuilderConfig) WithDeploymentInformer(
        return bc
 }
 
+func (bc *BuilderConfig) WithStatefulSetInformer(
+       statefulsetInformer appsinformers.StatefulSetInformer,
+) *BuilderConfig {
+       bc.statefulsetInformer = statefulsetInformer
+       return bc
+}
+
 func (bc *BuilderConfig) WithServiceInformer(
        serviceInformer coreinformers.ServiceInformer,
 ) *BuilderConfig {
diff --git a/submarine-cloud-v2/pkg/controller/parser.go 
b/submarine-cloud-v2/pkg/controller/parser.go
index ca570931..57279594 100644
--- a/submarine-cloud-v2/pkg/controller/parser.go
+++ b/submarine-cloud-v2/pkg/controller/parser.go
@@ -102,6 +102,17 @@ func ParseDeploymentYaml(relativePath string) 
(*appsv1.Deployment, error) {
        return &deployment, nil
 }
 
+// ParseStatefulSetYaml parse StatefulSets from yaml file.
+func ParseStatefulSetYaml(relativePath string) (*appsv1.StatefulSet, error) {
+       var statefulset appsv1.StatefulSet
+       marshaled, err := parseYaml(relativePath, "StatefulSet")
+       if err != nil {
+               return nil, err
+       }
+       json.Unmarshal(marshaled, &statefulset)
+       return &statefulset, nil
+}
+
 // ParseServiceYaml parse Service from yaml file.
 func ParseServiceYaml(relativePath string) (*v1.Service, error) {
        var service v1.Service
diff --git a/submarine-cloud-v2/pkg/controller/submarine_database.go 
b/submarine-cloud-v2/pkg/controller/submarine_database.go
index 07433fb3..bdec4bd8 100644
--- a/submarine-cloud-v2/pkg/controller/submarine_database.go
+++ b/submarine-cloud-v2/pkg/controller/submarine_database.go
@@ -44,7 +44,6 @@ func newSubmarineDatabasePersistentVolumeClaim(submarine 
*v1alpha1.Submarine) *c
 
 func newSubmarineDatabaseDeployment(submarine *v1alpha1.Submarine) 
*appsv1.Deployment {
        databaseImage := submarine.Spec.Database.Image
-       databaseReplicas := *submarine.Spec.Database.Replicas
 
        deployment, err := ParseDeploymentYaml(databaseYamlPath)
        if err != nil {
@@ -56,11 +55,27 @@ func newSubmarineDatabaseDeployment(submarine 
*v1alpha1.Submarine) *appsv1.Deplo
        if databaseImage != "" {
                deployment.Spec.Template.Spec.Containers[0].Image = 
databaseImage
        }
-       deployment.Spec.Replicas = &databaseReplicas
 
        return deployment
 }
 
+func newSubmarineDatabaseStatefulSet(submarine *v1alpha1.Submarine) 
*appsv1.StatefulSet {
+       statefulset, err := ParseStatefulSetYaml(databaseYamlPath)
+       if err != nil {
+               klog.Info("[Error] ParseStatefulSetYaml", err)
+       }
+       statefulset.ObjectMeta.OwnerReferences = []metav1.OwnerReference{
+               *metav1.NewControllerRef(submarine, 
v1alpha1.SchemeGroupVersion.WithKind("Submarine")),
+       }
+
+       databaseImage := submarine.Spec.Database.Image
+       if databaseImage != "" {
+               statefulset.Spec.Template.Spec.Containers[0].Image = 
databaseImage
+       }
+
+       return statefulset
+}
+
 func newSubmarineDatabaseService(submarine *v1alpha1.Submarine) 
*corev1.Service {
        service, err := ParseServiceYaml(databaseYamlPath)
        if err != nil {
@@ -100,15 +115,15 @@ func (c *Controller) createSubmarineDatabase(submarine 
*v1alpha1.Submarine) erro
                return fmt.Errorf(msg)
        }
 
-       // Step 2: Create Deployment
-       deployment, err := 
c.deploymentLister.Deployments(submarine.Namespace).Get(databaseName)
+       // Step 2: Create Statefulset
+       statefulset, err := 
c.statefulsetLister.StatefulSets(submarine.Namespace).Get(databaseName)
        // If the resource doesn't exist, we'll create it
        if errors.IsNotFound(err) {
-               deployment, err = 
c.kubeclientset.AppsV1().Deployments(submarine.Namespace).Create(context.TODO(),
 newSubmarineDatabaseDeployment(submarine), metav1.CreateOptions{})
+               statefulset, err = 
c.kubeclientset.AppsV1().StatefulSets(submarine.Namespace).Create(context.TODO(),
 newSubmarineDatabaseStatefulSet(submarine), metav1.CreateOptions{})
                if err != nil {
                        klog.Info(err)
                }
-               klog.Info("     Create Deployment: ", deployment.Name)
+               klog.Info("     Create StatefulSet: ", statefulset.Name)
        }
        // If an error occurs during Get/Create, we'll requeue the item so we 
can
        // attempt processing again later. This could have been caused by a
@@ -117,18 +132,12 @@ func (c *Controller) createSubmarineDatabase(submarine 
*v1alpha1.Submarine) erro
                return err
        }
 
-       if !metav1.IsControlledBy(deployment, submarine) {
-               msg := fmt.Sprintf(MessageResourceExists, deployment.Name)
+       if !metav1.IsControlledBy(statefulset, submarine) {
+               msg := fmt.Sprintf(MessageResourceExists, statefulset.Name)
                c.recorder.Event(submarine, corev1.EventTypeWarning, 
ErrResourceExists, msg)
                return fmt.Errorf(msg)
        }
 
-       // Update the replicas of the database deployment if it is not equal to 
spec
-       if submarine.Spec.Database.Replicas != nil && 
*submarine.Spec.Database.Replicas != *deployment.Spec.Replicas {
-               klog.V(4).Infof("Submarine %s database spec replicas: %d, 
actual replicas: %d", submarine.Name, *submarine.Spec.Database.Replicas, 
*deployment.Spec.Replicas)
-               _, err = 
c.kubeclientset.AppsV1().Deployments(submarine.Namespace).Update(context.TODO(),
 newSubmarineDatabaseDeployment(submarine), metav1.UpdateOptions{})
-       }
-
        if err != nil {
                return err
        }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@submarine.apache.org
For additional commands, e-mail: dev-h...@submarine.apache.org

Reply via email to