Leverage DB/ObjStore Routes for Prereq Health Checks

Resolved pre-commit issues

Removed loggers

Added TLS capabilities to minio route and handled 503 error

Added new example to test minio healthcheck with enableExternalRoute as true

Fix Pre-commit errors

Updated error message

Updated Error message

Updated as per comments

Fixed test case issue

config/internal/minio/route.yaml.tmpl

@@ -1,15 +1,18 @@
 kind: Route
 apiVersion: route.openshift.io/v1
 metadata:
-  name: minio-{{.Name}}
-  namespace: {{.Namespace}}
-  labels:
-    app: minio-{{.Name}}
-    component: data-science-pipelines
+ name: minio-{{.Name}}
+ namespace: {{.Namespace}}
+ labels:
+   app: minio-{{.Name}}
+   component: data-science-pipelines
 spec:
-  to:
-    kind: Service
-    name: minio-{{.Name}}
-    weight: 100
-  port:
-    targetPort: 9000
+ to:
+   kind: Service
+   name: minio-{{.Name}}
+   weight: 100
+ port:
+   targetPort: 9000
+ tls:
+   termination: Edge
+   insecureEdgeTerminationPolicy: Redirect

config/samples/dspa_healthcheck.yaml

@@ -0,0 +1,26 @@
+apiVersion: datasciencepipelinesapplications.opendatahub.io/v1alpha1
+kind: DataSciencePipelinesApplication
+metadata:
+  name: sample
+spec:
+  apiServer:
+    deploy: true
+    enableSamplePipeline: false
+    # If developing against a cluster using self-signed certs, then uncomment this field.
+    # cABundle:
+      # configMapName: kube-root-ca.crt
+      # configMapKey: ca.crt
+  # One of minio or externalStorage must be specified for objectStorage
+  # This example illustrates minimal deployment with minio
+  # This is NOT supported and should be used for dev testing/experimentation only.
+  # See dspa_simple_external_storage.yaml for an example with external connection.
+  objectStorage:
+    disableHealthCheck: false
+    enableExternalRoute: true
+    minio:
+      # Image field is required
+      image: 'quay.io/opendatahub/minio:RELEASE.2019-08-14T20-37-41Z-license-compliance'
+  # Optional
+  mlpipelineUI:
+    # Image field is required
+    image: 'quay.io/opendatahub/odh-ml-pipelines-frontend-container:beta-ui'

controllers/dspipeline_params.go

@@ -28,6 +28,7 @@ import (
 	dspa "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1"
 	"github.com/opendatahub-io/data-science-pipelines-operator/controllers/config"
 	"github.com/opendatahub-io/data-science-pipelines-operator/controllers/util"
+	routev1 "github.com/openshift/api/route/v1"
 	v1 "k8s.io/api/core/v1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -82,6 +83,7 @@ type ObjectStorageConnection struct {
 	Endpoint          string // scheme://host:port
 	AccessKeyID       string
 	SecretAccessKey   string
+	ExternalRouteURL  string
 }
 
 func (p *DSPAParams) UsingV2Pipelines(dsp *dspa.DataSciencePipelinesApplication) bool {
@@ -145,6 +147,28 @@ func (p *DSPAParams) ObjectStorageHealthCheckDisabled(dsp *dspa.DataSciencePipel
 	return false
 }
 
+// ExternalRouteEnabled will return true if an external route is enabled in the CR, otherwise false.
+func (p *DSPAParams) ExternalRouteEnabled(dsp *dspa.DataSciencePipelinesApplication) bool {
+	if dsp.Spec.ObjectStorage != nil {
+		return dsp.Spec.ObjectStorage.EnableExternalRoute
+	}
+	return false
+}
+
+func (p *DSPAParams) RetrieveAndSetExternalRoute(ctx context.Context, client client.Client, log logr.Logger) (*routev1.Route, error) {
+	// Retrieve the external route
+	route := &routev1.Route{}
+	namespacedName := types.NamespacedName{
+		Name:      "minio-" + p.Name,
+		Namespace: p.Namespace,
+	}
+	if err := client.Get(ctx, namespacedName, route); err != nil {
+		log.Info("Unable to retrieve route", "error", err)
+	}
+
+	return route, nil
+}
+
 func passwordGen(n int) string {
 	rand.Seed(time.Now().UnixNano())
 	var chars = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890")
@@ -207,7 +231,6 @@ func (p *DSPAParams) RetrieveOrCreateObjectStoreSecret(ctx context.Context, clie
 // If an external secret is specified, SetupDBParams will retrieve DB credentials from it.
 // If DSPO is managing a dynamically created secret, then SetupDBParams generates the creds.
 func (p *DSPAParams) SetupDBParams(ctx context.Context, dsp *dspa.DataSciencePipelinesApplication, client client.Client, log logr.Logger) error {
-
 	usingExternalDB := p.UsingExternalDB(dsp)
 	if usingExternalDB {
 		// Assume validation for CR ensures these values exist
@@ -282,7 +305,6 @@ func (p *DSPAParams) SetupDBParams(ctx context.Context, dsp *dspa.DataSciencePip
 // If an external secret is specified, SetupObjectParams will retrieve storage credentials from it.
 // If DSPO is managing a dynamically created secret, then SetupObjectParams generates the creds.
 func (p *DSPAParams) SetupObjectParams(ctx context.Context, dsp *dspa.DataSciencePipelinesApplication, client client.Client, log logr.Logger) error {
-
 	usingExternalObjectStorage := p.UsingExternalStorage(dsp)
 	if usingExternalObjectStorage {
 		// Assume validation for CR ensures these values exist
@@ -372,8 +394,21 @@ func (p *DSPAParams) SetupObjectParams(ctx context.Context, dsp *dspa.DataScienc
 		}
 		p.ObjectStorageConnection.AccessKeyID = accessKey
 		p.ObjectStorageConnection.SecretAccessKey = secretKey
-	}
 
+	}
+	if p.ExternalRouteEnabled(dsp) {
+		route, err := p.RetrieveAndSetExternalRoute(ctx, client, log)
+		if err != nil {
+			fmt.Printf("Unable to retrieve endpoint:%s", err)
+		}
+		p.ObjectStorageConnection.ExternalRouteURL = route.Spec.Host
+		p.ObjectStorageConnection.Endpoint = route.Spec.Host
+		p.ObjectStorageConnection.Secure = util.BoolPointer(true)
+		p.ObjectStorageConnection.Host = route.Spec.Host
+		p.ObjectStorageConnection.Scheme = "https"
+		//port should be empty when external route is specified
+		p.ObjectStorageConnection.Port = ""
+	}
 	endpoint := fmt.Sprintf(
 		"%s://%s",
 		p.ObjectStorageConnection.Scheme,

controllers/storage.go

@@ -132,7 +132,15 @@ var ConnectAndQueryObjStore = func(ctx context.Context, log logr.Logger, endpoin
 		// In the case that the Error is NoSuchKey (or NoSuchBucket), we can verify that the endpoint worked and the object just doesn't exist
 		case minio.ErrorResponse:
 			if err.Code == "NoSuchKey" || err.Code == "NoSuchBucket" {
-				return true, err
+				return true, nil
+			}
+			// This condition is added to handle the service unavailble error when the external route pod takes long time to send successful readiness checks
+			if err.Code == "503 Service Unavailable" {
+				errorMessage := "503 Service Unavailable. This could be a special condition when minio external route is used " +
+					"and health check is trying to reach the service, where the pod is up and running but takes long time " +
+					"to pass the successful readiness checks."
+				log.Info(errorMessage)
+				return false, errors.New(errorMessage)
 			}
 		}
 
@@ -141,7 +149,7 @@ var ConnectAndQueryObjStore = func(ctx context.Context, log logr.Logger, endpoin
 				"If using an tls S3 connection with  self-signed certs, you may specify a custom CABundle " +
 				"to mount on the DSP API Server via the DSPA cr under the spec.apiServer.cABundle field. If you have already " +
 				"provided a CABundle, verify the validity of the provided CABundle."
-			log.Error(err, errorMessage)
+			log.Info(errorMessage)
 			return false, errors.New(errorMessage)
 		}