From 69fbe6712898fae728a76f61478ae5070eab481b Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Tue, 9 Jan 2024 16:25:22 -0500 Subject: [PATCH 1/2] add conditions for mariadbdatabase by adding conditions, we allow the mariadbaccount to start its work before the mariadbdatabase is fully completed. this also brings MariaDBDatabase into the same condition style as other controllers in play. --- ...ariadb.openstack.org_mariadbdatabases.yaml | 43 ++++++++++++++ api/v1beta1/conditions.go | 2 + api/v1beta1/mariadbdatabase_types.go | 4 ++ api/v1beta1/zz_generated.deepcopy.go | 7 +++ ...ariadb.openstack.org_mariadbdatabases.yaml | 43 ++++++++++++++ controllers/mariadbaccount_controller.go | 16 ++++-- controllers/mariadbdatabase_controller.go | 56 ++++++++++++++++++- .../tests/database_create/02-assert.yaml | 50 +++++++++++++++++ 8 files changed, 215 insertions(+), 6 deletions(-) diff --git a/api/bases/mariadb.openstack.org_mariadbdatabases.yaml b/api/bases/mariadb.openstack.org_mariadbdatabases.yaml index b8d76956..9cd06a31 100644 --- a/api/bases/mariadb.openstack.org_mariadbdatabases.yaml +++ b/api/bases/mariadb.openstack.org_mariadbdatabases.yaml @@ -58,6 +58,49 @@ spec: properties: completed: type: boolean + conditions: + description: Deployment Conditions + items: + description: Condition defines an observation of a API resource + operational state. + properties: + lastTransitionTime: + description: Last time the condition transitioned from one status + to another. This should be when the underlying condition changed. + If that is not known, then using the time when the API field + changed is acceptable. + format: date-time + type: string + message: + description: A human readable message indicating details about + the transition. + type: string + reason: + description: The reason for the condition's last transition + in CamelCase. + type: string + severity: + description: Severity provides a classification of Reason code, + so the current situation is immediately understandable and + could act accordingly. It is meant for situations where Status=False + and it should be indicated if it is just informational, warning + (next reconciliation might fix it) or an error (e.g. DB create + issue and no actions to automatically resolve the issue can/should + be done). For conditions where Status=Unknown or Status=True + the Severity should be SeverityNone. + type: string + status: + description: Status of the condition, one of True, False, Unknown. + type: string + type: + description: Type of condition in CamelCase. + type: string + required: + - lastTransitionTime + - status + - type + type: object + type: array hash: additionalProperties: type: string diff --git a/api/v1beta1/conditions.go b/api/v1beta1/conditions.go index b00981b5..66ed0daf 100644 --- a/api/v1beta1/conditions.go +++ b/api/v1beta1/conditions.go @@ -77,6 +77,8 @@ const ( MariaDBServerReadyMessage = "MariaDB / Galera server ready" + MariaDBServerNotBootstrappedMessage = "MariaDB / Galera server not bootstrapped" + MariaDBAccountReadyInitMessage = "MariaDBAccount create / drop not started" MariaDBAccountReadyMessage = "MariaDBAccount creation complete" diff --git a/api/v1beta1/mariadbdatabase_types.go b/api/v1beta1/mariadbdatabase_types.go index 700547c4..51f3c857 100644 --- a/api/v1beta1/mariadbdatabase_types.go +++ b/api/v1beta1/mariadbdatabase_types.go @@ -17,6 +17,7 @@ limitations under the License. package v1beta1 import ( + condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -44,6 +45,9 @@ type MariaDBDatabaseSpec struct { // MariaDBDatabaseStatus defines the observed state of MariaDBDatabase type MariaDBDatabaseStatus struct { + // Deployment Conditions + Conditions condition.Conditions `json:"conditions,omitempty" optional:"true"` + Completed bool `json:"completed,omitempty"` // Map of hashes to track e.g. job status Hash map[string]string `json:"hash,omitempty"` diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 575c84a9..7e13b08a 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -394,6 +394,13 @@ func (in *MariaDBDatabaseSpec) DeepCopy() *MariaDBDatabaseSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MariaDBDatabaseStatus) DeepCopyInto(out *MariaDBDatabaseStatus) { *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make(condition.Conditions, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.Hash != nil { in, out := &in.Hash, &out.Hash *out = make(map[string]string, len(*in)) diff --git a/config/crd/bases/mariadb.openstack.org_mariadbdatabases.yaml b/config/crd/bases/mariadb.openstack.org_mariadbdatabases.yaml index b8d76956..9cd06a31 100644 --- a/config/crd/bases/mariadb.openstack.org_mariadbdatabases.yaml +++ b/config/crd/bases/mariadb.openstack.org_mariadbdatabases.yaml @@ -58,6 +58,49 @@ spec: properties: completed: type: boolean + conditions: + description: Deployment Conditions + items: + description: Condition defines an observation of a API resource + operational state. + properties: + lastTransitionTime: + description: Last time the condition transitioned from one status + to another. This should be when the underlying condition changed. + If that is not known, then using the time when the API field + changed is acceptable. + format: date-time + type: string + message: + description: A human readable message indicating details about + the transition. + type: string + reason: + description: The reason for the condition's last transition + in CamelCase. + type: string + severity: + description: Severity provides a classification of Reason code, + so the current situation is immediately understandable and + could act accordingly. It is meant for situations where Status=False + and it should be indicated if it is just informational, warning + (next reconciliation might fix it) or an error (e.g. DB create + issue and no actions to automatically resolve the issue can/should + be done). For conditions where Status=Unknown or Status=True + the Severity should be SeverityNone. + type: string + status: + description: Status of the condition, one of True, False, Unknown. + type: string + type: + description: Type of condition in CamelCase. + type: string + required: + - lastTransitionTime + - status + - type + type: object + type: array hash: additionalProperties: type: string diff --git a/controllers/mariadbaccount_controller.go b/controllers/mariadbaccount_controller.go index 72e62b45..9da6d603 100644 --- a/controllers/mariadbaccount_controller.go +++ b/controllers/mariadbaccount_controller.go @@ -167,8 +167,8 @@ func (r *MariaDBAccountReconciler) reconcileCreate( instance.Name, instance.ObjectMeta.Labels["mariaDBDatabaseName"])) return ctrl.Result{RequeueAfter: time.Duration(10) * time.Second}, nil - } else if err == nil && !mariadbDatabase.Status.Completed { - // found but not in completed status. + } else if err == nil && !mariadbDatabase.Status.Conditions.IsTrue(databasev1beta1.MariaDBDatabaseReadyCondition) { + // found but database not ready // for the create case, need to wait for the MariaDBDatabase to exists before we can continue; // requeue @@ -356,8 +356,8 @@ func (r *MariaDBAccountReconciler) reconcileDelete( controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) return ctrl.Result{}, nil - } else if err == nil && !mariadbDatabase.Status.Completed { - // found but not in completed status. + } else if err == nil && !mariadbDatabase.Status.Conditions.IsTrue(databasev1beta1.MariaDBDatabaseReadyCondition) { + // found but database is not ready // for the delete case, the database doesn't exist. so // that means we don't, either. remove finalizer from @@ -427,6 +427,14 @@ func (r *MariaDBAccountReconciler) reconcileDelete( if !dbGalera.Status.Bootstrapped { log.Info("DB bootstrap not complete. Requeue...") + + instance.Status.Conditions.MarkFalse( + databasev1beta1.MariaDBServerReadyCondition, + databasev1beta1.ReasonDBWaitingInitialized, + condition.SeverityInfo, + databasev1beta1.MariaDBServerNotBootstrappedMessage, + ) + return ctrl.Result{RequeueAfter: time.Second * 10}, nil } diff --git a/controllers/mariadbdatabase_controller.go b/controllers/mariadbdatabase_controller.go index 1cb466c2..973d309f 100644 --- a/controllers/mariadbdatabase_controller.go +++ b/controllers/mariadbdatabase_controller.go @@ -28,6 +28,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" helper "github.com/openstack-k8s-operators/lib-common/modules/common/helper" job "github.com/openstack-k8s-operators/lib-common/modules/common/job" databasev1beta1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" @@ -73,13 +74,49 @@ func (r *MariaDBDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Requ // Always patch the instance status when exiting this function so we can persist any changes. defer func() { + if instance.Status.Conditions.AllSubConditionIsTrue() { + // update the Ready condition based on the sub conditions + + instance.Status.Conditions.MarkTrue( + condition.ReadyCondition, condition.ReadyMessage) + } else { + // something is not ready so reset the Ready condition + instance.Status.Conditions.MarkUnknown( + condition.ReadyCondition, condition.InitReason, condition.ReadyInitMessage) + // and recalculate it based on the state of the rest of the conditions + instance.Status.Conditions.Set( + instance.Status.Conditions.Mirror(condition.ReadyCondition)) + } + + if instance.Status.Conditions.IsTrue(condition.ReadyCondition) { + // database creation finished... okay to set to completed + instance.Status.Completed = true + } + err := helper.PatchInstance(ctx, instance) + if err != nil { _err = err return } + }() + if instance.Status.Conditions == nil { + instance.Status.Conditions = condition.Conditions{} + + // initialize conditions used later as Status=Unknown + cl := condition.CreateList( + condition.UnknownCondition(databasev1beta1.MariaDBServerReadyCondition, condition.InitReason, databasev1beta1.MariaDBServerReadyInitMessage), + condition.UnknownCondition(databasev1beta1.MariaDBDatabaseReadyCondition, condition.InitReason, databasev1beta1.MariaDBDatabaseReadyInitMessage), + ) + + instance.Status.Conditions.Init(&cl) + + // Register overall status immediately to have an early feedback e.g. in the cli + return ctrl.Result{}, nil + } + // Fetch the Galera instance from which we'll pull the credentials dbGalera, err := r.getDatabaseObject(ctx, instance) @@ -133,6 +170,14 @@ func (r *MariaDBDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Requ if !dbGalera.Status.Bootstrapped { log.Info("DB bootstrap not complete. Requeue...") + + instance.Status.Conditions.MarkFalse( + databasev1beta1.MariaDBServerReadyCondition, + databasev1beta1.ReasonDBWaitingInitialized, + condition.SeverityInfo, + databasev1beta1.MariaDBServerNotBootstrappedMessage, + ) + return ctrl.Result{RequeueAfter: time.Second * 10}, nil } @@ -141,6 +186,11 @@ func (r *MariaDBDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Requ dbContainerImage = dbGalera.Spec.ContainerImage serviceAccount = dbGalera.RbacResourceName() + instance.Status.Conditions.MarkTrue( + databasev1beta1.MariaDBServerReadyCondition, + databasev1beta1.MariaDBServerReadyMessage, + ) + // Define a new Job object (hostname, password, containerImage) jobDef, err := mariadb.DbDatabaseJob(instance, dbName, dbSecret, dbContainerImage, serviceAccount) if err != nil { @@ -173,8 +223,10 @@ func (r *MariaDBDatabaseReconciler) Reconcile(ctx context.Context, req ctrl.Requ log.Info("Job hash added", "Job", jobDef.Name, "Hash", instance.Status.Hash[databasev1beta1.DbCreateHash]) } - // database creation finished... okay to set to completed - instance.Status.Completed = true + instance.Status.Conditions.MarkTrue( + databasev1beta1.MariaDBDatabaseReadyCondition, + databasev1beta1.MariaDBDatabaseReadyMessage, + ) return ctrl.Result{}, nil } diff --git a/tests/kuttl/tests/database_create/02-assert.yaml b/tests/kuttl/tests/database_create/02-assert.yaml index ed1dbe73..6fe01bb6 100644 --- a/tests/kuttl/tests/database_create/02-assert.yaml +++ b/tests/kuttl/tests/database_create/02-assert.yaml @@ -12,3 +12,53 @@ metadata: name: kuttldb-latin1-db-create status: succeeded: 1 +--- +apiVersion: mariadb.openstack.org/v1beta1 +kind: MariaDBDatabase +metadata: + name: kuttldb-utf8 + labels: + dbName: openstack +spec: + secret: osp-secret + name: kuttldb_utf8 +status: + conditions: + - message: Setup complete + reason: Ready + status: "True" + type: Ready + - message: MariaDBDatabase ready + reason: Ready + status: "True" + type: MariaDBDatabaseReady + - message: MariaDB / Galera server ready + reason: Ready + status: "True" + type: MariaDBServerReady + completed: true +--- +apiVersion: mariadb.openstack.org/v1beta1 +kind: MariaDBDatabase +metadata: + name: kuttldb-latin1 + labels: + dbName: openstack +spec: + secret: osp-secret + name: kuttldb_latin1 +status: + conditions: + - message: Setup complete + reason: Ready + status: "True" + type: Ready + - message: MariaDBDatabase ready + reason: Ready + status: "True" + type: MariaDBDatabaseReady + - message: MariaDB / Galera server ready + reason: Ready + status: "True" + type: MariaDBServerReady + completed: true From 4d759dd283b312905366b45f0d93059334738b2c Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 11 Jan 2024 16:59:50 -0500 Subject: [PATCH 2/2] link MariaDBAccount to the Database api This replaces the MariaDBDatabase.secret attribute with the use of MariaDBAccount instead. The MariaDBDatabase.secret attribute becomes optional and deprecated. Once all consuming operators have moved to the new Database API, MariaDBDatabase.secret can be removed entirely. also applies an interim fix to allow the encoding/collation to be present in the Database API, otherwise the database pod would fail. Amends MariaDBAccount to return from delete if the owning Galera resource is already deleted. --- ...ariadb.openstack.org_mariadbdatabases.yaml | 5 +- api/v1beta1/mariadbdatabase_funcs.go | 110 ++++++++++++++- api/v1beta1/mariadbdatabase_types.go | 9 +- api/v1beta1/zz_generated.deepcopy.go | 12 +- ...ariadb.openstack.org_mariadbdatabases.yaml | 5 +- controllers/mariadbaccount_controller.go | 19 +-- pkg/mariadb/database.go | 130 +++++++++++++----- templates/database.sh | 8 +- templates/delete_database.sh | 10 +- .../account_create/02-create-database.yaml | 1 - .../tests/database_create/02-assert.yaml | 17 --- .../database_create/02-create-database.yaml | 21 ++- .../tests/database_create/03-assert.yaml | 14 ++ .../tests/database_create/04-teardown.yaml | 6 + 14 files changed, 282 insertions(+), 85 deletions(-) diff --git a/api/bases/mariadb.openstack.org_mariadbdatabases.yaml b/api/bases/mariadb.openstack.org_mariadbdatabases.yaml index 9cd06a31..927dcc38 100644 --- a/api/bases/mariadb.openstack.org_mariadbdatabases.yaml +++ b/api/bases/mariadb.openstack.org_mariadbdatabases.yaml @@ -47,11 +47,8 @@ spec: description: Name of the database in MariaDB type: string secret: - description: Name of secret which contains DatabasePassword + description: Name of secret which contains DatabasePassword (deprecated) type: string - required: - - defaultCharacterSet - - defaultCollation type: object status: description: MariaDBDatabaseStatus defines the observed state of MariaDBDatabase diff --git a/api/v1beta1/mariadbdatabase_funcs.go b/api/v1beta1/mariadbdatabase_funcs.go index 8f5e818d..7e0d2255 100644 --- a/api/v1beta1/mariadbdatabase_funcs.go +++ b/api/v1beta1/mariadbdatabase_funcs.go @@ -121,6 +121,11 @@ func (d *Database) GetDatabase() *MariaDBDatabase { return d.database } +// GetAccount - returns the account +func (d *Database) GetAccount() *MariaDBAccount { + return d.account +} + // CreateOrPatchDB - create or patch the service DB instance // Deprecated. Use CreateOrPatchDBByName instead. If you want to use the // default the DB service instance of the deployment then pass "openstack" as @@ -162,6 +167,22 @@ func (d *Database) CreateOrPatchDBByName( } } + account := d.account + if account == nil { + account = &MariaDBAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: d.databaseUser, + Namespace: d.namespace, + Labels: map[string]string{ + "mariaDBDatabaseName": d.name, + }, + }, + Spec: MariaDBAccountSpec{ + UserName: d.databaseUser, + Secret: d.secret, + }, + } + } // set the database hostname on the db instance err := d.setDatabaseHostname(ctx, h, name) if err != nil { @@ -174,8 +195,6 @@ func (d *Database) CreateOrPatchDBByName( d.labels, ) - db.Spec.Secret = d.secret - err := controllerutil.SetControllerReference(h.GetBeforeObject(), db, h.GetScheme()) if err != nil { return err @@ -200,6 +219,36 @@ func (d *Database) CreateOrPatchDBByName( return ctrl.Result{RequeueAfter: time.Second * 5}, nil } + op_acc, err_acc := controllerutil.CreateOrPatch(ctx, h.GetClient(), account, func() error { + account.Labels = util.MergeStringMaps( + account.GetLabels(), + d.labels, + ) + + err := controllerutil.SetControllerReference(h.GetBeforeObject(), account, h.GetScheme()) + if err != nil { + return err + } + + // If the service object doesn't have our finalizer, add it. + controllerutil.AddFinalizer(account, h.GetFinalizer()) + + return nil + }) + + if err_acc != nil && !k8s_errors.IsNotFound(err_acc) { + return ctrl.Result{}, util.WrapErrorForObject( + fmt.Sprintf("Error create or update account object %s", account.Name), + account, + err_acc, + ) + } + + if op_acc != controllerutil.OperationResultNone { + util.LogForObject(h, fmt.Sprintf("Account object %s created or patched", account.Name), account) + return ctrl.Result{RequeueAfter: time.Second * 5}, nil + } + err = d.getDBWithName( ctx, h, @@ -211,7 +260,9 @@ func (d *Database) CreateOrPatchDBByName( return ctrl.Result{}, nil } -// WaitForDBCreatedWithTimeout - wait until the MariaDBDatabase is initialized and reports Status.Completed == true +// WaitForDBCreatedWithTimeout - wait until the MariaDBDatabase and MariaDBAccounts are +// initialized and reports Status.Conditions.IsTrue(MariaDBDatabaseReadyCondition) +// and Status.Conditions.IsTrue(MariaDBAccountReadyCondition) func (d *Database) WaitForDBCreatedWithTimeout( ctx context.Context, h *helper.Helper, @@ -226,7 +277,7 @@ func (d *Database) WaitForDBCreatedWithTimeout( return ctrl.Result{}, err } - if !d.database.Status.Completed || k8s_errors.IsNotFound(err) { + if !d.database.Status.Conditions.IsTrue(MariaDBDatabaseReadyCondition) { util.LogForObject( h, fmt.Sprintf("Waiting for service DB %s to be created", d.database.Name), @@ -236,6 +287,26 @@ func (d *Database) WaitForDBCreatedWithTimeout( return ctrl.Result{RequeueAfter: requeueAfter}, nil } + if !d.account.Status.Conditions.IsTrue(MariaDBAccountReadyCondition) { + util.LogForObject( + h, + fmt.Sprintf("Waiting for service account %s to be created", d.account.Name), + d.account, + ) + + return ctrl.Result{RequeueAfter: requeueAfter}, nil + } + + if k8s_errors.IsNotFound(err) { + util.LogForObject( + h, + fmt.Sprintf("DB or account objects not yet found %s", d.database.Name), + d.database, + ) + + return ctrl.Result{RequeueAfter: requeueAfter}, nil + } + return ctrl.Result{}, nil } @@ -262,6 +333,7 @@ func (d *Database) getDBWithName( if namespace == "" { namespace = h.GetBeforeObject().GetNamespace() } + err := h.GetClient().Get( ctx, types.NamespacedName{ @@ -269,6 +341,7 @@ func (d *Database) getDBWithName( Namespace: namespace, }, db) + if err != nil { if k8s_errors.IsNotFound(err) { return util.WrapErrorForObject( @@ -287,6 +360,35 @@ func (d *Database) getDBWithName( d.database = db + account := &MariaDBAccount{} + username := d.databaseUser + + err = h.GetClient().Get( + ctx, + types.NamespacedName{ + Name: username, + Namespace: namespace, + }, + account) + + if err != nil { + if k8s_errors.IsNotFound(err) { + return util.WrapErrorForObject( + fmt.Sprintf("Failed to get %s account %s ", username, namespace), + h.GetBeforeObject(), + err, + ) + } + + return util.WrapErrorForObject( + fmt.Sprintf("account error %s %s ", username, namespace), + h.GetBeforeObject(), + err, + ) + } + + d.account = account + return nil } diff --git a/api/v1beta1/mariadbdatabase_types.go b/api/v1beta1/mariadbdatabase_types.go index 51f3c857..6afcdf3f 100644 --- a/api/v1beta1/mariadbdatabase_types.go +++ b/api/v1beta1/mariadbdatabase_types.go @@ -31,16 +31,16 @@ const ( // MariaDBDatabaseSpec defines the desired state of MariaDBDatabase type MariaDBDatabaseSpec struct { - // Name of secret which contains DatabasePassword - Secret string `json:"secret,omitempty"` + // Name of secret which contains DatabasePassword (deprecated) + Secret *string `json:"secret,omitempty"` // Name of the database in MariaDB Name string `json:"name,omitempty"` // +kubebuilder:default=utf8 // Default character set for this database - DefaultCharacterSet string `json:"defaultCharacterSet"` + DefaultCharacterSet string `json:"defaultCharacterSet,omitempty"` // +kubebuilder:default=utf8_general_ci // Default collation for this database - DefaultCollation string `json:"defaultCollation"` + DefaultCollation string `json:"defaultCollation,omitempty"` } // MariaDBDatabaseStatus defines the observed state of MariaDBDatabase @@ -88,6 +88,7 @@ const ( // Database - type Database struct { database *MariaDBDatabase + account *MariaDBAccount databaseHostname string databaseName string databaseUser string diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 7e13b08a..b6e71507 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -49,6 +49,11 @@ func (in *Database) DeepCopyInto(out *Database) { *out = new(MariaDBDatabase) (*in).DeepCopyInto(*out) } + if in.account != nil { + in, out := &in.account, &out.account + *out = new(MariaDBAccount) + (*in).DeepCopyInto(*out) + } if in.labels != nil { in, out := &in.labels, &out.labels *out = make(map[string]string, len(*in)) @@ -322,7 +327,7 @@ func (in *MariaDBDatabase) DeepCopyInto(out *MariaDBDatabase) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) } @@ -379,6 +384,11 @@ func (in *MariaDBDatabaseList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MariaDBDatabaseSpec) DeepCopyInto(out *MariaDBDatabaseSpec) { *out = *in + if in.Secret != nil { + in, out := &in.Secret, &out.Secret + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MariaDBDatabaseSpec. diff --git a/config/crd/bases/mariadb.openstack.org_mariadbdatabases.yaml b/config/crd/bases/mariadb.openstack.org_mariadbdatabases.yaml index 9cd06a31..927dcc38 100644 --- a/config/crd/bases/mariadb.openstack.org_mariadbdatabases.yaml +++ b/config/crd/bases/mariadb.openstack.org_mariadbdatabases.yaml @@ -47,11 +47,8 @@ spec: description: Name of the database in MariaDB type: string secret: - description: Name of secret which contains DatabasePassword + description: Name of secret which contains DatabasePassword (deprecated) type: string - required: - - defaultCharacterSet - - defaultCollation type: object status: description: MariaDBDatabaseStatus defines the observed state of MariaDBDatabase diff --git a/controllers/mariadbaccount_controller.go b/controllers/mariadbaccount_controller.go index 9da6d603..83e1e827 100644 --- a/controllers/mariadbaccount_controller.go +++ b/controllers/mariadbaccount_controller.go @@ -411,16 +411,19 @@ func (r *MariaDBAccountReconciler) reconcileDelete( // remove local finalizer controllerutil.RemoveFinalizer(instance, helper.GetFinalizer()) - } - instance.Status.Conditions.Set(condition.FalseCondition( - databasev1beta1.MariaDBServerReadyCondition, - condition.ErrorReason, - condition.SeverityError, - "Error retrieving MariaDB/Galera instance %s", - err)) + // galera DB does not exist, so return + return ctrl.Result{}, nil + } else { + instance.Status.Conditions.Set(condition.FalseCondition( + databasev1beta1.MariaDBServerReadyCondition, + condition.ErrorReason, + condition.SeverityError, + "Error retrieving MariaDB/Galera instance %s", + err)) - return ctrl.Result{}, err + return ctrl.Result{}, err + } } var dbInstance, dbAdminSecret, dbContainerImage, serviceAccountName string diff --git a/pkg/mariadb/database.go b/pkg/mariadb/database.go index 59b45719..6ec80719 100644 --- a/pkg/mariadb/database.go +++ b/pkg/mariadb/database.go @@ -35,6 +35,51 @@ func DbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHostName s labels := map[string]string{ "owner": "mariadb-operator", "cr": database.Spec.Name, "app": "mariadbschema", } + + var scriptEnv []corev1.EnvVar + + if database.Spec.Secret != nil { + scriptEnv = []corev1.EnvVar{ + { + Name: "MYSQL_PWD", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: databaseSecret, + }, + Key: "DbRootPassword", + }, + }, + }, + // send deprecated Secret field but only if non-nil + { + Name: "DatabasePassword", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: *database.Spec.Secret, + }, + Key: "DatabasePassword", + }, + }, + }, + } + } else { + scriptEnv = []corev1.EnvVar{ + { + Name: "MYSQL_PWD", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: databaseSecret, + }, + Key: "DbRootPassword", + }, + }, + }, + } + } + job := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ // provided db name is used as metadata name where underscore is a not allowed @@ -54,30 +99,7 @@ func DbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHostName s Name: "mariadb-database-create", Image: containerImage, Command: []string{"/bin/sh", "-c", dbCmd}, - Env: []corev1.EnvVar{ - { - Name: "MYSQL_PWD", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: databaseSecret, - }, - Key: "DbRootPassword", - }, - }, - }, - { - Name: "DatabasePassword", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: database.Spec.Secret, - }, - Key: "DatabasePassword", - }, - }, - }, - }, + Env: scriptEnv, }, }, }, @@ -105,6 +127,52 @@ func DeleteDbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHost labels := map[string]string{ "owner": "mariadb-operator", "cr": database.Spec.Name, "app": "mariadbschema", } + + var scriptEnv []corev1.EnvVar + + if database.Spec.Secret != nil { + scriptEnv = []corev1.EnvVar{ + { + Name: "MYSQL_PWD", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: databaseSecret, + }, + Key: "DbRootPassword", + }, + }, + }, + // send deprecated Secret field but only if non-nil. otherwise + // the script should not try to drop usernames from mysql.user + { + Name: "DatabasePassword", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: *database.Spec.Secret, + }, + Key: "DatabasePassword", + }, + }, + }, + } + } else { + scriptEnv = []corev1.EnvVar{ + { + Name: "MYSQL_PWD", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: databaseSecret, + }, + Key: "DbRootPassword", + }, + }, + }, + } + } + job := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ Name: strings.Replace(database.Spec.Name, "_", "", -1) + "-database-delete", @@ -121,19 +189,7 @@ func DeleteDbDatabaseJob(database *databasev1beta1.MariaDBDatabase, databaseHost Name: "mariadb-database-create", Image: containerImage, Command: []string{"/bin/sh", "-c", delCmd}, - Env: []corev1.EnvVar{ - { - Name: "MYSQL_PWD", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: databaseSecret, - }, - Key: "DbRootPassword", - }, - }, - }, - }, + Env: scriptEnv, }, }, }, diff --git a/templates/database.sh b/templates/database.sh index 96484af9..1b0cfada 100755 --- a/templates/database.sh +++ b/templates/database.sh @@ -1,4 +1,8 @@ #!/bin/bash -export DatabasePassword=${DatabasePassword:?"Please specify a DatabasePassword variable."} -mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "CREATE DATABASE IF NOT EXISTS {{.DatabaseName}}; ALTER DATABASE {{.DatabaseName}} CHARACTER SET '{{.DefaultCharacterSet}}' COLLATE '{{.DefaultCollation}}'; GRANT ALL PRIVILEGES ON {{.DatabaseName}}.* TO '{{.DatabaseName}}'@'localhost' IDENTIFIED BY '$DatabasePassword';GRANT ALL PRIVILEGES ON {{.DatabaseName}}.* TO '{{.DatabaseName}}'@'%' IDENTIFIED BY '$DatabasePassword';" +mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "CREATE DATABASE IF NOT EXISTS {{.DatabaseName}}; ALTER DATABASE {{.DatabaseName}} CHARACTER SET '{{.DefaultCharacterSet}}' COLLATE '{{.DefaultCollation}}';" + +if [[ "${DatabasePassword}" != "" ]]; then + # legacy; create database with username + mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "GRANT ALL PRIVILEGES ON {{.DatabaseName}}.* TO '{{.DatabaseName}}'@'localhost' IDENTIFIED BY '$DatabasePassword';GRANT ALL PRIVILEGES ON {{.DatabaseName}}.* TO '{{.DatabaseName}}'@'%' IDENTIFIED BY '$DatabasePassword';" +fi diff --git a/templates/delete_database.sh b/templates/delete_database.sh index 5ef9806f..f7016277 100755 --- a/templates/delete_database.sh +++ b/templates/delete_database.sh @@ -1,3 +1,11 @@ #!/bin/bash -mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "DROP DATABASE IF EXISTS {{.DatabaseName}}; DROP USER IF EXISTS '{{.DatabaseName}}'@'localhost'; DROP USER IF EXISTS '{{.DatabaseName}}'@'%';" +mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "DROP DATABASE IF EXISTS {{.DatabaseName}};" + +if [[ "${DatabasePassword}" != "" ]]; then + # legacy; drop the username also + # we can't do this unconditionally here because we only want to delete the + # mysql account if the MariaDBDatabase was using the legacy "secret" attribute; + # otherwise this could be from a valid MariaDBAccount + mysql -h {{.DatabaseHostname}} -u {{.DatabaseAdminUsername}} -P 3306 -e "DROP USER IF EXISTS '{{.DatabaseName}}'@'localhost'; DROP USER IF EXISTS '{{.DatabaseName}}'@'%';" +fi diff --git a/tests/kuttl/tests/account_create/02-create-database.yaml b/tests/kuttl/tests/account_create/02-create-database.yaml index 45a80ff2..95a029bd 100644 --- a/tests/kuttl/tests/account_create/02-create-database.yaml +++ b/tests/kuttl/tests/account_create/02-create-database.yaml @@ -5,5 +5,4 @@ metadata: labels: dbName: openstack spec: - secret: osp-secret name: kuttldb_accounttest diff --git a/tests/kuttl/tests/database_create/02-assert.yaml b/tests/kuttl/tests/database_create/02-assert.yaml index 6fe01bb6..2ad1e430 100644 --- a/tests/kuttl/tests/database_create/02-assert.yaml +++ b/tests/kuttl/tests/database_create/02-assert.yaml @@ -1,18 +1,3 @@ ---- -apiVersion: batch/v1 -kind: Job -metadata: - name: kuttldb-utf8-db-create -status: - succeeded: 1 ---- -apiVersion: batch/v1 -kind: Job -metadata: - name: kuttldb-latin1-db-create -status: - succeeded: 1 ---- apiVersion: mariadb.openstack.org/v1beta1 kind: MariaDBDatabase metadata: @@ -20,7 +5,6 @@ metadata: labels: dbName: openstack spec: - secret: osp-secret name: kuttldb_utf8 status: conditions: @@ -45,7 +29,6 @@ metadata: labels: dbName: openstack spec: - secret: osp-secret name: kuttldb_latin1 status: conditions: diff --git a/tests/kuttl/tests/database_create/02-create-database.yaml b/tests/kuttl/tests/database_create/02-create-database.yaml index adaf5cec..04da7469 100644 --- a/tests/kuttl/tests/database_create/02-create-database.yaml +++ b/tests/kuttl/tests/database_create/02-create-database.yaml @@ -5,7 +5,6 @@ metadata: labels: dbName: openstack spec: - secret: osp-secret name: kuttldb_utf8 --- apiVersion: mariadb.openstack.org/v1beta1 @@ -15,7 +14,25 @@ metadata: labels: dbName: openstack spec: - secret: osp-secret name: kuttldb_latin1 defaultCharacterSet: latin1 defaultCollation: latin1_general_ci +--- +# test optional, deprecated secret field +apiVersion: v1 +data: + DatabasePassword: ZGJzZWNyZXQx +kind: Secret +metadata: + name: some-db-secret +type: Opaque +--- +apiVersion: mariadb.openstack.org/v1beta1 +kind: MariaDBDatabase +metadata: + name: kuttldb-legacy-secret + labels: + dbName: openstack +spec: + name: kuttldb_legacy_secret + secret: some-db-secret diff --git a/tests/kuttl/tests/database_create/03-assert.yaml b/tests/kuttl/tests/database_create/03-assert.yaml index 8be4ac8d..eafeddbb 100644 --- a/tests/kuttl/tests/database_create/03-assert.yaml +++ b/tests/kuttl/tests/database_create/03-assert.yaml @@ -13,3 +13,17 @@ commands: - script: | oc rsh -n ${NAMESPACE} -c galera openstack-galera-0 /bin/sh -c 'mysql -uroot -p${DB_ROOT_PASSWORD} -Nse "select @@character_set_database;" kuttldb_utf8' | grep -o -w utf8 oc rsh -n ${NAMESPACE} -c galera openstack-galera-0 /bin/sh -c 'mysql -uroot -p${DB_ROOT_PASSWORD} -Nse "select @@collation_database;" kuttldb_utf8' | grep -o -w utf8_general_ci +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +commands: + # for legacy secret non-present, test that a mariadb username was *not* made + - script: | + ${MARIADB_KUTTL_DIR:-tests/kuttl/tests}/../common/scripts/check_db_account.sh openstack-galera-0 kuttldb_utf8 kuttldb_utf_8 12345678 --reverse +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +commands: + # for legacy secret present, test that a mariadb username was made + - script: | + ${MARIADB_KUTTL_DIR:-tests/kuttl/tests}/../common/scripts/check_db_account.sh openstack-galera-0 kuttldb_legacy_secret kuttldb_legacy_secret dbsecret1 diff --git a/tests/kuttl/tests/database_create/04-teardown.yaml b/tests/kuttl/tests/database_create/04-teardown.yaml index 1b19d23a..c1ee147b 100644 --- a/tests/kuttl/tests/database_create/04-teardown.yaml +++ b/tests/kuttl/tests/database_create/04-teardown.yaml @@ -10,6 +10,12 @@ delete: - apiVersion: mariadb.openstack.org/v1beta1 kind: MariaDBDatabase name: kuttldb-utf8 +- apiVersion: mariadb.openstack.org/v1beta1 + kind: MariaDBDatabase + name: kuttldb-legacy-secret +- apiVersion: v1 + kind: Secret + name: some-db-secret commands: - script: | oc delete -n $NAMESPACE pvc mysql-db-openstack-galera-0