Skip to content

Commit

Permalink
Merge pull request #454 from hbelmiro/issue-220
Browse files Browse the repository at this point in the history
Created optional operator-managed Route objects for deployed DBs and ObjStores
  • Loading branch information
openshift-merge-bot[bot] authored Nov 9, 2023
2 parents 40a8930 + a1701fb commit 9cb08d8
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 3 deletions.
4 changes: 4 additions & 0 deletions api/v1alpha1/dspipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,10 @@ type ObjectStorage struct {
// +kubebuilder:default:=false
// +kubebuilder:validation:Optional
DisableHealthCheck bool `json:"disableHealthCheck"`
// Enable an external route so the object storage is reachable from outside the cluster. Default: false
// +kubebuilder:default:=false
// +kubebuilder:validation:Optional
EnableExternalRoute bool `json:"enableExternalRoute"`
}

type Minio struct {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,11 @@ spec:
default: false
description: 'Default: false'
type: boolean
enableExternalRoute:
default: false
description: 'Enable an external route so the object storage is
reachable from outside the cluster. Default: false'
type: boolean
externalStorage:
properties:
bucket:
Expand Down
15 changes: 15 additions & 0 deletions config/internal/minio/route.yaml.tmpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
kind: Route
apiVersion: route.openshift.io/v1
metadata:
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
10 changes: 7 additions & 3 deletions controllers/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ import (
)

const storageSecret = "minio/secret.yaml.tmpl"
const storageRoute = "minio/route.yaml.tmpl"

var minioTemplates = []string{
"minio/deployment.yaml.tmpl",
"minio/pvc.yaml.tmpl",
"minio/service.yaml.tmpl",
"minio/minio-sa.yaml.tmpl",
storageRoute,
}

func joinHostPort(host, port string) (string, error) {
Expand Down Expand Up @@ -212,9 +214,11 @@ func (r *DSPAReconciler) ReconcileStorage(ctx context.Context, dsp *dspav1alpha1
}
log.Info("Applying object storage resources.")
for _, template := range minioTemplates {
err := r.Apply(dsp, params, template)
if err != nil {
return err
if dsp.Spec.ObjectStorage.EnableExternalRoute || template != storageRoute {
err := r.Apply(dsp, params, template)
if err != nil {
return err
}
}
}
// If no storage was not specified, deploy minio by default.
Expand Down
80 changes: 80 additions & 0 deletions controllers/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/minio/minio-go/v7/pkg/credentials"
dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1"

routev1 "github.com/openshift/api/route/v1"
"github.com/stretchr/testify/assert"
appsv1 "k8s.io/api/apps/v1"
"k8s.io/apimachinery/pkg/api/resource"
Expand Down Expand Up @@ -89,6 +90,85 @@ func TestDeployStorage(t *testing.T) {
created, err = reconciler.IsResourceCreated(ctx, deployment, expectedStorageName, testNamespace)
assert.True(t, created)
assert.Nil(t, err)

// Assert ObjectStorage Route doesn't exist
route := &routev1.Route{}
created, err = reconciler.IsResourceCreated(ctx, route, expectedStorageName, testNamespace)
assert.False(t, created)
assert.Nil(t, err)
}

func TestDeployStorageWithExternalRouteEnabled(t *testing.T) {
testNamespace := "testnamespace"
testDSPAName := "testdspa"
expectedStorageName := "minio-testdspa"

// Construct DSPA Spec with deployed Minio Object Storage
dspa := &dspav1alpha1.DataSciencePipelinesApplication{
Spec: dspav1alpha1.DSPASpec{
Database: &dspav1alpha1.Database{
DisableHealthCheck: false,
MariaDB: &dspav1alpha1.MariaDB{
Deploy: true,
},
},
ObjectStorage: &dspav1alpha1.ObjectStorage{
DisableHealthCheck: false,
EnableExternalRoute: true,
Minio: &dspav1alpha1.Minio{
Deploy: true,
Image: "someimage",
Resources: &dspav1alpha1.ResourceRequirements{ //TODO: fails without this block. Why?
Requests: &dspav1alpha1.Resources{
CPU: resource.MustParse("250m"),
Memory: resource.MustParse("500Mi"),
},
Limits: &dspav1alpha1.Resources{
CPU: resource.MustParse("500m"),
Memory: resource.MustParse("1Gi"),
},
},
},
},
},
}

// Enrich DSPA with name+namespace
dspa.Name = testDSPAName
dspa.Namespace = testNamespace

// Create Context, Fake Controller and Params
ctx, params, reconciler := CreateNewTestObjects()
err := params.ExtractParams(ctx, dspa, reconciler.Client, reconciler.Log)
assert.Nil(t, err)

// Assert ObjectStorage Deployment doesn't yet exist
deployment := &appsv1.Deployment{}
created, err := reconciler.IsResourceCreated(ctx, deployment, expectedStorageName, testNamespace)
assert.False(t, created)
assert.Nil(t, err)

// Assert ObjectStorage Route doesn't yet exist
route := &routev1.Route{}
created, err = reconciler.IsResourceCreated(ctx, route, expectedStorageName, testNamespace)
assert.False(t, created)
assert.Nil(t, err)

// Run test reconciliation
err = reconciler.ReconcileStorage(ctx, dspa, params)
assert.Nil(t, err)

// Assert ObjectStorage Deployment now exists
deployment = &appsv1.Deployment{}
created, err = reconciler.IsResourceCreated(ctx, deployment, expectedStorageName, testNamespace)
assert.True(t, created)
assert.Nil(t, err)

// Assert ObjectStorage Route now exists
route = &routev1.Route{}
created, err = reconciler.IsResourceCreated(ctx, route, expectedStorageName, testNamespace)
assert.True(t, created)
assert.Nil(t, err)
}

func TestDontDeployStorage(t *testing.T) {
Expand Down

0 comments on commit 9cb08d8

Please sign in to comment.