[feature] remove hard-coded configs in KFP V2 launcher

[thesuperzapper]

Feature Area

/area backend

Related Issues

• [feature] Allow KFP to specify Minio instance when configuring pipeline root #6517

The Problem

Currently, we are hardcoding a few important configs in KFP V2's launcher and cache code.
This is making it difficult for distributions to support KFP v2 in a generic way.

Launcher - The minio credentials secret is hard-coded as mlpipeline-minio-artifact:

pipelines/backend/src/v2/objectstore/object_store.go

Line 292 in 35f7507

const minioArtifactSecretName = "mlpipeline-minio-artifact"

Launcher - The minio credentials secret keys are hard-coded as accesskey and secretkey:

pipelines/backend/src/v2/objectstore/object_store.go

Lines 324 to 325 in 35f7507

	accessKey := string(secret.Data["accesskey"])
	secretKey := string(secret.Data["secretkey"])

Launcher - The minio endpoint is hard-coded as minio-service.kubeflow:9000:

pipelines/backend/src/v2/objectstore/object_store.go

Line 291 in 35f7507

const defaultMinioEndpointInMultiUserMode = "minio-service.kubeflow:9000"

Cache - The ml-pipeline endpoint is hard-coded as ml-pipeline.kubeflow:8887:

pipelines/backend/src/v2/cacheutils/cache.go

Line 26 in 35f7507

defaultKfpApiEndpoint = "ml-pipeline.kubeflow:8887"

The Solution

We want to allow the above configs to be changed from their defaults, preferably on a per-namespace basis.

I propose we add these configs to the ConfigMap/kfp-launcher which already exists in each profile namespace to set defaultPipelineRoot.

For example, a new ConfigMap/kfp-launcher in a profile namespace might look like this:

data:
  defaultPipelineRoot: "minio://mlpipeline/v2/artifacts"
 
  ## minio endpoint config
  minioEndpoint: "minio.example.com:9000"
 
  ## minio auth configs
  minioAuthSecret: "mlpipeline-minio-artifact"
  minioAuthSecretAccessKeyKey: "access_key"
  minioAuthSecretAccessKeyKey: "access_key"

  ## ml pipeline endpoint config
  mlPipelineEndpoint: "ml-pipeline.kubeflow:8887"

The question now becomes how to propagate these configs to the kfp-launcher container in each workflow Pod.
I think there are two options:

• OPTION 1: have the kfp-launcher container read the ConfigMap/kfp-launcher
  • when it executes, it can pass the minio configs down as it calls objectstore.OpenBucket
  • setting mlPipelineEndpoint has to be done before it runs cacheutils.NewClient()
• OPTION 2: read the ConfigMap/kfp-launcher in RootDAG at the same time as we read the defaultPipelineRoot
  • (this approach reduces the number of API calls to read ConfigMap/kfp-launcher)

Other Thoughts

We could make this slightly more advanced to allow for "selecting" credentials based on the bucket & key-prefix being used, because not all pipelines have to use the default bucket_root, so might need different credentials.

For example, we could extend ConfigMap/kfp-launcher like this:

data:
  defaultPipelineRoot: "minio://my-bucket/v2/artifacts"

  ## minio endpoint config
  minioEndpoint: "minio.example.com:9000"

  ## ml pipeline endpoint config
  mlPipelineEndpoint: "ml-pipeline.kubeflow:8887"

  ## bucket auth configs (as a YAML string)
  bucketAuth: |
    ## list of "auth providers" for minio
    minio:

      ## specifies auth for objects under "minio://my-bucket/v2/artifacts/"
      - bucketName: "my-bucket"
        keyPrefix: "v2/artifacts/"
        authFromSecret:
          secretName: "my-secret-in-profile-namespace"
          accessKeyKey: "access_key"
          secretKeyKey: "secret_key"

    ## similar, but for s3 buckets
    s3: []

    ## similar, but for gcs buckets
    gcs: []

---

Love this idea? Give it a 👍.

[thesuperzapper]

@chensun @zijianjoy I think this is very important before cutting the 2.0.0 for the backend.

[zijianjoy]

/assign @zijianjoy

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ca-scribner]

Did this get resolved before the 1.8 release?

[thesuperzapper]

@chensun can you confirm what the status on this is?

[TobiasGoerke]

Just seeing this issue after I created #10318 and implemented #10319.

If implemented, this would enable users to define arbitrary s3 endpoints for artifact storage.
Appropriate envs (AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY and AWS_REGION) still need to be added to the pipeline pods on some other way.

Does this help?

[HumairAK]

Thank you for this @TobiasGoerke I think this is an improvement. Though the problem of the hardcoded configs still remains. Adding the endpoint as part of the query string seems more like a workaround to me for this issue, and as you said the issue of propagating credentials remains.

There seems to be already a precedent with passing around mlmd endpoint info via a configmap, seems to make sense to do the same for the s3 endpoint as suggested by @thesuperzapper. In this same config we can specify the s3 credentials secret name/keys as well.

[HumairAK]

@zijianjoy this is a priority item for our team so we'll be looking to resolve this in our downstream, and if you'd like I can help with implementing it here as well. If you are already in progress with this and would like to see it to completion, we'd be interested in your approach so we can be aligned in our solution while we wait for the change in upstream!

Some follow ups:

1. Is there an obvious preference over option 1/2?

Option 1 it seems easier to implement as well as easier to extend, one does not need to always update mlmd client code when adding new properties to the config. However, as previously mentioned it requires that every executor pod fetches the configmap, is this a major concern?

Option 2 we'd follow the same pattern as we do for reading in the pipelineroot. So addtiional metadata fields for the provider creds would need to be added. In the driver we would find the matching auth provider config in the kfp launcher configmap and maybe store that as a json string. We then query MLMD for the configs within the launcher.

I would be interested to hear other options/suggestions under consideration.

2. Configmap structure

I like @thesuperzapper more flexible configmap suggestions, but I have some further suggestions/amendments:

    defaultPipelineRoot: "minio://my-bucket/v2/artifacts"
    providers:
        minio:
          # can be another minio instance
          endpoint: "minio.example.com:9000"
          disableSSL: true
          # by default use this secret for this provider
          defaultProviderSecretRef:
            secretName: "my-secret-in-profile-namespace"
            accessKeyKey: "access_key"
            secretKeyKey: "secret_key"
          # optional, first matching prefix in pipelineRoot is used
          # in this example v2/artifacts would be used if using the 
          # defaultPipelineRoot above
          authConfigs:
            - bucketName: "my-bucket"
              keyPrefix: "v2/artifacts/123"
              secretRef:
                secretName: "my-secret-in-profile-namespace-1"
                accessKeyKey: "access_key"
                secretKeyKey: "secret_key"
            - bucketName: "my-bucket"
              keyPrefix: "v2/artifacts/"
              secretRef:
                secretName: "my-secret-in-profile-namespace-2"
                accessKeyKey: "access_key"
                secretKeyKey: "secret_key"

        # Having the ability to override the regions here allows us to leverage other s3 compatible providers
        # and not just aws s3
        s3:
          # allows for pointing to s3 compatible non aws solutions like ceph
          endpoint: "https://s3.amazonaws.com"
          disableSSL: true
          region: us-east-2
          defaultProviderSecretRef:
            secretName: "my-aws-secret-in-profile-namespace"
            accessKeyKey: "access_key"
            secretKeyKey: "secret_key"
          authConfigs: []
        gs: { }

3. mlPipelineEndpoint

I don't see the need to add this in kfp-launcher configmap, to me it makes more sense to specify this in the API server as a config/env var and pass it down as parameters within driver/kfp-launcher/etc. just like the mlmd address/port.

[thesuperzapper]

@HumairAK I see that you have at least partially fixed this in your fork of KFP:

• UPSTREAM: <carry>: add ns scoped s3 support. opendatahub-io/data-science-pipelines#4
• fix: handle cached input in objstore opendatahub-io/data-science-pipelines#27

Would you be willing to raise a PR upstream, so we can all standardize on a fix?

[HumairAK]

@thesuperzapper that's the plan, I'll send one out soon

[HumairAK]

@thesuperzapper I've provided a WIP that is mostly complete, just need to do more testing, and fix a obj store region related bug

I welcome feedback on implementation/approach there:

#10625

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[hbelmiro]

Don't stale.

[rimolive]

/lifecycle frozen

[gregsheremeta]

@HumairAK @thesuperzapper should this be closed as complete? Referenced PRs are all merged

[HumairAK]

Hardcoded configs for object store have all been made configurable, and docs are provided here. What now remains is parameterizing the pipeline server endpoint, which is still hardcoded to use the ml-pipeline service: https://github.com/kubeflow/pipelines/blob/2.2.0/backend/src/v2/cacheutils/cache.go#L127-L140. The service doesn't require to be located in kubeflow namespace, so it doesn't prohibit users from doing a kfp installation in other non kubeflow namespaces, so it's less of an issue in my mind, but still worth doing.

I made an attempt here. But an easier route might be to just over-write the default ML_PIPELINE_SERVICE_HOST / ML_PIPELINE_SERVICE_PORT_GRPC, this way we can keep the IR env agnostic.

I'm fine with having a separated granular issue for it though.

[shivanibhargove]

Hi,
We are also facing the similar issue where our kubeflow is not installed in kubeflow namespace and thus its failing as default is given as kubeflow.