search

trustyai-explainability / trustyai-service-operator

TrustyAI's Kubernetes operator
Apache License 2.0
3 stars 20 forks source link

feat: Add overlays #283

Closed ruivieira closed 2 months ago

ruivieira commented 2 months ago

Add overlays to support selective controller configuration:

openshift-ci[bot] commented 2 months ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yhwang

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

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/trustyai-explainability/trustyai-service-operator/blob/dev/lm-eval/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
github-actions[bot] commented 2 months ago

PR image build and manifest generation completed successfully!

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

📦 Driver image: quay.io/trustyai/ta-lmes-driver:a97f57cef0a8569883732689434314b27887dfce

🗂️ CI manifests

ruivieira commented 2 months ago

Nice overlays! Just one minor question: In your change, the base is TAS+LMES and ODH inherits the base. RHOAI overrides it to TAS only. The other way around is to keep the base as TAS only and change ODH to TAS+LMES, then for RHOAI, it can inherit from the base directly.

These two approaches all deliver the same outcome. I guess the key point is: the base should be TAS only or TAS + LMES. This may be also related to how the e2e test deploys the operator. I will leave this to you. I am good with either way.

Thanks @yhwang that's a good point.

Personally, I think we should keep the base as LMES+TAS. ODH can just inherit from base. We then provide LMES only for specific use cases. Any downstream can then configure the controllers in their respective forks. In this case, the overlay for LMES+TAS is in fact redundant.

I will change the GHA to build the ta-lmes-job in a separate PR.

openshift-ci[bot] commented 2 months ago

New changes are detected. LGTM label has been removed.

zdtsw commented 2 months ago

i am good with this PR, looks neat and only small changes required from Platform when enable this feature.