opendatahub-io / data-science-pipelines-operator

Apache License 2.0
11 stars 47 forks source link

Feature Request: Make image optional in DataSciencePipelinesApplication #94

Closed strangiato closed 10 months ago

strangiato commented 1 year ago

The following is the sample DataSciencePipelinesApplication object which has several required image fields:

apiVersion: datasciencepipelinesapplications.opendatahub.io/v1alpha1
kind: DataSciencePipelinesApplication
metadata:
  name: sample
spec:
  # 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:
    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'

Instead of requiring the image fields, it would be great if these fields were optional. Ideally these fields should be set by the operator itself (without them needing them in the yaml object) and allow the option to overwrite the default image if they are specified.

This allows the operator to automatically manage these images as the operator is updated. If the operator is updated and the preferred mlpipelineUI image is updated, the operator will be able to automatically update it to the latest version of the image. If the image is manually specified by the user, the instance will be created and never updated which could cause the images for those items to become stale.

HumairAK commented 1 year ago

Hey @strangiato , thanks for the suggestion.

Note that mlpipelineUI is a UI from upstream kubeflow, and will very soon (likely by next month) be deprecated -> removed from the odh offering when odh-dashboard implements it's own UI.

As for Minio, we did make this default initially, we ended up removing it because having a default requires users to mention an upstream image in any downstream fork of DSPO as part of their default setup, and to avoid this they would need to provide their own downstream minio image (or some sort of a stub), in the defaults, and minio may not be their preferred object storage solution.

But to me that's a bit of a trivial issue and is not worth forcing users to specify their own minio image, I think you raise a good point and we should consider just making it default. @gmfrasca thoughts?

HumairAK commented 10 months ago

Closing due to staleness and lack of capacity, if this continues to be an issue/concern please feel free to re-open or create a new issue.