Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat[frontend]: implement artifact-repositories configmap support #11354

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

droctothorpe
Copy link
Contributor

@droctothorpe droctothorpe commented Nov 5, 2024

Description of your changes:

Argo Workflows supports the ability to specify an artifact repository for archived logs uniquely for each namespace rather than just globally as documented here.

This PR adds support for this functionality to the KFP UI. If a workflow runs and...

  1. The corresponding pods are deleted
  2. The corresponding workflow is deleted
  3. There is an artifact-repositories configmap in the target namespace that provides a unique, namespaced keyFormat

...this PR ensures that logs are still accessible in the UI. This functionality is disabled by default to avoid unnecessary network calls (for end users who don't leverage namespace-specific artifact-repositories). It can be toggled on through configs.ts or an environment variable.

Co-authored with @quinnovator 🌟 .

Fixes: #11339

Checklist:

Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign zijianjoy for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: droctothorpe <[email protected]>
Co-authored-by: quinnovator <[email protected]>
@github-actions github-actions bot added ci-passed All CI tests on a pull request have passed and removed ci-passed All CI tests on a pull request have passed labels Nov 5, 2024
@github-actions github-actions bot added the ci-passed All CI tests on a pull request have passed label Nov 5, 2024
@HumairAK HumairAK self-assigned this Nov 6, 2024
@droctothorpe
Copy link
Contributor Author

droctothorpe commented Nov 6, 2024

@HumairAK to recreate the edge case:

  1. Deploy KFP to a local K8s cluster.
  2. Set ARGO_ARCHIVE_LOGS=true in configs.ts.
  3. cd into the frontend directory.
  4. Run NAMESPACE=kubeflow npm run start:proxy-and-server to start the UI on your host machine.
  5. Create the following configmap in the kubeflow namespace:
metadata:
  annotations:
    workflows.argoproj.io/default-artifact-repository: artifact-repositories
  name: artifact-repositories
  namespace: kubeflow
data:
  artifact-repositories: |-
    archiveLogs: true
    s3:
      accessKeySecret:
        key: accesskey
        name: mlpipeline-minio-artifact
      bucket: mlpipeline
      endpoint: minio-service.kubeflow:9000
      insecure: true
      keyFormat: foo
      secretKeySecret:
        key: secretkey
        name: mlpipeline-minio-artifact
kind: ConfigMap
  1. Run a simple pipeline.
  2. Once it completes, delete the AWF workflow custom resource associated with the run.
  3. Try to access the logs of the run in the UI running on your host machine. On master, this operation will fail. On this branch, it will succeed.
  4. The next few steps are optional. Port-forward the minio pod.
  5. Navigate to the minio UI.
  6. Login (username: minio, password: minio123).
  7. Note how the logs are stored in mlpipeline/foo rather than the default keyFormat specified in configs.ts. Argo grants precedence to the keyFormat specified in the artifact-repositories configmap over the keyFormat specified in workflow-controller-configmap.

@droctothorpe
Copy link
Contributor Author

droctothorpe commented Nov 8, 2024

I updated the instructions to reproduce to edge case to make manual validation a lot easier by running the UI on your host machine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-passed All CI tests on a pull request have passed size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[frontend] The UI does not account for the artifact-repositories configmap
2 participants