Skip to content

Commit

Permalink
Database code improvements
Browse files Browse the repository at this point in the history 
Changes:
 - Document different configurations of the database secret.
 - Handle different database configurations when reconciling.
 - General code and logging cleanup.
  • Loading branch information
@gabrieljackson
gabrieljackson committed Nov 14, 2019
1 parent ac438f1 commit 300a40c
Show file tree
Hide file tree
Showing 11 changed files with 319 additions and 100 deletions.
26 changes: 20 additions & 6 deletions deploy/crds/mattermost_v1alpha1_clusterinstallation_crd.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,20 +142,20 @@ spec:
description: Defines the backup retention policy.
type: string
backupRestoreSecretName:
description: Defines the secret to be used for uploading/restoring
backup.
description: Defines the secret to be used when performing a database
restore.
type: string
backupSchedule:
description: Defines the interval for backups in cron expression
format.
type: string
backupSecretName:
description: Defines the secret to be used for uploading/restoring
backup.
type: string
backupURL:
description: Defines the object storage url for uploading backups.
type: string
secret:
description: If the user wants to use an existing Secret or an external DB.
This can be inside the same k8s cluster or outside like AWS RDS
type: string
initBucketURL:
description: Defines the AWS S3 bucket where the Database Backup
is stored. The operator will download the file to restore the
Expand All @@ -171,6 +171,20 @@ spec:
description: Defines the resource requests and limits for the database
pods.
type: object
secret:
description: 'Optionally enter the name of an already-existing Secret
for connecting to the database. This secret should be configured
as follows: User-Managed Database - Key: DB_CONNECTION_STRING
| Value: <FULL_DATABASE_CONNECTION_STRING> Operator-Managed Database -
Key: ROOT_PASSWORD | Value: <ROOT_DATABASE_PASSWORD> - Key:
USER | Value: <USER_NAME> - Key: PASSWORD | Value: <USER_PASSWORD> -
Key: DATABASE Value: <DATABASE_NAME> Notes: If you define all
secret values for both User-Managed and Operator-Managed database
types, the User-Managed connection string will take precedence
and the Operator-Managed values will be ignored. If the secret
is left blank, the default behavior is to use an Operator-Managed database
with strong randomly-generated database credentials.'
type: string
storageSize:
description: Defines the storage size for the database. ie 50Gi
pattern: ^([+-]?[0-9.]+)([eEinumkKMGTP]*[-+]?[0-9]*)$
Expand Down
22 changes: 20 additions & 2 deletions pkg/apis/mattermost/v1alpha1/clusterinstallation_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,27 @@ type Database struct {
// Defines the secret to be used for uploading/restoring backup.
// +optional
BackupSecretName string `json:"backupSecretName,omitempty"`
// Optionally enter the name of an already existing Secret for use by the MySQL operator
// Optionally enter the name of an already-existing Secret for connecting to
// the database. This secret should be configured as follows:
//
// User-Managed Database
// - Key: DB_CONNECTION_STRING | Value: <FULL_DATABASE_CONNECTION_STRING>
// Operator-Managed Database
// - Key: ROOT_PASSWORD | Value: <ROOT_DATABASE_PASSWORD>
// - Key: USER | Value: <USER_NAME>
// - Key: PASSWORD | Value: <USER_PASSWORD>
// - Key: DATABASE Value: <DATABASE_NAME>
//
// Notes:
// If you define all secret values for both User-Managed and
// Operator-Managed database types, the User-Managed connection string will
// take precedence and the Operator-Managed values will be ignored. If the
// secret is left blank, the default behavior is to use an Operator-Managed
// database with strong randomly-generated database credentials.
// +optional
Secret string `json:"secret,omitempty"`
// Defines the secret to be used when performing a database restore.
// +optional
Secret string `json:"secret,omitempty"`
BackupRestoreSecretName string `json:"backupRestoreSecretName,omitempty"`
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/mattermost/v1alpha1/clusterinstallation_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ func (mattermost *ClusterInstallation) GenerateDeployment(deploymentName, ingres
LocalObjectReference: corev1.LocalObjectReference{
Name: mattermost.Spec.Database.Secret,
},
Key: "externalDB",
Key: "DB_CONNECTION_STRING",
},
}
} else {
Expand Down
8 changes: 7 additions & 1 deletion pkg/components/minio/minio.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func Instance(mattermost *mattermostv1alpha1.ClusterInstallation) *minioOperator

// Secret returns the secret name created to use togehter with Minio deployment
func Secret(mattermost *mattermostv1alpha1.ClusterInstallation) *corev1.Secret {
secretName := fmt.Sprintf("%s-minio", mattermost.Name)
secretName := DefaultMinioSecretName(mattermost.Name)
data := make(map[string][]byte)
data["accesskey"] = utils.New16ID()
data["secretkey"] = utils.New28ID()
Expand All @@ -73,3 +73,9 @@ func Secret(mattermost *mattermostv1alpha1.ClusterInstallation) *corev1.Secret {
data,
)
}

// DefaultMinioSecretName returns the default minio secret name based on
// the provided installation name.
func DefaultMinioSecretName(installationName string) string {
return fmt.Sprintf("%s-minio", installationName)
}
8 changes: 7 additions & 1 deletion pkg/components/mysql/mysql.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func Cluster(mattermost *mattermostv1alpha1.ClusterInstallation) *mysqlOperator.
Spec: mysqlOperator.MysqlClusterSpec{
MysqlVersion: "5.7",
Replicas: utils.NewInt32(mattermost.Spec.Database.Replicas),
SecretName: fmt.Sprintf("%s-mysql-root-password", mattermost.Name),
SecretName: DefaultDatabaseSecretName(mattermost.Name),
VolumeSpec: mysqlOperator.VolumeSpec{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimSpec{
AccessModes: []corev1.PersistentVolumeAccessMode{
Expand Down Expand Up @@ -63,3 +63,9 @@ func Cluster(mattermost *mattermostv1alpha1.ClusterInstallation) *mysqlOperator.

return mysql
}

// DefaultDatabaseSecretName returns the default database secret name based on
// the provided installation name.
func DefaultDatabaseSecretName(installationName string) string {
return fmt.Sprintf("%s-mysql-root-password", installationName)
}
20 changes: 18 additions & 2 deletions pkg/controller/clusterinstallation/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
v1beta1 "k8s.io/api/extensions/v1beta1"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/handler"
Expand Down Expand Up @@ -216,11 +217,26 @@ func (r *ReconcileClusterInstallation) Reconcile(request reconcile.Request) (rec
}

func (r *ReconcileClusterInstallation) checkDatabase(mattermost *mattermostv1alpha1.ClusterInstallation, reqLogger logr.Logger) error {
// Check for an existing secret and determine which type it is (User-Managed
// or Operator-Manged). See the Database spec to learn more on this.
if mattermost.Spec.Database.Secret != "" {
err := r.checkSecret(mattermost.Spec.Database.Secret, "externalDB", mattermost.Namespace)
foundSecret := &corev1.Secret{}
err := r.client.Get(context.TODO(), types.NamespacedName{Name: mattermost.Spec.Database.Secret, Namespace: mattermost.Namespace}, foundSecret)
if err != nil {
return errors.Wrap(err, "Error getting the external database secret.")
return errors.Wrap(err, "error getting database secret")
}

if _, ok := foundSecret.Data["DB_CONNECTION_STRING"]; ok {
// No MySQL cluster setup is needed so return here.
return nil
}

err = getDatabaseInfoFromSecret(foundSecret).IsValid()
if err != nil {
return fmt.Errorf("database secret %s is not valid for either user-managed or operator-managed database types", mattermost.Spec.Database.Secret)
}

// Proceed to MySQL cluster setup.
}

switch mattermost.Spec.Database.Type {
Expand Down
39 changes: 4 additions & 35 deletions pkg/controller/clusterinstallation/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func TestReconcile(t *testing.T) {

// Create the resources that would normally be created by other operators
// running on the kubernetes cluster.
err = prepAllDependencyTestResources(r, ci)
err = prepAllDependencyTestResources(r.client, ci)
require.NoError(t, err)

// Mock request to simulate Reconcile() being called on an event for a
Expand Down Expand Up @@ -395,39 +395,7 @@ func TestReconcile(t *testing.T) {
})
}

func prepAllDependencyTestResources(r *ReconcileClusterInstallation, ci *mattermostv1alpha1.ClusterInstallation) error {
dbSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: ci.Name + "-mysql-root-password",
Namespace: ci.Namespace,
},
Data: map[string][]byte{
"ROOT_PASSWORD": []byte("mysupersecure"),
"USER": []byte("mmuser"),
"PASSWORD": []byte("mysupersecure1"),
"DATABASE": []byte("mattermost"),
},
}
err := r.client.Create(context.TODO(), dbSecret)
if err != nil {
return err
}

MinioSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: ci.Name + "-minio",
Namespace: ci.Namespace,
},
Data: map[string][]byte{
"accesskey": []byte("mysupersecure"),
"secretkey": []byte("mysupersecurekey"),
},
}
err = r.client.Create(context.TODO(), MinioSecret)
if err != nil {
return err
}

func prepAllDependencyTestResources(client client.Client, ci *mattermostv1alpha1.ClusterInstallation) error {
minioService := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: ci.Name + "-minio-hl-svc",
Expand All @@ -438,5 +406,6 @@ func prepAllDependencyTestResources(r *ReconcileClusterInstallation, ci *matterm
ClusterIP: corev1.ClusterIPNone,
},
}
return r.client.Create(context.TODO(), minioService)

return client.Create(context.TODO(), minioService)
}
32 changes: 20 additions & 12 deletions pkg/controller/clusterinstallation/mattermost.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,24 +91,32 @@ func (r *ReconcileClusterInstallation) checkMattermostIngress(mattermost *matter

func (r *ReconcileClusterInstallation) checkMattermostDeployment(mattermost *mattermostv1alpha1.ClusterInstallation, resourceName, ingressName, imageName string, reqLogger logr.Logger) error {
var externalDB, isLicensed bool
var dbData databaseInfo
var err error
if mattermost.Spec.Database.Secret == "" {
dbData, err = r.getOrCreateMySQLSecrets(mattermost, reqLogger)
if err != nil {
return errors.Wrap(err, "Error getting mysql database password.")
}
dbInfo := &databaseInfo{}

err = dbData.Valid()
if mattermost.Spec.Database.Secret == "" {
dbInfo, err = r.getOrCreateMySQLSecrets(mattermost, reqLogger)
if err != nil {
return err
return errors.Wrap(err, "unable to get database information")
}
} else {
err = r.checkSecret(mattermost.Spec.Database.Secret, "externalDB", mattermost.Namespace)
foundSecret := &corev1.Secret{}
err = r.client.Get(context.TODO(), types.NamespacedName{Name: mattermost.Spec.Database.Secret, Namespace: mattermost.Namespace}, foundSecret)
if err != nil {
return errors.Wrap(err, "Error getting the external database secret.")
return errors.Wrap(err, "error getting database secret")
}

if _, ok := foundSecret.Data["DB_CONNECTION_STRING"]; ok {
externalDB = true
}

if !externalDB {
dbInfo = getDatabaseInfoFromSecret(foundSecret)
err = dbInfo.IsValid()
if err != nil {
return errors.Wrap(err, "database secret is not valid")
}
}
externalDB = true
}

var minioURL string
Expand All @@ -129,7 +137,7 @@ func (r *ReconcileClusterInstallation) checkMattermostDeployment(mattermost *mat
isLicensed = true
}

desired := mattermost.GenerateDeployment(resourceName, ingressName, imageName, dbData.userName, dbData.userPassword, dbData.dbName, externalDB, isLicensed, minioURL)
desired := mattermost.GenerateDeployment(resourceName, ingressName, imageName, dbInfo.userName, dbInfo.userPassword, dbInfo.dbName, externalDB, isLicensed, minioURL)
err = r.createDeploymentIfNotExists(mattermost, desired, reqLogger)
if err != nil {
return err
Expand Down
Loading

0 comments on commit 300a40c

Please sign in to comment.