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

Reverts of unneeded commits; and upstream code sync #179

Merged

Conversation

israel-hdez
Copy link

What this PR does / why we need it:

This reverts the following PRs that are no longer need:

The PR also does a code-sync with the upstream repository. Without the sync, kserve#3316 would not be picked and CI tests would be failing. So, the reverts and the sync need to be done at the same time to ensure stability.

Which issue(s) this PR fixes:

This is the remainder work to close: https://issues.redhat.com/browse/RHOAIENG-1084.

Feature/Issue validation/testing:

CI E2E tests should be enough to ensure correctness of the build.

Jooho and others added 8 commits January 18, 2024 18:38
…ve#3363)

* generate docs

Signed-off-by: Yuan Tang <[email protected]>

* fix

Signed-off-by: Yuan Tang <[email protected]>

* fix

Signed-off-by: Yuan Tang <[email protected]>

---------

Signed-off-by: Yuan Tang <[email protected]>
1. graph poetry project was not included in previous checks.
    2. As poetry lock --check is deprecated, used `poetry check --lock`.

Signed-off-by: Andrews Arokiam <[email protected]>
* initial commit for graph raw deployment

Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>

* addd hpa support as well for inference graph raw deployment

Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>

* Just for local

Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>

* Just local

Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>

* Fix local setup

Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>

* Sleep change

Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>

* Fix self-signed-ca installation (kserve#3165)

Signed-off-by: Sivanantham Chinnaiyan <[email protected]>
Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>

* refactored

Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>

* Fix logging message

Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>

* adding unit tests

Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>

* adding unit tests

Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>

* removed temporary dev env changes

Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>

* Rawdeployment mode type

Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>

* restoring from master branch

Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>

* Remove affinity test

Signed-off-by: Tanvi Thakur <[email protected]>

* add with resource test

Signed-off-by: Tanvi Thakur <[email protected]>

* added controller test for graph

Signed-off-by: Mopuri, Bharath <[email protected]>

* addressed code review comments

Signed-off-by: Mopuri, Bharath <[email protected]>

* Added copy right statement for new files

Signed-off-by: Mopuri, Bharath <[email protected]>

* Removed dependency for InferenceGraph kind on componentExtensionSpec which is specific to inference service spec

Signed-off-by: Mopuri, Bharath <[email protected]>

* restored Makefile from master branch

Signed-off-by: Mopuri, Bharath <[email protected]>

* fixed codescan issue for AtoI function

Signed-off-by: Mopuri, Bharath <[email protected]>

* fixed test failures

Signed-off-by: Mopuri, Bharath <[email protected]>

* Added unit test for hpa reconciler

Signed-off-by: Mopuri, Bharath <[email protected]>

* Added Ready status when Deployment is available in raw mode

Signed-off-by: Mopuri, Bharath <[email protected]>

* Removed unused code

Signed-off-by: Mopuri, Bharath <[email protected]>

* e2e test for ISVC in raw mode

Signed-off-by: Mopuri, Bharath <[email protected]>

* improved e2e tests for inference graph raw deployment

Signed-off-by: Mopuri, Bharath <[email protected]>

* fixed python lint errors

Signed-off-by: Mopuri, Bharath <[email protected]>

* added annotations for ig spec that triggers raw deployment

Signed-off-by: Mopuri, Bharath <[email protected]>

* made names unique b/w runs

Signed-off-by: Mopuri, Bharath <[email protected]>

* Add test

Signed-off-by: Tanvi Thakur <[email protected]>

* Adding unit test

Signed-off-by: Tanvi Thakur <[email protected]>

* fixed e2e test failure for ig.  IG is not moving to ready state because of not handling raw deployment mode condition correctly

Signed-off-by: Mopuri, Bharath <[email protected]>

* changed marker for graph test

Signed-off-by: Mopuri, Bharath <[email protected]>

* corrected test validation for raw deployment mode knative resources

Signed-off-by: Mopuri, Bharath <[email protected]>

* correct rebase errors

Signed-off-by: Mopuri, Bharath <[email protected]>

* hpa field moved from annotations to inferencegraphspec as fields

Signed-off-by: Mopuri, Bharath <[email protected]>

* commiting make generate output due to InferenceGraphSpec changes

Signed-off-by: Mopuri, Bharath <[email protected]>

---------

Signed-off-by: Mopuri, Bharath <[email protected]>
Signed-off-by: Tanvi Thakur <[email protected]>
Signed-off-by: Sivanantham Chinnaiyan <[email protected]>
Co-authored-by: Mopuri, Bharath <[email protected]>
Co-authored-by: Tanvi Thakur <[email protected]>
Co-authored-by: Sivanantham <[email protected]>
When Istio is installed with its CNI plugin, KServe inference services are not capable to start. This is because the storage initializer is an init-container and the network is not available when the CNI plugin is enabled.

The typical recommendation to fix the issue is to remove init-containers and move any initialization code to a regular container. This approach would not work well with KServe, because the serving runtimes assume the model is already present on the filesystem and moving the storage initializer as a regular container will cause race conditions (the runtime will succeed loading only if the storage initializer manages to pull the model before the runtime starts).

There are alternative approaches documented in https://istio.io/latest/docs/setup/additional-setup/cni/#compatibility-with-application-init-containers. All alternatives have the downside that the traffic won't be captured by Istio and won't benefit from Istio features, which should be OK for KServe storage-initializer case.

These changes use the approach for running the storage initializer using the same UserID as the Istio sidecar. The UID is copied from the sidecar container to cover Istio derivatives, like Maistra.

Signed-off-by: Edgar Hernández <[email protected]>
This reverts commit 17fb6b9.

Signed-off-by: Edgar Hernández <[email protected]>
…-cni"

This reverts commit 4e7f0df

Signed-off-by: Edgar Hernández <[email protected]>
@openshift-ci openshift-ci bot requested review from anishasthana and Jooho January 26, 2024 21:05
Copy link

openshift-ci bot commented Jan 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: israel-hdez

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

@israel-hdez israel-hdez changed the title Reverts and sync Reverts of unneeded commits; and upstream code sync Jan 26, 2024
@openshift-ci openshift-ci bot added the lgtm label Jan 29, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit f462609 into opendatahub-io:master Jan 29, 2024
18 checks passed
@israel-hdez israel-hdez deleted the reverts-and-sync branch January 29, 2024 17:54
spolti referenced this pull request in spolti/kserve Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants