Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions pkg/controller/inference/playground_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/log"
Expand Down Expand Up @@ -81,6 +82,24 @@ func (r *PlaygroundReconciler) Reconcile(ctx context.Context, req ctrl.Request)

logger.V(10).Info("reconcile Playground", "Playground", klog.KObj(playground))

service := &inferenceapi.Service{}
if err := r.Get(ctx, types.NamespacedName{Name: playground.Name, Namespace: playground.Namespace}, service); err == nil {
if !controllerutil.HasControllerReference(service) {
condition := metav1.Condition{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should collect all the condition handlers to one place just like setPlaygroundCondition but this can be a follow up when implementing the Pending status for waiting for model creation.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ok

Type: inferenceapi.PlaygroundProgressing,
Status: metav1.ConditionFalse,
Reason: "AbortProcessing",
Message: "Playground owns the same name with an existing Service",
}
apimeta.SetStatusCondition(&playground.Status.Conditions, condition)
if err := r.Client.Status().Update(ctx, playground); err != nil {
return ctrl.Result{}, err
}
// No need to requeue. Once the Service is deleted, the Playground will be reconciled.
return ctrl.Result{}, nil
}
}

var serviceApplyConfiguration *inferenceclientgo.ServiceApplyConfiguration

model := &coreapi.OpenModel{}
Expand All @@ -94,7 +113,6 @@ func (r *PlaygroundReconciler) Reconcile(ctx context.Context, req ctrl.Request)
}

// TODO: handle MultiModelsClaims in the future.

if err := setControllerReferenceForService(playground, serviceApplyConfiguration, r.Scheme); err != nil {
return ctrl.Result{}, err
}
Expand All @@ -104,7 +122,7 @@ func (r *PlaygroundReconciler) Reconcile(ctx context.Context, req ctrl.Request)
}

// Handle status.
service := &inferenceapi.Service{}
service = &inferenceapi.Service{}
if err := r.Get(ctx, types.NamespacedName{Name: playground.Name, Namespace: playground.Namespace}, service); err != nil {
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -272,7 +290,7 @@ func buildWorkerTemplate(model *coreapi.OpenModel, playground *inferenceapi.Play

func setPlaygroundCondition(playground *inferenceapi.Playground, service *inferenceapi.Service) {
// For the start up.
if len(playground.Status.Conditions) == 0 {
if len(playground.Status.Conditions) == 0 || !apimeta.IsStatusConditionTrue(service.Status.Conditions, inferenceapi.PlaygroundProgressing) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should also be refactored once introducing the pending for model creation condition. Because not progressing doesn't mean it's waiting for inferenceService ready.

condition := metav1.Condition{
Type: inferenceapi.PlaygroundProgressing,
Status: metav1.ConditionTrue,
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/inference/service_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
func (r *ServiceReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&inferenceapi.Service{}).
Watches(&lws.LeaderWorkerSet{}, &handler.EnqueueRequestForObject{},
Watches(&lws.LeaderWorkerSet{}, handler.EnqueueRequestForOwner(r.Scheme, r.RESTMapper(), &inferenceapi.Service{}, handler.OnlyControllerOwner()),
builder.WithPredicates(predicate.Funcs{
UpdateFunc: func(e event.UpdateEvent) bool {
oldBar := e.ObjectOld.(*lws.LeaderWorkerSet)
Expand Down
35 changes: 35 additions & 0 deletions test/integration/controller/inference/playground_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,41 @@ var _ = ginkgo.Describe("playground controller test", func() {
},
},
}),
ginkgo.Entry("playground is created when service exists with the same name", &testValidatingCase{
makePlayground: func() *inferenceapi.Playground {
return util.MockASamplePlayground(ns.Name)
},
updates: []*update{
{
playgroundUpdateFn: func(playground *inferenceapi.Playground) {
// Create a service with the same name as the playground.
service := wrapper.MakeService(playground.Name, playground.Namespace).
ModelsClaim([]string{"llama3-8b"}, []string{}, nil).
WorkerTemplate().
Obj()
gomega.Expect(k8sClient.Create(ctx, service)).To(gomega.Succeed())
gomega.Expect(k8sClient.Create(ctx, playground)).To(gomega.Succeed())
},
checkPlayground: func(ctx context.Context, k8sClient client.Client, playground *inferenceapi.Playground) {
validation.ValidatePlaygroundStatusEqualTo(ctx, k8sClient, playground, inferenceapi.PlaygroundProgressing, "AbortProcessing", metav1.ConditionFalse)
},
},
{
// Delete the service, playground should be updated to Pending.
playgroundUpdateFn: func(playground *inferenceapi.Playground) {
service := wrapper.MakeService(playground.Name, playground.Namespace).
ModelsClaim([]string{"llama3-8b"}, []string{}, nil).
WorkerTemplate().
Obj()
gomega.Expect(k8sClient.Delete(ctx, service)).To(gomega.Succeed())
},
checkPlayground: func(ctx context.Context, k8sClient client.Client, playground *inferenceapi.Playground) {
validation.ValidatePlayground(ctx, k8sClient, playground)
validation.ValidatePlaygroundStatusEqualTo(ctx, k8sClient, playground, inferenceapi.PlaygroundProgressing, "Pending", metav1.ConditionTrue)
},
},
},
}),
ginkgo.Entry("create the model after playground is created", &testValidatingCase{
makePlayground: func() *inferenceapi.Playground {
return util.MockASamplePlayground(ns.Name)
Expand Down
82 changes: 82 additions & 0 deletions test/util/format/format.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
/*
Copyright 2022 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

// Copy from https://github.com/kubernetes/kubernetes/blob/master/test/utils/format/format.go

// Package format is an extension of Gomega's format package which
// improves printing of objects that can be serialized well as YAML,
// like the structs in the Kubernetes API.
//
// Just importing it is enough to activate this special YAML support
// in Gomega.
package format

import (
"reflect"
"strings"

"github.com/onsi/gomega/format"

"sigs.k8s.io/yaml"
)

func init() {
format.RegisterCustomFormatter(handleYAML)
}

// Object makes Gomega's [format.Object] available without having to import that
// package.
func Object(object interface{}, indentation uint) string {
return format.Object(object, indentation)
}

// handleYAML formats all values as YAML where the result
// is likely to look better as YAML:
// - pointer to struct or struct where all fields
// have `json` tags
// - slices containing such a value
// - maps where the key or value are such a value
func handleYAML(object interface{}) (string, bool) {
value := reflect.ValueOf(object)
if !useYAML(value.Type()) {
return "", false
}
y, err := yaml.Marshal(object)
if err != nil {
return "", false
}
return "\n" + strings.TrimSpace(string(y)), true
}

func useYAML(t reflect.Type) bool {
switch t.Kind() {
case reflect.Pointer, reflect.Slice, reflect.Array:
return useYAML(t.Elem())
case reflect.Map:
return useYAML(t.Key()) || useYAML(t.Elem())
case reflect.Struct:
// All fields must have a `json` tag.
for i := 0; i < t.NumField(); i++ {
field := t.Field(i)
if _, ok := field.Tag.Lookup("json"); !ok {
return false
}
}
return true
default:
return false
}
}
5 changes: 3 additions & 2 deletions test/util/validation/validate_playground.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/inftyai/llmaz/pkg/controller_helper/backend"
modelSource "github.com/inftyai/llmaz/pkg/controller_helper/model_source"
"github.com/inftyai/llmaz/test/util"
"github.com/inftyai/llmaz/test/util/format"
)

func ValidatePlayground(ctx context.Context, k8sClient client.Client, playground *inferenceapi.Playground) {
Expand Down Expand Up @@ -136,10 +137,10 @@ func ValidatePlaygroundStatusEqualTo(ctx context.Context, k8sClient client.Clien
return err
}
if condition := apimeta.FindStatusCondition(newPlayground.Status.Conditions, conditionType); condition == nil {
return errors.New("condition not found")
return fmt.Errorf("condition not found: %s", format.Object(newPlayground, 1))
} else {
if condition.Reason != reason || condition.Status != status {
return errors.New("reason or status not right")
return fmt.Errorf("expected reason %q or status %q, but got %s", reason, status, format.Object(condition, 1))
}
}
return nil
Expand Down