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

parameterize and upate argo v2 images. #549

Merged
merged 1 commit into from
Jan 20, 2024

Conversation

HumairAK
Copy link
Contributor

@HumairAK HumairAK commented Jan 19, 2024

  • Add an odh overlay to deploy argo + dspo together, to be utilized by a DSC
  • Parameterize argo images in argo pods
  • update image sources to point to v2 images
  • remove scheduled workflow crd from argo deployment (we already deploy it with dspo)
  • remove priority class from workflow controller because the ODH operator SA does not have permissions to deploy/set this.
is forbidden: no PriorityClass with name ds-pipelines-workflow-controller-priorityclass was found

Copy link
Contributor

openshift-ci bot commented Jan 19, 2024

[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 ask for approval from humairak. 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

@dsp-developers
Copy link
Contributor

A new image has been built to help with testing out this PR: quay.io/opendatahub/data-science-pipelines-operator:pr-549
An OCP cluster where you are logged in as cluster admin is required.

To use this image run the following:

cd $(mktemp -d)
git clone git@github.com:opendatahub-io/data-science-pipelines-operator.git
cd data-science-pipelines-operator/
git fetch origin pull/549/head
git checkout -b pullrequest d0cee33acc6440795ab3a1f424cb55ae80457c96
make deploy IMG="quay.io/opendatahub/data-science-pipelines-operator:pr-549"

More instructions here on how to deploy and test a Data Science Pipelines Application.

@dsp-developers
Copy link
Contributor

Change to PR detected. A new PR build was completed.
A new image has been built to help with testing out this PR: quay.io/opendatahub/data-science-pipelines-operator:pr-549

@dsp-developers
Copy link
Contributor

Change to PR detected. A new PR build was completed.
A new image has been built to help with testing out this PR: quay.io/opendatahub/data-science-pipelines-operator:pr-549

@dsp-developers
Copy link
Contributor

Change to PR detected. A new PR build was completed.
A new image has been built to help with testing out this PR: quay.io/opendatahub/data-science-pipelines-operator:pr-549

@dsp-developers
Copy link
Contributor

Change to PR detected. A new PR build was completed.
A new image has been built to help with testing out this PR: quay.io/opendatahub/data-science-pipelines-operator:pr-549

@HumairAK HumairAK force-pushed the update_images branch 2 times, most recently from d654929 to cdcab83 Compare January 20, 2024 18:31
@dsp-developers
Copy link
Contributor

Change to PR detected. A new PR build was completed.
A new image has been built to help with testing out this PR: quay.io/opendatahub/data-science-pipelines-operator:pr-549

@dsp-developers
Copy link
Contributor

Change to PR detected. A new PR build was completed.
A new image has been built to help with testing out this PR: quay.io/opendatahub/data-science-pipelines-operator:pr-549

@dsp-developers
Copy link
Contributor

Change to PR detected. A new PR build was completed.
A new image has been built to help with testing out this PR: quay.io/opendatahub/data-science-pipelines-operator:pr-549

@dsp-developers
Copy link
Contributor

Change to PR detected. A new PR build was completed.
A new image has been built to help with testing out this PR: quay.io/opendatahub/data-science-pipelines-operator:pr-549

@dsp-developers
Copy link
Contributor

Change to PR detected. A new PR build was completed.
A new image has been built to help with testing out this PR: quay.io/opendatahub/data-science-pipelines-operator:pr-549

Signed-off-by: Humair Khan <HumairAK@users.noreply.github.com>
@dsp-developers
Copy link
Contributor

Change to PR detected. A new PR build was completed.
A new image has been built to help with testing out this PR: quay.io/opendatahub/data-science-pipelines-operator:pr-549

@HumairAK HumairAK merged commit 3f5369f into opendatahub-io:main Jan 20, 2024
5 of 6 checks passed
@@ -199,6 +199,17 @@ undeploy-kind: ## Undeploy controller from the K8s cluster specified in ~/.kube/
&& kustomize edit set namespace ${OPERATOR_NS}
kustomize build config/overlays/kind-tests | kubectl delete --ignore-not-found=$(ignore-not-found) -f -

.PHONY: deployODH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should document this (and the other deployment commands if they're not already in there) in the README

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can make this part of: https://issues.redhat.com/browse/RHOAIENG-1718

Comment on lines 19 to +20
# - deployment.workflow-controller.yaml
- priorityclass.yaml
# - priorityclass.yaml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should remove these if they're not needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I wasn't sure, do you know the reasoning behind this one? I don't see why we would want this

IMAGES_MLMDENVOY=quay.io/opendatahub/ds-pipelines-metadata-envoy:v1.6.1
IMAGES_MLMDGRPC=quay.io/opendatahub/ds-pipelines-metadata-grpc:v1.6.1
IMAGES_MLMDWRITER=quay.io/opendatahub/ds-pipelines-metadata-writer:v1.6.1
IMAGES_CRDVIEWER=gcr.io/ml-pipeline/viewer-crd-controller:2.0.0-rc.2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think we can remove this

workflowController:
deploy: true
image: gcr.io/ml-pipeline/workflow-controller:v3.3.10-license-compliance
image: quay.io/opendatahub/ds-pipelines-frontend:latest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v1 and v2 frontend images are different, does this now point to the v2 frontend? does the v1 DSPA sample need to be updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right, it does need to be updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants