Merge pull request opendatahub-io#133 from gmfrasca/storage-readiness-conditions

Add Database and ObjectStorage Health Checks

Author: openshift-merge-robot

api/v1alpha1/dspipeline_types.go

@@ -122,6 +122,9 @@ type MlPipelineUI struct {
 type Database struct {
 	*MariaDB    `json:"mariaDB,omitempty"`
 	*ExternalDB `json:"externalDB,omitempty"`
+	// +kubebuilder:default:=false
+	// +kubebuilder:validation:Optional
+	DisableHealthCheck bool `json:"disableHealthCheck"`
 }
 
 type MariaDB struct {
@@ -151,6 +154,9 @@ type ExternalDB struct {
 type ObjectStorage struct {
 	*Minio           `json:"minio,omitempty"`
 	*ExternalStorage `json:"externalStorage,omitempty"`
+	// +kubebuilder:default:=false
+	// +kubebuilder:validation:Optional
+	DisableHealthCheck bool `json:"disableHealthCheck"`
 }
 
 type Minio struct {

api/v1alpha1/zz_generated.deepcopy.go

@@ -138,6 +138,9 @@ spec:
                   mariaDB:
                     deploy: true
                 properties:
+                  disableHealthCheck:
+                    default: false
+                    type: boolean
                   externalDB:
                     properties:
                       host:
@@ -431,6 +434,9 @@ spec:
                 type: object
               objectStorage:
                 properties:
+                  disableHealthCheck:
+                    default: false
+                    type: boolean
                   externalStorage:
                     properties:
                       bucket:

config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml

@@ -74,6 +74,7 @@ spec:
     # otherwise operator will not deploy DSPA
     configMap: ds-pipeline-ui-configmap
   database:
+    disableHealthCheck: false
     mariaDB:   # mutually exclusive with externalDB
       deploy: true
       image: registry.redhat.io/rhel8/mariadb-103:1-188
@@ -101,6 +102,7 @@ spec:
 #        name: somesecret
 #        key: somekey
   objectStorage:
+    disableHealthCheck: false
     minio:  # mutually exclusive with externalStorage
       deploy: true
       image: quay.io/opendatahub/minio:RELEASE.2019-08-14T20-37-41Z-license-compliance

config/samples/dspa_all_fields.yaml

@@ -0,0 +1,19 @@
+# A simple DSPA with the Database and ObjectStore Health Checks Disabled
+#
+# Since the default database and storage options leverage internal Services,
+# a locally-run DSPO that manages an external cluster (common development practice)
+# would not be able to run the pre-deploy health checks on these prerequisite components
+# and therefore the DSPA will never fully deploy without disabling them, as this DSPA sample does
+apiVersion: datasciencepipelinesapplications.opendatahub.io/v1alpha1
+kind: DataSciencePipelinesApplication
+metadata:
+  name: sample
+spec:
+  database:
+    disableHealthCheck: true
+  objectStorage:
+    disableHealthCheck: true
+    minio:
+      image: 'quay.io/opendatahub/minio:RELEASE.2019-08-14T20-37-41Z-license-compliance'
+  mlpipelineUI:
+    image: 'quay.io/opendatahub/odh-ml-pipelines-frontend-container:beta-ui'

config/samples/dspa_local_dev.yaml

@@ -69,6 +69,8 @@ const (
 
 // DSPA Status Condition Types
 const (
+	DatabaseAvailable      = "DatabaseAvailable"
+	ObjectStoreAvailable   = "ObjectStoreAvailable"
 	APIServerReady         = "APIServerReady"
 	PersistenceAgentReady  = "PersistenceAgentReady"
 	ScheduledWorkflowReady = "ScheduledWorkflowReady"
@@ -81,9 +83,10 @@ const (
 // kubectl get output, and in summarizing
 // occurrences of causes
 const (
-	MinimumReplicasAvailable = "MinimumReplicasAvailable"
-	FailingToDeploy          = "FailingToDeploy"
-	Deploying                = "Deploying"
+	MinimumReplicasAvailable    = "MinimumReplicasAvailable"
+	FailingToDeploy             = "FailingToDeploy"
+	Deploying                   = "Deploying"
+	ComponentDeploymentNotFound = "ComponentDeploymentNotFound"
 )
 
 // Any required Configmap paths can be added here,

controllers/config/defaults.go

@@ -17,6 +17,11 @@ package controllers
 
 import (
 	"context"
+	"database/sql"
+	b64 "encoding/base64"
+	"fmt"
+
+	_ "github.com/go-sql-driver/mysql"
 
 	dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1"
 )
@@ -31,6 +36,52 @@ var dbTemplates = []string{
 	dbSecret,
 }
 
+// extract to var for mocking in testing
+var ConnectAndQueryDatabase = func(host, port, username, password, dbname string) bool {
+	connectionString := fmt.Sprintf("%s:%s@tcp(%s:%s)/%s", username, password, host, port, dbname)
+	db, err := sql.Open("mysql", connectionString)
+	if err != nil {
+		return false
+	}
+	defer db.Close()
+
+	testStatement := "SELECT 1;"
+	_, err = db.Exec(testStatement)
+	return err == nil
+}
+
+func (r *DSPAReconciler) isDatabaseAccessible(ctx context.Context, dsp *dspav1alpha1.DataSciencePipelinesApplication,
+	params *DSPAParams) bool {
+	log := r.Log.WithValues("namespace", dsp.Namespace).WithValues("dspa_name", dsp.Name)
+
+	if params.DatabaseHealthCheckDisabled(dsp) {
+		log.V(1).Info("Database health check disabled, assuming database is available and ready.")
+		return true
+	}
+
+	log.Info("Performing Database Health Check")
+	databaseSpecified := dsp.Spec.Database != nil
+	usingExternalDB := params.UsingExternalDB(dsp)
+	usingMariaDB := !databaseSpecified || dsp.Spec.Database.MariaDB != nil
+	if !usingMariaDB && !usingExternalDB {
+		log.Info("Could not connect to Database: Unsupported Type")
+		return false
+	}
+
+	decodePass, _ := b64.StdEncoding.DecodeString(params.DBConnection.Password)
+	dbHealthCheckPassed := ConnectAndQueryDatabase(params.DBConnection.Host,
+		params.DBConnection.Port,
+		params.DBConnection.Username,
+		string(decodePass),
+		params.DBConnection.DBName)
+	if dbHealthCheckPassed {
+		log.Info("Database Health Check Successful")
+	} else {
+		log.Info("Unable to connect to Database")
+	}
+	return dbHealthCheckPassed
+}
+
 func (r *DSPAReconciler) ReconcileDatabase(ctx context.Context, dsp *dspav1alpha1.DataSciencePipelinesApplication,
 	params *DSPAParams) error {
 
@@ -40,8 +91,12 @@ func (r *DSPAReconciler) ReconcileDatabase(ctx context.Context, dsp *dspav1alpha
 	// DB field can be specified as an empty obj, confirm that subfields are also specified
 	// By default if Database is empty, we deploy mariadb
 	externalDBSpecified := params.UsingExternalDB(dsp)
-	mariaDBSpecified := !databaseSpecified || dsp.Spec.Database.MariaDB != nil
-	deployMariaDB := !databaseSpecified || (mariaDBSpecified && dsp.Spec.Database.MariaDB.Deploy)
+	mariaDBSpecified := dsp.Spec.Database.MariaDB != nil
+	defaultDBRequired := !databaseSpecified || (!externalDBSpecified && !mariaDBSpecified)
+
+	deployMariaDB := mariaDBSpecified && dsp.Spec.Database.MariaDB.Deploy
+	// Default DB is currently MariaDB as well, but storing these bools seperately in case that changes
+	deployDefaultDB := !databaseSpecified || defaultDBRequired
 
 	// If external db is specified, it takes precedence
 	if externalDBSpecified {
@@ -52,7 +107,7 @@ func (r *DSPAReconciler) ReconcileDatabase(ctx context.Context, dsp *dspav1alpha
 		if err != nil {
 			return err
 		}
-	} else if deployMariaDB {
+	} else if deployMariaDB || deployDefaultDB {
 		log.Info("Applying mariaDB resources.")
 		for _, template := range dbTemplates {
 			err := r.Apply(dsp, params, template)
@@ -65,6 +120,8 @@ func (r *DSPAReconciler) ReconcileDatabase(ctx context.Context, dsp *dspav1alpha
 		// desired state.
 		if !databaseSpecified {
 			dsp.Spec.Database = &dspav1alpha1.Database{}
+		}
+		if !databaseSpecified || defaultDBRequired {
 			dsp.Spec.Database.MariaDB = params.MariaDB.DeepCopy()
 			dsp.Spec.Database.MariaDB.Deploy = true
 			if err := r.Update(ctx, dsp); err != nil {