Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

Change fallback OVMS URL to point to our built image #768

Merged
merged 1 commit into from
May 4, 2023

Conversation

Xaenalt
Copy link
Member

@Xaenalt Xaenalt commented Mar 30, 2023

Image includes Nvidia GPU inferencing support

@Xaenalt Xaenalt force-pushed the ovms-gpu branch 3 times, most recently from 11be14c to 99ee11c Compare April 18, 2023 19:14
@Xaenalt Xaenalt changed the title Change OVMS URL to point to our built image Change fallback OVMS URL to point to our built image Apr 18, 2023
@Xaenalt
Copy link
Member Author

Xaenalt commented Apr 18, 2023

I'm also not completely convinced we need this per se too, we may want the fallback image to point elsewhere, but I'll let the reviewers decide on that

Copy link
Contributor

@Jooho Jooho left a comment

Choose a reason for hiding this comment

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

/lgtm

@LaVLaS
Copy link
Contributor

LaVLaS commented Apr 20, 2023

@Xaenalt We need to upstream this repo and host these images in quay.io/opendatahub instead of pulling from a downstream image repository for a build system that is already in use upstream

@Xaenalt
Copy link
Member Author

Xaenalt commented Apr 20, 2023

I don't have write access to quay.io/opendatahub to make that change sadly. Also, if there's a robot account, we could configure automatic pushes to these places from the openshift-ci builds too

model-mesh/base/params.env Outdated Show resolved Hide resolved
@LaVLaS
Copy link
Contributor

LaVLaS commented May 2, 2023

/retest

/approve

@openshift-ci openshift-ci bot added the approved label May 2, 2023
@openshift-ci
Copy link

openshift-ci bot commented May 2, 2023

@Xaenalt: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/49-images 11be14c link true /test 49-images

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@Jooho
Copy link
Contributor

Jooho commented May 3, 2023

The fail does not look due to the openvino.

 FAILURE after 1203.757s: operator-tests/odh-manifests/basictests/codeflare-stack.sh:21: executing 'oc get pod -n opendatahub |grep mcad-controller | awk '{print $2}'' expecting any result and text '1/1'; re-trying every 10s until completion or 1200.000s: the command timed out

@Xaenalt
Copy link
Member Author

Xaenalt commented May 3, 2023

Let me rebase it onto main, that might help

@LaVLaS
Copy link
Contributor

LaVLaS commented May 3, 2023

I didn't investigate the root cause but it looks like the operator was not installed successfully OR the odh-core kfdef was never deployed so there are no components

@LaVLaS
Copy link
Contributor

LaVLaS commented May 3, 2023

@Xaenalt A rebase should kick off a new test run

Image includes Nvidia GPU inferencing support
@LaVLaS
Copy link
Contributor

LaVLaS commented May 4, 2023

/lgtm

@Jooho
Copy link
Contributor

Jooho commented May 4, 2023

/approve

@openshift-ci
Copy link

openshift-ci bot commented May 4, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Jooho, LaVLaS

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

The pull request process is described 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

@openshift-merge-robot openshift-merge-robot merged commit 59e7748 into opendatahub-io:master May 4, 2023
@@ -3,6 +3,6 @@ monitoring-namespace=
odh-mm-rest-proxy=quay.io/opendatahub/rest-proxy:v0.11.0-alpha
odh-modelmesh-runtime-adapter=quay.io/opendatahub/modelmesh-runtime-adapter:v0.11.0-alpha
odh-modelmesh=quay.io/opendatahub/modelmesh:v0.11.0-alpha
odh-openvino=quay.io/opendatahub/openvino_model_server:2022.3-gpu
odh-openvino=quay.io/opendatahub/openvino_model_server@sha256:20dbfbaf53d1afbd47c612d953984238cb0e207972ed544a5ea662c2404f276d

This comment was marked as resolved.

Copy link
Contributor

Choose a reason for hiding this comment

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

params.env file is for optional input from kfdef file. Openvino is not changed frequently so it is ok to use sha256 directly. however, for the others, we are thinking to use the doc to support a disconnected environment. In other word, the image information in this file is just for default value but we can change it with sha256 with kfdef to support a disconnected environment.

newName: openvino/model_server
newTag: "2022.3"
newName: quay.io/opendatahub/openvino_model_server
newTag: "2022.3-release"

This comment was marked as resolved.

Copy link
Contributor

Choose a reason for hiding this comment

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

this information has nothing to do with a disconnected environment. The mirror command line is not referring this information.

Copy link
Contributor

Choose a reason for hiding this comment

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

but the ImageContentSourcePolicy generated by oc mirror, or, in my case generated manually, needs sha256 type digests, not tags, in manifests. Note my content "restricted network environment installs (on-prem) with mirrored repos and ImageContentSourcePolicy"

@@ -26,8 +26,8 @@ images:
newTag: 0.5.2

- name: ovms-1
Copy link
Contributor

Choose a reason for hiding this comment

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

that name is nowhere in the image field of https://github.com/opendatahub-io/odh-manifests/blob/master/model-mesh/runtimes/ovms-1.x.yaml#L37 instead, there is a kustomize parameter value in there from https://github.com/opendatahub-io/odh-manifests/blob/master/model-mesh/base/params.env#L6 Same goes for the other three ServingRunetime yamls referenced ...

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

Successfully merging this pull request may close these issues.

5 participants