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 <[email protected]>
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 <[email protected]>
Signed-off-by: Kevin <[email protected]>
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: [email protected]
For additional commands, e-mail: [email protected]
