-
Notifications
You must be signed in to change notification settings - Fork 49
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
Conversation
HumairAK
commented
Jan 19, 2024
•
edited
Loading
edited
- 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.
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
A new image has been built to help with testing out this PR: 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. |
d0cee33
to
38a67cf
Compare
Change to PR detected. A new PR build was completed. |
38a67cf
to
f1f5bd7
Compare
Change to PR detected. A new PR build was completed. |
f1f5bd7
to
79e95c7
Compare
Change to PR detected. A new PR build was completed. |
79e95c7
to
6126a2b
Compare
Change to PR detected. A new PR build was completed. |
d654929
to
cdcab83
Compare
Change to PR detected. A new PR build was completed. |
cdcab83
to
5e30bb2
Compare
Change to PR detected. A new PR build was completed. |
5e30bb2
to
cbb4515
Compare
Change to PR detected. A new PR build was completed. |
cbb4515
to
d46943a
Compare
Change to PR detected. A new PR build was completed. |
d46943a
to
3e85b6e
Compare
Change to PR detected. A new PR build was completed. |
Signed-off-by: Humair Khan <HumairAK@users.noreply.github.com>
3e85b6e
to
b2a21fe
Compare
Change to PR detected. A new PR build was completed. |
@@ -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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
# - deployment.workflow-controller.yaml | ||
- priorityclass.yaml | ||
# - priorityclass.yaml |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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