adding unit tests for operator deployable

Author: enoodle

pkg/operator/operands/deployable/deployable.go

@@ -66,15 +66,15 @@ func (d *DeployableOperands) Deploy(
 		Controller: ptr.To(true),
 	}
 
-	if createObjectsInCluster(ctx, runtimeClient, reconcilerAsOwnerReference, objectsToCreate) != nil {
+	if err := createObjectsInCluster(ctx, runtimeClient, reconcilerAsOwnerReference, objectsToCreate); err != nil {
 		return err
 	}
 
-	if deleteObjectsInCluster(ctx, runtimeClient, objectsToDelete) != nil {
+	if err := deleteObjectsInCluster(ctx, runtimeClient, objectsToDelete); err != nil {
 		return err
 	}
 
-	if updateObjectsInCluster(ctx, runtimeClient, reconcilerAsOwnerReference, objectsToUpdate) != nil {
+	if err := updateObjectsInCluster(ctx, runtimeClient, reconcilerAsOwnerReference, objectsToUpdate); err != nil {
 		return err
 	}
 	return nil

pkg/operator/operands/deployable/deployable_test.go

@@ -5,6 +5,7 @@ package deployable
 
 import (
 	"context"
+	"errors"
 	"testing"
 
 	kaiv1 "github.com/NVIDIA/KAI-scheduler/pkg/apis/kai/v1"
@@ -24,6 +25,7 @@ import (
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
 )
 
 func TestDeployable(t *testing.T) {
@@ -62,92 +64,154 @@ var _ = Describe("Deployable", func() {
 			deployable *DeployableOperands
 			fakeClient client.Client
 		)
-		BeforeEach(func() {
-			operand := &fakeOperand{}
+		Context("object creation successfull", func() {
 
-			deployable = New([]operands.Operand{operand}, known_types.KAIConfigRegisteredCollectible)
-			fakeClient = getFakeClient(fakeClientBuilder, known_types.KAIConfigRegisteredCollectible)
-		})
-		It("should deploy operands desired state", func() {
-			Expect(deployable.Deploy(context.TODO(), fakeClient, kaiConfig, kaiConfig)).To(Succeed())
-			podsList := &v1.PodList{}
-			err := fakeClient.List(context.TODO(), podsList)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(len(podsList.Items)).To(Equal(1))
+			BeforeEach(func() {
+				operand := &fakeOperand{}
 
-			configMapList := &v1.ConfigMapList{}
-			err = fakeClient.List(context.TODO(), configMapList)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(len(configMapList.Items)).To(Equal(1))
-		})
-		It("should not deploy operands on custom field inheritor", func() {
-			desiredWithAnnotation := &v1.ConfigMap{
-				TypeMeta: metav1.TypeMeta{
-					Kind:       "ConfigMap",
-					APIVersion: "v1",
-				},
-				ObjectMeta: metav1.ObjectMeta{
-					Name:        "foo",
-					Namespace:   "bar",
-					Annotations: map[string]string{"A": "a"},
-					OwnerReferences: []metav1.OwnerReference{
-						{
-							APIVersion: kaiConfig.GetObjectKind().GroupVersionKind().GroupVersion().String(),
-							Kind:       kaiConfig.GetObjectKind().GroupVersionKind().Kind,
-							Name:       kaiConfig.GetName(),
-							UID:        kaiConfig.GetUID(),
-							Controller: ptr.To(true),
+				deployable = New([]operands.Operand{operand}, known_types.KAIConfigRegisteredCollectible)
+				fakeClient = getFakeClient(fakeClientBuilder, known_types.KAIConfigRegisteredCollectible)
+			})
+			It("should deploy operands desired state", func() {
+				Expect(deployable.Deploy(context.TODO(), fakeClient, kaiConfig, kaiConfig)).To(Succeed())
+				podsList := &v1.PodList{}
+				err := fakeClient.List(context.TODO(), podsList)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(len(podsList.Items)).To(Equal(1))
+
+				configMapList := &v1.ConfigMapList{}
+				err = fakeClient.List(context.TODO(), configMapList)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(len(configMapList.Items)).To(Equal(1))
+			})
+			It("should not deploy operands on custom field inheritor", func() {
+				desiredWithAnnotation := &v1.ConfigMap{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "ConfigMap",
+						APIVersion: "v1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:        "foo",
+						Namespace:   "bar",
+						Annotations: map[string]string{"A": "a"},
+						OwnerReferences: []metav1.OwnerReference{
+							{
+								APIVersion: kaiConfig.GetObjectKind().GroupVersionKind().GroupVersion().String(),
+								Kind:       kaiConfig.GetObjectKind().GroupVersionKind().Kind,
+								Name:       kaiConfig.GetName(),
+								UID:        kaiConfig.GetUID(),
+								Controller: ptr.To(true),
+							},
 						},
 					},
-				},
-			}
-			Expect(fakeClient.Create(context.TODO(), desiredWithAnnotation)).To(Succeed())
-
-			deployable.RegisterFieldsInheritFromClusterObjects(&v1.ConfigMap{}, func(current, desired client.Object) {
-				currentCm := current.(*v1.ConfigMap)
-				desiredCm := desired.(*v1.ConfigMap)
-				if desiredCm.Annotations == nil {
-					desiredCm.Annotations = map[string]string{}
 				}
-				maps.Copy(desiredCm.Annotations, currentCm.Annotations)
+				Expect(fakeClient.Create(context.TODO(), desiredWithAnnotation)).To(Succeed())
+
+				deployable.RegisterFieldsInheritFromClusterObjects(&v1.ConfigMap{}, func(current, desired client.Object) {
+					currentCm := current.(*v1.ConfigMap)
+					desiredCm := desired.(*v1.ConfigMap)
+					if desiredCm.Annotations == nil {
+						desiredCm.Annotations = map[string]string{}
+					}
+					maps.Copy(desiredCm.Annotations, currentCm.Annotations)
+				})
+				Expect(deployable.Deploy(context.TODO(), fakeClient, kaiConfig, kaiConfig)).To(Succeed())
+
+				cmList := &v1.ConfigMapList{}
+				err := fakeClient.List(context.TODO(), cmList)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(len(cmList.Items)).To(Equal(1))
+				Expect(len(cmList.Items[0].Annotations)).To(Equal(1))
 			})
-			Expect(deployable.Deploy(context.TODO(), fakeClient, kaiConfig, kaiConfig)).To(Succeed())
 
-			cmList := &v1.ConfigMapList{}
-			err := fakeClient.List(context.TODO(), cmList)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(len(cmList.Items)).To(Equal(1))
-			Expect(len(cmList.Items[0].Annotations)).To(Equal(1))
+			It("should delete other resources", func() {
+				otherConfigMap := &v1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "foo2",
+						Namespace: "bar2",
+						OwnerReferences: []metav1.OwnerReference{
+							{
+								APIVersion: kaiConfig.GetObjectKind().GroupVersionKind().GroupVersion().String(),
+								Kind:       kaiConfig.GetObjectKind().GroupVersionKind().Kind,
+								Name:       kaiConfig.GetName(),
+								UID:        kaiConfig.GetUID(),
+								Controller: ptr.To(true),
+							},
+						},
+					},
+				}
+				Expect(fakeClient.Create(context.TODO(), otherConfigMap)).To(Succeed())
+
+				Expect(deployable.Deploy(context.TODO(), fakeClient, kaiConfig, kaiConfig)).To(Succeed())
+
+				configMapList := &v1.ConfigMapList{}
+				err := fakeClient.List(context.TODO(), configMapList)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(len(configMapList.Items)).To(Equal(1))
+
+				for _, item := range configMapList.Items {
+					Expect(item.Name).ToNot(Equal(otherConfigMap.Name))
+				}
+			})
 		})
 
-		It("should delete other resources", func() {
-			otherConfigMap := &v1.ConfigMap{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "foo2",
-					Namespace: "bar2",
-					OwnerReferences: []metav1.OwnerReference{
-						{
-							APIVersion: kaiConfig.GetObjectKind().GroupVersionKind().GroupVersion().String(),
-							Kind:       kaiConfig.GetObjectKind().GroupVersionKind().Kind,
-							Name:       kaiConfig.GetName(),
-							UID:        kaiConfig.GetUID(),
-							Controller: ptr.To(true),
+		Context("Object creation fails", func() {
+			var (
+				fakeClient  client.Client
+				createCalls int
+				updateCalls int
+				createError error
+				updateError error
+			)
+			BeforeEach(func() {
+				createCalls = 0
+				updateCalls = 0
+				createError = nil
+				updateError = nil
+
+				operand := &fakeOperand{}
+				deployable = New([]operands.Operand{operand}, known_types.KAIConfigRegisteredCollectible)
+
+				fakeClient = getFakeClient(fakeClientBuilder.
+					WithInterceptorFuncs(interceptor.Funcs{
+						Create: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
+							createCalls++
+							return createError
 						},
-					},
-				},
-			}
-			Expect(fakeClient.Create(context.TODO(), otherConfigMap)).To(Succeed())
+						Update: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.UpdateOption) error {
+							updateCalls++
+							return updateError
+						},
+					}), known_types.KAIConfigRegisteredCollectible)
+			})
 
-			Expect(deployable.Deploy(context.TODO(), fakeClient, kaiConfig, kaiConfig)).To(Succeed())
+			It("should create object successfully", func() {
+				Expect(deployable.Deploy(context.TODO(), fakeClient, kaiConfig, kaiConfig)).To(Succeed())
+				Expect(createCalls).To(Equal(2)) // Pod and ConfigMap creates
+				Expect(updateCalls).To(Equal(0))
+			})
 
-			configMapList := &v1.ConfigMapList{}
-			err := fakeClient.List(context.TODO(), configMapList)
-			Expect(err).ToNot(HaveOccurred())
-			Expect(len(configMapList.Items)).To(Equal(1))
+			It("should update object if create fails due to existing resource", func() {
+				createError = errors.New("already exists")
+				defer func() { createError = nil }()
+				Expect(deployable.Deploy(context.TODO(), fakeClient, kaiConfig, kaiConfig)).To(Succeed())
+				Expect(createCalls).To(Equal(2))
+				Expect(updateCalls).To(Equal(2))
+			})
 
-			for _, item := range configMapList.Items {
-				Expect(item.Name).ToNot(Equal(otherConfigMap.Name))
-			}
+			It("should fail if both create and update fail", func() {
+				createError = errors.New("already exists")
+				updateError = errors.New("update failed")
+				defer func() {
+					createError = nil
+					updateError = nil
+				}()
+
+				err := deployable.Deploy(context.TODO(), fakeClient, kaiConfig, kaiConfig)
+				Expect(err).To(HaveOccurred())
+				Expect(createCalls).To(Equal(1))
+				Expect(updateCalls).To(Equal(1))
+			})
 		})
 	})
 
@@ -198,6 +262,64 @@ var _ = Describe("Deployable", func() {
 		})
 	})
 
+	Describe("Monitor", func() {
+		var (
+			deployable *DeployableOperands
+			fakeClient client.Client
+		)
+		BeforeEach(func() {
+			operand := &fakeOperand{}
+			deployable = New([]operands.Operand{operand}, known_types.KAIConfigRegisteredCollectible)
+			fakeClient = getFakeClient(fakeClientBuilder, known_types.KAIConfigRegisteredCollectible)
+		})
+
+		It("should not return error when monitoring", func() {
+			Expect(deployable.Monitor(context.TODO(), fakeClient, kaiConfig)).To(Succeed())
+		})
+
+		It("should return error if operand's Monitor fails", func() {
+			errOperand := &fakeOperandWithMonitorError{monitorErr: errors.New("monitor failed")}
+			deployable = New([]operands.Operand{errOperand}, known_types.KAIConfigRegisteredCollectible)
+			Expect(deployable.Monitor(context.TODO(), fakeClient, kaiConfig)).To(MatchError(ContainSubstring("monitor failed")))
+			Expect(errOperand.monitorErr).To(HaveOccurred())
+		})
+	})
+
+	Describe("HasMissingDependencies", func() {
+		var (
+			deployable *DeployableOperands
+			fakeClient client.Reader
+		)
+		BeforeEach(func() {
+			operand := &fakeOperand{}
+			deployable = New([]operands.Operand{operand}, known_types.KAIConfigRegisteredCollectible)
+			fakeClient = getFakeClient(fakeClientBuilder, known_types.KAIConfigRegisteredCollectible)
+		})
+
+		It("should return no missing dependencies if all operands report none", func() {
+			missing, err := deployable.HasMissingDependencies(context.TODO(), fakeClient, kaiConfig)
+			Expect(missing).To(BeEmpty())
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		It("should aggregate missing dependencies from operands", func() {
+			operand1 := &fakeOperandWithDeps{missingDeps: "dep1", name: "operand1"}
+			operand2 := &fakeOperandWithDeps{missingDeps: "dep2", name: "operand2"}
+			deployable = New([]operands.Operand{operand1, operand2}, known_types.KAIConfigRegisteredCollectible)
+			missing, err := deployable.HasMissingDependencies(context.TODO(), fakeClient, kaiConfig)
+			Expect(missing).To(ContainSubstring("operand1 is missing dep1"))
+			Expect(missing).To(ContainSubstring("operand2 is missing dep2"))
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		It("should handle error from operand's HasMissingDependencies", func() {
+			errOperand := &fakeOperandWithDeps{hasDepErr: errors.New("dependency check failed"), name: "errOperand"}
+			deployable = New([]operands.Operand{errOperand}, known_types.KAIConfigRegisteredCollectible)
+			_, err := deployable.HasMissingDependencies(context.TODO(), fakeClient, kaiConfig)
+			Expect(err).To(MatchError(ContainSubstring("dependency check failed")))
+		})
+	})
+
 	Describe("SortObjectByCreationOrder", func() {
 		var (
 			orderDefinition []string
@@ -375,3 +497,48 @@ func getFakeClient(builder *fake.ClientBuilder, collectables []*known_types.Coll
 	}
 	return builder.Build()
 }
+
+type fakeOperandWithMonitorError struct {
+	monitorErr error
+}
+
+func (f *fakeOperandWithMonitorError) Monitor(_ context.Context, _ client.Reader, _ *kaiv1.Config) error {
+	return f.monitorErr
+}
+
+func (f *fakeOperandWithMonitorError) Name() string { return "fakeOperandWithMonitorError" }
+func (f *fakeOperandWithMonitorError) DesiredState(_ context.Context, _ client.Reader, _ *kaiv1.Config) ([]client.Object, error) {
+	return nil, nil
+}
+func (f *fakeOperandWithMonitorError) IsDeployed(_ context.Context, _ client.Reader) (bool, error) {
+	return true, nil
+}
+func (f *fakeOperandWithMonitorError) IsAvailable(_ context.Context, _ client.Reader) (bool, error) {
+	return true, nil
+}
+func (f *fakeOperandWithMonitorError) HasMissingDependencies(context.Context, client.Reader, *kaiv1.Config) (string, error) {
+	return "", nil
+}
+
+type fakeOperandWithDeps struct {
+	missingDeps string
+	hasDepErr   error
+	name        string
+}
+
+func (f *fakeOperandWithDeps) HasMissingDependencies(_ context.Context, _ client.Reader, _ *kaiv1.Config) (string, error) {
+	return f.missingDeps, f.hasDepErr
+}
+func (f *fakeOperandWithDeps) Name() string { return f.name }
+func (f *fakeOperandWithDeps) DesiredState(_ context.Context, _ client.Reader, _ *kaiv1.Config) ([]client.Object, error) {
+	return nil, nil
+}
+func (f *fakeOperandWithDeps) IsDeployed(_ context.Context, _ client.Reader) (bool, error) {
+	return true, nil
+}
+func (f *fakeOperandWithDeps) IsAvailable(_ context.Context, _ client.Reader) (bool, error) {
+	return true, nil
+}
+func (f *fakeOperandWithDeps) Monitor(_ context.Context, _ client.Reader, _ *kaiv1.Config) error {
+	return nil
+}