Add new conditions to Playground #120
Conversation
InftyAI-Agent
added
needs-triage
kerthcet
force-pushed
the
cleanup/add-conditions
branch
from
ef070d6 to
6e9b3a8
Compare
Signed-off-by: kerthcet <kerthcet@gmail.com>
kerthcet
force-pushed
the
cleanup/add-conditions
branch
from
6e9b3a8 to
2269c16
Compare
|
/kind cleanup leave LGTM to @carlory |
InftyAI-Agent
added
cleanup
|
|
||
| // Set the available to false | ||
| new_condition := metav1.Condition{ | ||
| Type: inferenceapi.PlaygroundAvailable, | ||
| Status: metav1.ConditionFalse, | ||
| Reason: "PlaygroundNotReady", |
There was a problem hiding this comment.
PlaygroundAvailable=false implies the reason, so ServiceNotAvailable maybe better?
There was a problem hiding this comment.
Added the details to the message: Waiting for inference Service ready.
| validation.ValidatePlayground(ctx, k8sClient, playground) | ||
| validation.ValidatePlaygroundStatusEqualTo(ctx, k8sClient, playground, inferenceapi.PlaygroundProgressing, "Pending", metav1.ConditionTrue) | ||
| }, | ||
| }, | ||
| { |
There was a problem hiding this comment.
I'm not sure whether the test in playground should update the service instead, I know that the service status will be updated via the lws by service controller.
The current intergation setup all controllers. It may be unreasonable. One side effect is that updating the service is impossible because the service status will be updated by the service controller. we have to update the lws object.
In kubernetes, the intergation tests for a given controller, other controllers will not be enabled.
There was a problem hiding this comment.
Well, Playground integrates with Service closely, where I believe integration tests should join here. In Kubernetes, different controllers didn't work with each other, so there's no need to put them together.
Signed-off-by: kerthcet <kerthcet@gmail.com>
|
|
||
| // Set the available to false | ||
| new_condition := metav1.Condition{ | ||
| Type: inferenceapi.PlaygroundAvailable, | ||
| Status: metav1.ConditionFalse, | ||
| Reason: "PlaygroundNotReady", |
There was a problem hiding this comment.
Added the details to the message: Waiting for inference Service ready.
| validation.ValidatePlayground(ctx, k8sClient, playground) | ||
| validation.ValidatePlaygroundStatusEqualTo(ctx, k8sClient, playground, inferenceapi.PlaygroundProgressing, "Pending", metav1.ConditionTrue) | ||
| }, | ||
| }, | ||
| { |
There was a problem hiding this comment.
Well, Playground integrates with Service closely, where I believe integration tests should join here. In Kubernetes, different controllers didn't work with each other, so there's no need to put them together.
|
/lgtm |
InftyAI-Agent
added
the
lgtm
What this PR does / why we need it
For better observation and higher test coverage.
Which issue(s) this PR fixes
Fixes #117 #113
Special notes for your reviewer
Does this PR introduce a user-facing change?