Skip to content

Conversation

@carlory
Copy link
Member

@carlory carlory commented Aug 28, 2024

What this PR does / why we need it

Let's image there's no Model in the cluster, I created a Playground, because no Model exists, inference Pods will not be created. However, if I created the corresponding Model, the Playground should be triggered to create Services and then Pods. Right now, this is not true.

The solution is quite simple, make Playground watch for model creation events.

Which issue(s) this PR fixes

Fixes #92

Special notes for your reviewer

Does this PR introduce a user-facing change?

Fix playground should be triggered to create Services and then Pods once the model is created

@InftyAI-Agent InftyAI-Agent added needs-triage Indicates an issue or PR lacks a label and requires one. needs-priority Indicates a PR lacks a label and requires one. do-not-merge/needs-kind Indicates a PR lacks a label and requires one. labels Aug 28, 2024
@carlory
Copy link
Member Author

carlory commented Aug 28, 2024

/cc @kerthcet

@carlory
Copy link
Member Author

carlory commented Aug 28, 2024

/kind bug

@InftyAI-Agent InftyAI-Agent requested a review from kerthcet August 28, 2024 08:32
@InftyAI-Agent InftyAI-Agent added bug Categorizes issue or PR as related to a bug. and removed do-not-merge/needs-kind Indicates a PR lacks a label and requires one. labels Aug 28, 2024
@carlory carlory force-pushed the PlaygroundReconciler branch 2 times, most recently from d265621 to 95258d1 Compare August 28, 2024 10:54
},
checkPlayground: func(ctx context.Context, k8sClient client.Client, playground *inferenceapi.Playground) {
validation.ValidatePlayground(ctx, k8sClient, playground)
gomega.Consistently(func() error {
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if no condition is expected.

Copy link
Member

Choose a reason for hiding this comment

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

Yes for now, but this can be optimized like pending for model creation in the follow up if you're interested or together with this PR, whatever you like.

So when there's no condition with Playground or model is not created, Playground is Pending, but with different reasons, one is Waiting for inferenceService ready, another is Waiting for model to be created.

Copy link
Member Author

Choose a reason for hiding this comment

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

n the follow up if you're interested or together with this PR, whatever you like.

I will do it in another PR.

@carlory carlory force-pushed the PlaygroundReconciler branch from 95258d1 to 61c31f6 Compare August 28, 2024 11:17
},
checkPlayground: func(ctx context.Context, k8sClient client.Client, playground *inferenceapi.Playground) {
validation.ValidatePlayground(ctx, k8sClient, playground)
gomega.Consistently(func() error {
Copy link
Member

Choose a reason for hiding this comment

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

Yes for now, but this can be optimized like pending for model creation in the follow up if you're interested or together with this PR, whatever you like.

So when there's no condition with Playground or model is not created, Playground is Pending, but with different reasons, one is Waiting for inferenceService ready, another is Waiting for model to be created.

@carlory carlory force-pushed the PlaygroundReconciler branch from 61c31f6 to ad9973f Compare August 29, 2024 11:10
Copy link
Member

@kerthcet kerthcet left a comment

Choose a reason for hiding this comment

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

Generally LGTM

@carlory carlory force-pushed the PlaygroundReconciler branch from ad9973f to ba653c6 Compare August 29, 2024 12:51
@kerthcet
Copy link
Member

/lgtm
/approve

Thanks!

@InftyAI-Agent InftyAI-Agent added lgtm Looks good to me, indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 29, 2024
@InftyAI-Agent InftyAI-Agent merged commit 6031da5 into InftyAI:main Aug 29, 2024
@carlory carlory deleted the PlaygroundReconciler branch August 30, 2024 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bug Categorizes issue or PR as related to a bug. lgtm Looks good to me, indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a label and requires one. needs-triage Indicates an issue or PR lacks a label and requires one.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Playground will not reconcile once model created

3 participants