Improve model adapter reconcile workflow stability (#260)

* Remove self maintained pointer utils

* Refactor status update methods

* Improve model adapter stability

api/model/v1alpha1/modeladapter_types.go

@@ -64,17 +64,21 @@ type ModelAdapterPhase string
 const (
 	// ModelAdapterPending means the CR has been created and that's the initial status
 	ModelAdapterPending ModelAdapterPhase = "Pending"
-	// ModelAdapterScheduling means the ModelAdapter is pending scheduling
-	ModelAdapterScheduling ModelAdapterPhase = "Scheduling"
-	// ModelAdapterBinding means the controller loads ModelAdapter on a selected pod
-	ModelAdapterBinding ModelAdapterPhase = "Binding"
+	// ModelAdapterScheduled means the ModelAdapter is pending scheduling
+	ModelAdapterScheduled ModelAdapterPhase = "Scheduled"
+	// ModelAdapterBound means the controller loads ModelAdapter on a selected pod
+	ModelAdapterBound ModelAdapterPhase = "Bound"
+	// ModelAdapterResourceCreated means the model adapter owned resources have been created
+	ModelAdapterResourceCreated ModelAdapterPhase = "ResourceCreated"
 	// ModelAdapterRunning means ModelAdapter has been running on the pod
 	ModelAdapterRunning ModelAdapterPhase = "Running"
 	// ModelAdapterFailed means ModelAdapter has terminated in a failure
 	ModelAdapterFailed ModelAdapterPhase = "Failed"
-	// ModelAdapterScaling means ModelAdapter is scaling, could be scaling in or out. won't be enabled until we allow multiple replicas
+	// ModelAdapterUnknown means ModelAdapter clean up some stable resources
+	ModelAdapterUnknown ModelAdapterPhase = "Unknown"
+	// ModelAdapterScaled means ModelAdapter is scaled, could be scaling in or out. won't be enabled until we allow multiple replicas
 	// TODO: not implemented yet.
-	ModelAdapterScaling ModelAdapterPhase = "Scaling"
+	ModelAdapterScaled ModelAdapterPhase = "Scaled"
 )
 
 // ModelAdapterStatus defines the observed state of ModelAdapter
@@ -97,11 +101,10 @@ type ModelAdapterConditionType string
 
 const (
 	ModelAdapterConditionTypeInitialized     ModelAdapterConditionType = "Initialized"
-	ModelAdapterConditionTypeSelectorMatched ModelAdapterConditionType = "SelectorMatched"
 	ModelAdapterConditionTypeScheduled       ModelAdapterConditionType = "Scheduled"
+	ModelAdapterConditionTypeBound           ModelAdapterConditionType = "Bound"
 	ModelAdapterConditionTypeResourceCreated ModelAdapterConditionType = "ResourceCreated"
 	ModelAdapterConditionReady               ModelAdapterConditionType = "Ready"
-	ModelAdapterConditionCleanup             ModelAdapterConditionType = "Cleanup"
 )
 
 // +genclient

pkg/controller/modeladapter/resources.go

@@ -22,9 +22,10 @@ import (
 	discoveryv1 "k8s.io/api/discovery/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	"k8s.io/utils/ptr"
 )
 
-func buildModelAdapterEndpointSlice(instance *modelv1alpha1.ModelAdapter, pod *corev1.Pod) (*discoveryv1.EndpointSlice, error) {
+func buildModelAdapterEndpointSlice(instance *modelv1alpha1.ModelAdapter, pod *corev1.Pod) *discoveryv1.EndpointSlice {
 	serviceLabels := map[string]string{
 		"kubernetes.io/service-name": instance.Name,
 	}
@@ -37,9 +38,9 @@ func buildModelAdapterEndpointSlice(instance *modelv1alpha1.ModelAdapter, pod *c
 
 	ports := []discoveryv1.EndpointPort{
 		{
-			Name:     stringPtr("http"),
-			Protocol: protocolPtr(corev1.ProtocolTCP),
-			Port:     int32Ptr(8000),
+			Name:     ptr.To("http"),
+			Protocol: ptr.To(corev1.ProtocolTCP),
+			Port:     ptr.To(int32(8000)),
 		},
 	}
 
@@ -56,10 +57,10 @@ func buildModelAdapterEndpointSlice(instance *modelv1alpha1.ModelAdapter, pod *c
 		AddressType: discoveryv1.AddressTypeIPv4,
 		Endpoints:   addresses,
 		Ports:       ports,
-	}, nil
+	}
 }
 
-func buildModelAdapterService(instance *modelv1alpha1.ModelAdapter) (*corev1.Service, error) {
+func buildModelAdapterService(instance *modelv1alpha1.ModelAdapter) *corev1.Service {
 	labels := map[string]string{
 		"model.aibrix.ai/name":         instance.Spec.BaseModel,
 		"adapter.model.aibrix.ai/name": instance.Name,
@@ -94,5 +95,5 @@ func buildModelAdapterService(instance *modelv1alpha1.ModelAdapter) (*corev1.Ser
 			PublishNotReadyAddresses: true,
 			Ports:                    ports,
 		},
-	}, nil
+	}
 }

pkg/controller/modeladapter/resources_test.go

@@ -43,10 +43,7 @@ func TestBuildModelAdapterEndpointSlice(t *testing.T) {
 	}
 
 	// Call the function to test
-	endpointSlice, err := buildModelAdapterEndpointSlice(instance, pod)
-
-	// Assert no errors
-	assert.NoError(t, err)
+	endpointSlice := buildModelAdapterEndpointSlice(instance, pod)
 
 	// Check EndpointSlice metadata
 	assert.Equal(t, "test-instance", endpointSlice.Name)
@@ -83,10 +80,7 @@ func TestBuildModelAdapterService(t *testing.T) {
 	}
 
 	// Call the function to test
-	service, err := buildModelAdapterService(instance)
-
-	// Assert no errors
-	assert.NoError(t, err)
+	service := buildModelAdapterService(instance)
 
 	// Check Service metadata
 	assert.Equal(t, "test-instance", service.Name)

pkg/controller/modeladapter/scheduling/leastadapters.go

@@ -50,6 +50,6 @@ func (r leastAdapters) SelectPod(ctx context.Context, pods []v1.Pod) (*v1.Pod, e
 		}
 	}
 
-	klog.Infof("pod selected with least model adapters: %s", selectedPod.Name)
+	klog.InfoS("pod selected with least model adapters", "pod", klog.KObj(&selectedPod))
 	return &selectedPod, nil
 }

pkg/controller/modeladapter/utils.go

@@ -23,27 +23,10 @@ import (
 	"os"
 	"strings"
 
-	corev1 "k8s.io/api/core/v1"
-
 	modelv1alpha1 "github.com/aibrix/aibrix/api/model/v1alpha1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func stringPtr(s string) *string {
-	return &s
-}
-
-func protocolPtr(p corev1.Protocol) *corev1.Protocol {
-	return &p
-}
-
-func int32Ptr(i int32) *int32 {
-	return &i
-}
-
-func mapPtr(m map[string]string) *map[string]string {
-	return &m
-}
-
 func validateModelAdapter(instance *modelv1alpha1.ModelAdapter) error {
 	if instance.Spec.ArtifactURL == "" {
 		return fmt.Errorf("artifactURL is required")
@@ -128,11 +111,33 @@ func extractHuggingFacePath(artifactURL string) (string, error) {
 	return path, nil
 }
 
-func stringInSlice(slice []string, str string) bool {
+func StringInSlice(slice []string, str string) bool {
 	for _, v := range slice {
 		if v == str {
 			return true
 		}
 	}
 	return false
 }
+
+// RemoveInstanceFromList removes a string from a slice of strings
+func RemoveInstanceFromList(slice []string, strToRemove string) []string {
+	var result []string
+	for _, s := range slice {
+		if s != strToRemove {
+			result = append(result, s)
+		}
+	}
+	return result
+}
+
+// NewCondition creates a new condition.
+func NewCondition(condType string, status metav1.ConditionStatus, reason, msg string) metav1.Condition {
+	return metav1.Condition{
+		Type:               condType,
+		Status:             status,
+		LastTransitionTime: metav1.Now(),
+		Reason:             reason,
+		Message:            msg,
+	}
+}

pkg/controller/modeladapter/utils_test.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
 
 	modelv1alpha1 "github.com/aibrix/aibrix/api/model/v1alpha1"
 )
@@ -36,7 +37,7 @@ func TestValidateModelAdapter(t *testing.T) {
 				PodSelector: &metav1.LabelSelector{
 					MatchLabels: map[string]string{"app": "test"},
 				},
-				Replicas: int32Ptr(1),
+				Replicas: ptr.To(int32(1)),
 			},
 		}