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: new pulling mechanism for job statuses #314

Merged

Conversation

yhwang
Copy link
Collaborator

@yhwang yhwang commented Oct 10, 2024

Update the driver to keep running even the user program finishes. The driver provides two APIs:

  • GetStatus(): retrieve job status
  • Shutdown(): properly tear down the driver

In the controller side, it uses pod/exec resource to run the driver command to invoke the driver APIs to retrieve the job status and shutdown the driver when job is done.

@yhwang yhwang requested a review from ruivieira October 10, 2024 03:06
@yhwang yhwang added the lm-eval Issues related to LM-Eval label Oct 10, 2024
@yhwang yhwang force-pushed the pulling-mechanism branch 2 times, most recently from fa56a16 to 7a07350 Compare October 10, 2024 03:30
Update the driver to keep running even the user program
finishes. The driver provides two APIs:
- GetStatus(): retrieve job status
- Shutdown(): properly tear down the driver

In the controller side, it uses `pod/exec` resource
to run the driver command to invoke the driver APIs
to retrieve the job status and shutdown the driver
when job is done.

Signed-off-by: Yihong Wang <[email protected]>
@yhwang yhwang force-pushed the pulling-mechanism branch from 7a07350 to 741224e Compare October 10, 2024 04:40
@yhwang
Copy link
Collaborator Author

yhwang commented Oct 10, 2024

Some info about the pulling mechanism:
The controller uses pod/exec to run the driver command on the job pod to retrieve the job status and results periodically (using the lmes-pod-checking-interval setting). The driver command uses the domain socket as the IPC to exchange the information with the driver that runs the evaluation job on the pod. When a job is done, the controller uses the same way to run the driver command on the pod to shut down the running driver.
The communication between the controller and job pods goes through the Kube API server. No more gRPC and ingress traffic on the operator. Therefore, I removed all of the gRPC-related files/code.

Copy link
Contributor

@tedhtchang tedhtchang left a comment

Choose a reason for hiding this comment

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

/LGTM I tested with my suspend PR

ENABLED_SERVICES=LMES CONTROLLER_TOOLS_VERSION=v0.16.3 IMG=quay.io/tedchang/trustyai-service-operator:latest make run

In another terminal:

cat << EOF| kubectl apply -f -
apiVersion: trustyai.opendatahub.io/v1alpha1
kind: LMEvalJob
metadata:
  labels:
    app.kubernetes.io/name: fms-lm-eval-service
    app.kubernetes.io/managed-by: kustomize
  name: evaljob-sample
  namespace: default
spec:
  suspend: false
  model: hf
  modelArgs:
  - name: pretrained
    value: EleutherAI/pythia-70m
  taskList:
    taskNames:
    - unfair_tos
  logSamples: true
  limit: "5"
EOF

Verified job and pod in running state

oc get po,lmevaljob
NAME                 READY   STATUS    RESTARTS   AGE
pod/evaljob-sample   1/1     Running   0          2m46s

NAME                                               STATE
lmevaljob.trustyai.opendatahub.io/evaljob-sample   Running

Copy link

openshift-ci bot commented Oct 11, 2024

@tedhtchang: changing LGTM is restricted to collaborators

In response to this:

/LGTM I tested with my suspend PR

ENABLED_SERVICES=LMES CONTROLLER_TOOLS_VERSION=v0.16.3 IMG=quay.io/tedchang/trustyai-service-operator:latest make run

In another terminal:

cat << EOF| kubectl apply -f -
apiVersion: trustyai.opendatahub.io/v1alpha1
kind: LMEvalJob
metadata:
 labels:
   app.kubernetes.io/name: fms-lm-eval-service
   app.kubernetes.io/managed-by: kustomize
 name: evaljob-sample
 namespace: default
spec:
 suspend: false
 model: hf
 modelArgs:
 - name: pretrained
   value: EleutherAI/pythia-70m
 taskList:
   taskNames:
   - unfair_tos
 logSamples: true
 limit: "5"
EOF

Verified job and pod in running state

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-sigs/prow repository.

Copy link

github-actions bot commented Oct 11, 2024

PR image build and manifest generation completed successfully!

📦 PR image: quay.io/trustyai/trustyai-service-operator-ci:741224e023229d75d963321d5e09c73456706865

📦 LMES driver image: quay.io/trustyai/ta-lmes-driver:741224e023229d75d963321d5e09c73456706865

📦 LMES job image: quay.io/trustyai/ta-lmes-job:741224e023229d75d963321d5e09c73456706865

🗂️ CI manifests

Copy link
Member

@ruivieira ruivieira left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @yhwang!

Copy link

openshift-ci bot commented Oct 12, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ruivieira, tedhtchang

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

@ruivieira ruivieira merged commit ab6bc98 into trustyai-explainability:dev/lm-eval Oct 13, 2024
4 of 5 checks passed
@yhwang yhwang deleted the pulling-mechanism branch October 14, 2024 16:55
ruivieira added a commit that referenced this pull request Oct 22, 2024
* Add lm-eval-service controller (#258)

* feat: Initial database support (#246)

* Initial database support

- Add status checking
- Add better storage flags
- Add spec.storage.format validation
- Add DDL
-Add HIBERNATE format to DB (test)
- Update service image
- Revert identifier to DATABASE
- Update CR options (remove mandatory data)

* Remove default DDL generation env var

* Update service image to latest tag

* Add migration awareness

* Add updating pods for migration

* Change JDBC url from mysql to mariadb

* Fix TLS mount

* Revert images

* Remove redundant logic

* Fix comments

* feat: Add TLS certificate mount on ModelMesh (#255)

* feat: Add TLS certificate mount on ModelMesh

* Revert from http to https until kserve/modelmesh#147 is merged

* Add lm-eval-service controller

refactor the existing TrustyAIService controller and
add LMEvalService controller

Signed-off-by: Yihong Wang <[email protected]>

---------

Signed-off-by: Yihong Wang <[email protected]>
Co-authored-by: Rui Vieira <[email protected]>

* fix: Fix typo in operator's arguments (#261)

Operator's arguments changed from `--eanble-services` to `--enable-services`.
trustyai.opendatahub.io_lmevaljobs.yaml and zz_generated.deepcopy.go regenerated.

* feat: Add LMES driver build to GHA (#272)

* sync: sync dev/lm-eval with main branch (#271)

* feat: Initial database support (#246)

* Initial database support

- Add status checking
- Add better storage flags
- Add spec.storage.format validation
- Add DDL
-Add HIBERNATE format to DB (test)
- Update service image
- Revert identifier to DATABASE
- Update CR options (remove mandatory data)

* Remove default DDL generation env var

* Update service image to latest tag

* Add migration awareness

* Add updating pods for migration

* Change JDBC url from mysql to mariadb

* Fix TLS mount

* Revert images

* Remove redundant logic

* Fix comments

* feat: Add TLS certificate mount on ModelMesh (#255)

* feat: Add TLS certificate mount on ModelMesh

* Revert from http to https until kserve/modelmesh#147 is merged

* Pin oc version, ubi version (#263)

* Restore checkout of trustyai-exp (#265)

* Add operator installation robustness (#266)

* fix: Skip InferenceService patching for KServe RawDeployment (#262)

* feat: ConfigMap key to disable KServe Serverless configuration (#267)

* feat: Add support for custom certificates in database connection (#259)

* Add TLS endpoint for ModelMesh payload processors. (#268)

Keep non-TLS endpoint for KServe Serverless (disabled by default)

---------

Signed-off-by: Yihong Wang <[email protected]>
Co-authored-by: Rui Vieira <[email protected]>
Co-authored-by: Rob Geada <[email protected]>

* Weekly sync up of dev/lm-eval branch (#278)

* feat: Initial database support (#246)

* Initial database support

- Add status checking
- Add better storage flags
- Add spec.storage.format validation
- Add DDL
-Add HIBERNATE format to DB (test)
- Update service image
- Revert identifier to DATABASE
- Update CR options (remove mandatory data)

* Remove default DDL generation env var

* Update service image to latest tag

* Add migration awareness

* Add updating pods for migration

* Change JDBC url from mysql to mariadb

* Fix TLS mount

* Revert images

* Remove redundant logic

* Fix comments

* feat: Add TLS certificate mount on ModelMesh (#255)

* feat: Add TLS certificate mount on ModelMesh

* Revert from http to https until kserve/modelmesh#147 is merged

* Pin oc version, ubi version (#263)

* Restore checkout of trustyai-exp (#265)

* Add operator installation robustness (#266)

* fix: Skip InferenceService patching for KServe RawDeployment (#262)

* feat: ConfigMap key to disable KServe Serverless configuration (#267)

* feat: Add support for custom certificates in database connection (#259)

* Add TLS endpoint for ModelMesh payload processors. (#268)

Keep non-TLS endpoint for KServe Serverless (disabled by default)

* fix: Correct maxSurge and maxUnavailable (#275)

* feat: Add support for custom DB names (#257)

* feat: Add support for custom DB names

* fix: Correct custom DB name

---------

Signed-off-by: Yihong Wang <[email protected]>
Co-authored-by: Rui Vieira <[email protected]>
Co-authored-by: Rob Geada <[email protected]>

* Driver updates job's status periodically (#280)

The driver periodically update the LMEvalJob.Status.Message field
with the outputs from the lm-eval. The message pattern the driver
captures is like `Running text generation:  81%|`. Then users
can use this information to check the progress of the job.

Signed-off-by: Yihong Wang <[email protected]>

* Add Dockerfile for LMES job image (#276)

Add Dockerfile for LMES job image and the needed files

Signed-off-by: Yihong Wang <[email protected]>

* feat: Add overlays (#283)

* feat: Add overlays

* Remove redundant lmes-tas overlay. Change job image name.

* Add job image build (#284)

* Change job image use midstream lm-evaluation-harness (#285)

* feat: support batch size (#290)

Add batch size support in the LMEvalJob which
leverages the `--batch_size` in the `lm-evaluation-harness`.
This only affects the local models. The `--bath_size` doesn't
work for remote inference APIs.

Signed-off-by: Yihong Wang <[email protected]>

* Add the `openai` package into the lmes job image (#292)

update the LMES job's Dockerfile to include the
`openai` package.

Signed-off-by: Yihong Wang <[email protected]>

* fix: fix dependency error in the job image (#296)

Split up the unitxt and openai dependencies to
avoid the conflict.

Signed-off-by: Yihong Wang <[email protected]>

* feat: add device detection in lmes driver (#298)

Added a new feature in LMES driver to detect the available
devices by using the PyTorch API. This feature can be disabled
by passing the `--detect-device false` option.

Signed-off-by: Yihong Wang <[email protected]>

* feat: support unitxt recipes (#301)

Add new fields in the CRD to support unitxt recipes and
leverage the driver to create corresponding yaml files
of the unitxt recipes.

Signed-off-by: Yihong Wang <[email protected]>

* feat: support custom dataset (#309)

Updated the CRD data struct to allow users to specify a custom Unitxt card in
JSON format. The custom Unitxt card is equivalent to a custom dataset
definition. Also restructured and updated the CRD to support Volumes,
VolumeMounts, Env, Resources, Labels, and Annotations.

Signed-off-by: Yihong Wang <[email protected]>

* feat: new pulling mechanism for job statuses (#314)

Update the driver to keep running even the user program
finishes. The driver provides two APIs:
- GetStatus(): retrieve job status
- Shutdown(): properly tear down the driver

In the controller side, it uses `pod/exec` resource
to run the driver command to invoke the driver APIs
to retrieve the job status and shutdown the driver
when job is done.

Signed-off-by: Yihong Wang <[email protected]>

* Move operator's cmd/operator/main.go to cmd/main.go to keep operator-sdk compatibility (#295)

* Remove hardcoded job's user ID (#322)

* Fix mkdir command in Job dockerfile (#330)

* Refactor some lmesreconcile methods (#323)

* Refactor lmes reconcile  optoins

Signed-off-by: ted chang <[email protected]>

* Update controllers/lmes/lmevaljob_controller.go

Co-authored-by: Yihong Wang <[email protected]>

* Update controllers/lmes/lmevaljob_controller.go

Co-authored-by: Yihong Wang <[email protected]>
Signed-off-by: ted chang <[email protected]>

---------

Signed-off-by: ted chang <[email protected]>
Co-authored-by: Yihong Wang <[email protected]>

* tidy: clean up lmes-job image (#333)

remove BAM related packages and patch.

Signed-off-by: Yihong Wang <[email protected]>

* Enable job suspend for Kueue (#317)

* Refactor lmes reconcile  optoins

Signed-off-by: ted chang <[email protected]>

* Update controllers/lmes/lmevaljob_controller.go

Co-authored-by: Yihong Wang <[email protected]>

* Update controllers/lmes/lmevaljob_controller.go

Co-authored-by: Yihong Wang <[email protected]>
Signed-off-by: ted chang <[email protected]>

* Enable job suspend for Kueue

Signed-off-by: ted chang <[email protected]>

---------

Signed-off-by: ted chang <[email protected]>
Co-authored-by: Yihong Wang <[email protected]>

* Add overlay placeholders for main merge (#334)

* sync: sync up dev/lm-eval branch with main branch (#336)

* [CI] Run tests from trustyai-tests (#279)

* Change Dockerfile to clone trustyai-tests

* Add PYTEST_MARKERS env and remove TESTS_REGEX

* RHOAIENG-12274: Update operator's overlays (#287)

* Update operator's overlays

* Update kustomization.yaml

* Add devflag printout to GH Action comment (#289)

* Add timeout loop to DSC install (#305)

* RHOAIENG-13625: Add DBAvailable status to CR (#304)

* Add DBAvailable status to CR

* Remove probes

* Add KServe destination rule for Inference Services in the ServiceMesh (#315)

* Add DestinationRule creation for KServe serverless

* Add permissions for destination rules

* Add role for destination rules

* Add missing role for creating destination rules

* Fix spacing in DestinationRule template

* Add check if DestinationRule CRD is present before creating it (#316)

* Add check for DestinationRule CRD

* Add API extensions to operator's scheme

* Add permission for CRD resource

* Fix operator metrics service target port (#320)

* Add readiness probes (#312)

* Enable KServe serverless in the rhoai overlay (#321)

* Update overlay images (#331)

* Add correct CA cert to JDBC (#324)

* Add correct CA cert to JDBC

* Add require SSL

* Support for VirtualServices for InferenceLogger traffic (#332)

* Generate KServe Inference Logger in conformance with DestinationRule and VirtualService

* Add VirtualService creation for models in the mesh

* Add permissions for VirtualServices

* Update manifests for VirtualServices

* Fix VirtualServiceName variable

* fix yaml linter after the sync

Signed-off-by: Yihong Wang <[email protected]>

* tidy the go.mod and go.sum as well

Signed-off-by: Yihong Wang <[email protected]>

---------

Signed-off-by: Yihong Wang <[email protected]>
Co-authored-by: Adolfo Aguirrezabal <[email protected]>
Co-authored-by: Rui Vieira <[email protected]>
Co-authored-by: Rob Geada <[email protected]>
Co-authored-by: Rui Vieira <[email protected]>

---------

Signed-off-by: Yihong Wang <[email protected]>
Signed-off-by: ted chang <[email protected]>
Co-authored-by: Yihong Wang <[email protected]>
Co-authored-by: Rob Geada <[email protected]>
Co-authored-by: ted chang <[email protected]>
Co-authored-by: Adolfo Aguirrezabal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm lm-eval Issues related to LM-Eval ok-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LM-Eval jobs can't connect to the gRPC server if deployed in a namespace other than the operator's
3 participants