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