diff --git a/api/v1alpha1/appdeployment_types.go b/api/v1alpha1/appdeployment_types.go index e17f9d4..b32be59 100644 --- a/api/v1alpha1/appdeployment_types.go +++ b/api/v1alpha1/appdeployment_types.go @@ -21,6 +21,20 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + AppDeploymentOwnerKey = ".appDeployment.metadata.controller" + + AppDeploymentFinalizerName = "finalizer.appdeployment.devinfra.goms.io" + + // phase types + AppDeploymentPhaseEmpty = "" + AppDeploymentPhasePending = "Pending" + AppDeploymentPhaseDeploying = "Deploying" + AppDeploymentPhaseReady = "Ready" + AppDeploymentPhaseDeleting = "Deleting" + AppDeploymentPhaseDeleted = "Deleted" +) + // EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. diff --git a/api/v1alpha1/cache_types.go b/api/v1alpha1/cache_types.go index 10fa978..fb85017 100644 --- a/api/v1alpha1/cache_types.go +++ b/api/v1alpha1/cache_types.go @@ -20,6 +20,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + CacheOwnerKey = ".metadata.controller.cache" +) + // EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. diff --git a/api/v1alpha1/operation_types.go b/api/v1alpha1/operation_types.go index ba849ef..2788ef2 100644 --- a/api/v1alpha1/operation_types.go +++ b/api/v1alpha1/operation_types.go @@ -21,6 +21,19 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + OperationOwnerKey = ".operation.metadata.controller" + + OperationFinalizerName = "finalizer.operation.controller.azure.com" + OperationAcquiredAnnotationKey = "operation.controller.azure.com/acquired" + + OperationPhaseEmpty = "" + OperationPhaseReconciling = "Reconciling" + OperationPhaseReconciled = "Reconciled" + OperationPhaseDeleting = "Deleting" + OperationPhaseDeleted = "Deleted" +) + // EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. diff --git a/api/v1alpha1/requirement_types.go b/api/v1alpha1/requirement_types.go index 1fd6465..c3854a9 100644 --- a/api/v1alpha1/requirement_types.go +++ b/api/v1alpha1/requirement_types.go @@ -20,6 +20,30 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + RequirementOwnerKey = ".requirement.metadata.controller" + + RequirementFinalizerName = "finalizer.requirement.devinfra.goms.io" + + RequirementConditionRequirementInitialized = "RequirementInitialized" + RequirementConditionCacheResourceFound = "CacheCRFound" + RequirementConditionCachedOperationAcquired = "CachedOpAcquired" + RequirementConditionOperationReady = "OperationReady" + + RequirementConditionReasonNoOperationAvailable = "NoOperationAvailable" + RequirementConditionReasonCacheCRNotFound = "CacheCRNotFound" + RequirementConditionReasonCacheCRFound = "CacheCRFound" + RequirementConditionReasonCacheHit = "CacheHit" + RequirementConditionReasonCacheMiss = "CacheMiss" + + RequirementPhaseEmpty = "" + RequirementPhaseCacheChecking = "CacheChecking" + RequirementPhaseOperating = "Operating" + RequirementPhaseReady = "Ready" + RequirementPhaseDeleted = "Deleted" + RequirementPhaseDeleting = "Deleting" +) + // EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. diff --git a/internal/controller/appdeployment_controller.go b/internal/controller/appdeployment_controller.go index 0eb2361..66ae478 100644 --- a/internal/controller/appdeployment_controller.go +++ b/internal/controller/appdeployment_controller.go @@ -26,10 +26,11 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/log" + klog "sigs.k8s.io/controller-runtime/pkg/log" "github.com/Azure/operation-cache-controller/api/v1alpha1" - apdutil "github.com/Azure/operation-cache-controller/internal/utils/controller/appdeployment" + "github.com/Azure/operation-cache-controller/internal/handler" + "github.com/Azure/operation-cache-controller/internal/log" "github.com/Azure/operation-cache-controller/internal/utils/reconciler" ) @@ -57,23 +58,22 @@ type AppDeploymentReconciler struct { // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.20.4/pkg/reconcile func (r *AppDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - logger := log.FromContext(ctx).WithValues(apdutil.LogKeyAppDeploymentName, req.NamespacedName) + logger := klog.FromContext(ctx).WithValues(log.AppDeploymentJobName, req.NamespacedName) appdeployment := &v1alpha1.AppDeployment{} if err := r.Get(ctx, req.NamespacedName, appdeployment); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) } - adapter := NewAppDeploymentAdapter(ctx, appdeployment, logger, r.Client, r.recorder) - return r.ReconcileHandler(ctx, adapter) + return r.ReconcileHandler(ctx, handler.NewAppDeploymentHandler(ctx, appdeployment, logger, r.Client, r.recorder)) } -func (r *AppDeploymentReconciler) ReconcileHandler(ctx context.Context, adapter AppDeploymentAdapterInterface) (ctrl.Result, error) { +func (r *AppDeploymentReconciler) ReconcileHandler(ctx context.Context, h handler.AppDeploymentHandlerInterface) (ctrl.Result, error) { operations := []reconciler.ReconcileOperation{ - adapter.EnsureApplicationValid, - adapter.EnsureFinalizer, - adapter.EnsureFinalizerDeleted, - adapter.EnsureDependenciesReady, - adapter.EnsureDeployingFinished, - adapter.EnsureTeardownFinished, + h.EnsureApplicationValid, + h.EnsureFinalizer, + h.EnsureFinalizerDeleted, + h.EnsureDependenciesReady, + h.EnsureDeployingFinished, + h.EnsureTeardownFinished, } for _, operation := range operations { @@ -88,22 +88,21 @@ func (r *AppDeploymentReconciler) ReconcileHandler(ctx context.Context, adapter return ctrl.Result{}, nil } -var appDeploymentOwnerKey = ".appDeployment.metadata.controller" +func appDeploymentIndexerFunc(rawObj client.Object) []string { + job := rawObj.(*batchv1.Job) + owner := metav1.GetControllerOf(job) + if owner == nil { + return nil + } + if owner.APIVersion != v1alpha1.GroupVersion.String() || owner.Kind != "AppDeployment" { + return nil + } + return []string{owner.Name} +} // SetupWithManager sets up the controller with the Manager. func (r *AppDeploymentReconciler) SetupWithManager(mgr ctrl.Manager) error { - if err := mgr.GetFieldIndexer().IndexField(context.Background(), &batchv1.Job{}, appDeploymentOwnerKey, - func(rawObj client.Object) []string { - job := rawObj.(*batchv1.Job) - owner := metav1.GetControllerOf(job) - if owner == nil { - return nil - } - if owner.APIVersion != v1alpha1.GroupVersion.String() || owner.Kind != "AppDeployment" { - return nil - } - return []string{owner.Name} - }); err != nil { + if err := mgr.GetFieldIndexer().IndexField(context.Background(), &batchv1.Job{}, v1alpha1.AppDeploymentOwnerKey, appDeploymentIndexerFunc); err != nil { return err } diff --git a/internal/controller/appdeployment_controller_test.go b/internal/controller/appdeployment_controller_test.go index 6aa444d..710b686 100644 --- a/internal/controller/appdeployment_controller_test.go +++ b/internal/controller/appdeployment_controller_test.go @@ -33,8 +33,9 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/Azure/operation-cache-controller/api/v1alpha1" - ctrlmocks "github.com/Azure/operation-cache-controller/internal/controller/mocks" - mockpkg "github.com/Azure/operation-cache-controller/internal/mocks" + "github.com/Azure/operation-cache-controller/internal/handler" + hmocks "github.com/Azure/operation-cache-controller/internal/handler/mocks" + utilsmock "github.com/Azure/operation-cache-controller/internal/utils/mocks" "github.com/Azure/operation-cache-controller/internal/utils/reconciler" ) @@ -87,9 +88,9 @@ var _ = Describe("AppDeployment Controller", func() { const resourceName = "test-resource" var ( mockRecorderCtrl *gomock.Controller - mockRecorder *mockpkg.MockEventRecorder + mockRecorder *utilsmock.MockEventRecorder mockAdapterCtrl *gomock.Controller - mockAdapter *ctrlmocks.MockAppDeploymentAdapterInterface + mockAdapter *hmocks.MockAppDeploymentHandlerInterface ) ctx := context.Background() @@ -121,9 +122,9 @@ var _ = Describe("AppDeployment Controller", func() { Expect(k8sClient.Create(ctx, resource)).To(Succeed()) } mockRecorderCtrl = gomock.NewController(GinkgoT()) - mockRecorder = mockpkg.NewMockEventRecorder(mockRecorderCtrl) + mockRecorder = utilsmock.NewMockEventRecorder(mockRecorderCtrl) mockAdapterCtrl = gomock.NewController(GinkgoT()) - mockAdapter = ctrlmocks.NewMockAppDeploymentAdapterInterface(mockAdapterCtrl) + mockAdapter = hmocks.NewMockAppDeploymentHandlerInterface(mockAdapterCtrl) }) AfterEach(func() { @@ -142,7 +143,7 @@ var _ = Describe("AppDeployment Controller", func() { Scheme: k8sClient.Scheme(), recorder: mockRecorder, } - ctx = context.WithValue(ctx, appdeploymentAdapterContextKey{}, mockAdapter) + ctx = context.WithValue(ctx, handler.AppdeploymentHandlerContextKey{}, mockAdapter) mockAdapter.EXPECT().EnsureApplicationValid(gomock.Any()).Return(reconciler.OperationResult{}, nil) mockAdapter.EXPECT().EnsureFinalizer(gomock.Any()).Return(reconciler.OperationResult{}, nil) @@ -165,7 +166,7 @@ var _ = Describe("AppDeployment Controller", func() { Scheme: k8sClient.Scheme(), recorder: mockRecorder, } - ctx = context.WithValue(ctx, appdeploymentAdapterContextKey{}, mockAdapter) + ctx = context.WithValue(ctx, handler.AppdeploymentHandlerContextKey{}, mockAdapter) mockAdapter.EXPECT().EnsureApplicationValid(gomock.Any()).Return(reconciler.OperationResult{ CancelRequest: true, @@ -184,7 +185,7 @@ var _ = Describe("AppDeployment Controller", func() { Scheme: k8sClient.Scheme(), recorder: mockRecorder, } - ctx = context.WithValue(ctx, appdeploymentAdapterContextKey{}, mockAdapter) + ctx = context.WithValue(ctx, handler.AppdeploymentHandlerContextKey{}, mockAdapter) mockAdapter.EXPECT().EnsureApplicationValid(gomock.Any()).Return(reconciler.OperationResult{}, errors.NewServiceUnavailable("test error")) @@ -194,4 +195,82 @@ var _ = Describe("AppDeployment Controller", func() { Expect(errors.IsServiceUnavailable(err)).To(BeTrue(), "expected error is ServiceUnavailable") }) }) + + Context("appDeploymentIndexerFunc tests", func() { + It("should return nil for Job without owner", func() { + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "job-without-owner", + Namespace: "default", + }, + } + + result := appDeploymentIndexerFunc(job) + Expect(result).To(BeNil()) + }) + + It("should return nil for Job with non-AppDeployment owner", func() { + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "job-with-wrong-owner", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Pod", + Name: "owner-pod", + UID: "12345", + Controller: &[]bool{true}[0], + }, + }, + }, + } + + result := appDeploymentIndexerFunc(job) + Expect(result).To(BeNil()) + }) + + It("should return owner name for Job with AppDeployment owner", func() { + ownerName := "test-appdeployment" + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "job-with-appdeployment-owner", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: v1alpha1.GroupVersion.String(), + Kind: "AppDeployment", + Name: ownerName, + UID: "67890", + Controller: &[]bool{true}[0], + }, + }, + }, + } + + result := appDeploymentIndexerFunc(job) + Expect(result).To(Equal([]string{ownerName})) + }) + + It("should return nil for Job with AppDeployment owner reference that is not controller", func() { + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "job-with-non-controller-appdeployment", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: v1alpha1.GroupVersion.String(), + Kind: "AppDeployment", + Name: "test-appdeployment", + UID: "13579", + Controller: nil, // Not a controller reference + }, + }, + }, + } + + result := appDeploymentIndexerFunc(job) + Expect(result).To(BeNil()) + }) + }) }) diff --git a/internal/controller/cache_controller.go b/internal/controller/cache_controller.go index be073ba..a4961aa 100644 --- a/internal/controller/cache_controller.go +++ b/internal/controller/cache_controller.go @@ -29,6 +29,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" "github.com/Azure/operation-cache-controller/api/v1alpha1" + "github.com/Azure/operation-cache-controller/internal/handler" "github.com/Azure/operation-cache-controller/internal/utils/reconciler" ) @@ -65,18 +66,17 @@ func (r *CacheReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl return ctrl.Result{}, client.IgnoreNotFound(err) } - adapter := NewCacheAdapter(ctx, cache, logger, r.Client, r.Scheme, r.recorder, ctrl.SetControllerReference) - return r.reconcileHandler(ctx, adapter) + return r.reconcileHandler(ctx, handler.NewCacheHandler(ctx, cache, logger, r.Client, r.Scheme, r.recorder, ctrl.SetControllerReference)) } -func (r *CacheReconciler) reconcileHandler(ctx context.Context, adapter CacheAdapterInterface) (ctrl.Result, error) { +func (r *CacheReconciler) reconcileHandler(ctx context.Context, h handler.CacheHandlerInterface) (ctrl.Result, error) { logger := log.FromContext(ctx) operations := []reconciler.ReconcileOperation{ - adapter.CheckCacheExpiry, - adapter.EnsureCacheInitialized, - adapter.CalculateKeepAliveCount, - adapter.AdjustCache, + h.CheckCacheExpiry, + h.EnsureCacheInitialized, + h.CalculateKeepAliveCount, + h.AdjustCache, } for _, operation := range operations { @@ -94,8 +94,6 @@ func (r *CacheReconciler) reconcileHandler(ctx context.Context, adapter CacheAda return ctrl.Result{RequeueAfter: defaultCacheCheckInterval}, nil } -var cacheOwnerKey = ".metadata.controller.cache" - func cacheOperationIndexerFunc(obj client.Object) []string { // grab the operation object, extract the owner... operation := obj.(*v1alpha1.Operation) @@ -113,7 +111,7 @@ func cacheOperationIndexerFunc(obj client.Object) []string { // SetupWithManager sets up the controller with the Manager. func (r *CacheReconciler) SetupWithManager(mgr ctrl.Manager) error { // +gocover:ignore:block init controller - if err := mgr.GetFieldIndexer().IndexField(context.Background(), &v1alpha1.Operation{}, cacheOwnerKey, cacheOperationIndexerFunc); err != nil { // +gocover:ignore:block init controller + if err := mgr.GetFieldIndexer().IndexField(context.Background(), &v1alpha1.Operation{}, v1alpha1.CacheOwnerKey, cacheOperationIndexerFunc); err != nil { // +gocover:ignore:block init controller return err } // +gocover:ignore:block init controller diff --git a/internal/controller/cache_controller_test.go b/internal/controller/cache_controller_test.go index 03bef64..454f56e 100644 --- a/internal/controller/cache_controller_test.go +++ b/internal/controller/cache_controller_test.go @@ -31,7 +31,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/Azure/operation-cache-controller/api/v1alpha1" - "github.com/Azure/operation-cache-controller/internal/controller/mocks" + "github.com/Azure/operation-cache-controller/internal/handler/mocks" "github.com/Azure/operation-cache-controller/internal/utils/reconciler" ) @@ -62,7 +62,7 @@ func TestReconcileHandler(t *testing.T) { Scheme: scheme, } mockCacheAdapterCtrl := gomock.NewController(t) - cacheAdapter := mocks.NewMockCacheAdapterInterface(mockCacheAdapterCtrl) + cacheAdapter := mocks.NewMockCacheHandlerInterface(mockCacheAdapterCtrl) cacheAdapter.EXPECT().CheckCacheExpiry(ctx).Return(reconciler.OperationResult{}, nil) cacheAdapter.EXPECT().EnsureCacheInitialized(ctx).Return(reconciler.OperationResult{}, nil) cacheAdapter.EXPECT().CalculateKeepAliveCount(ctx).Return(reconciler.OperationResult{}, nil) @@ -83,7 +83,7 @@ func TestReconcileHandler(t *testing.T) { Scheme: scheme, } mockCacheAdapterCtrl := gomock.NewController(t) - cacheAdapter := mocks.NewMockCacheAdapterInterface(mockCacheAdapterCtrl) + cacheAdapter := mocks.NewMockCacheHandlerInterface(mockCacheAdapterCtrl) cacheAdapter.EXPECT().CheckCacheExpiry(ctx).Return(reconciler.OperationResult{CancelRequest: true}, nil) res, err := cacheReconciler.reconcileHandler(ctx, cacheAdapter) assert.NoError(t, err) @@ -101,7 +101,7 @@ func TestReconcileHandler(t *testing.T) { Scheme: scheme, } mockCacheAdapterCtrl := gomock.NewController(t) - cacheAdapter := mocks.NewMockCacheAdapterInterface(mockCacheAdapterCtrl) + cacheAdapter := mocks.NewMockCacheHandlerInterface(mockCacheAdapterCtrl) cacheAdapter.EXPECT().CheckCacheExpiry(ctx).Return(reconciler.OperationResult{}, assert.AnError) _, err := cacheReconciler.reconcileHandler(ctx, cacheAdapter) assert.NotNil(t, err) diff --git a/internal/controller/operation_controller.go b/internal/controller/operation_controller.go index 11fee5e..ee7127e 100644 --- a/internal/controller/operation_controller.go +++ b/internal/controller/operation_controller.go @@ -28,6 +28,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" "github.com/Azure/operation-cache-controller/api/v1alpha1" + "github.com/Azure/operation-cache-controller/internal/handler" "github.com/Azure/operation-cache-controller/internal/utils/reconciler" ) @@ -59,17 +60,17 @@ func (r *OperationReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, client.IgnoreNotFound(err) } - adapter := NewOperationAdapter(ctx, operation, logger, r.Client, r.recorder) + adapter := handler.NewOperationHandler(ctx, operation, logger, r.Client, r.recorder) return r.ReconcileHandler(ctx, adapter) } -func (r *OperationReconciler) ReconcileHandler(ctx context.Context, adapter OperationAdapterInterface) (ctrl.Result, error) { +func (r *OperationReconciler) ReconcileHandler(ctx context.Context, h handler.OperationHandlerInterface) (ctrl.Result, error) { operations := []reconciler.ReconcileOperation{ - adapter.EnsureFinalizer, - adapter.EnsureFinalizerRemoved, - adapter.EnsureNotExpired, - adapter.EnsureAllAppsAreReady, - adapter.EnsureAllAppsAreDeleted, + h.EnsureFinalizer, + h.EnsureFinalizerRemoved, + h.EnsureNotExpired, + h.EnsureAllAppsAreReady, + h.EnsureAllAppsAreDeleted, } for _, operation := range operations { @@ -85,29 +86,27 @@ func (r *OperationReconciler) ReconcileHandler(ctx context.Context, adapter Oper return ctrl.Result{}, nil } -var operationOwnerKey = ".operation.metadata.controller" +func operationIndexerFunc(rawObj client.Object) []string { + // grab the AppDeployment object, extract the owner + adp := rawObj.(*v1alpha1.AppDeployment) + owner := metav1.GetControllerOf(adp) + if owner == nil { + return nil + } + // Make sure the owner is a Operation object + if owner.APIVersion != v1alpha1.GroupVersion.String() || owner.Kind != "Operation" { + return nil + } + return []string{owner.Name} +} // SetupWithManager sets up the controller with the Manager. func (r *OperationReconciler) SetupWithManager(mgr ctrl.Manager) error { - if err := mgr.GetFieldIndexer().IndexField(context.Background(), &v1alpha1.AppDeployment{}, operationOwnerKey, - func(rawObj client.Object) []string { - // grab the AppDeployment object, extract the owner - adp := rawObj.(*v1alpha1.AppDeployment) - owner := metav1.GetControllerOf(adp) - if owner == nil { - return nil - } - // Make sure the owner is a Operation object - if owner.APIVersion != v1alpha1.GroupVersion.String() || owner.Kind != "Operation" { - return nil - } - return []string{owner.Name} - }); err != nil { + if err := mgr.GetFieldIndexer().IndexField(context.Background(), + &v1alpha1.AppDeployment{}, v1alpha1.OperationOwnerKey, operationIndexerFunc); err != nil { return err } - r.recorder = mgr.GetEventRecorderFor("Operation") - return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.Operation{}). Owns(&v1alpha1.AppDeployment{}). diff --git a/internal/controller/operation_controller_test.go b/internal/controller/operation_controller_test.go index 783889a..28cdb86 100644 --- a/internal/controller/operation_controller_test.go +++ b/internal/controller/operation_controller_test.go @@ -19,10 +19,12 @@ package controller import ( "context" "fmt" + "testing" "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -32,7 +34,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "github.com/Azure/operation-cache-controller/api/v1alpha1" - "github.com/Azure/operation-cache-controller/internal/controller/mocks" + "github.com/Azure/operation-cache-controller/internal/handler" + "github.com/Azure/operation-cache-controller/internal/handler/mocks" "github.com/Azure/operation-cache-controller/internal/utils/reconciler" ) @@ -104,7 +107,7 @@ var _ = Describe("Operation Controller", func() { mockClientCtrl *gomock.Controller mockRecorderCtrl *gomock.Controller mockAdapterCtrl *gomock.Controller - mockAdapter *mocks.MockOperationAdapterInterface + mockAdapter *mocks.MockOperationHandlerInterface operationReconciler *OperationReconciler @@ -118,7 +121,7 @@ var _ = Describe("Operation Controller", func() { mockClientCtrl = gomock.NewController(GinkgoT()) mockRecorderCtrl = gomock.NewController(GinkgoT()) mockAdapterCtrl = gomock.NewController(GinkgoT()) - mockAdapter = mocks.NewMockOperationAdapterInterface(mockAdapterCtrl) + mockAdapter = mocks.NewMockOperationHandlerInterface(mockAdapterCtrl) operationReconciler = &OperationReconciler{ Client: k8sClient, Scheme: k8sClient.Scheme(), @@ -141,7 +144,7 @@ var _ = Describe("Operation Controller", func() { mockAdapter.EXPECT().EnsureAllAppsAreReady(gomock.Any()).Return(reconciler.ContinueOperationResult(), nil) mockAdapter.EXPECT().EnsureAllAppsAreDeleted(gomock.Any()).Return(reconciler.ContinueOperationResult(), nil) - res, err := operationReconciler.Reconcile(context.WithValue(ctx, operationAdapterContextKey{}, mockAdapter), reconcile.Request{ + res, err := operationReconciler.Reconcile(context.WithValue(ctx, handler.OperationContextKey{}, mockAdapter), reconcile.Request{ NamespacedName: key, }) Expect(err).NotTo(HaveOccurred()) @@ -154,7 +157,7 @@ var _ = Describe("Operation Controller", func() { testErr := fmt.Errorf("test-error") mockAdapter.EXPECT().EnsureFinalizer(gomock.Any()).Return(reconciler.ContinueOperationResult(), testErr) - res, err := operationReconciler.Reconcile(context.WithValue(ctx, operationAdapterContextKey{}, mockAdapter), reconcile.Request{ + res, err := operationReconciler.Reconcile(context.WithValue(ctx, handler.OperationContextKey{}, mockAdapter), reconcile.Request{ NamespacedName: key, }) Expect(err).To(HaveOccurred()) @@ -169,7 +172,7 @@ var _ = Describe("Operation Controller", func() { CancelRequest: true, }, nil) - res, err := operationReconciler.Reconcile(context.WithValue(ctx, operationAdapterContextKey{}, mockAdapter), reconcile.Request{ + res, err := operationReconciler.Reconcile(context.WithValue(ctx, handler.OperationContextKey{}, mockAdapter), reconcile.Request{ NamespacedName: key, }) Expect(err).NotTo(HaveOccurred()) @@ -241,3 +244,102 @@ var _ = Describe("Operation Controller", func() { }) }) }) + +func TestOperationIndexerFunc(t *testing.T) { + // Test case 1: AppDeployment without an owner reference + t.Run("AppDeployment without an owner reference", func(t *testing.T) { + appDeployment := &v1alpha1.AppDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-without-owner", + Namespace: "default", + }, + } + result := operationIndexerFunc(appDeployment) + assert.Nil(t, result, "Expected nil for AppDeployment without owner reference") + }) + + // Test case 2: AppDeployment with a non-controller owner reference + t.Run("AppDeployment with a non-controller owner reference", func(t *testing.T) { + appDeployment := &v1alpha1.AppDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-with-non-controller-owner", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: v1alpha1.GroupVersion.String(), + Kind: "Operation", + Name: "test-operation", + UID: "12345", + Controller: nil, // Not a controller reference + }, + }, + }, + } + result := operationIndexerFunc(appDeployment) + assert.Nil(t, result, "Expected nil for AppDeployment with non-controller owner reference") + }) + + // Test case 3: AppDeployment with an owner that's not an Operation + t.Run("AppDeployment with an owner that's not an Operation", func(t *testing.T) { + appDeployment := &v1alpha1.AppDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-with-wrong-owner-kind", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: v1alpha1.GroupVersion.String(), + Kind: "SomeOtherKind", + Name: "test-owner", + UID: "12345", + Controller: &[]bool{true}[0], + }, + }, + }, + } + result := operationIndexerFunc(appDeployment) + assert.Nil(t, result, "Expected nil for AppDeployment with wrong owner kind") + }) + + // Test case 4: AppDeployment with a valid Operation controller reference + t.Run("AppDeployment with a valid Operation controller reference", func(t *testing.T) { + operationName := "test-operation" + appDeployment := &v1alpha1.AppDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-with-operation-owner", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: v1alpha1.GroupVersion.String(), + Kind: "Operation", + Name: operationName, + UID: "12345", + Controller: &[]bool{true}[0], + }, + }, + }, + } + result := operationIndexerFunc(appDeployment) + assert.Equal(t, []string{operationName}, result, "Expected owner name for AppDeployment with Operation controller reference") + }) + + // Test case 5: AppDeployment with wrong API version owner reference + t.Run("AppDeployment with wrong API version owner reference", func(t *testing.T) { + appDeployment := &v1alpha1.AppDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "app-with-wrong-api-version", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "wrong.api/v1", + Kind: "Operation", + Name: "test-operation", + UID: "12345", + Controller: &[]bool{true}[0], + }, + }, + }, + } + result := operationIndexerFunc(appDeployment) + assert.Nil(t, result, "Expected nil for AppDeployment with wrong API version") + }) +} diff --git a/internal/controller/requirement_controller.go b/internal/controller/requirement_controller.go index ff8cd6f..b384451 100644 --- a/internal/controller/requirement_controller.go +++ b/internal/controller/requirement_controller.go @@ -29,6 +29,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" "github.com/Azure/operation-cache-controller/api/v1alpha1" + "github.com/Azure/operation-cache-controller/internal/handler" "github.com/Azure/operation-cache-controller/internal/utils/reconciler" ) @@ -62,16 +63,15 @@ func (r *RequirementReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, client.IgnoreNotFound(err) } - adapter := NewRequirementAdapter(ctx, requirement, logger, r.Client, r.recorder) - return r.ReconcileHandler(ctx, adapter) + return r.ReconcileHandler(ctx, handler.NewRequirementHandler(ctx, requirement, logger, r.Client, r.recorder)) } -func (r *RequirementReconciler) ReconcileHandler(ctx context.Context, adapter RequirementAdapterInterface) (ctrl.Result, error) { +func (r *RequirementReconciler) ReconcileHandler(ctx context.Context, h handler.RequirementHandlerInterface) (ctrl.Result, error) { operations := []reconciler.ReconcileOperation{ - adapter.EnsureNotExpired, - adapter.EnsureInitialized, - adapter.EnsureCacheExisted, - adapter.EnsureCachedOperationAcquired, - adapter.EnsureOperationReady, + h.EnsureNotExpired, + h.EnsureInitialized, + h.EnsureCacheExisted, + h.EnsureCachedOperationAcquired, + h.EnsureOperationReady, } for _, operation := range operations { @@ -90,29 +90,27 @@ func (r *RequirementReconciler) ReconcileHandler(ctx context.Context, adapter Re return ctrl.Result{RequeueAfter: defaultCheckInterval}, nil } -var requirementOwnerKey = ".requirement.metadata.controller" +func requirementIndexerFunc(rawObj client.Object) []string { + // grab the Operation object, extract the owner + op := rawObj.(*v1alpha1.Operation) + owner := metav1.GetControllerOf(op) + if owner == nil { + return nil + } + // Make sure the owner is a Requirement object + if owner.APIVersion != v1alpha1.GroupVersion.String() || owner.Kind != "Requirement" { + return nil + } + return []string{owner.Name} +} // SetupWithManager sets up the controller with the Manager. func (r *RequirementReconciler) SetupWithManager(mgr ctrl.Manager) error { - if err := mgr.GetFieldIndexer().IndexField(context.Background(), &v1alpha1.Operation{}, requirementOwnerKey, - func(rawObj client.Object) []string { - // grab the Operation object, extract the owner - op := rawObj.(*v1alpha1.Operation) - owner := metav1.GetControllerOf(op) - if owner == nil { - return nil - } - // Make sure the owner is a Requirement object - if owner.APIVersion != v1alpha1.GroupVersion.String() || owner.Kind != "Requirement" { - return nil - } - return []string{owner.Name} - }); err != nil { + if err := mgr.GetFieldIndexer().IndexField(context.Background(), + &v1alpha1.Operation{}, v1alpha1.RequirementOwnerKey, requirementIndexerFunc); err != nil { return err } - r.recorder = mgr.GetEventRecorderFor("Requirement") - return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.Requirement{}). Owns(&v1alpha1.Operation{}). diff --git a/internal/controller/requirement_controller_test.go b/internal/controller/requirement_controller_test.go index c9632d7..79f4d62 100644 --- a/internal/controller/requirement_controller_test.go +++ b/internal/controller/requirement_controller_test.go @@ -35,7 +35,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/Azure/operation-cache-controller/api/v1alpha1" - "github.com/Azure/operation-cache-controller/internal/controller/mocks" + "github.com/Azure/operation-cache-controller/internal/handler" + "github.com/Azure/operation-cache-controller/internal/handler/mocks" "github.com/Azure/operation-cache-controller/internal/utils/reconciler" ) @@ -122,7 +123,7 @@ var _ = Describe("Requirement Controller", func() { mockClientCtrl *gomock.Controller mockRecorderCtrl *gomock.Controller mockAdapterCtrl *gomock.Controller - mockAdapter *mocks.MockRequirementAdapterInterface + mockAdapter *mocks.MockRequirementHandlerInterface requirementReconciler *RequirementReconciler @@ -136,7 +137,7 @@ var _ = Describe("Requirement Controller", func() { mockClientCtrl = gomock.NewController(GinkgoT()) mockRecorderCtrl = gomock.NewController(GinkgoT()) mockAdapterCtrl = gomock.NewController(GinkgoT()) - mockAdapter = mocks.NewMockRequirementAdapterInterface(mockAdapterCtrl) + mockAdapter = mocks.NewMockRequirementHandlerInterface(mockAdapterCtrl) requirementReconciler = &RequirementReconciler{ Client: k8sClient, @@ -157,7 +158,7 @@ var _ = Describe("Requirement Controller", func() { mockAdapter.EXPECT().EnsureCachedOperationAcquired(gomock.Any()).Return(reconciler.ContinueOperationResult(), nil) mockAdapter.EXPECT().EnsureOperationReady(gomock.Any()).Return(reconciler.ContinueOperationResult(), nil) - result, err := requirementReconciler.Reconcile(context.WithValue(context.Background(), requirementAdapterContextKey{}, mockAdapter), ctrl.Request{ + result, err := requirementReconciler.Reconcile(context.WithValue(context.Background(), handler.RequiremenContextKey{}, mockAdapter), ctrl.Request{ NamespacedName: key, }) Expect(err).NotTo(HaveOccurred()) @@ -169,7 +170,7 @@ var _ = Describe("Requirement Controller", func() { testErr := fmt.Errorf("test-error") mockAdapter.EXPECT().EnsureNotExpired(gomock.Any()).Return(reconciler.RequeueWithError(testErr)) - result, err := requirementReconciler.Reconcile(context.WithValue(ctx, requirementAdapterContextKey{}, mockAdapter), ctrl.Request{ + result, err := requirementReconciler.Reconcile(context.WithValue(ctx, handler.RequiremenContextKey{}, mockAdapter), ctrl.Request{ NamespacedName: key, }) Expect(err).To(MatchError(testErr)) @@ -184,7 +185,7 @@ var _ = Describe("Requirement Controller", func() { CancelRequest: true, }, nil) - result, err := requirementReconciler.Reconcile(context.WithValue(ctx, requirementAdapterContextKey{}, mockAdapter), ctrl.Request{ + result, err := requirementReconciler.Reconcile(context.WithValue(ctx, handler.RequiremenContextKey{}, mockAdapter), ctrl.Request{ NamespacedName: key, }) Expect(err).NotTo(HaveOccurred()) @@ -269,4 +270,76 @@ var _ = Describe("Requirement Controller", func() { Expect(result).Should(Equal(reconcile.Result{RequeueAfter: reconciler.DefaultRequeueDelay})) }) }) + + Context("Testing requirementIndexerFunc", func() { + It("Should return the owner name for an Operation owned by a Requirement", func() { + ownerName := "test-requirement-owner" + operation := &v1alpha1.Operation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-operation", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: v1alpha1.GroupVersion.String(), + Kind: "Requirement", + Name: ownerName, + Controller: &[]bool{true}[0], + }, + }, + }, + } + + indexKeys := requirementIndexerFunc(operation) + Expect(indexKeys).To(HaveLen(1)) + Expect(indexKeys[0]).To(Equal(ownerName)) + }) + + It("Should return nil if the Operation has no owner", func() { + operation := &v1alpha1.Operation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-operation-no-owner", + }, + } + + indexKeys := requirementIndexerFunc(operation) + Expect(indexKeys).To(BeNil()) + }) + + It("Should return nil if the Operation is owned by a non-Requirement resource", func() { + operation := &v1alpha1.Operation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-operation-wrong-owner", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Pod", + Name: "some-pod", + Controller: &[]bool{true}[0], + }, + }, + }, + } + + indexKeys := requirementIndexerFunc(operation) + Expect(indexKeys).To(BeNil()) + }) + + It("Should return nil if the owner is a Requirement but not the controller", func() { + operation := &v1alpha1.Operation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-operation-not-controlled", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: v1alpha1.GroupVersion.String(), + Kind: "Requirement", + Name: "non-controller-requirement", + Controller: &[]bool{false}[0], + }, + }, + }, + } + + indexKeys := requirementIndexerFunc(operation) + Expect(indexKeys).To(BeNil()) + }) + }) }) diff --git a/internal/controller/appdeployment_adapter.go b/internal/handler/appdeployment.go similarity index 68% rename from internal/controller/appdeployment_adapter.go rename to internal/handler/appdeployment.go index d7bed0b..a86ff9b 100644 --- a/internal/controller/appdeployment_adapter.go +++ b/internal/handler/appdeployment.go @@ -1,4 +1,4 @@ -package controller +package handler import ( "context" @@ -14,14 +14,15 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/Azure/operation-cache-controller/api/v1alpha1" - apdutil "github.com/Azure/operation-cache-controller/internal/utils/controller/appdeployment" + "github.com/Azure/operation-cache-controller/internal/log" + ctrlutils "github.com/Azure/operation-cache-controller/internal/utils/controller" "github.com/Azure/operation-cache-controller/internal/utils/reconciler" ) -type appdeploymentAdapterContextKey struct{} +type AppdeploymentHandlerContextKey struct{} -//go:generate mockgen -destination=./mocks/mock_appdeployment_adapter.go -package=mocks github.com/Azure/operation-cache-controller/internal/controller AppDeploymentAdapterInterface -type AppDeploymentAdapterInterface interface { +//go:generate mockgen -destination=./mocks/mock_appdeployment.go -package=mocks github.com/Azure/operation-cache-controller/internal/handler AppDeploymentHandlerInterface +type AppDeploymentHandlerInterface interface { EnsureApplicationValid(ctx context.Context) (reconciler.OperationResult, error) EnsureFinalizer(ctx context.Context) (reconciler.OperationResult, error) EnsureFinalizerDeleted(ctx context.Context) (reconciler.OperationResult, error) @@ -30,26 +31,30 @@ type AppDeploymentAdapterInterface interface { EnsureTeardownFinished(ctx context.Context) (reconciler.OperationResult, error) } -type AppDeploymentAdapter struct { +type AppDeploymentHandler struct { appDeployment *v1alpha1.AppDeployment logger logr.Logger client client.Client recorder record.EventRecorder + + apdutil ctrlutils.AppDeploymentHelper } -func NewAppDeploymentAdapter(ctx context.Context, appDeployment *v1alpha1.AppDeployment, logger logr.Logger, client client.Client, recorder record.EventRecorder) AppDeploymentAdapterInterface { - if appdeploymentAdapter, ok := ctx.Value(appdeploymentAdapterContextKey{}).(AppDeploymentAdapterInterface); ok { - return appdeploymentAdapter +func NewAppDeploymentHandler(ctx context.Context, appDeployment *v1alpha1.AppDeployment, logger logr.Logger, client client.Client, recorder record.EventRecorder) AppDeploymentHandlerInterface { + if appdeploymentHandler, ok := ctx.Value(AppdeploymentHandlerContextKey{}).(AppDeploymentHandlerInterface); ok { + return appdeploymentHandler } - return &AppDeploymentAdapter{ + return &AppDeploymentHandler{ appDeployment: appDeployment, logger: logger, recorder: recorder, client: client, + + apdutil: ctrlutils.NewAppDeploymentHelper(), } } -func (a *AppDeploymentAdapter) phaseIs(phase ...string) bool { +func (a *AppDeploymentHandler) phaseIs(phase ...string) bool { for _, p := range phase { if a.appDeployment.Status.Phase == p { return true @@ -58,50 +63,50 @@ func (a *AppDeploymentAdapter) phaseIs(phase ...string) bool { return false } -func (a *AppDeploymentAdapter) EnsureApplicationValid(ctx context.Context) (reconciler.OperationResult, error) { +func (a *AppDeploymentHandler) EnsureApplicationValid(ctx context.Context) (reconciler.OperationResult, error) { a.logger.V(1).Info("Operation EnsureApplicationValid") - if err := apdutil.Validate(a.appDeployment); err != nil { + if err := ctrlutils.Validate(a.appDeployment); err != nil { a.recorder.Event(a.appDeployment, "Error", "InvalidApplication", err.Error()) return reconciler.RequeueWithError(err) } // initialize the appdeployment status - if a.phaseIs(apdutil.PhaseEmpty) { + if a.phaseIs(v1alpha1.AppDeploymentPhaseEmpty) { a.logger.V(1).Info("Initializing appdeployment status") - a.appDeployment.Status.Phase = apdutil.PhasePending - apdutil.ClearConditions(ctx, a.appDeployment) + a.appDeployment.Status.Phase = v1alpha1.AppDeploymentPhasePending + a.apdutil.ClearConditions(ctx, a.appDeployment) return reconciler.RequeueOnErrorOrContinue(a.client.Status().Update(ctx, a.appDeployment)) } return reconciler.ContinueProcessing() } -func (a *AppDeploymentAdapter) EnsureFinalizer(ctx context.Context) (reconciler.OperationResult, error) { +func (a *AppDeploymentHandler) EnsureFinalizer(ctx context.Context) (reconciler.OperationResult, error) { a.logger.V(1).Info("Operation EnsureFinalizer") - if a.appDeployment.ObjectMeta.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(a.appDeployment, apdutil.FinalizerName) { - controllerutil.AddFinalizer(a.appDeployment, apdutil.FinalizerName) + if a.appDeployment.ObjectMeta.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(a.appDeployment, v1alpha1.AppDeploymentFinalizerName) { + controllerutil.AddFinalizer(a.appDeployment, v1alpha1.AppDeploymentFinalizerName) } return reconciler.RequeueOnErrorOrContinue(a.client.Update(ctx, a.appDeployment)) } -func (a *AppDeploymentAdapter) EnsureFinalizerDeleted(ctx context.Context) (reconciler.OperationResult, error) { +func (a *AppDeploymentHandler) EnsureFinalizerDeleted(ctx context.Context) (reconciler.OperationResult, error) { a.logger.V(1).Info("Operation EnsureFinalizerDeleted") - if !a.appDeployment.ObjectMeta.DeletionTimestamp.IsZero() && controllerutil.ContainsFinalizer(a.appDeployment, apdutil.FinalizerName) { - if a.phaseIs(apdutil.PhaseDeleted) { + if !a.appDeployment.ObjectMeta.DeletionTimestamp.IsZero() && controllerutil.ContainsFinalizer(a.appDeployment, v1alpha1.AppDeploymentFinalizerName) { + if a.phaseIs(v1alpha1.AppDeploymentPhaseDeleted) { a.logger.V(1).Info("All app deleted removing finalizer") - controllerutil.RemoveFinalizer(a.appDeployment, apdutil.FinalizerName) + controllerutil.RemoveFinalizer(a.appDeployment, v1alpha1.AppDeploymentFinalizerName) return reconciler.RequeueOnErrorOrContinue(a.client.Update(ctx, a.appDeployment)) } - if !a.phaseIs(apdutil.PhaseDeleting) { + if !a.phaseIs(v1alpha1.AppDeploymentPhaseDeleting) { a.logger.V(1).Info("App is not deleted yet, setting phase to deleting") - a.appDeployment.Status.Phase = apdutil.PhaseDeleting + a.appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeleting return reconciler.RequeueOnErrorOrContinue(a.client.Status().Update(ctx, a.appDeployment)) } } return reconciler.ContinueProcessing() } -func (a *AppDeploymentAdapter) EnsureDependenciesReady(ctx context.Context) (reconciler.OperationResult, error) { - if !a.phaseIs(apdutil.PhasePending) { +func (a *AppDeploymentHandler) EnsureDependenciesReady(ctx context.Context) (reconciler.OperationResult, error) { + if !a.phaseIs(v1alpha1.AppDeploymentPhasePending) { return reconciler.ContinueProcessing() } a.logger.V(1).Info("Operation EnsureDependenciesReady") @@ -109,17 +114,17 @@ func (a *AppDeploymentAdapter) EnsureDependenciesReady(ctx context.Context) (rec for _, dep := range a.appDeployment.Spec.Dependencies { // check if dependency is ready appdeployment := &v1alpha1.AppDeployment{} - realAppName := apdutil.OperationScopedAppDeployment(dep, a.appDeployment.Spec.OpId) + realAppName := ctrlutils.OperationScopedAppDeployment(dep, a.appDeployment.Spec.OpId) if err := a.client.Get(ctx, client.ObjectKey{Namespace: a.appDeployment.Namespace, Name: realAppName}, appdeployment); err != nil { a.logger.V(1).Error(err, "dependency not found", "dependency", realAppName) return reconciler.RequeueWithError(fmt.Errorf("dependency not found: %s ", realAppName)) } - if appdeployment.Status.Phase != apdutil.PhaseReady { + if appdeployment.Status.Phase != v1alpha1.AppDeploymentPhaseReady { return reconciler.RequeueWithError(fmt.Errorf("dependency is not ready: %s", realAppName)) } } // all dependencies are ready - a.appDeployment.Status.Phase = apdutil.PhaseDeploying + a.appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeploying return reconciler.RequeueOnErrorOrContinue(a.client.Status().Update(ctx, a.appDeployment)) } @@ -127,7 +132,7 @@ var ( errJobNotCompleted = fmt.Errorf("job not completed") ) -func (a *AppDeploymentAdapter) createJob(ctx context.Context, jobTemplate *batchv1.Job) error { +func (a *AppDeploymentHandler) createJob(ctx context.Context, jobTemplate *batchv1.Job) error { if err := ctrl.SetControllerReference(a.appDeployment, jobTemplate, a.client.Scheme()); err != nil { return fmt.Errorf("failed to set controller reference for job %s: %w", jobTemplate.Name, err) } @@ -137,7 +142,7 @@ func (a *AppDeploymentAdapter) createJob(ctx context.Context, jobTemplate *batch return nil } -func (a *AppDeploymentAdapter) initializeJobAndAwaitCompletion(ctx context.Context, jobTemplate *batchv1.Job) error { +func (a *AppDeploymentHandler) initializeJobAndAwaitCompletion(ctx context.Context, jobTemplate *batchv1.Job) error { job := &batchv1.Job{} // check if the job exists if err := a.client.Get(ctx, client.ObjectKey{Namespace: a.appDeployment.Namespace, Name: jobTemplate.Name}, job); err != nil { @@ -153,9 +158,9 @@ func (a *AppDeploymentAdapter) initializeJobAndAwaitCompletion(ctx context.Conte } // check if the job is running - switch apdutil.CheckJobStatus(ctx, job) { + switch ctrlutils.CheckJobStatus(ctx, job) { // if job is failed then delete the job and create a new one - case apdutil.JobStatusFailed: + case ctrlutils.JobStatusFailed: // delete the failed job if err := a.client.Delete(ctx, job, client.PropagationPolicy(metav1.DeletePropagationBackground)); client.IgnoreNotFound(err) != nil { a.recorder.Event(a.appDeployment, "Error", "FailedDeleteJob", err.Error()) @@ -170,7 +175,7 @@ func (a *AppDeploymentAdapter) initializeJobAndAwaitCompletion(ctx context.Conte } // if job is succeeded then delete the job - case apdutil.JobStatusSucceeded: + case ctrlutils.JobStatusSucceeded: // delete the succeeded job if err := a.client.Delete(ctx, job, client.PropagationPolicy(metav1.DeletePropagationBackground)); client.IgnoreNotFound(err) != nil { return fmt.Errorf("failed to delete succeeded job %s: %w", job.Name, err) @@ -185,20 +190,20 @@ func (a *AppDeploymentAdapter) initializeJobAndAwaitCompletion(ctx context.Conte // if job is exist && running then requeue and waiting for the job complete // if job is exist && failed then delete the job and create a new one // if job is exist && succeeded then update the appdeployment status to ready -func (a *AppDeploymentAdapter) EnsureDeployingFinished(ctx context.Context) (reconciler.OperationResult, error) { +func (a *AppDeploymentHandler) EnsureDeployingFinished(ctx context.Context) (reconciler.OperationResult, error) { a.logger.V(1).Info("Operation EnsureDeployingFinished") - if !a.phaseIs(apdutil.PhaseDeploying) { + if !a.phaseIs(v1alpha1.AppDeploymentPhaseDeploying) { return reconciler.ContinueProcessing() } - provisionJob := apdutil.ProvisionJobFromAppDeploymentSpec(a.appDeployment) + provisionJob := ctrlutils.ProvisionJobFromAppDeploymentSpec(a.appDeployment) err := a.initializeJobAndAwaitCompletion(ctx, provisionJob) switch err { case nil: // provision job is succeeded move the appdeployment to ready phase - a.appDeployment.Status.Phase = apdutil.PhaseReady + a.appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseReady return reconciler.RequeueOnErrorOrContinue(a.client.Status().Update(ctx, a.appDeployment)) case errJobNotCompleted: - a.logger.V(1).WithValues(apdutil.LogKeyJobName, provisionJob.Name).Info("provision job is not completed yet") + a.logger.V(1).WithValues(log.AppDeploymentJobName, provisionJob.Name).Info("provision job is not completed yet") return reconciler.Requeue() default: a.logger.Error(err, "provision job failed %s", provisionJob.Name) @@ -206,23 +211,23 @@ func (a *AppDeploymentAdapter) EnsureDeployingFinished(ctx context.Context) (rec } } -func (a *AppDeploymentAdapter) EnsureTeardownFinished(ctx context.Context) (reconciler.OperationResult, error) { +func (a *AppDeploymentHandler) EnsureTeardownFinished(ctx context.Context) (reconciler.OperationResult, error) { a.logger.V(1).Info("Operation EnsureTeardownFinished") - if !a.phaseIs(apdutil.PhaseDeleting) { + if !a.phaseIs(v1alpha1.AppDeploymentPhaseDeleting) { return reconciler.ContinueProcessing() } - teardownJob := apdutil.TeardownJobFromAppDeploymentSpec(a.appDeployment) + teardownJob := ctrlutils.TeardownJobFromAppDeploymentSpec(a.appDeployment) err := a.initializeJobAndAwaitCompletion(ctx, teardownJob) switch err { case nil: // teardown job is succeeded move the appdeployment to deleted phase - a.appDeployment.Status.Phase = apdutil.PhaseDeleted + a.appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeleted return reconciler.RequeueOnErrorOrContinue(a.client.Status().Update(ctx, a.appDeployment)) case errJobNotCompleted: - a.logger.V(1).WithValues(apdutil.LogKeyJobName, teardownJob.Name).Info("teardown job is not completed yet") + a.logger.V(1).WithValues(log.AppDeploymentJobName, teardownJob.Name).Info("teardown job is not completed yet") return reconciler.Requeue() default: - a.logger.WithValues(apdutil.LogKeyJobName, teardownJob.Name).Error(err, "teardown job failed %s") + a.logger.WithValues(log.AppDeploymentJobName, teardownJob.Name).Error(err, "teardown job failed %s") return reconciler.RequeueWithError(err) } } diff --git a/internal/controller/appdeployment_adapter_test.go b/internal/handler/appdeployment_test.go similarity index 58% rename from internal/controller/appdeployment_adapter_test.go rename to internal/handler/appdeployment_test.go index 3fd3495..5a764cc 100644 --- a/internal/controller/appdeployment_adapter_test.go +++ b/internal/handler/appdeployment_test.go @@ -1,4 +1,4 @@ -package controller +package handler import ( "context" @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" k8serr "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -16,13 +17,35 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" "github.com/Azure/operation-cache-controller/api/v1alpha1" - mockpkg "github.com/Azure/operation-cache-controller/internal/mocks" - apdutil "github.com/Azure/operation-cache-controller/internal/utils/controller/appdeployment" + mockpkg "github.com/Azure/operation-cache-controller/internal/utils/mocks" "github.com/Azure/operation-cache-controller/internal/utils/reconciler" ) const testOpId = "test-op-id" +func newTestJobSpec() batchv1.JobSpec { + return batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{}, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "test-container", + Image: "test-image", + Command: []string{ + "echo", + "hello", + }, + Args: []string{ + "world", + }, + }, + }, + }, + }, + } +} + var validAppDeployment = &v1alpha1.AppDeployment{ Spec: v1alpha1.AppDeploymentSpec{ Provision: newTestJobSpec(), @@ -42,7 +65,7 @@ func TestNewAppDeploymentAdapter(t *testing.T) { mockClient := mockpkg.NewMockClient(mockCtrl) mockRecorder := mockpkg.NewMockEventRecorder(mockCtrl) - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) assert.NotNil(t, adapter) } @@ -68,7 +91,7 @@ func TestAppDeploymentAdapter_EnsureApplicationValid(t *testing.T) { t.Run("Happy path: application valid", func(t *testing.T) { appDeployment := validAppDeployment.DeepCopy() - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) assert.NotNil(t, adapter) res, err := adapter.EnsureApplicationValid(ctx) assert.NoError(t, err) @@ -77,8 +100,8 @@ func TestAppDeploymentAdapter_EnsureApplicationValid(t *testing.T) { t.Run("Happy path: application invalid and not in empty phase", func(t *testing.T) { appDeployment := validAppDeployment.DeepCopy() - appDeployment.Status.Phase = apdutil.PhaseDeploying - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeploying + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) assert.NotNil(t, adapter) res, err := adapter.EnsureApplicationValid(ctx) assert.NoError(t, err) @@ -87,7 +110,7 @@ func TestAppDeploymentAdapter_EnsureApplicationValid(t *testing.T) { t.Run("Sad path: application return error", func(t *testing.T) { appDeployment := &v1alpha1.AppDeployment{} - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) assert.NotNil(t, adapter) res, err := adapter.EnsureApplicationValid(ctx) assert.Error(t, err) @@ -107,7 +130,7 @@ func TestAppDeploymentAdapter_EnsureFinalizer(t *testing.T) { mockRecorder := mockpkg.NewMockEventRecorder(mockCtrl) // mockStatusWriter := mockpkg.NewMockStatusWriter(mockCtrl) - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) assert.NotNil(t, adapter) t.Run("Happy path: finalizer not present", func(t *testing.T) { @@ -149,7 +172,7 @@ func TestAppDeploymentAdapter_EnsureFinalizerDeleted(t *testing.T) { mockRecorder = mockpkg.NewMockEventRecorder(mockCtrl) appDeployment := validAppDeployment.DeepCopy() - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) res, err := adapter.EnsureFinalizerDeleted(ctx) assert.NoError(t, err) @@ -165,11 +188,11 @@ func TestAppDeploymentAdapter_EnsureFinalizerDeleted(t *testing.T) { mockRecorder = mockpkg.NewMockEventRecorder(mockCtrl) appDeployment := validAppDeployment.DeepCopy() - appDeployment.Finalizers = []string{apdutil.FinalizerName} + appDeployment.Finalizers = []string{v1alpha1.AppDeploymentFinalizerName} appDeployment.DeletionTimestamp = &metav1.Time{Time: time.Now()} - appDeployment.Status.Phase = apdutil.PhaseDeleted + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeleted - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) mockClient.EXPECT().Update(ctx, gomock.Any()).Return(nil) res, err := adapter.EnsureFinalizerDeleted(ctx) assert.NoError(t, err) @@ -187,11 +210,11 @@ func TestAppDeploymentAdapter_EnsureFinalizerDeleted(t *testing.T) { mockRecorder = mockpkg.NewMockEventRecorder(mockCtrl) appDeployment := validAppDeployment.DeepCopy() - appDeployment.Finalizers = []string{apdutil.FinalizerName} + appDeployment.Finalizers = []string{v1alpha1.AppDeploymentFinalizerName} appDeployment.DeletionTimestamp = &metav1.Time{Time: time.Now()} - appDeployment.Status.Phase = apdutil.PhaseDeleted + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeleted - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) mockClient.EXPECT().Update(ctx, gomock.Any()).Return(assert.AnError) res, err := adapter.EnsureFinalizerDeleted(ctx) @@ -213,13 +236,13 @@ func TestAppDeploymentAdapter_EnsureFinalizerDeleted(t *testing.T) { mockClient.EXPECT().Status().Return(mockStatusWriter).AnyTimes() appDeployment := validAppDeployment.DeepCopy() - appDeployment.Finalizers = []string{apdutil.FinalizerName} + appDeployment.Finalizers = []string{v1alpha1.AppDeploymentFinalizerName} appDeployment.DeletionTimestamp = &metav1.Time{Time: time.Now()} - appDeployment.Status.Phase = apdutil.PhasePending + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhasePending mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) res, err := adapter.EnsureFinalizerDeleted(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{ @@ -240,7 +263,7 @@ func TestAppDeploymentAdapter_EnsureDependenciesReady(t *testing.T) { t.Run("Happy path: skip dependencies check", func(t *testing.T) { appDeployment := validAppDeployment.DeepCopy() - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) res, err := adapter.EnsureDependenciesReady(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{}, res) @@ -250,16 +273,16 @@ func TestAppDeploymentAdapter_EnsureDependenciesReady(t *testing.T) { mockStatusWriter := mockpkg.NewMockStatusWriter(mockCtrl) mockClient.EXPECT().Status().Return(mockStatusWriter).AnyTimes() appDeployment := validAppDeployment.DeepCopy() - appDeployment.Status.Phase = apdutil.PhasePending + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhasePending appDeployment.Spec.OpId = testOpId appDeployment.Spec.Dependencies = []string{ "test-app-1", } - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) dependendApp := &v1alpha1.AppDeployment{ Status: v1alpha1.AppDeploymentStatus{ - Phase: apdutil.PhaseReady, + Phase: v1alpha1.AppDeploymentPhaseReady, }, } @@ -280,12 +303,12 @@ func TestAppDeploymentAdapter_EnsureDependenciesReady(t *testing.T) { mockStatusWriter := mockpkg.NewMockStatusWriter(mockCtrl) mockClient.EXPECT().Status().Return(mockStatusWriter).AnyTimes() appDeployment := validAppDeployment.DeepCopy() - appDeployment.Status.Phase = apdutil.PhasePending + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhasePending appDeployment.Spec.OpId = testOpId appDeployment.Spec.Dependencies = []string{ "test-app-1", } - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&v1alpha1.AppDeployment{}), gomock.Any()).Return(assert.AnError).Times(1) @@ -298,16 +321,16 @@ func TestAppDeploymentAdapter_EnsureDependenciesReady(t *testing.T) { mockStatusWriter := mockpkg.NewMockStatusWriter(mockCtrl) mockClient.EXPECT().Status().Return(mockStatusWriter).AnyTimes() appDeployment := validAppDeployment.DeepCopy() - appDeployment.Status.Phase = apdutil.PhasePending + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhasePending appDeployment.Spec.OpId = testOpId appDeployment.Spec.Dependencies = []string{ "test-app-1", } - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) dependendApp := &v1alpha1.AppDeployment{ Status: v1alpha1.AppDeploymentStatus{ - Phase: apdutil.PhasePending, + Phase: v1alpha1.AppDeploymentPhasePending, }, } @@ -324,6 +347,109 @@ func TestAppDeploymentAdapter_EnsureDependenciesReady(t *testing.T) { }) } +func TestAppDeploymentAdapter_EnsureDependenciesReady_MultipleDepencies(t *testing.T) { + ctx := context.Background() + logger := log.FromContext(ctx) + + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mockClient := mockpkg.NewMockClient(mockCtrl) + mockRecorder := mockpkg.NewMockEventRecorder(mockCtrl) + mockStatusWriter := mockpkg.NewMockStatusWriter(mockCtrl) + mockClient.EXPECT().Status().Return(mockStatusWriter).AnyTimes() + + appDeployment := validAppDeployment.DeepCopy() + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhasePending + appDeployment.Spec.OpId = testOpId + appDeployment.Spec.Dependencies = []string{ + "test-app-1", + "test-app-2", + } + + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) + + readyApp := &v1alpha1.AppDeployment{ + Status: v1alpha1.AppDeploymentStatus{ + Phase: v1alpha1.AppDeploymentPhaseReady, + }, + } + + mockClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&v1alpha1.AppDeployment{}), gomock.Any()). + DoAndReturn(func(ctx context.Context, key client.ObjectKey, obj runtime.Object, opts ...client.GetOption) error { + *obj.(*v1alpha1.AppDeployment) = *readyApp + return nil + }).Times(2) + + mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() + + res, err := adapter.EnsureDependenciesReady(ctx) + assert.NoError(t, err) + assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: false}, res) +} + +func TestAppDeploymentAdapter_CreateJob_Errors(t *testing.T) { + ctx := context.Background() + logger := log.FromContext(ctx) + + t.Run("Error setting controller reference", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + mockClient := mockpkg.NewMockClient(mockCtrl) + mockRecorder := mockpkg.NewMockEventRecorder(mockCtrl) + + appDeployment := validAppDeployment.DeepCopy() + adapter := &AppDeploymentHandler{ + appDeployment: appDeployment, + logger: logger, + client: mockClient, + recorder: mockRecorder, + } + + jobTemplate := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + } + + mockClient.EXPECT().Scheme().Return(runtime.NewScheme()) // Empty scheme will cause error + + err := adapter.createJob(ctx, jobTemplate) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to set controller reference") + }) + + t.Run("Error creating job", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + mockClient := mockpkg.NewMockClient(mockCtrl) + mockRecorder := mockpkg.NewMockEventRecorder(mockCtrl) + + scheme := runtime.NewScheme() + _ = v1alpha1.AddToScheme(scheme) + mockClient.EXPECT().Scheme().Return(scheme).AnyTimes() + + appDeployment := validAppDeployment.DeepCopy() + adapter := &AppDeploymentHandler{ + appDeployment: appDeployment, + logger: logger, + client: mockClient, + recorder: mockRecorder, + } + + jobTemplate := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + }, + } + + expectedErr := errors.New("create job error") + mockClient.EXPECT().Create(ctx, gomock.Any()).Return(expectedErr) + + err := adapter.createJob(ctx, jobTemplate) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create job") + }) +} + func TestAppDeploymentAdapter_EnsureDeployingFinished(t *testing.T) { ctx := context.Background() logger := log.FromContext(ctx) @@ -335,7 +461,7 @@ func TestAppDeploymentAdapter_EnsureDeployingFinished(t *testing.T) { mockRecorderCtrl := gomock.NewController(t) mockRecorder := mockpkg.NewMockEventRecorder(mockRecorderCtrl) - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) assert.NotNil(t, adapter) res, err := adapter.EnsureDeployingFinished(ctx) @@ -353,9 +479,9 @@ func TestAppDeploymentAdapter_EnsureDeployingFinished(t *testing.T) { mockClient.EXPECT().Status().Return(mockStatusWriter).AnyTimes() appDeployment := validAppDeployment.DeepCopy() - appDeployment.Status.Phase = apdutil.PhaseDeploying + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeploying - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&batchv1.Job{})). DoAndReturn(func(ctx context.Context, key client.ObjectKey, obj runtime.Object, opts ...client.GetOption) error { *obj.(*batchv1.Job) = batchv1.Job{ @@ -396,9 +522,9 @@ func TestAppDeploymentAdapter_EnsureDeployingFinished(t *testing.T) { mockClient.EXPECT().Scheme().Return(scheme).AnyTimes() appDeployment := validAppDeployment.DeepCopy() - appDeployment.Status.Phase = apdutil.PhaseDeploying + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeploying - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&batchv1.Job{})). Return(k8serr.NewNotFound(batchv1.Resource("job"), "test-job")) @@ -438,9 +564,9 @@ func TestAppDeploymentAdapter_EnsureDeployingFinished(t *testing.T) { mockClient.EXPECT().Scheme().Return(scheme).AnyTimes() appDeployment := validAppDeployment.DeepCopy() - appDeployment.Status.Phase = apdutil.PhaseDeploying + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeploying - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&batchv1.Job{})).DoAndReturn( func(ctx context.Context, key client.ObjectKey, obj runtime.Object, opts ...client.GetOption) error { *obj.(*batchv1.Job) = failedJob @@ -455,6 +581,126 @@ func TestAppDeploymentAdapter_EnsureDeployingFinished(t *testing.T) { }) } +func TestAppDeploymentAdapter_EnsureDeployingFinished_JobErrors(t *testing.T) { + ctx := context.Background() + logger := log.FromContext(ctx) + + t.Run("Error getting job status", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + mockClient := mockpkg.NewMockClient(mockCtrl) + mockRecorder := mockpkg.NewMockEventRecorder(mockCtrl) + + appDeployment := validAppDeployment.DeepCopy() + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeploying + + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) + + expectedErr := errors.New("get job error") + mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(expectedErr) + + result, err := adapter.EnsureDeployingFinished(ctx) + assert.Error(t, err) + assert.Contains(t, err.Error(), "get job error") + assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, result) + }) +} + +func TestAppDeploymentAdapter_EnsureDeployingFinished_FailedJobRecreateErrors(t *testing.T) { + ctx := context.Background() + logger := log.FromContext(ctx) + + t.Run("Error setting controller reference when recreating failed job", func(t *testing.T) { + failedJob := batchv1.Job{ + Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{ + { + Type: batchv1.JobFailed, + Status: "True", + }, + }, + Failed: 1, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + Namespace: "default", + }, + } + + mockCtrl := gomock.NewController(t) + mockClient := mockpkg.NewMockClient(mockCtrl) + mockRecorderCtrl := gomock.NewController(t) + mockRecorder := mockpkg.NewMockEventRecorder(mockRecorderCtrl) + + scheme := runtime.NewScheme() + // Not adding v1alpha1 to scheme to force controller reference error + mockClient.EXPECT().Scheme().Return(scheme).AnyTimes() + + appDeployment := validAppDeployment.DeepCopy() + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeploying + + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) + mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&batchv1.Job{})).DoAndReturn( + func(ctx context.Context, key client.ObjectKey, obj runtime.Object, opts ...client.GetOption) error { + *obj.(*batchv1.Job) = failedJob + return nil + }) + mockClient.EXPECT().Delete(ctx, gomock.Any(), gomock.Any()).Return(nil) + + // No need to expect Create since it should fail at SetControllerReference + + result, err := adapter.EnsureDeployingFinished(ctx) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to set controller reference for job") + assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, result) + }) + + t.Run("Error creating job when recreating failed job", func(t *testing.T) { + failedJob := batchv1.Job{ + Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{ + { + Type: batchv1.JobFailed, + Status: "True", + }, + }, + Failed: 1, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-job", + Namespace: "default", + }, + } + + mockCtrl := gomock.NewController(t) + mockClient := mockpkg.NewMockClient(mockCtrl) + mockRecorderCtrl := gomock.NewController(t) + mockRecorder := mockpkg.NewMockEventRecorder(mockRecorderCtrl) + + scheme := runtime.NewScheme() + _ = v1alpha1.AddToScheme(scheme) + mockClient.EXPECT().Scheme().Return(scheme).AnyTimes() + + appDeployment := validAppDeployment.DeepCopy() + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeploying + + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) + mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&batchv1.Job{})).DoAndReturn( + func(ctx context.Context, key client.ObjectKey, obj runtime.Object, opts ...client.GetOption) error { + *obj.(*batchv1.Job) = failedJob + return nil + }) + mockClient.EXPECT().Delete(ctx, gomock.Any(), gomock.Any()).Return(nil) + + expectedErr := errors.New("create job error") + mockClient.EXPECT().Create(ctx, gomock.Any()).Return(expectedErr) + + result, err := adapter.EnsureDeployingFinished(ctx) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to create job") + assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, result) + }) +} + func TestAppDeploymentAdapter_EnsureTeardownFinished(t *testing.T) { ctx := context.Background() @@ -502,7 +748,7 @@ func TestAppDeploymentAdapter_EnsureTeardownFinished(t *testing.T) { mockClient := mockpkg.NewMockClient(mockCtrl) mockRecorder := mockpkg.NewMockEventRecorder(mockCtrl) - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) assert.NotNil(t, adapter) res, err := adapter.EnsureTeardownFinished(ctx) assert.NoError(t, err) @@ -511,7 +757,7 @@ func TestAppDeploymentAdapter_EnsureTeardownFinished(t *testing.T) { t.Run("Happy path: teardown finished", func(t *testing.T) { appDeployment := validAppDeployment.DeepCopy() - appDeployment.Status.Phase = apdutil.PhaseDeleting + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeleting logger := log.FromContext(ctx) mockCtrl := gomock.NewController(t) @@ -522,7 +768,7 @@ func TestAppDeploymentAdapter_EnsureTeardownFinished(t *testing.T) { mockStatusWriter := mockpkg.NewMockStatusWriter(mockStatusCtrl) mockClient.EXPECT().Status().Return(mockStatusWriter).AnyTimes() - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) assert.NotNil(t, adapter) mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&batchv1.Job{})). @@ -539,7 +785,7 @@ func TestAppDeploymentAdapter_EnsureTeardownFinished(t *testing.T) { }) t.Run("Happy path: teardown create new job", func(t *testing.T) { appDeployment := validAppDeployment.DeepCopy() - appDeployment.Status.Phase = apdutil.PhaseDeleting + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeleting logger := log.FromContext(ctx) mockCtrl := gomock.NewController(t) @@ -553,7 +799,7 @@ func TestAppDeploymentAdapter_EnsureTeardownFinished(t *testing.T) { _ = v1alpha1.AddToScheme(scheme) mockClient.EXPECT().Scheme().Return(scheme).AnyTimes() - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) assert.NotNil(t, adapter) mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&batchv1.Job{})). @@ -568,7 +814,7 @@ func TestAppDeploymentAdapter_EnsureTeardownFinished(t *testing.T) { }) t.Run("Happy path: teardown job failed, create new job", func(t *testing.T) { appDeployment := validAppDeployment.DeepCopy() - appDeployment.Status.Phase = apdutil.PhaseDeleting + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeleting logger := log.FromContext(ctx) mockCtrl := gomock.NewController(t) @@ -582,7 +828,7 @@ func TestAppDeploymentAdapter_EnsureTeardownFinished(t *testing.T) { _ = v1alpha1.AddToScheme(scheme) mockClient.EXPECT().Scheme().Return(scheme).AnyTimes() - adapter := NewAppDeploymentAdapter(ctx, appDeployment, logger, mockClient, mockRecorder) + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) assert.NotNil(t, adapter) mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&batchv1.Job{})). @@ -599,3 +845,100 @@ func TestAppDeploymentAdapter_EnsureTeardownFinished(t *testing.T) { assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, res) }) } + +func TestAppDeploymentAdapter_EnsureTeardownFinished_JobErrors(t *testing.T) { + ctx := context.Background() + logger := log.FromContext(ctx) + + t.Run("Error deleting succeeded job", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + mockClient := mockpkg.NewMockClient(mockCtrl) + mockRecorder := mockpkg.NewMockEventRecorder(mockCtrl) + mockStatusWriter := mockpkg.NewMockStatusWriter(mockCtrl) + mockClient.EXPECT().Status().Return(mockStatusWriter).AnyTimes() + + appDeployment := validAppDeployment.DeepCopy() + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeleting + + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) + + succeededJob := &batchv1.Job{ + Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{ + { + Type: batchv1.JobComplete, + Status: "True", + }, + { + Type: batchv1.JobFailed, + Status: "False", + }, + }, + Succeeded: 1, + }, + } + + mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&batchv1.Job{})).DoAndReturn( + func(ctx context.Context, key client.ObjectKey, obj runtime.Object, opts ...client.GetOption) error { + *obj.(*batchv1.Job) = *succeededJob + return nil + }) + + expectedErr := errors.New("delete job error") + mockClient.EXPECT().Delete(ctx, gomock.Any(), gomock.Any()).Return(expectedErr) + + result, err := adapter.EnsureTeardownFinished(ctx) + assert.Error(t, err) + assert.Contains(t, err.Error(), "delete job error") + assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, result) + }) + + t.Run("Error getting job", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + mockClient := mockpkg.NewMockClient(mockCtrl) + mockRecorder := mockpkg.NewMockEventRecorder(mockCtrl) + mockStatusWriter := mockpkg.NewMockStatusWriter(mockCtrl) + mockClient.EXPECT().Status().Return(mockStatusWriter).AnyTimes() + + appDeployment := validAppDeployment.DeepCopy() + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeleting + + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) + + expectedErr := errors.New("get job error") + mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&batchv1.Job{})).Return(expectedErr) + + result, err := adapter.EnsureTeardownFinished(ctx) + assert.Error(t, err) + assert.Contains(t, err.Error(), "get job error") + assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, result) + }) + + t.Run("Error creating new job", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + mockClient := mockpkg.NewMockClient(mockCtrl) + mockRecorder := mockpkg.NewMockEventRecorder(mockCtrl) + mockStatusWriter := mockpkg.NewMockStatusWriter(mockCtrl) + mockClient.EXPECT().Status().Return(mockStatusWriter).AnyTimes() + scheme := runtime.NewScheme() + _ = v1alpha1.AddToScheme(scheme) + mockClient.EXPECT().Scheme().Return(scheme).AnyTimes() + + appDeployment := validAppDeployment.DeepCopy() + appDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseDeleting + + adapter := NewAppDeploymentHandler(ctx, appDeployment, logger, mockClient, mockRecorder) + + mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&batchv1.Job{})). + Return(k8serr.NewNotFound(batchv1.Resource("job"), "test-job")) + + expectedErr := errors.New("create job error") + mockClient.EXPECT().Create(ctx, gomock.Any()).Return(expectedErr) + mockRecorder.EXPECT().Event(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Times(1) + + result, err := adapter.EnsureTeardownFinished(ctx) + assert.Error(t, err) + assert.Contains(t, err.Error(), "create job error") + assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, result) + }) +} diff --git a/internal/controller/cache_adapter.go b/internal/handler/cache.go similarity index 82% rename from internal/controller/cache_adapter.go rename to internal/handler/cache.go index 8d4f808..5b494ff 100644 --- a/internal/controller/cache_adapter.go +++ b/internal/handler/cache.go @@ -1,4 +1,4 @@ -package controller +package handler import ( "context" @@ -17,31 +17,33 @@ import ( "github.com/Azure/operation-cache-controller/api/v1alpha1" ctrlutils "github.com/Azure/operation-cache-controller/internal/utils/controller" - oputils "github.com/Azure/operation-cache-controller/internal/utils/controller/operation" + randutils "github.com/Azure/operation-cache-controller/internal/utils/rand" "github.com/Azure/operation-cache-controller/internal/utils/reconciler" ) -//go:generate mockgen -destination=./mocks/mock_cache_adapter.go -package=mocks github.com/Azure/operation-cache-controller/internal/controller CacheAdapterInterface -type CacheAdapterInterface interface { +//go:generate mockgen -destination=./mocks/mock_cache.go -package=mocks github.com/Azure/operation-cache-controller/internal/handler CacheHandlerInterface +type CacheHandlerInterface interface { CheckCacheExpiry(ctx context.Context) (reconciler.OperationResult, error) EnsureCacheInitialized(ctx context.Context) (reconciler.OperationResult, error) CalculateKeepAliveCount(ctx context.Context) (reconciler.OperationResult, error) AdjustCache(ctx context.Context) (reconciler.OperationResult, error) } -type CacheAdapter struct { +type CacheHandler struct { cache *v1alpha1.Cache logger logr.Logger client client.Client scheme *runtime.Scheme recorder record.EventRecorder + cacheUtils ctrlutils.CacheHelper + oputils ctrlutils.OperationHelper setControllerReferenceFunc func(owner, controlled metav1.Object, scheme *runtime.Scheme, opts ...controllerutil.OwnerReferenceOption) error } -func NewCacheAdapter(ctx context.Context, +func NewCacheHandler(ctx context.Context, cache *v1alpha1.Cache, logger logr.Logger, client client.Client, scheme *runtime.Scheme, recorder record.EventRecorder, - fn func(owner, controlled metav1.Object, scheme *runtime.Scheme, opts ...controllerutil.OwnerReferenceOption) error) CacheAdapterInterface { - return &CacheAdapter{ + fn func(owner, controlled metav1.Object, scheme *runtime.Scheme, opts ...controllerutil.OwnerReferenceOption) error) CacheHandlerInterface { + return &CacheHandler{ cache: cache, logger: logger, client: client, @@ -52,7 +54,7 @@ func NewCacheAdapter(ctx context.Context, } // updateStatus updates the status of the cache cr -func (c *CacheAdapter) updateStatus(ctx context.Context) error { +func (c *CacheHandler) updateStatus(ctx context.Context) error { if err := c.client.Status().Update(ctx, c.cache); err != nil { return fmt.Errorf("unable to update cache status: %w", err) } @@ -60,7 +62,7 @@ func (c *CacheAdapter) updateStatus(ctx context.Context) error { } // CheckCacheExpiry checks if the cache cr is expired. If it is, the cr is deleted. -func (c *CacheAdapter) CheckCacheExpiry(ctx context.Context) (reconciler.OperationResult, error) { +func (c *CacheHandler) CheckCacheExpiry(ctx context.Context) (reconciler.OperationResult, error) { if c.cache.Spec.ExpireTime == "" { return reconciler.ContinueProcessing() } @@ -81,26 +83,26 @@ func (c *CacheAdapter) CheckCacheExpiry(ctx context.Context) (reconciler.Operati } // EnsureCacheInitialized ensures the cache cr is initialized -func (c *CacheAdapter) EnsureCacheInitialized(ctx context.Context) (reconciler.OperationResult, error) { +func (c *CacheHandler) EnsureCacheInitialized(ctx context.Context) (reconciler.OperationResult, error) { // initialize the AvailableCaches in status if it is nil if c.cache.Status.AvailableCaches == nil { c.cache.Status.AvailableCaches = []string{} } if c.cache.Status.CacheKey == "" { - c.cache.Status.CacheKey = ctrlutils.NewCacheKeyFromApplications(c.cache.Spec.OperationTemplate.Applications) + c.cache.Status.CacheKey = c.cacheUtils.NewCacheKeyFromApplications(c.cache.Spec.OperationTemplate.Applications) } return reconciler.RequeueOnErrorOrContinue(c.updateStatus(ctx)) } // CalculateKeepAliveCount calculates the keepAliveCount for the cache cr -func (c *CacheAdapter) CalculateKeepAliveCount(ctx context.Context) (reconciler.OperationResult, error) { +func (c *CacheHandler) CalculateKeepAliveCount(ctx context.Context) (reconciler.OperationResult, error) { // before we have cache service to provide the keepAliveCount, we use fixed value c.cache.Status.KeepAliveCount = 5 return reconciler.RequeueOnErrorOrContinue(c.updateStatus(ctx)) } -func (c *CacheAdapter) createOperationsAsync(ctx context.Context, ops []*v1alpha1.Operation) error { +func (c *CacheHandler) createOperationsAsync(ctx context.Context, ops []*v1alpha1.Operation) error { wg := sync.WaitGroup{} errChan := make(chan error, len(ops)) for _, op := range ops { @@ -119,7 +121,7 @@ func (c *CacheAdapter) createOperationsAsync(ctx context.Context, ops []*v1alpha return errs } -func (c *CacheAdapter) deleteOperationsAsync(ctx context.Context, ops []*v1alpha1.Operation) error { +func (c *CacheHandler) deleteOperationsAsync(ctx context.Context, ops []*v1alpha1.Operation) error { wg := sync.WaitGroup{} errChan := make(chan error, len(ops)) for _, op := range ops { @@ -138,11 +140,7 @@ func (c *CacheAdapter) deleteOperationsAsync(ctx context.Context, ops []*v1alpha return errs } -func operationReady(op *v1alpha1.Operation) bool { - return op.Status.Phase == oputils.PhaseReconciled -} - -func (c *CacheAdapter) initOperationFromCache(operationName string) *v1alpha1.Operation { +func (c *CacheHandler) initOperationFromCache(operationName string) *v1alpha1.Operation { op := &v1alpha1.Operation{} annotations := op.GetAnnotations() @@ -170,14 +168,14 @@ func (c *CacheAdapter) initOperationFromCache(operationName string) *v1alpha1.Op return op } -func (c *CacheAdapter) AdjustCache(ctx context.Context) (reconciler.OperationResult, error) { +func (c *CacheHandler) AdjustCache(ctx context.Context) (reconciler.OperationResult, error) { var ownedOps v1alpha1.OperationList - if err := c.client.List(ctx, &ownedOps, client.InNamespace(c.cache.Namespace), client.MatchingFields{cacheOwnerKey: c.cache.Name}); err != nil { + if err := c.client.List(ctx, &ownedOps, client.InNamespace(c.cache.Namespace), client.MatchingFields{v1alpha1.CacheOwnerKey: c.cache.Name}); err != nil { return reconciler.RequeueWithError(err) } availableCaches := []string{} for _, op := range ownedOps.Items { - if operationReady(&op) { + if c.oputils.IsOperationReady(&op) { availableCaches = append(availableCaches, op.Name) } } @@ -193,7 +191,7 @@ func (c *CacheAdapter) AdjustCache(ctx context.Context) (reconciler.OperationRes availableCacheNumToRemove := cacheBalance opsToRemove := []*v1alpha1.Operation{} for _, op := range ownedOps.Items { - if !operationReady(&op) { + if !c.oputils.IsOperationReady(&op) { opsToRemove = append(opsToRemove, &op) } else { if availableCacheNumToRemove > 0 { @@ -212,7 +210,7 @@ func (c *CacheAdapter) AdjustCache(ctx context.Context) (reconciler.OperationRes opsToCreate := []*v1alpha1.Operation{} opsNumToCreate := keepAliveCount - len(ownedOps.Items) for range opsNumToCreate { - opName := fmt.Sprintf("cached-operation-%s-%s", c.cache.Status.CacheKey[:8], strings.ToLower(ctrlutils.GenerateRandomString(5))) + opName := fmt.Sprintf("cached-operation-%s-%s", c.cache.Status.CacheKey[:8], strings.ToLower(randutils.GenerateRandomString(5))) opToCreate := c.initOperationFromCache(opName) if err := c.setControllerReferenceFunc(c.cache, opToCreate, c.scheme); err != nil { return reconciler.RequeueWithError(err) diff --git a/internal/controller/cache_adapter_test.go b/internal/handler/cache_test.go similarity index 92% rename from internal/controller/cache_adapter_test.go rename to internal/handler/cache_test.go index d5c16bf..16670fc 100644 --- a/internal/controller/cache_adapter_test.go +++ b/internal/handler/cache_test.go @@ -1,4 +1,4 @@ -package controller +package handler import ( "context" @@ -17,11 +17,12 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" "github.com/Azure/operation-cache-controller/api/v1alpha1" - mockpkg "github.com/Azure/operation-cache-controller/internal/mocks" ctrlutils "github.com/Azure/operation-cache-controller/internal/utils/controller" - oputils "github.com/Azure/operation-cache-controller/internal/utils/controller/operation" + mockpkg "github.com/Azure/operation-cache-controller/internal/utils/mocks" ) +var cacheHelper = ctrlutils.NewCacheHelper() + func TestNewCacheAdapter(t *testing.T) { t.Run("NewCacheAdapter", func(t *testing.T) { testCache := &v1alpha1.Cache{} @@ -35,7 +36,7 @@ func TestNewCacheAdapter(t *testing.T) { ) mockClient = mockpkg.NewMockClient(mockClientCtrl) mockRecorder = mockpkg.NewMockEventRecorder(mockRecorderCtrl) - adapter := NewCacheAdapter(context.Background(), testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) + adapter := NewCacheHandler(context.Background(), testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) assert.NotNil(t, adapter) }) } @@ -82,7 +83,7 @@ func TestCacheCheckCacheExpiry(t *testing.T) { }, Status: v1alpha1.CacheStatus{}, } - adapter := NewCacheAdapter(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) + adapter := NewCacheHandler(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) assert.NotNil(t, adapter) res, err := adapter.CheckCacheExpiry(ctx) @@ -101,7 +102,7 @@ func TestCacheCheckCacheExpiry(t *testing.T) { }, Status: v1alpha1.CacheStatus{}, } - adapter := NewCacheAdapter(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) + adapter := NewCacheHandler(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) assert.NotNil(t, adapter) mockClient.EXPECT().Delete(ctx, gomock.Any()).Return(nil) @@ -119,7 +120,7 @@ func TestCacheCheckCacheExpiry(t *testing.T) { Spec: v1alpha1.CacheSpec{}, Status: v1alpha1.CacheStatus{}, } - adapter := NewCacheAdapter(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) + adapter := NewCacheHandler(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) assert.NotNil(t, adapter) res, err := adapter.CheckCacheExpiry(ctx) @@ -141,7 +142,7 @@ func TestCacheCheckCacheExpiry(t *testing.T) { }, Status: v1alpha1.CacheStatus{}, } - adapter := NewCacheAdapter(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) + adapter := NewCacheHandler(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) assert.NotNil(t, adapter) res, err := adapter.CheckCacheExpiry(ctx) @@ -172,7 +173,7 @@ func TestCacheEnsureCacheInitialized(t *testing.T) { mockStatusWriter = mockpkg.NewMockStatusWriter(mockStatusWriterCtrl) testApps := getTestApps() - testCacheKey := ctrlutils.NewCacheKeyFromApplications(testApps) + testCacheKey := cacheHelper.NewCacheKeyFromApplications(testApps) t.Run("happy path", func(t *testing.T) { testCache := &v1alpha1.Cache{ @@ -187,7 +188,7 @@ func TestCacheEnsureCacheInitialized(t *testing.T) { }, Status: v1alpha1.CacheStatus{}, } - adapter := NewCacheAdapter(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) + adapter := NewCacheHandler(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) assert.NotNil(t, adapter) mockClient.EXPECT().Status().Return(mockStatusWriter) mockStatusWriter.EXPECT().Update(ctx, gomock.Any()).Return(nil) @@ -232,7 +233,7 @@ func TestCacheCalculateKeepAliveCount(t *testing.T) { }, Status: v1alpha1.CacheStatus{}, } - adapter := NewCacheAdapter(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) + adapter := NewCacheHandler(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) assert.NotNil(t, adapter) mockClient.EXPECT().Status().Return(mockStatusWriter) mockStatusWriter.EXPECT().Update(ctx, gomock.Any()).Return(nil) @@ -264,7 +265,7 @@ func TestCacheAdjustCache(t *testing.T) { mockStatusWriter = mockpkg.NewMockStatusWriter(mockStatusWriterCtrl) testApps := getTestApps() - testCacheKey := ctrlutils.NewCacheKeyFromApplications(testApps) + testCacheKey := cacheHelper.NewCacheKeyFromApplications(testApps) newOperation := &v1alpha1.Operation{ Spec: v1alpha1.OperationSpec{ @@ -276,7 +277,7 @@ func TestCacheAdjustCache(t *testing.T) { Labels: map[string]string{ctrlutils.LabelNameCacheKey: testCacheKey}, }, Status: v1alpha1.OperationStatus{ - Phase: oputils.PhaseEmpty, + Phase: v1alpha1.OperationPhaseEmpty, }, } @@ -290,7 +291,7 @@ func TestCacheAdjustCache(t *testing.T) { Labels: map[string]string{ctrlutils.LabelNameCacheKey: testCacheKey}, }, Status: v1alpha1.OperationStatus{ - Phase: oputils.PhaseReconciled, + Phase: v1alpha1.OperationPhaseReconciled, }, } // TODO assert operation status @@ -320,7 +321,7 @@ func TestCacheAdjustCache(t *testing.T) { KeepAliveCount: 2, }, } - adapter := NewCacheAdapter(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) + adapter := NewCacheHandler(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) assert.NotNil(t, adapter) mockClient.EXPECT().List(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).SetArg(1, resOperations).Return(nil) mockClient.EXPECT().Status().Return(mockStatusWriter) @@ -358,7 +359,7 @@ func TestCacheAdjustCache(t *testing.T) { KeepAliveCount: 2, }, } - adapter := NewCacheAdapter(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) + adapter := NewCacheHandler(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, ctrl.SetControllerReference) assert.NotNil(t, adapter) mockClient.EXPECT().List(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).SetArg(1, resOperations).Return(nil) mockClient.EXPECT().Delete(ctx, gomock.Any()).Return(nil).Times(3) @@ -393,7 +394,7 @@ func TestCacheAdjustCache(t *testing.T) { KeepAliveCount: 3, }, } - adapter := NewCacheAdapter(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, func(owner, controlled metav1.Object, scheme *runtime.Scheme, opts ...controllerutil.OwnerReferenceOption) error { + adapter := NewCacheHandler(ctx, testCache, testlogger, mockClient, scheme, mockRecorder, func(owner, controlled metav1.Object, scheme *runtime.Scheme, opts ...controllerutil.OwnerReferenceOption) error { return nil }) assert.NotNil(t, adapter) diff --git a/internal/controller/mocks/mock_appdeployment_adapter.go b/internal/handler/mocks/mock_appdeployment.go similarity index 62% rename from internal/controller/mocks/mock_appdeployment_adapter.go rename to internal/handler/mocks/mock_appdeployment.go index 0aae904..96c989f 100644 --- a/internal/controller/mocks/mock_appdeployment_adapter.go +++ b/internal/handler/mocks/mock_appdeployment.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/Azure/operation-cache-controller/internal/controller (interfaces: AppDeploymentAdapterInterface) +// Source: github.com/Azure/operation-cache-controller/internal/handler (interfaces: AppDeploymentHandlerInterface) // // Generated by this command: // -// mockgen -destination=./mocks/mock_appdeployment_adapter.go -package=mocks github.com/Azure/operation-cache-controller/internal/controller AppDeploymentAdapterInterface +// mockgen -destination=./mocks/mock_appdeployment.go -package=mocks github.com/Azure/operation-cache-controller/internal/handler AppDeploymentHandlerInterface // // Package mocks is a generated GoMock package. @@ -17,32 +17,32 @@ import ( gomock "go.uber.org/mock/gomock" ) -// MockAppDeploymentAdapterInterface is a mock of AppDeploymentAdapterInterface interface. -type MockAppDeploymentAdapterInterface struct { +// MockAppDeploymentHandlerInterface is a mock of AppDeploymentHandlerInterface interface. +type MockAppDeploymentHandlerInterface struct { ctrl *gomock.Controller - recorder *MockAppDeploymentAdapterInterfaceMockRecorder + recorder *MockAppDeploymentHandlerInterfaceMockRecorder isgomock struct{} } -// MockAppDeploymentAdapterInterfaceMockRecorder is the mock recorder for MockAppDeploymentAdapterInterface. -type MockAppDeploymentAdapterInterfaceMockRecorder struct { - mock *MockAppDeploymentAdapterInterface +// MockAppDeploymentHandlerInterfaceMockRecorder is the mock recorder for MockAppDeploymentHandlerInterface. +type MockAppDeploymentHandlerInterfaceMockRecorder struct { + mock *MockAppDeploymentHandlerInterface } -// NewMockAppDeploymentAdapterInterface creates a new mock instance. -func NewMockAppDeploymentAdapterInterface(ctrl *gomock.Controller) *MockAppDeploymentAdapterInterface { - mock := &MockAppDeploymentAdapterInterface{ctrl: ctrl} - mock.recorder = &MockAppDeploymentAdapterInterfaceMockRecorder{mock} +// NewMockAppDeploymentHandlerInterface creates a new mock instance. +func NewMockAppDeploymentHandlerInterface(ctrl *gomock.Controller) *MockAppDeploymentHandlerInterface { + mock := &MockAppDeploymentHandlerInterface{ctrl: ctrl} + mock.recorder = &MockAppDeploymentHandlerInterfaceMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockAppDeploymentAdapterInterface) EXPECT() *MockAppDeploymentAdapterInterfaceMockRecorder { +func (m *MockAppDeploymentHandlerInterface) EXPECT() *MockAppDeploymentHandlerInterfaceMockRecorder { return m.recorder } // EnsureApplicationValid mocks base method. -func (m *MockAppDeploymentAdapterInterface) EnsureApplicationValid(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockAppDeploymentHandlerInterface) EnsureApplicationValid(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureApplicationValid", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -51,13 +51,13 @@ func (m *MockAppDeploymentAdapterInterface) EnsureApplicationValid(ctx context.C } // EnsureApplicationValid indicates an expected call of EnsureApplicationValid. -func (mr *MockAppDeploymentAdapterInterfaceMockRecorder) EnsureApplicationValid(ctx any) *gomock.Call { +func (mr *MockAppDeploymentHandlerInterfaceMockRecorder) EnsureApplicationValid(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureApplicationValid", reflect.TypeOf((*MockAppDeploymentAdapterInterface)(nil).EnsureApplicationValid), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureApplicationValid", reflect.TypeOf((*MockAppDeploymentHandlerInterface)(nil).EnsureApplicationValid), ctx) } // EnsureDependenciesReady mocks base method. -func (m *MockAppDeploymentAdapterInterface) EnsureDependenciesReady(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockAppDeploymentHandlerInterface) EnsureDependenciesReady(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureDependenciesReady", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -66,13 +66,13 @@ func (m *MockAppDeploymentAdapterInterface) EnsureDependenciesReady(ctx context. } // EnsureDependenciesReady indicates an expected call of EnsureDependenciesReady. -func (mr *MockAppDeploymentAdapterInterfaceMockRecorder) EnsureDependenciesReady(ctx any) *gomock.Call { +func (mr *MockAppDeploymentHandlerInterfaceMockRecorder) EnsureDependenciesReady(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureDependenciesReady", reflect.TypeOf((*MockAppDeploymentAdapterInterface)(nil).EnsureDependenciesReady), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureDependenciesReady", reflect.TypeOf((*MockAppDeploymentHandlerInterface)(nil).EnsureDependenciesReady), ctx) } // EnsureDeployingFinished mocks base method. -func (m *MockAppDeploymentAdapterInterface) EnsureDeployingFinished(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockAppDeploymentHandlerInterface) EnsureDeployingFinished(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureDeployingFinished", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -81,13 +81,13 @@ func (m *MockAppDeploymentAdapterInterface) EnsureDeployingFinished(ctx context. } // EnsureDeployingFinished indicates an expected call of EnsureDeployingFinished. -func (mr *MockAppDeploymentAdapterInterfaceMockRecorder) EnsureDeployingFinished(ctx any) *gomock.Call { +func (mr *MockAppDeploymentHandlerInterfaceMockRecorder) EnsureDeployingFinished(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureDeployingFinished", reflect.TypeOf((*MockAppDeploymentAdapterInterface)(nil).EnsureDeployingFinished), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureDeployingFinished", reflect.TypeOf((*MockAppDeploymentHandlerInterface)(nil).EnsureDeployingFinished), ctx) } // EnsureFinalizer mocks base method. -func (m *MockAppDeploymentAdapterInterface) EnsureFinalizer(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockAppDeploymentHandlerInterface) EnsureFinalizer(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureFinalizer", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -96,13 +96,13 @@ func (m *MockAppDeploymentAdapterInterface) EnsureFinalizer(ctx context.Context) } // EnsureFinalizer indicates an expected call of EnsureFinalizer. -func (mr *MockAppDeploymentAdapterInterfaceMockRecorder) EnsureFinalizer(ctx any) *gomock.Call { +func (mr *MockAppDeploymentHandlerInterfaceMockRecorder) EnsureFinalizer(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureFinalizer", reflect.TypeOf((*MockAppDeploymentAdapterInterface)(nil).EnsureFinalizer), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureFinalizer", reflect.TypeOf((*MockAppDeploymentHandlerInterface)(nil).EnsureFinalizer), ctx) } // EnsureFinalizerDeleted mocks base method. -func (m *MockAppDeploymentAdapterInterface) EnsureFinalizerDeleted(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockAppDeploymentHandlerInterface) EnsureFinalizerDeleted(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureFinalizerDeleted", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -111,13 +111,13 @@ func (m *MockAppDeploymentAdapterInterface) EnsureFinalizerDeleted(ctx context.C } // EnsureFinalizerDeleted indicates an expected call of EnsureFinalizerDeleted. -func (mr *MockAppDeploymentAdapterInterfaceMockRecorder) EnsureFinalizerDeleted(ctx any) *gomock.Call { +func (mr *MockAppDeploymentHandlerInterfaceMockRecorder) EnsureFinalizerDeleted(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureFinalizerDeleted", reflect.TypeOf((*MockAppDeploymentAdapterInterface)(nil).EnsureFinalizerDeleted), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureFinalizerDeleted", reflect.TypeOf((*MockAppDeploymentHandlerInterface)(nil).EnsureFinalizerDeleted), ctx) } // EnsureTeardownFinished mocks base method. -func (m *MockAppDeploymentAdapterInterface) EnsureTeardownFinished(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockAppDeploymentHandlerInterface) EnsureTeardownFinished(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureTeardownFinished", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -126,7 +126,7 @@ func (m *MockAppDeploymentAdapterInterface) EnsureTeardownFinished(ctx context.C } // EnsureTeardownFinished indicates an expected call of EnsureTeardownFinished. -func (mr *MockAppDeploymentAdapterInterfaceMockRecorder) EnsureTeardownFinished(ctx any) *gomock.Call { +func (mr *MockAppDeploymentHandlerInterfaceMockRecorder) EnsureTeardownFinished(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureTeardownFinished", reflect.TypeOf((*MockAppDeploymentAdapterInterface)(nil).EnsureTeardownFinished), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureTeardownFinished", reflect.TypeOf((*MockAppDeploymentHandlerInterface)(nil).EnsureTeardownFinished), ctx) } diff --git a/internal/controller/mocks/mock_cache_adapter.go b/internal/handler/mocks/mock_cache.go similarity index 60% rename from internal/controller/mocks/mock_cache_adapter.go rename to internal/handler/mocks/mock_cache.go index 8888728..7c6686a 100644 --- a/internal/controller/mocks/mock_cache_adapter.go +++ b/internal/handler/mocks/mock_cache.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/Azure/operation-cache-controller/internal/controller (interfaces: CacheAdapterInterface) +// Source: github.com/Azure/operation-cache-controller/internal/handler (interfaces: CacheHandlerInterface) // // Generated by this command: // -// mockgen -destination=./mocks/mock_cache_adapter.go -package=mocks github.com/Azure/operation-cache-controller/internal/controller CacheAdapterInterface +// mockgen -destination=./mocks/mock_cache.go -package=mocks github.com/Azure/operation-cache-controller/internal/handler CacheHandlerInterface // // Package mocks is a generated GoMock package. @@ -17,32 +17,32 @@ import ( gomock "go.uber.org/mock/gomock" ) -// MockCacheAdapterInterface is a mock of CacheAdapterInterface interface. -type MockCacheAdapterInterface struct { +// MockCacheHandlerInterface is a mock of CacheHandlerInterface interface. +type MockCacheHandlerInterface struct { ctrl *gomock.Controller - recorder *MockCacheAdapterInterfaceMockRecorder + recorder *MockCacheHandlerInterfaceMockRecorder isgomock struct{} } -// MockCacheAdapterInterfaceMockRecorder is the mock recorder for MockCacheAdapterInterface. -type MockCacheAdapterInterfaceMockRecorder struct { - mock *MockCacheAdapterInterface +// MockCacheHandlerInterfaceMockRecorder is the mock recorder for MockCacheHandlerInterface. +type MockCacheHandlerInterfaceMockRecorder struct { + mock *MockCacheHandlerInterface } -// NewMockCacheAdapterInterface creates a new mock instance. -func NewMockCacheAdapterInterface(ctrl *gomock.Controller) *MockCacheAdapterInterface { - mock := &MockCacheAdapterInterface{ctrl: ctrl} - mock.recorder = &MockCacheAdapterInterfaceMockRecorder{mock} +// NewMockCacheHandlerInterface creates a new mock instance. +func NewMockCacheHandlerInterface(ctrl *gomock.Controller) *MockCacheHandlerInterface { + mock := &MockCacheHandlerInterface{ctrl: ctrl} + mock.recorder = &MockCacheHandlerInterfaceMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockCacheAdapterInterface) EXPECT() *MockCacheAdapterInterfaceMockRecorder { +func (m *MockCacheHandlerInterface) EXPECT() *MockCacheHandlerInterfaceMockRecorder { return m.recorder } // AdjustCache mocks base method. -func (m *MockCacheAdapterInterface) AdjustCache(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockCacheHandlerInterface) AdjustCache(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AdjustCache", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -51,13 +51,13 @@ func (m *MockCacheAdapterInterface) AdjustCache(ctx context.Context) (reconciler } // AdjustCache indicates an expected call of AdjustCache. -func (mr *MockCacheAdapterInterfaceMockRecorder) AdjustCache(ctx any) *gomock.Call { +func (mr *MockCacheHandlerInterfaceMockRecorder) AdjustCache(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AdjustCache", reflect.TypeOf((*MockCacheAdapterInterface)(nil).AdjustCache), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AdjustCache", reflect.TypeOf((*MockCacheHandlerInterface)(nil).AdjustCache), ctx) } // CalculateKeepAliveCount mocks base method. -func (m *MockCacheAdapterInterface) CalculateKeepAliveCount(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockCacheHandlerInterface) CalculateKeepAliveCount(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CalculateKeepAliveCount", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -66,13 +66,13 @@ func (m *MockCacheAdapterInterface) CalculateKeepAliveCount(ctx context.Context) } // CalculateKeepAliveCount indicates an expected call of CalculateKeepAliveCount. -func (mr *MockCacheAdapterInterfaceMockRecorder) CalculateKeepAliveCount(ctx any) *gomock.Call { +func (mr *MockCacheHandlerInterfaceMockRecorder) CalculateKeepAliveCount(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CalculateKeepAliveCount", reflect.TypeOf((*MockCacheAdapterInterface)(nil).CalculateKeepAliveCount), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CalculateKeepAliveCount", reflect.TypeOf((*MockCacheHandlerInterface)(nil).CalculateKeepAliveCount), ctx) } // CheckCacheExpiry mocks base method. -func (m *MockCacheAdapterInterface) CheckCacheExpiry(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockCacheHandlerInterface) CheckCacheExpiry(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CheckCacheExpiry", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -81,13 +81,13 @@ func (m *MockCacheAdapterInterface) CheckCacheExpiry(ctx context.Context) (recon } // CheckCacheExpiry indicates an expected call of CheckCacheExpiry. -func (mr *MockCacheAdapterInterfaceMockRecorder) CheckCacheExpiry(ctx any) *gomock.Call { +func (mr *MockCacheHandlerInterfaceMockRecorder) CheckCacheExpiry(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckCacheExpiry", reflect.TypeOf((*MockCacheAdapterInterface)(nil).CheckCacheExpiry), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CheckCacheExpiry", reflect.TypeOf((*MockCacheHandlerInterface)(nil).CheckCacheExpiry), ctx) } // EnsureCacheInitialized mocks base method. -func (m *MockCacheAdapterInterface) EnsureCacheInitialized(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockCacheHandlerInterface) EnsureCacheInitialized(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureCacheInitialized", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -96,7 +96,7 @@ func (m *MockCacheAdapterInterface) EnsureCacheInitialized(ctx context.Context) } // EnsureCacheInitialized indicates an expected call of EnsureCacheInitialized. -func (mr *MockCacheAdapterInterfaceMockRecorder) EnsureCacheInitialized(ctx any) *gomock.Call { +func (mr *MockCacheHandlerInterfaceMockRecorder) EnsureCacheInitialized(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureCacheInitialized", reflect.TypeOf((*MockCacheAdapterInterface)(nil).EnsureCacheInitialized), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureCacheInitialized", reflect.TypeOf((*MockCacheHandlerInterface)(nil).EnsureCacheInitialized), ctx) } diff --git a/internal/controller/mocks/mock_operation_adapter.go b/internal/handler/mocks/mock_operation.go similarity index 61% rename from internal/controller/mocks/mock_operation_adapter.go rename to internal/handler/mocks/mock_operation.go index 607d140..8de6443 100644 --- a/internal/controller/mocks/mock_operation_adapter.go +++ b/internal/handler/mocks/mock_operation.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/Azure/operation-cache-controller/internal/controller (interfaces: OperationAdapterInterface) +// Source: github.com/Azure/operation-cache-controller/internal/handler (interfaces: OperationHandlerInterface) // // Generated by this command: // -// mockgen -destination=./mocks/mock_operation_adapter.go -package=mocks github.com/Azure/operation-cache-controller/internal/controller OperationAdapterInterface +// mockgen -destination=./mocks/mock_operation.go -package=mocks github.com/Azure/operation-cache-controller/internal/handler OperationHandlerInterface // // Package mocks is a generated GoMock package. @@ -17,32 +17,32 @@ import ( gomock "go.uber.org/mock/gomock" ) -// MockOperationAdapterInterface is a mock of OperationAdapterInterface interface. -type MockOperationAdapterInterface struct { +// MockOperationHandlerInterface is a mock of OperationHandlerInterface interface. +type MockOperationHandlerInterface struct { ctrl *gomock.Controller - recorder *MockOperationAdapterInterfaceMockRecorder + recorder *MockOperationHandlerInterfaceMockRecorder isgomock struct{} } -// MockOperationAdapterInterfaceMockRecorder is the mock recorder for MockOperationAdapterInterface. -type MockOperationAdapterInterfaceMockRecorder struct { - mock *MockOperationAdapterInterface +// MockOperationHandlerInterfaceMockRecorder is the mock recorder for MockOperationHandlerInterface. +type MockOperationHandlerInterfaceMockRecorder struct { + mock *MockOperationHandlerInterface } -// NewMockOperationAdapterInterface creates a new mock instance. -func NewMockOperationAdapterInterface(ctrl *gomock.Controller) *MockOperationAdapterInterface { - mock := &MockOperationAdapterInterface{ctrl: ctrl} - mock.recorder = &MockOperationAdapterInterfaceMockRecorder{mock} +// NewMockOperationHandlerInterface creates a new mock instance. +func NewMockOperationHandlerInterface(ctrl *gomock.Controller) *MockOperationHandlerInterface { + mock := &MockOperationHandlerInterface{ctrl: ctrl} + mock.recorder = &MockOperationHandlerInterfaceMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockOperationAdapterInterface) EXPECT() *MockOperationAdapterInterfaceMockRecorder { +func (m *MockOperationHandlerInterface) EXPECT() *MockOperationHandlerInterfaceMockRecorder { return m.recorder } // EnsureAllAppsAreDeleted mocks base method. -func (m *MockOperationAdapterInterface) EnsureAllAppsAreDeleted(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockOperationHandlerInterface) EnsureAllAppsAreDeleted(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureAllAppsAreDeleted", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -51,13 +51,13 @@ func (m *MockOperationAdapterInterface) EnsureAllAppsAreDeleted(ctx context.Cont } // EnsureAllAppsAreDeleted indicates an expected call of EnsureAllAppsAreDeleted. -func (mr *MockOperationAdapterInterfaceMockRecorder) EnsureAllAppsAreDeleted(ctx any) *gomock.Call { +func (mr *MockOperationHandlerInterfaceMockRecorder) EnsureAllAppsAreDeleted(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureAllAppsAreDeleted", reflect.TypeOf((*MockOperationAdapterInterface)(nil).EnsureAllAppsAreDeleted), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureAllAppsAreDeleted", reflect.TypeOf((*MockOperationHandlerInterface)(nil).EnsureAllAppsAreDeleted), ctx) } // EnsureAllAppsAreReady mocks base method. -func (m *MockOperationAdapterInterface) EnsureAllAppsAreReady(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockOperationHandlerInterface) EnsureAllAppsAreReady(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureAllAppsAreReady", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -66,13 +66,13 @@ func (m *MockOperationAdapterInterface) EnsureAllAppsAreReady(ctx context.Contex } // EnsureAllAppsAreReady indicates an expected call of EnsureAllAppsAreReady. -func (mr *MockOperationAdapterInterfaceMockRecorder) EnsureAllAppsAreReady(ctx any) *gomock.Call { +func (mr *MockOperationHandlerInterfaceMockRecorder) EnsureAllAppsAreReady(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureAllAppsAreReady", reflect.TypeOf((*MockOperationAdapterInterface)(nil).EnsureAllAppsAreReady), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureAllAppsAreReady", reflect.TypeOf((*MockOperationHandlerInterface)(nil).EnsureAllAppsAreReady), ctx) } // EnsureFinalizer mocks base method. -func (m *MockOperationAdapterInterface) EnsureFinalizer(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockOperationHandlerInterface) EnsureFinalizer(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureFinalizer", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -81,13 +81,13 @@ func (m *MockOperationAdapterInterface) EnsureFinalizer(ctx context.Context) (re } // EnsureFinalizer indicates an expected call of EnsureFinalizer. -func (mr *MockOperationAdapterInterfaceMockRecorder) EnsureFinalizer(ctx any) *gomock.Call { +func (mr *MockOperationHandlerInterfaceMockRecorder) EnsureFinalizer(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureFinalizer", reflect.TypeOf((*MockOperationAdapterInterface)(nil).EnsureFinalizer), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureFinalizer", reflect.TypeOf((*MockOperationHandlerInterface)(nil).EnsureFinalizer), ctx) } // EnsureFinalizerRemoved mocks base method. -func (m *MockOperationAdapterInterface) EnsureFinalizerRemoved(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockOperationHandlerInterface) EnsureFinalizerRemoved(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureFinalizerRemoved", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -96,13 +96,13 @@ func (m *MockOperationAdapterInterface) EnsureFinalizerRemoved(ctx context.Conte } // EnsureFinalizerRemoved indicates an expected call of EnsureFinalizerRemoved. -func (mr *MockOperationAdapterInterfaceMockRecorder) EnsureFinalizerRemoved(ctx any) *gomock.Call { +func (mr *MockOperationHandlerInterfaceMockRecorder) EnsureFinalizerRemoved(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureFinalizerRemoved", reflect.TypeOf((*MockOperationAdapterInterface)(nil).EnsureFinalizerRemoved), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureFinalizerRemoved", reflect.TypeOf((*MockOperationHandlerInterface)(nil).EnsureFinalizerRemoved), ctx) } // EnsureNotExpired mocks base method. -func (m *MockOperationAdapterInterface) EnsureNotExpired(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockOperationHandlerInterface) EnsureNotExpired(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureNotExpired", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -111,7 +111,7 @@ func (m *MockOperationAdapterInterface) EnsureNotExpired(ctx context.Context) (r } // EnsureNotExpired indicates an expected call of EnsureNotExpired. -func (mr *MockOperationAdapterInterfaceMockRecorder) EnsureNotExpired(ctx any) *gomock.Call { +func (mr *MockOperationHandlerInterfaceMockRecorder) EnsureNotExpired(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureNotExpired", reflect.TypeOf((*MockOperationAdapterInterface)(nil).EnsureNotExpired), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureNotExpired", reflect.TypeOf((*MockOperationHandlerInterface)(nil).EnsureNotExpired), ctx) } diff --git a/internal/controller/mocks/mock_requirement_adapter.go b/internal/handler/mocks/mock_requirement.go similarity index 61% rename from internal/controller/mocks/mock_requirement_adapter.go rename to internal/handler/mocks/mock_requirement.go index cd5daff..7f6c56f 100644 --- a/internal/controller/mocks/mock_requirement_adapter.go +++ b/internal/handler/mocks/mock_requirement.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/Azure/operation-cache-controller/internal/controller (interfaces: RequirementAdapterInterface) +// Source: github.com/Azure/operation-cache-controller/internal/handler (interfaces: RequirementHandlerInterface) // // Generated by this command: // -// mockgen -destination=./mocks/mock_requirement_adapter.go -package=mocks github.com/Azure/operation-cache-controller/internal/controller RequirementAdapterInterface +// mockgen -destination=./mocks/mock_requirement.go -package=mocks github.com/Azure/operation-cache-controller/internal/handler RequirementHandlerInterface // // Package mocks is a generated GoMock package. @@ -17,32 +17,32 @@ import ( gomock "go.uber.org/mock/gomock" ) -// MockRequirementAdapterInterface is a mock of RequirementAdapterInterface interface. -type MockRequirementAdapterInterface struct { +// MockRequirementHandlerInterface is a mock of RequirementHandlerInterface interface. +type MockRequirementHandlerInterface struct { ctrl *gomock.Controller - recorder *MockRequirementAdapterInterfaceMockRecorder + recorder *MockRequirementHandlerInterfaceMockRecorder isgomock struct{} } -// MockRequirementAdapterInterfaceMockRecorder is the mock recorder for MockRequirementAdapterInterface. -type MockRequirementAdapterInterfaceMockRecorder struct { - mock *MockRequirementAdapterInterface +// MockRequirementHandlerInterfaceMockRecorder is the mock recorder for MockRequirementHandlerInterface. +type MockRequirementHandlerInterfaceMockRecorder struct { + mock *MockRequirementHandlerInterface } -// NewMockRequirementAdapterInterface creates a new mock instance. -func NewMockRequirementAdapterInterface(ctrl *gomock.Controller) *MockRequirementAdapterInterface { - mock := &MockRequirementAdapterInterface{ctrl: ctrl} - mock.recorder = &MockRequirementAdapterInterfaceMockRecorder{mock} +// NewMockRequirementHandlerInterface creates a new mock instance. +func NewMockRequirementHandlerInterface(ctrl *gomock.Controller) *MockRequirementHandlerInterface { + mock := &MockRequirementHandlerInterface{ctrl: ctrl} + mock.recorder = &MockRequirementHandlerInterfaceMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockRequirementAdapterInterface) EXPECT() *MockRequirementAdapterInterfaceMockRecorder { +func (m *MockRequirementHandlerInterface) EXPECT() *MockRequirementHandlerInterfaceMockRecorder { return m.recorder } // EnsureCacheExisted mocks base method. -func (m *MockRequirementAdapterInterface) EnsureCacheExisted(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockRequirementHandlerInterface) EnsureCacheExisted(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureCacheExisted", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -51,13 +51,13 @@ func (m *MockRequirementAdapterInterface) EnsureCacheExisted(ctx context.Context } // EnsureCacheExisted indicates an expected call of EnsureCacheExisted. -func (mr *MockRequirementAdapterInterfaceMockRecorder) EnsureCacheExisted(ctx any) *gomock.Call { +func (mr *MockRequirementHandlerInterfaceMockRecorder) EnsureCacheExisted(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureCacheExisted", reflect.TypeOf((*MockRequirementAdapterInterface)(nil).EnsureCacheExisted), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureCacheExisted", reflect.TypeOf((*MockRequirementHandlerInterface)(nil).EnsureCacheExisted), ctx) } // EnsureCachedOperationAcquired mocks base method. -func (m *MockRequirementAdapterInterface) EnsureCachedOperationAcquired(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockRequirementHandlerInterface) EnsureCachedOperationAcquired(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureCachedOperationAcquired", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -66,13 +66,13 @@ func (m *MockRequirementAdapterInterface) EnsureCachedOperationAcquired(ctx cont } // EnsureCachedOperationAcquired indicates an expected call of EnsureCachedOperationAcquired. -func (mr *MockRequirementAdapterInterfaceMockRecorder) EnsureCachedOperationAcquired(ctx any) *gomock.Call { +func (mr *MockRequirementHandlerInterfaceMockRecorder) EnsureCachedOperationAcquired(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureCachedOperationAcquired", reflect.TypeOf((*MockRequirementAdapterInterface)(nil).EnsureCachedOperationAcquired), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureCachedOperationAcquired", reflect.TypeOf((*MockRequirementHandlerInterface)(nil).EnsureCachedOperationAcquired), ctx) } // EnsureInitialized mocks base method. -func (m *MockRequirementAdapterInterface) EnsureInitialized(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockRequirementHandlerInterface) EnsureInitialized(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureInitialized", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -81,13 +81,13 @@ func (m *MockRequirementAdapterInterface) EnsureInitialized(ctx context.Context) } // EnsureInitialized indicates an expected call of EnsureInitialized. -func (mr *MockRequirementAdapterInterfaceMockRecorder) EnsureInitialized(ctx any) *gomock.Call { +func (mr *MockRequirementHandlerInterfaceMockRecorder) EnsureInitialized(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureInitialized", reflect.TypeOf((*MockRequirementAdapterInterface)(nil).EnsureInitialized), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureInitialized", reflect.TypeOf((*MockRequirementHandlerInterface)(nil).EnsureInitialized), ctx) } // EnsureNotExpired mocks base method. -func (m *MockRequirementAdapterInterface) EnsureNotExpired(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockRequirementHandlerInterface) EnsureNotExpired(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureNotExpired", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -96,13 +96,13 @@ func (m *MockRequirementAdapterInterface) EnsureNotExpired(ctx context.Context) } // EnsureNotExpired indicates an expected call of EnsureNotExpired. -func (mr *MockRequirementAdapterInterfaceMockRecorder) EnsureNotExpired(ctx any) *gomock.Call { +func (mr *MockRequirementHandlerInterfaceMockRecorder) EnsureNotExpired(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureNotExpired", reflect.TypeOf((*MockRequirementAdapterInterface)(nil).EnsureNotExpired), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureNotExpired", reflect.TypeOf((*MockRequirementHandlerInterface)(nil).EnsureNotExpired), ctx) } // EnsureOperationReady mocks base method. -func (m *MockRequirementAdapterInterface) EnsureOperationReady(ctx context.Context) (reconciler.OperationResult, error) { +func (m *MockRequirementHandlerInterface) EnsureOperationReady(ctx context.Context) (reconciler.OperationResult, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "EnsureOperationReady", ctx) ret0, _ := ret[0].(reconciler.OperationResult) @@ -111,7 +111,7 @@ func (m *MockRequirementAdapterInterface) EnsureOperationReady(ctx context.Conte } // EnsureOperationReady indicates an expected call of EnsureOperationReady. -func (mr *MockRequirementAdapterInterfaceMockRecorder) EnsureOperationReady(ctx any) *gomock.Call { +func (mr *MockRequirementHandlerInterfaceMockRecorder) EnsureOperationReady(ctx any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureOperationReady", reflect.TypeOf((*MockRequirementAdapterInterface)(nil).EnsureOperationReady), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EnsureOperationReady", reflect.TypeOf((*MockRequirementHandlerInterface)(nil).EnsureOperationReady), ctx) } diff --git a/internal/controller/operation_adapter.go b/internal/handler/operation.go similarity index 73% rename from internal/controller/operation_adapter.go rename to internal/handler/operation.go index 678a2fb..b1772b0 100644 --- a/internal/controller/operation_adapter.go +++ b/internal/handler/operation.go @@ -1,4 +1,4 @@ -package controller +package handler import ( "context" @@ -15,15 +15,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/Azure/operation-cache-controller/api/v1alpha1" - ctlrutils "github.com/Azure/operation-cache-controller/internal/utils/controller" - apdutils "github.com/Azure/operation-cache-controller/internal/utils/controller/appdeployment" - oputils "github.com/Azure/operation-cache-controller/internal/utils/controller/operation" + ctrlutils "github.com/Azure/operation-cache-controller/internal/utils/controller" ) -type operationAdapterContextKey struct{} +type OperationContextKey struct{} -//go:generate mockgen -destination=./mocks/mock_operation_adapter.go -package=mocks github.com/Azure/operation-cache-controller/internal/controller OperationAdapterInterface -type OperationAdapterInterface interface { +//go:generate mockgen -destination=./mocks/mock_operation.go -package=mocks github.com/Azure/operation-cache-controller/internal/handler OperationHandlerInterface +type OperationHandlerInterface interface { EnsureNotExpired(ctx context.Context) (reconciler.OperationResult, error) EnsureFinalizer(ctx context.Context) (reconciler.OperationResult, error) EnsureFinalizerRemoved(ctx context.Context) (reconciler.OperationResult, error) @@ -31,27 +29,34 @@ type OperationAdapterInterface interface { EnsureAllAppsAreDeleted(ctx context.Context) (reconciler.OperationResult, error) } -type OperationAdapter struct { +type OperationHandler struct { operation *v1alpha1.Operation logger logr.Logger client client.Client recorder record.EventRecorder + + apdutils ctrlutils.AppDeploymentHelper + oputils ctrlutils.OperationHelper + cacheutils ctrlutils.CacheHelper } -func NewOperationAdapter(ctx context.Context, operation *v1alpha1.Operation, logger logr.Logger, client client.Client, recorder record.EventRecorder) OperationAdapterInterface { - if operationAdapter, ok := ctx.Value(operationAdapterContextKey{}).(OperationAdapterInterface); ok { - return operationAdapter +func NewOperationHandler(ctx context.Context, operation *v1alpha1.Operation, logger logr.Logger, client client.Client, recorder record.EventRecorder) OperationHandlerInterface { + if operationHandler, ok := ctx.Value(OperationContextKey{}).(OperationHandlerInterface); ok { + return operationHandler } - return &OperationAdapter{ + return &OperationHandler{ operation: operation, logger: logger, client: client, recorder: recorder, + + apdutils: ctrlutils.NewAppDeploymentHelper(), + oputils: ctrlutils.NewOperationHelper(), } } -func (o *OperationAdapter) phaseIn(phases ...string) bool { +func (o *OperationHandler) phaseIn(phases ...string) bool { for _, phase := range phases { if phase == o.operation.Status.Phase { @@ -61,12 +66,12 @@ func (o *OperationAdapter) phaseIn(phases ...string) bool { return false } -func (o *OperationAdapter) EnsureNotExpired(ctx context.Context) (reconciler.OperationResult, error) { +func (o *OperationHandler) EnsureNotExpired(ctx context.Context) (reconciler.OperationResult, error) { o.logger.V(1).Info("Operation EnsureNotExpired") if len(o.operation.Spec.ExpireAt) == 0 { return reconciler.ContinueProcessing() } - if o.phaseIn(oputils.PhaseDeleted, oputils.PhaseDeleting) { + if o.phaseIn(v1alpha1.OperationPhaseDeleted, v1alpha1.OperationPhaseDeleting) { return reconciler.ContinueProcessing() } expireTime, err := time.Parse(time.RFC3339, o.operation.Spec.ExpireAt) @@ -89,17 +94,17 @@ func (o *OperationAdapter) EnsureNotExpired(ctx context.Context) (reconciler.Ope return reconciler.ContinueProcessing() } -func (o *OperationAdapter) EnsureAllAppsAreReady(ctx context.Context) (reconciler.OperationResult, error) { +func (o *OperationHandler) EnsureAllAppsAreReady(ctx context.Context) (reconciler.OperationResult, error) { o.logger.V(1).Info("Operation EnsureAllAppsAreReady") - if o.phaseIn(oputils.PhaseDeleted, oputils.PhaseDeleting) { + if o.phaseIn(v1alpha1.OperationPhaseDeleted, v1alpha1.OperationPhaseDeleting) { return reconciler.ContinueProcessing() } - if o.phaseIn(oputils.PhaseEmpty) { + if o.phaseIn(v1alpha1.OperationPhaseEmpty) { o.logger.V(1).Info("initializing operation status") - oputils.ClearConditions(o.operation) - o.operation.Status.OperationID = oputils.NewOperationId() + o.oputils.ClearConditions(o.operation) + o.operation.Status.OperationID = o.oputils.NewOperationId() } - if o.phaseIn(oputils.PhaseReconciling) { + if o.phaseIn(v1alpha1.OperationPhaseReconciling) { err := o.reconcilingApplications(ctx) if err != nil { o.logger.Error(err, "reconciling applications failed") @@ -107,55 +112,55 @@ func (o *OperationAdapter) EnsureAllAppsAreReady(ctx context.Context) (reconcile return reconciler.RequeueWithError(err) } - o.operation.Status.Phase = oputils.PhaseReconciled + o.operation.Status.Phase = v1alpha1.OperationPhaseReconciled return reconciler.RequeueOnErrorOrStop(o.client.Status().Update(ctx, o.operation)) } // check the diff between the expected and actual apps, set phase to reconciling and requeue if changes - expectedCacheKey := ctlrutils.NewCacheKeyFromApplications(o.operation.Spec.Applications) + expectedCacheKey := o.cacheutils.NewCacheKeyFromApplications(o.operation.Spec.Applications) if o.operation.Status.CacheKey != expectedCacheKey { o.operation.Status.CacheKey = expectedCacheKey - o.operation.Status.Phase = oputils.PhaseReconciling + o.operation.Status.Phase = v1alpha1.OperationPhaseReconciling } return reconciler.RequeueOnErrorOrContinue(o.client.Status().Update(ctx, o.operation)) } -func (o *OperationAdapter) EnsureFinalizer(ctx context.Context) (reconciler.OperationResult, error) { +func (o *OperationHandler) EnsureFinalizer(ctx context.Context) (reconciler.OperationResult, error) { o.logger.V(1).Info("operation EnsureFinalizer") - if o.operation.ObjectMeta.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(o.operation, oputils.FinalizerName) { - controllerutil.AddFinalizer(o.operation, oputils.FinalizerName) + if o.operation.ObjectMeta.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(o.operation, v1alpha1.OperationFinalizerName) { + controllerutil.AddFinalizer(o.operation, v1alpha1.OperationFinalizerName) } return reconciler.RequeueOnErrorOrContinue(o.client.Update(ctx, o.operation)) } -func (o *OperationAdapter) EnsureFinalizerRemoved(ctx context.Context) (reconciler.OperationResult, error) { +func (o *OperationHandler) EnsureFinalizerRemoved(ctx context.Context) (reconciler.OperationResult, error) { o.logger.V(1).Info("operation EnsureFinalizerDeleted") - if !o.operation.ObjectMeta.DeletionTimestamp.IsZero() && controllerutil.ContainsFinalizer(o.operation, oputils.FinalizerName) { - if o.phaseIn(oputils.PhaseDeleted) { + if !o.operation.ObjectMeta.DeletionTimestamp.IsZero() && controllerutil.ContainsFinalizer(o.operation, v1alpha1.OperationFinalizerName) { + if o.phaseIn(v1alpha1.OperationPhaseDeleted) { o.logger.V(1).Info("All app deleted removing finalizer") - controllerutil.RemoveFinalizer(o.operation, oputils.FinalizerName) + controllerutil.RemoveFinalizer(o.operation, v1alpha1.OperationFinalizerName) return reconciler.RequeueOnErrorOrContinue(o.client.Update(ctx, o.operation)) } - if !o.phaseIn(oputils.PhaseDeleting) { + if !o.phaseIn(v1alpha1.OperationPhaseDeleting) { o.logger.V(1).Info("App is not deleted yet, setting phase to deleting") - o.operation.Status.Phase = oputils.PhaseDeleting + o.operation.Status.Phase = v1alpha1.OperationPhaseDeleting return reconciler.RequeueOnErrorOrContinue(o.client.Status().Update(ctx, o.operation)) } } return reconciler.ContinueProcessing() } -func (o *OperationAdapter) EnsureAllAppsAreDeleted(ctx context.Context) (reconciler.OperationResult, error) { +func (o *OperationHandler) EnsureAllAppsAreDeleted(ctx context.Context) (reconciler.OperationResult, error) { o.logger.V(1).Info("Operation EnsureAllAppsAreDeleted") - if o.phaseIn(oputils.PhaseDeleting) { + if o.phaseIn(v1alpha1.OperationPhaseDeleting) { // deleting logic here - o.operation.Status.Phase = oputils.PhaseDeleted + o.operation.Status.Phase = v1alpha1.OperationPhaseDeleted return reconciler.RequeueOnErrorOrStop(o.client.Status().Update(ctx, o.operation)) } return reconciler.ContinueProcessing() } -func (o *OperationAdapter) reconcilingApplications(ctx context.Context) error { +func (o *OperationHandler) reconcilingApplications(ctx context.Context) error { logger := o.logger.WithValues("operation", "reconcilingApplications") currentAppDeployments, err := o.listCurrentAppDeployments(ctx) if err != nil { @@ -172,7 +177,7 @@ func (o *OperationAdapter) reconcilingApplications(ctx context.Context) error { logger.V(1).Info("expected app deployment", "appName", app.Name, "opId", app.Spec.OpId, "provision", app.Spec.Provision, "teardown", app.Spec.Teardown, "dependencies", app.Spec.Dependencies) } - added, removed, updated := oputils.DiffAppDeployments(expectedAppDeployments, currentAppDeployments, oputils.CompareProvisionJobs) + added, removed, updated := o.oputils.DiffAppDeployments(expectedAppDeployments, currentAppDeployments, o.oputils.CompareProvisionJobs) for _, app := range added { logger.V(1).Info(fmt.Sprintf("app to be added %s", app.Name), "opId", app.Spec.OpId, "provision", app.Spec.Provision, "teardown", app.Spec.Teardown, "dependencies", app.Spec.Dependencies) if err := ctrl.SetControllerReference(o.operation, &app, o.client.Scheme()); err != nil { @@ -204,7 +209,7 @@ func (o *OperationAdapter) reconcilingApplications(ctx context.Context) error { return fmt.Errorf("failed to get app deployment: %w", err) } // check if all dependencies are ready - if appdeployment.Status.Phase != apdutils.PhaseReady { + if appdeployment.Status.Phase != v1alpha1.AppDeploymentPhaseReady { return fmt.Errorf("app deployment is not ready: name %s, status, %s", app.Name, app.Status.Phase) } } @@ -212,11 +217,11 @@ func (o *OperationAdapter) reconcilingApplications(ctx context.Context) error { return nil } -func (o *OperationAdapter) expectedAppDeployments() []v1alpha1.AppDeployment { +func (o *OperationHandler) expectedAppDeployments() []v1alpha1.AppDeployment { return lo.Map(o.operation.Spec.Applications, func(app v1alpha1.ApplicationSpec, index int) v1alpha1.AppDeployment { return v1alpha1.AppDeployment{ ObjectMeta: metav1.ObjectMeta{ - Name: apdutils.OperationScopedAppDeployment(app.Name, o.operation.Status.OperationID), + Name: ctrlutils.OperationScopedAppDeployment(app.Name, o.operation.Status.OperationID), Namespace: o.operation.Namespace, }, Spec: v1alpha1.AppDeploymentSpec{ @@ -229,9 +234,9 @@ func (o *OperationAdapter) expectedAppDeployments() []v1alpha1.AppDeployment { }) } -func (o *OperationAdapter) listCurrentAppDeployments(ctx context.Context) ([]v1alpha1.AppDeployment, error) { +func (o *OperationHandler) listCurrentAppDeployments(ctx context.Context) ([]v1alpha1.AppDeployment, error) { appDeploymentList := &v1alpha1.AppDeploymentList{} - if err := o.client.List(ctx, appDeploymentList, client.MatchingFields{operationOwnerKey: o.operation.Name}); err != nil { + if err := o.client.List(ctx, appDeploymentList, client.MatchingFields{v1alpha1.OperationOwnerKey: o.operation.Name}); err != nil { return nil, fmt.Errorf("failed to list appDeployments: %w", err) } return lo.Map(appDeploymentList.Items, func(app v1alpha1.AppDeployment, index int) v1alpha1.AppDeployment { diff --git a/internal/controller/operation_adapter_test.go b/internal/handler/operation_test.go similarity index 83% rename from internal/controller/operation_adapter_test.go rename to internal/handler/operation_test.go index a57783e..a95c00c 100644 --- a/internal/controller/operation_adapter_test.go +++ b/internal/handler/operation_test.go @@ -1,4 +1,4 @@ -package controller +package handler import ( "context" @@ -6,9 +6,7 @@ import ( "time" "github.com/Azure/operation-cache-controller/api/v1alpha1" - mockpkg "github.com/Azure/operation-cache-controller/internal/mocks" - adputils "github.com/Azure/operation-cache-controller/internal/utils/controller/appdeployment" - oputils "github.com/Azure/operation-cache-controller/internal/utils/controller/operation" + mockpkg "github.com/Azure/operation-cache-controller/internal/utils/mocks" "github.com/Azure/operation-cache-controller/internal/utils/reconciler" "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" @@ -61,7 +59,7 @@ var ( Dependencies: []string{"test-app2"}, }, Status: v1alpha1.AppDeploymentStatus{ - Phase: adputils.PhaseReady, + Phase: v1alpha1.AppDeploymentPhaseReady, }, }, { @@ -75,7 +73,7 @@ var ( Teardown: newTestJobSpec(), }, Status: v1alpha1.AppDeploymentStatus{ - Phase: adputils.PhaseReady, + Phase: v1alpha1.AppDeploymentPhaseReady, }, }, }, @@ -112,7 +110,7 @@ var ( Teardown: newTestJobSpec(), }, Status: v1alpha1.AppDeploymentStatus{ - Phase: adputils.PhaseReady, + Phase: v1alpha1.AppDeploymentPhaseReady, }, }, { @@ -126,14 +124,14 @@ var ( Teardown: newTestJobSpec(), }, Status: v1alpha1.AppDeploymentStatus{ - Phase: adputils.PhaseReady, + Phase: v1alpha1.AppDeploymentPhaseReady, }, }, }, } ) -func TestNewOperationAdapter(t *testing.T) { +func TestNewOperationHandler(t *testing.T) { ctx := context.Background() logger := log.FromContext(ctx) @@ -143,11 +141,11 @@ func TestNewOperationAdapter(t *testing.T) { mockRecorder := mockpkg.NewMockEventRecorder(mockRecorderCtrl) operation := emptyOperation.DeepCopy() - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) assert.NotNil(t, adapter) } -func TestOperationAdapter_EnsureNotExpired(t *testing.T) { +func TestOperationHandler_EnsureNotExpired(t *testing.T) { ctx := context.Background() logger := log.FromContext(ctx) @@ -159,7 +157,7 @@ func TestOperationAdapter_EnsureNotExpired(t *testing.T) { t.Run("happy path: continue processing when expire is not set", func(t *testing.T) { operation := validOperation.DeepCopy() operation.Spec.ExpireAt = time.Now().Add(time.Hour).Format(time.RFC3339) - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) res, err := adapter.EnsureNotExpired(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{}, res) @@ -167,15 +165,15 @@ func TestOperationAdapter_EnsureNotExpired(t *testing.T) { t.Run("happy path: continue processing when operation is in deleting phase", func(t *testing.T) { operation := validOperation.DeepCopy() operation.Spec.ExpireAt = time.Now().Add(time.Hour).Format(time.RFC3339) - operation.Status.Phase = oputils.PhaseDeleting + operation.Status.Phase = v1alpha1.OperationPhaseDeleting - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) res, err := adapter.EnsureNotExpired(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{}, res) - operation.Status.Phase = oputils.PhaseDeleted - adapter = NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + operation.Status.Phase = v1alpha1.OperationPhaseDeleted + adapter = NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) res, err = adapter.EnsureNotExpired(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{}, res) @@ -183,7 +181,7 @@ func TestOperationAdapter_EnsureNotExpired(t *testing.T) { t.Run("happy path: continue processing when expire time is in the future", func(t *testing.T) { operation := validOperation.DeepCopy() operation.Spec.ExpireAt = time.Now().Add(time.Hour).Format(time.RFC3339) - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) res, err := adapter.EnsureNotExpired(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{}, res) @@ -193,7 +191,7 @@ func TestOperationAdapter_EnsureNotExpired(t *testing.T) { operation.Spec.ExpireAt = "invalid-time" mockRecorder.EXPECT().Event(operation, "Warning", "InvalidExpireTime", "Failed to parse expire time") - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) res, err := adapter.EnsureNotExpired(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{}, res) @@ -205,7 +203,7 @@ func TestOperationAdapter_EnsureNotExpired(t *testing.T) { mockClient.EXPECT().Delete(ctx, operation, gomock.Any()).Return(nil) - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) res, err := adapter.EnsureNotExpired(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{}, res) @@ -218,14 +216,14 @@ func TestOperationAdapter_EnsureNotExpired(t *testing.T) { mockRecorder.EXPECT().Event(operation, "Warning", "DeleteFailed", "Failed to delete expired operation") - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) res, err := adapter.EnsureNotExpired(ctx) assert.Error(t, err) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, res) }) } -func TestOperationAdapter_EnsureAllAppsAreReady(t *testing.T) { +func TestOperationHandler_EnsureAllAppsAreReady(t *testing.T) { ctx := context.Background() logger := log.FromContext(ctx) @@ -236,15 +234,15 @@ func TestOperationAdapter_EnsureAllAppsAreReady(t *testing.T) { mockRecorder := mockpkg.NewMockEventRecorder(mockRecorderCtrl) operation := validOperation.DeepCopy() - operation.Status.Phase = oputils.PhaseDeleting + operation.Status.Phase = v1alpha1.OperationPhaseDeleting - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) res, err := adapter.EnsureAllAppsAreReady(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{}, res) - operation.Status.Phase = oputils.PhaseDeleted - adapter = NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + operation.Status.Phase = v1alpha1.OperationPhaseDeleted + adapter = NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) res, err = adapter.EnsureAllAppsAreReady(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{}, res) @@ -261,12 +259,12 @@ func TestOperationAdapter_EnsureAllAppsAreReady(t *testing.T) { operation := emptyOperation.DeepCopy() mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) res, err := adapter.EnsureAllAppsAreReady(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay}, res) assert.NotEmpty(t, operation.Status.CacheKey) - assert.Equal(t, operation.Status.Phase, oputils.PhaseReconciling) + assert.Equal(t, operation.Status.Phase, v1alpha1.OperationPhaseReconciling) }) t.Run("happy path: continue processing when operation is in reconciling phase", func(t *testing.T) { @@ -276,7 +274,7 @@ func TestOperationAdapter_EnsureAllAppsAreReady(t *testing.T) { mockRecorder := mockpkg.NewMockEventRecorder(mockRecorderCtrl) operation := validOperation.DeepCopy() - operation.Status.Phase = oputils.PhaseReconciling + operation.Status.Phase = v1alpha1.OperationPhaseReconciling appList := emptyAppDeploymentList.DeepCopy() mockClient.EXPECT().List(ctx, appList, gomock.Any()).DoAndReturn(func(ctx context.Context, list *v1alpha1.AppDeploymentList, opts ...interface{}) error { @@ -295,11 +293,11 @@ func TestOperationAdapter_EnsureAllAppsAreReady(t *testing.T) { mockClient.EXPECT().Update(ctx, gomock.Any()).Return(nil) mockRecorder.EXPECT().Event(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) res, err := adapter.EnsureAllAppsAreReady(ctx) assert.ErrorContains(t, err, "app deployment is not ready") assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, res) - assert.Equal(t, operation.Status.Phase, oputils.PhaseReconciling) + assert.Equal(t, operation.Status.Phase, v1alpha1.OperationPhaseReconciling) }) @@ -314,7 +312,7 @@ func TestOperationAdapter_EnsureAllAppsAreReady(t *testing.T) { // set operation to reconciling phase operation := validOperation.DeepCopy() - operation.Status.Phase = oputils.PhaseReconciling + operation.Status.Phase = v1alpha1.OperationPhaseReconciling appList := emptyAppDeploymentList.DeepCopy() mockClient.EXPECT().List(ctx, appList, gomock.Any()).DoAndReturn(func(ctx context.Context, list *v1alpha1.AppDeploymentList, opts ...interface{}) error { @@ -324,23 +322,23 @@ func TestOperationAdapter_EnsureAllAppsAreReady(t *testing.T) { scheme := runtime.NewScheme() mockClient.EXPECT().Scheme().Return(scheme).AnyTimes() readyAppDeployment := &v1alpha1.AppDeployment{} - readyAppDeployment.Status.Phase = adputils.PhaseReady + readyAppDeployment.Status.Phase = v1alpha1.AppDeploymentPhaseReady mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, key client.ObjectKey, obj runtime.Object, opt ...interface{}) error { *obj.(*v1alpha1.AppDeployment) = *readyAppDeployment return nil }).AnyTimes() mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) res, err := adapter.EnsureAllAppsAreReady(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, CancelRequest: true}, res) - assert.Equal(t, operation.Status.Phase, oputils.PhaseReconciled) + assert.Equal(t, operation.Status.Phase, v1alpha1.OperationPhaseReconciled) }) } -func TestOperationAdapter_EnsureFinalizer(t *testing.T) { +func TestOperationHandler_EnsureFinalizer(t *testing.T) { ctx := context.Background() logger := log.FromContext(ctx) @@ -352,7 +350,7 @@ func TestOperationAdapter_EnsureFinalizer(t *testing.T) { t.Run("happy path: continue processing when finalizer is not set", func(t *testing.T) { operation := validOperation.DeepCopy() operation.ObjectMeta.Finalizers = nil - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) mockClient.EXPECT().Update(ctx, operation).Return(nil) res, err := adapter.EnsureFinalizer(ctx) @@ -361,7 +359,7 @@ func TestOperationAdapter_EnsureFinalizer(t *testing.T) { }) } -func TestOperationAdapter_EnsureFinalizerRemoved(t *testing.T) { +func TestOperationHandler_EnsureFinalizerRemoved(t *testing.T) { ctx := context.Background() logger := log.FromContext(ctx) @@ -376,7 +374,7 @@ func TestOperationAdapter_EnsureFinalizerRemoved(t *testing.T) { t.Run("happy path: continue processing when finalizer is not set", func(t *testing.T) { operation := validOperation.DeepCopy() operation.ObjectMeta.Finalizers = nil - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) res, err := adapter.EnsureFinalizerRemoved(ctx) assert.NoError(t, err) @@ -386,10 +384,10 @@ func TestOperationAdapter_EnsureFinalizerRemoved(t *testing.T) { t.Run("happy path: continue processing when operation is in deleted phase", func(t *testing.T) { operation := validOperation.DeepCopy() - operation.ObjectMeta.Finalizers = []string{oputils.FinalizerName} + operation.ObjectMeta.Finalizers = []string{v1alpha1.OperationFinalizerName} operation.ObjectMeta.DeletionTimestamp = &metav1.Time{Time: time.Now()} - operation.Status.Phase = oputils.PhaseDeleted - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + operation.Status.Phase = v1alpha1.OperationPhaseDeleted + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) mockClient.EXPECT().Update(ctx, operation).Return(nil) @@ -401,9 +399,9 @@ func TestOperationAdapter_EnsureFinalizerRemoved(t *testing.T) { t.Run("happy path: continue processing when operation is not in deleting phase", func(t *testing.T) { operation := validOperation.DeepCopy() - operation.ObjectMeta.Finalizers = []string{oputils.FinalizerName} + operation.ObjectMeta.Finalizers = []string{v1alpha1.OperationFinalizerName} operation.ObjectMeta.DeletionTimestamp = &metav1.Time{Time: time.Now()} - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) res, err := adapter.EnsureFinalizerRemoved(ctx) @@ -412,7 +410,7 @@ func TestOperationAdapter_EnsureFinalizerRemoved(t *testing.T) { }) } -func TestOperationAdapter_EnsureAllAppsAreDeleted(t *testing.T) { +func TestOperationHandler_EnsureAllAppsAreDeleted(t *testing.T) { ctx := context.Background() logger := log.FromContext(ctx) @@ -426,21 +424,21 @@ func TestOperationAdapter_EnsureAllAppsAreDeleted(t *testing.T) { t.Run("happy path: continue processing when operation is in deleting phase", func(t *testing.T) { operation := validOperation.DeepCopy() - operation.Status.Phase = oputils.PhaseDeleting - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + operation.Status.Phase = v1alpha1.OperationPhaseDeleting + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) res, err := adapter.EnsureAllAppsAreDeleted(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, CancelRequest: true}, res) - assert.Equal(t, operation.Status.Phase, oputils.PhaseDeleted) + assert.Equal(t, operation.Status.Phase, v1alpha1.OperationPhaseDeleted) }) t.Run("happy path: continue processing when operation is in empty phase", func(t *testing.T) { operation := emptyOperation.DeepCopy() - adapter := NewOperationAdapter(ctx, operation, logger, mockClient, mockRecorder) + adapter := NewOperationHandler(ctx, operation, logger, mockClient, mockRecorder) res, err := adapter.EnsureAllAppsAreDeleted(ctx) assert.NoError(t, err) diff --git a/internal/controller/requirement_adapter.go b/internal/handler/requirement.go similarity index 70% rename from internal/controller/requirement_adapter.go rename to internal/handler/requirement.go index 1201f8d..a8ef720 100644 --- a/internal/controller/requirement_adapter.go +++ b/internal/handler/requirement.go @@ -1,4 +1,4 @@ -package controller +package handler import ( "context" @@ -15,17 +15,14 @@ import ( "github.com/Azure/operation-cache-controller/api/v1alpha1" ctlutils "github.com/Azure/operation-cache-controller/internal/utils/controller" - cacheutils "github.com/Azure/operation-cache-controller/internal/utils/controller/cache" - oputils "github.com/Azure/operation-cache-controller/internal/utils/controller/operation" - rqutils "github.com/Azure/operation-cache-controller/internal/utils/controller/requirement" "github.com/Azure/operation-cache-controller/internal/utils/ptr" "github.com/Azure/operation-cache-controller/internal/utils/reconciler" ) -type requirementAdapterContextKey struct{} +type RequiremenContextKey struct{} -//go:generate mockgen -destination=./mocks/mock_requirement_adapter.go -package=mocks github.com/Azure/operation-cache-controller/internal/controller RequirementAdapterInterface -type RequirementAdapterInterface interface { +//go:generate mockgen -destination=./mocks/mock_requirement.go -package=mocks github.com/Azure/operation-cache-controller/internal/handler RequirementHandlerInterface +type RequirementHandlerInterface interface { EnsureNotExpired(ctx context.Context) (reconciler.OperationResult, error) EnsureInitialized(ctx context.Context) (reconciler.OperationResult, error) EnsureCacheExisted(ctx context.Context) (reconciler.OperationResult, error) @@ -33,27 +30,35 @@ type RequirementAdapterInterface interface { EnsureOperationReady(ctx context.Context) (reconciler.OperationResult, error) } -type RequirementAdapter struct { +type RequirementHandler struct { requirement *v1alpha1.Requirement logger logr.Logger client client.Client recorder record.EventRecorder + + cacheutils ctlutils.CacheHelper + oputils ctlutils.OperationHelper + rqutils ctlutils.RequirementHelper } -func NewRequirementAdapter(ctx context.Context, requirement *v1alpha1.Requirement, logger logr.Logger, client client.Client, recorder record.EventRecorder) RequirementAdapterInterface { - if requirementAdapter, ok := ctx.Value(requirementAdapterContextKey{}).(RequirementAdapterInterface); ok { - return requirementAdapter +func NewRequirementHandler(ctx context.Context, requirement *v1alpha1.Requirement, logger logr.Logger, client client.Client, recorder record.EventRecorder) RequirementHandlerInterface { + if requirementHandler, ok := ctx.Value(RequiremenContextKey{}).(RequirementHandlerInterface); ok { + return requirementHandler } - return &RequirementAdapter{ + return &RequirementHandler{ requirement: requirement, logger: logger, client: client, recorder: recorder, + + cacheutils: ctlutils.NewCacheHelper(), + oputils: ctlutils.NewOperationHelper(), + rqutils: ctlutils.NewRequirementHelper(), } } -func (o *RequirementAdapter) phaseIn(phases ...string) bool { +func (o *RequirementHandler) phaseIn(phases ...string) bool { for _, phase := range phases { if phase == o.requirement.Status.Phase { @@ -63,7 +68,7 @@ func (o *RequirementAdapter) phaseIn(phases ...string) bool { return false } -func (r *RequirementAdapter) EnsureNotExpired(ctx context.Context) (reconciler.OperationResult, error) { +func (r *RequirementHandler) EnsureNotExpired(ctx context.Context) (reconciler.OperationResult, error) { r.logger.V(1).Info("operation: EnsureNotExpired") if len(r.requirement.Spec.ExpireAt) == 0 { return reconciler.ContinueProcessing() @@ -88,22 +93,22 @@ func (r *RequirementAdapter) EnsureNotExpired(ctx context.Context) (reconciler.O return reconciler.ContinueProcessing() } -func (r *RequirementAdapter) EnsureInitialized(ctx context.Context) (reconciler.OperationResult, error) { +func (r *RequirementHandler) EnsureInitialized(ctx context.Context) (reconciler.OperationResult, error) { r.logger.V(1).Info("operation: EnsureInitialized") - if !r.phaseIn(rqutils.PhaseEmpty) { + if !r.phaseIn(v1alpha1.RequirementPhaseEmpty) { return reconciler.ContinueProcessing() } - r.requirement.Status.CacheKey = ctlutils.NewCacheKeyFromApplications(r.requirement.Spec.Template.Applications) - rqutils.ClearConditions(r.requirement) + r.requirement.Status.CacheKey = r.cacheutils.NewCacheKeyFromApplications(r.requirement.Spec.Template.Applications) + r.rqutils.ClearConditions(r.requirement) if r.requirement.Spec.EnableCache { - r.requirement.Status.Phase = rqutils.PhaseCacheChecking + r.requirement.Status.Phase = v1alpha1.RequirementPhaseCacheChecking } else { - r.requirement.Status.Phase = rqutils.PhaseOperating + r.requirement.Status.Phase = v1alpha1.RequirementPhaseOperating } return reconciler.RequeueOnErrorOrContinue(r.client.Status().Update(ctx, r.requirement)) } -func (r *RequirementAdapter) ownerReference() metav1.OwnerReference { +func (r *RequirementHandler) ownerReference() metav1.OwnerReference { return metav1.OwnerReference{ APIVersion: r.requirement.APIVersion, Kind: r.requirement.Kind, @@ -114,27 +119,27 @@ func (r *RequirementAdapter) ownerReference() metav1.OwnerReference { } } -func (r *RequirementAdapter) setCacheNotExistedStatus() { - r.requirement.Status.Phase = rqutils.PhaseOperating - _ = rqutils.UpdateCondition(r.requirement, rqutils.ConditionCacheResourceFound, metav1.ConditionFalse, rqutils.ConditionReasonCacheCRFound, "Cache CR found") +func (r *RequirementHandler) setCacheNotExistedStatus() { + r.requirement.Status.Phase = v1alpha1.RequirementPhaseOperating + _ = r.rqutils.UpdateCondition(r.requirement, v1alpha1.RequirementConditionCacheResourceFound, metav1.ConditionFalse, v1alpha1.RequirementConditionReasonCacheCRFound, "Cache CR found") } -func (r *RequirementAdapter) setCacheHitStatus() { - r.requirement.Status.Phase = rqutils.PhaseReady - _ = rqutils.UpdateCondition(r.requirement, rqutils.ConditionOperationReady, metav1.ConditionTrue, rqutils.ConditionReasonCacheHit, "Cached Operation acquired") +func (r *RequirementHandler) setCacheHitStatus() { + r.requirement.Status.Phase = v1alpha1.RequirementPhaseReady + _ = r.rqutils.UpdateCondition(r.requirement, v1alpha1.RequirementConditionOperationReady, metav1.ConditionTrue, v1alpha1.RequirementConditionReasonCacheHit, "Cached Operation acquired") } -func (r *RequirementAdapter) setCacheMissStatus() { - r.requirement.Status.Phase = rqutils.PhaseOperating - _ = rqutils.UpdateCondition(r.requirement, rqutils.ConditionCachedOperationAcquired, metav1.ConditionTrue, rqutils.ConditionReasonCacheMiss, "No cached operation available") +func (r *RequirementHandler) setCacheMissStatus() { + r.requirement.Status.Phase = v1alpha1.RequirementPhaseOperating + _ = r.rqutils.UpdateCondition(r.requirement, v1alpha1.RequirementConditionCachedOperationAcquired, metav1.ConditionTrue, v1alpha1.RequirementConditionReasonCacheMiss, "No cached operation available") } -func (r *RequirementAdapter) defaultCacheName() string { +func (r *RequirementHandler) defaultCacheName() string { return fmt.Sprintf("cache-%s", r.requirement.Status.CacheKey) } -func (r *RequirementAdapter) EnsureCacheExisted(ctx context.Context) (reconciler.OperationResult, error) { - if !r.phaseIn(rqutils.PhaseCacheChecking) { +func (r *RequirementHandler) EnsureCacheExisted(ctx context.Context) (reconciler.OperationResult, error) { + if !r.phaseIn(v1alpha1.RequirementPhaseCacheChecking) { return reconciler.ContinueProcessing() } @@ -161,7 +166,7 @@ func (r *RequirementAdapter) EnsureCacheExisted(ctx context.Context) (reconciler cache.Namespace = r.requirement.Namespace cache.Spec = v1alpha1.CacheSpec{ OperationTemplate: r.requirement.Spec.Template, - ExpireTime: cacheutils.DefaultCacheExpireTime(), + ExpireTime: r.cacheutils.DefaultCacheExpireTime(), } err = r.client.Create(ctx, cache) if err != nil { @@ -171,14 +176,14 @@ func (r *RequirementAdapter) EnsureCacheExisted(ctx context.Context) (reconciler return reconciler.RequeueOnErrorOrContinue(r.client.Status().Update(ctx, r.requirement)) } // extend cache expire time every time when cache is checked - cache.Spec.ExpireTime = cacheutils.DefaultCacheExpireTime() + cache.Spec.ExpireTime = r.cacheutils.DefaultCacheExpireTime() _ = r.client.Update(ctx, cache) - r.requirement.Status.OperationName = cacheutils.RandomSelectCachedOperation(cache) + r.requirement.Status.OperationName = r.cacheutils.RandomSelectCachedOperation(cache) return reconciler.RequeueOnErrorOrContinue(r.client.Status().Update(ctx, r.requirement)) } -func (r *RequirementAdapter) EnsureCachedOperationAcquired(ctx context.Context) (reconciler.OperationResult, error) { - if !r.phaseIn(rqutils.PhaseCacheChecking) { +func (r *RequirementHandler) EnsureCachedOperationAcquired(ctx context.Context) (reconciler.OperationResult, error) { + if !r.phaseIn(v1alpha1.RequirementPhaseCacheChecking) { return reconciler.ContinueProcessing() } r.logger.V(1).Info("operation: EnsureCachedOperationAcquired") @@ -193,7 +198,7 @@ func (r *RequirementAdapter) EnsureCachedOperationAcquired(ctx context.Context) return reconciler.RequeueOnErrorOrContinue(fmt.Errorf("failed to get operation %s: %w", r.requirement.Status.OperationName, err)) } // already acquired - if _, ok := operation.Annotations[oputils.AcquiredAnnotationKey]; ok { + if _, ok := operation.Annotations[v1alpha1.OperationAcquiredAnnotationKey]; ok { if len(operation.OwnerReferences) != 0 { if operation.OwnerReferences[0].UID != r.requirement.UID { // return error if owner is not this requirement @@ -218,13 +223,13 @@ func (r *RequirementAdapter) EnsureCachedOperationAcquired(ctx context.Context) return reconciler.RequeueOnErrorOrContinue(r.client.Status().Update(ctx, r.requirement)) } -func (r *RequirementAdapter) acquireCachedOperation(ctx context.Context, operation *v1alpha1.Operation) error { - operation.Annotations[oputils.AcquiredAnnotationKey] = time.Now().Format(time.RFC3339) +func (r *RequirementHandler) acquireCachedOperation(ctx context.Context, operation *v1alpha1.Operation) error { + operation.Annotations[v1alpha1.OperationAcquiredAnnotationKey] = time.Now().Format(time.RFC3339) operation.OwnerReferences = []metav1.OwnerReference{r.ownerReference()} return r.client.Update(ctx, operation) } -func (r *RequirementAdapter) getOperation() (*v1alpha1.Operation, error) { +func (r *RequirementHandler) getOperation() (*v1alpha1.Operation, error) { namespacedName := types.NamespacedName{ Name: r.requirement.Status.OperationName, Namespace: r.requirement.Namespace, @@ -237,7 +242,7 @@ func (r *RequirementAdapter) getOperation() (*v1alpha1.Operation, error) { return operation, nil } -func (r *RequirementAdapter) updateOperation() error { +func (r *RequirementHandler) updateOperation() error { op, err := r.getOperation() if err != nil { return err @@ -246,7 +251,7 @@ func (r *RequirementAdapter) updateOperation() error { return r.client.Update(context.Background(), op) } -func (r *RequirementAdapter) createOperation() error { +func (r *RequirementHandler) createOperation() error { operation := &v1alpha1.Operation{ ObjectMeta: metav1.ObjectMeta{ Name: r.requirement.Status.OperationName, @@ -260,35 +265,35 @@ func (r *RequirementAdapter) createOperation() error { return r.client.Create(context.Background(), operation) } -func (r *RequirementAdapter) EnsureOperationReady(ctx context.Context) (reconciler.OperationResult, error) { +func (r *RequirementHandler) EnsureOperationReady(ctx context.Context) (reconciler.OperationResult, error) { r.logger.V(1).Info("operation: EnsureOperationReady") - if r.phaseIn(rqutils.PhaseReady) { + if r.phaseIn(v1alpha1.RequirementPhaseReady) { // check if application changed - cacheKey := ctlutils.NewCacheKeyFromApplications(r.requirement.Spec.Template.Applications) + cacheKey := r.cacheutils.NewCacheKeyFromApplications(r.requirement.Spec.Template.Applications) if r.requirement.Status.CacheKey != cacheKey { r.logger.Info("application changed, updating operation", "oldCacheKey", r.requirement.Status.CacheKey, "newCacheKey", cacheKey) if err := r.updateOperation(); err != nil { return reconciler.RequeueWithError(err) } r.requirement.Status.CacheKey = cacheKey - r.requirement.Status.Phase = rqutils.PhaseOperating + r.requirement.Status.Phase = v1alpha1.RequirementPhaseOperating return reconciler.RequeueOnErrorOrContinue(r.client.Status().Update(ctx, r.requirement)) } return reconciler.ContinueProcessing() } - if !r.phaseIn(rqutils.PhaseOperating) { + if !r.phaseIn(v1alpha1.RequirementPhaseOperating) { return reconciler.ContinueProcessing() } - if rqutils.IsCacheMissed(r.requirement) { + if r.rqutils.IsCacheMissed(r.requirement) { r.logger.V(1).Info("cache missed, creating operation") r.requirement.Status.OperationName = r.requirement.Name + "-" + "operation" } // check operation status if op, err := r.getOperation(); err == nil { r.logger.V(1).Info("requirement operation found", "operation", op.Name) - if op.Status.Phase == oputils.PhaseReconciled { + if op.Status.Phase == v1alpha1.OperationPhaseReconciled { r.logger.Info("operation is reconciled, set requirement to ready", "operationName", op.Name, "operationId", op.Status.OperationID) - r.requirement.Status.Phase = rqutils.PhaseReady + r.requirement.Status.Phase = v1alpha1.RequirementPhaseReady r.requirement.Status.OperationId = op.Status.OperationID return reconciler.RequeueOnErrorOrContinue(r.client.Status().Update(ctx, r.requirement)) } diff --git a/internal/controller/requirement_adapter_test.go b/internal/handler/requirement_test.go similarity index 79% rename from internal/controller/requirement_adapter_test.go rename to internal/handler/requirement_test.go index 8ab3abb..476b910 100644 --- a/internal/controller/requirement_adapter_test.go +++ b/internal/handler/requirement_test.go @@ -1,4 +1,4 @@ -package controller +package handler import ( "context" @@ -17,14 +17,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" "github.com/Azure/operation-cache-controller/api/v1alpha1" - mockpkg "github.com/Azure/operation-cache-controller/internal/mocks" ctlutils "github.com/Azure/operation-cache-controller/internal/utils/controller" - oputils "github.com/Azure/operation-cache-controller/internal/utils/controller/operation" - rqutils "github.com/Azure/operation-cache-controller/internal/utils/controller/requirement" + mockpkg "github.com/Azure/operation-cache-controller/internal/utils/mocks" "github.com/Azure/operation-cache-controller/internal/utils/reconciler" ) var ( + cacheutils = ctlutils.NewCacheHelper() testOperationName = "test-operation" emptyRequirement = &v1alpha1.Requirement{} validRequirement = &v1alpha1.Requirement{ @@ -55,7 +54,7 @@ var ( } ) -func TestNewRequirementAdapter(t *testing.T) { +func TestNewRequirementHandler(t *testing.T) { t.Run("When creating a new Requirement Adapter", func(t *testing.T) { ctx := context.Background() logger := log.FromContext(ctx) @@ -66,7 +65,7 @@ func TestNewRequirementAdapter(t *testing.T) { mockRecorder := mockpkg.NewMockEventRecorder(mockRecorderCtrl) requirement := emptyRequirement.DeepCopy() - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) require.NotNil(t, adapter) }) } @@ -82,7 +81,7 @@ func TestRequirementAdapter_EnsureNotExpired(t *testing.T) { t.Run("happy path: continue processing when expire is not set", func(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.Spec.ExpireAt = time.Now().Add(time.Hour).Format(time.RFC3339) - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureNotExpired(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{}, res) @@ -91,7 +90,7 @@ func TestRequirementAdapter_EnsureNotExpired(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.Spec.ExpireAt = time.Now().Add(time.Hour).Format(time.RFC3339) - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureNotExpired(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{}, res) @@ -102,7 +101,7 @@ func TestRequirementAdapter_EnsureNotExpired(t *testing.T) { requirement.Spec.ExpireAt = "invalid-time" mockRecorder.EXPECT().Event(requirement, "Warning", "InvalidExpireTime", "Failed to parse expire time") - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureNotExpired(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{}, res) @@ -113,7 +112,7 @@ func TestRequirementAdapter_EnsureNotExpired(t *testing.T) { requirement.Spec.ExpireAt = time.Now().Add(-time.Hour).Format(time.RFC3339) mockClient.EXPECT().Delete(ctx, requirement, gomock.Any()).Return(nil) - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureNotExpired(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{}, res) @@ -125,7 +124,7 @@ func TestRequirementAdapter_EnsureNotExpired(t *testing.T) { mockClient.EXPECT().Delete(ctx, requirement, gomock.Any()).Return(assert.AnError) mockRecorder.EXPECT().Event(requirement, "Warning", "DeleteFailed", "Failed to delete expired requirement") - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureNotExpired(ctx) assert.Error(t, err) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, res) @@ -146,31 +145,31 @@ func TestRequirementAdapter_EnsureInitialized(t *testing.T) { t.Run("happy path: continue processing when requirement is in empty phase and cache disabled", func(t *testing.T) { requirement := validRequirement.DeepCopy() - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) res, err := adapter.EnsureInitialized(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay}, res) - assert.Equal(t, rqutils.PhaseOperating, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseOperating, requirement.Status.Phase) }) t.Run("happy path: continue processing when requirement is in empty phase and cache enabled", func(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.Spec.EnableCache = true - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) res, err := adapter.EnsureInitialized(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay}, res) - assert.Equal(t, rqutils.PhaseCacheChecking, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseCacheChecking, requirement.Status.Phase) }) t.Run("happy path: continue processing requirement is not in empty phase", func(t *testing.T) { requirement := validRequirement.DeepCopy() - requirement.Status.Phase = rqutils.PhaseOperating - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + requirement.Status.Phase = v1alpha1.RequirementPhaseOperating + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureInitialized(ctx) assert.NoError(t, err) @@ -192,7 +191,7 @@ func TestRequirementAdapter_EnsureCacheExisted(t *testing.T) { t.Run("happy path: continue processing when cache is not enabled", func(t *testing.T) { requirement := validRequirement.DeepCopy() - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureCacheExisted(ctx) assert.NoError(t, err) @@ -201,22 +200,22 @@ func TestRequirementAdapter_EnsureCacheExisted(t *testing.T) { t.Run("happy path: continue processing when candidate operation exist", func(t *testing.T) { requirement := validRequirement.DeepCopy() - requirement.Status.Phase = rqutils.PhaseCacheChecking + requirement.Status.Phase = v1alpha1.RequirementPhaseCacheChecking requirement.Status.OperationName = testOperationName - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureCacheExisted(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{}, res) - assert.Equal(t, rqutils.PhaseCacheChecking, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseCacheChecking, requirement.Status.Phase) }) t.Run("happy path: when get a candidate operation", func(t *testing.T) { requirement := validRequirement.DeepCopy() - requirement.Status.Phase = rqutils.PhaseCacheChecking - requirement.Status.CacheKey = ctlutils.NewCacheKeyFromApplications(requirement.Spec.Template.Applications) + requirement.Status.Phase = v1alpha1.RequirementPhaseCacheChecking + requirement.Status.CacheKey = cacheutils.NewCacheKeyFromApplications(requirement.Spec.Template.Applications) - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) cache := validCache.DeepCopy() mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(cache), gomock.Any()).DoAndReturn(func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { @@ -229,14 +228,14 @@ func TestRequirementAdapter_EnsureCacheExisted(t *testing.T) { res, err := adapter.EnsureCacheExisted(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay}, res) - assert.Equal(t, rqutils.PhaseCacheChecking, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseCacheChecking, requirement.Status.Phase) }) t.Run("sad path: cache key is not set", func(t *testing.T) { requirement := validRequirement.DeepCopy() - requirement.Status.Phase = rqutils.PhaseCacheChecking + requirement.Status.Phase = v1alpha1.RequirementPhaseCacheChecking requirement.Status.CacheKey = "" - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureCacheExisted(ctx) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, res) assert.ErrorContains(t, err, "empty cache key") @@ -244,10 +243,10 @@ func TestRequirementAdapter_EnsureCacheExisted(t *testing.T) { t.Run("sad path: failed to get cache", func(t *testing.T) { requirement := validRequirement.DeepCopy() - requirement.Status.Phase = rqutils.PhaseCacheChecking - requirement.Status.CacheKey = ctlutils.NewCacheKeyFromApplications(requirement.Spec.Template.Applications) + requirement.Status.Phase = v1alpha1.RequirementPhaseCacheChecking + requirement.Status.CacheKey = cacheutils.NewCacheKeyFromApplications(requirement.Spec.Template.Applications) - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) cache := validCache.DeepCopy() mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(cache), gomock.Any()).Return(assert.AnError) @@ -259,10 +258,10 @@ func TestRequirementAdapter_EnsureCacheExisted(t *testing.T) { t.Run("happy path: cache not found, create a new cache", func(t *testing.T) { requirement := validRequirement.DeepCopy() - requirement.Status.Phase = rqutils.PhaseCacheChecking - requirement.Status.CacheKey = ctlutils.NewCacheKeyFromApplications(requirement.Spec.Template.Applications) + requirement.Status.Phase = v1alpha1.RequirementPhaseCacheChecking + requirement.Status.CacheKey = cacheutils.NewCacheKeyFromApplications(requirement.Spec.Template.Applications) errCacheNotFound := apierrors.NewNotFound(schema.GroupResource{Group: "appsv1", Resource: "Cache"}, "cache not found") - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(errCacheNotFound) mockClient.EXPECT().Create(ctx, gomock.Any()).Return(nil) @@ -270,15 +269,15 @@ func TestRequirementAdapter_EnsureCacheExisted(t *testing.T) { res, err := adapter.EnsureCacheExisted(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay}, res) - assert.Equal(t, rqutils.PhaseOperating, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseOperating, requirement.Status.Phase) }) t.Run("happy path: cache is not available", func(t *testing.T) { requirement := validRequirement.DeepCopy() - requirement.Status.Phase = rqutils.PhaseCacheChecking - requirement.Status.CacheKey = ctlutils.NewCacheKeyFromApplications(requirement.Spec.Template.Applications) + requirement.Status.Phase = v1alpha1.RequirementPhaseCacheChecking + requirement.Status.CacheKey = cacheutils.NewCacheKeyFromApplications(requirement.Spec.Template.Applications) - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) cache := validCache.DeepCopy() cache.Status.AvailableCaches = nil @@ -310,7 +309,7 @@ func TestRequirementAdapter_EnsureCachedOperationAcquired(t *testing.T) { t.Run("happy path: continue processing when not in cache checking phase", func(t *testing.T) { requirement := validRequirement.DeepCopy() - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureCachedOperationAcquired(ctx) assert.NoError(t, err) @@ -320,24 +319,24 @@ func TestRequirementAdapter_EnsureCachedOperationAcquired(t *testing.T) { t.Run("happy path: continue processing when operation is not set", func(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.Status.OperationName = "" - requirement.Status.Phase = rqutils.PhaseCacheChecking + requirement.Status.Phase = v1alpha1.RequirementPhaseCacheChecking - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) res, err := adapter.EnsureCachedOperationAcquired(ctx) assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay}, res) - assert.Equal(t, rqutils.PhaseOperating, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseOperating, requirement.Status.Phase) }) t.Run("happy path: continue processing when operation is already acquired", func(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.UID = testRequirementUID requirement.Status.OperationName = testOperationName - requirement.Status.Phase = rqutils.PhaseCacheChecking + requirement.Status.Phase = v1alpha1.RequirementPhaseCacheChecking operation := validOperation.DeepCopy() operation.Annotations = map[string]string{ - oputils.AcquiredAnnotationKey: "2021-09-01T00:00:00Z", + v1alpha1.OperationAcquiredAnnotationKey: "2021-09-01T00:00:00Z", } operation.OwnerReferences = []metav1.OwnerReference{ { @@ -354,11 +353,11 @@ func TestRequirementAdapter_EnsureCachedOperationAcquired(t *testing.T) { }) mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureCachedOperationAcquired(ctx) assert.NoError(t, err) - assert.Equal(t, rqutils.PhaseReady, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseReady, requirement.Status.Phase) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, CancelRequest: true}, res) }) @@ -366,10 +365,10 @@ func TestRequirementAdapter_EnsureCachedOperationAcquired(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.UID = testRequirementUID requirement.Status.OperationName = testOperationName - requirement.Status.Phase = rqutils.PhaseCacheChecking + requirement.Status.Phase = v1alpha1.RequirementPhaseCacheChecking operation := validOperation.DeepCopy() operation.Annotations = map[string]string{ - oputils.AcquiredAnnotationKey: "2021-09-01T00:00:00Z", + v1alpha1.OperationAcquiredAnnotationKey: "2021-09-01T00:00:00Z", } operation.OwnerReferences = []metav1.OwnerReference{ { @@ -386,11 +385,11 @@ func TestRequirementAdapter_EnsureCachedOperationAcquired(t *testing.T) { }) mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureCachedOperationAcquired(ctx) assert.NoError(t, err) - assert.Equal(t, rqutils.PhaseOperating, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseOperating, requirement.Status.Phase) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay}, res) }) @@ -398,8 +397,8 @@ func TestRequirementAdapter_EnsureCachedOperationAcquired(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.UID = testRequirementUID requirement.Status.OperationName = testOperationName - requirement.Status.Phase = rqutils.PhaseCacheChecking - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + requirement.Status.Phase = v1alpha1.RequirementPhaseCacheChecking + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) operation := validOperation.DeepCopy() operation.Annotations = map[string]string{} @@ -414,21 +413,21 @@ func TestRequirementAdapter_EnsureCachedOperationAcquired(t *testing.T) { assert.NoError(t, err) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay}, res) assert.Equal(t, operation.Name, requirement.Status.OperationName) - assert.Equal(t, rqutils.PhaseReady, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseReady, requirement.Status.Phase) }) t.Run("sad path: failed to get operation", func(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.UID = testRequirementUID requirement.Status.OperationName = testOperationName - requirement.Status.Phase = rqutils.PhaseCacheChecking - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + requirement.Status.Phase = v1alpha1.RequirementPhaseCacheChecking + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&v1alpha1.Operation{}), gomock.Any()).Return(assert.AnError) res, err := adapter.EnsureCachedOperationAcquired(ctx) assert.ErrorIs(t, err, assert.AnError) - assert.Equal(t, rqutils.PhaseOperating, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseOperating, requirement.Status.Phase) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay}, res) }) @@ -436,8 +435,8 @@ func TestRequirementAdapter_EnsureCachedOperationAcquired(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.UID = testRequirementUID requirement.Status.OperationName = testOperationName - requirement.Status.Phase = rqutils.PhaseCacheChecking - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + requirement.Status.Phase = v1alpha1.RequirementPhaseCacheChecking + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) operation := validOperation.DeepCopy() operation.Annotations = map[string]string{} @@ -450,7 +449,7 @@ func TestRequirementAdapter_EnsureCachedOperationAcquired(t *testing.T) { res, err := adapter.EnsureCachedOperationAcquired(ctx) assert.Error(t, err) assert.Equal(t, requirement.Status.OperationName, operation.Name) - assert.Equal(t, rqutils.PhaseOperating, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseOperating, requirement.Status.Phase) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay}, res) }) } @@ -469,7 +468,7 @@ func TestRequirementAdapter_EnsureOperationReady(t *testing.T) { t.Run("happy path: continue processing when not in ready and operating phase", func(t *testing.T) { requirement := validRequirement.DeepCopy() - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureOperationReady(ctx) assert.NoError(t, err) @@ -479,9 +478,9 @@ func TestRequirementAdapter_EnsureOperationReady(t *testing.T) { t.Run("happy path: continue processing when in ready phase but cachekey is not changed", func(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.Status.OperationName = testOperationName - requirement.Status.CacheKey = ctlutils.NewCacheKeyFromApplications(requirement.Spec.Template.Applications) - requirement.Status.Phase = rqutils.PhaseReady - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + requirement.Status.CacheKey = cacheutils.NewCacheKeyFromApplications(requirement.Spec.Template.Applications) + requirement.Status.Phase = v1alpha1.RequirementPhaseReady + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureOperationReady(ctx) assert.NoError(t, err) @@ -491,7 +490,7 @@ func TestRequirementAdapter_EnsureOperationReady(t *testing.T) { t.Run("happy path: ready phase but cachekey is changed", func(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.Status.OperationName = testOperationName - requirement.Status.Phase = rqutils.PhaseReady + requirement.Status.Phase = v1alpha1.RequirementPhaseReady requirement.Status.CacheKey = "test-cache-key" operaition := validOperation.DeepCopy() @@ -502,53 +501,53 @@ func TestRequirementAdapter_EnsureOperationReady(t *testing.T) { mockClient.EXPECT().Update(ctx, gomock.AssignableToTypeOf(&v1alpha1.Operation{})).Return(nil) mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureOperationReady(ctx) assert.NoError(t, err) - assert.Equal(t, rqutils.PhaseOperating, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseOperating, requirement.Status.Phase) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay}, res) }) t.Run("sad path: failed to get operation", func(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.Status.OperationName = testOperationName - requirement.Status.Phase = rqutils.PhaseReady - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + requirement.Status.Phase = v1alpha1.RequirementPhaseReady + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&v1alpha1.Operation{}), gomock.Any()).Return(assert.AnError) res, err := adapter.EnsureOperationReady(ctx) assert.ErrorIs(t, err, assert.AnError) - assert.Equal(t, rqutils.PhaseReady, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseReady, requirement.Status.Phase) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, res) }) t.Run("happy path: continue processing when operation is not ready", func(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.Status.OperationName = testOperationName - requirement.Status.Phase = rqutils.PhaseOperating + requirement.Status.Phase = v1alpha1.RequirementPhaseOperating operation := validOperation.DeepCopy() - operation.Status.Phase = oputils.PhaseReconciling + operation.Status.Phase = v1alpha1.OperationPhaseReconciling mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&v1alpha1.Operation{}), gomock.Any()).DoAndReturn(func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { *obj.(*v1alpha1.Operation) = *operation return nil }) - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureOperationReady(ctx) assert.NoError(t, err) - assert.Equal(t, rqutils.PhaseOperating, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseOperating, requirement.Status.Phase) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, res) }) t.Run("happy path: continue processing when operation is ready", func(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.Status.OperationName = testOperationName - requirement.Status.Phase = rqutils.PhaseOperating + requirement.Status.Phase = v1alpha1.RequirementPhaseOperating operation := validOperation.DeepCopy() - operation.Status.Phase = oputils.PhaseReconciled + operation.Status.Phase = v1alpha1.OperationPhaseReconciled mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&v1alpha1.Operation{}), gomock.Any()).DoAndReturn(func(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { *obj.(*v1alpha1.Operation) = *operation @@ -556,21 +555,21 @@ func TestRequirementAdapter_EnsureOperationReady(t *testing.T) { }) mockStatusWriter.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) res, err := adapter.EnsureOperationReady(ctx) assert.NoError(t, err) - assert.Equal(t, rqutils.PhaseReady, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseReady, requirement.Status.Phase) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay}, res) }) t.Run("happy path: operation not found, create one", func(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.Status.OperationName = testOperationName - requirement.Status.Phase = rqutils.PhaseOperating + requirement.Status.Phase = v1alpha1.RequirementPhaseOperating scheme := runtime.NewScheme() _ = v1alpha1.AddToScheme(scheme) - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&v1alpha1.Operation{}), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{Group: "appsv1", Resource: "Operation"}, "operation not found")) mockClient.EXPECT().Scheme().Return(scheme) @@ -578,15 +577,15 @@ func TestRequirementAdapter_EnsureOperationReady(t *testing.T) { res, err := adapter.EnsureOperationReady(ctx) assert.NoError(t, err) - assert.Equal(t, rqutils.PhaseOperating, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseOperating, requirement.Status.Phase) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, res) }) t.Run("sad path: failed to create operation", func(t *testing.T) { requirement := validRequirement.DeepCopy() requirement.Status.OperationName = testOperationName - requirement.Status.Phase = rqutils.PhaseOperating - adapter := NewRequirementAdapter(ctx, requirement, logger, mockClient, mockRecorder) + requirement.Status.Phase = v1alpha1.RequirementPhaseOperating + adapter := NewRequirementHandler(ctx, requirement, logger, mockClient, mockRecorder) schema := runtime.NewScheme() _ = v1alpha1.AddToScheme(schema) mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&v1alpha1.Operation{}), gomock.Any()).Return(assert.AnError) @@ -595,7 +594,7 @@ func TestRequirementAdapter_EnsureOperationReady(t *testing.T) { res, err := adapter.EnsureOperationReady(ctx) assert.ErrorIs(t, err, assert.AnError) - assert.Equal(t, rqutils.PhaseOperating, requirement.Status.Phase) + assert.Equal(t, v1alpha1.RequirementPhaseOperating, requirement.Status.Phase) assert.Equal(t, reconciler.OperationResult{RequeueDelay: reconciler.DefaultRequeueDelay, RequeueRequest: true}, res) }) } diff --git a/internal/log/const.go b/internal/log/const.go new file mode 100644 index 0000000..d5ceeb9 --- /dev/null +++ b/internal/log/const.go @@ -0,0 +1,7 @@ +package log + +const ( + // log keys + AppDeploymentJobName = "jobName" + AppDeploymentName = "appDeploymentName" +) diff --git a/internal/utils/controller/appdeployment/conditions.go b/internal/utils/controller/appdeployment/conditions.go deleted file mode 100644 index 87d9613..0000000 --- a/internal/utils/controller/appdeployment/conditions.go +++ /dev/null @@ -1,14 +0,0 @@ -package appdeployment - -import ( - "context" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/Azure/operation-cache-controller/api/v1alpha1" -) - -func ClearConditions(ctx context.Context, appdeployment *v1alpha1.AppDeployment) { - // Clear all conditions - appdeployment.Status.Conditions = []metav1.Condition{} -} diff --git a/internal/utils/controller/appdeployment/const.go b/internal/utils/controller/appdeployment/const.go deleted file mode 100644 index 1d7a874..0000000 --- a/internal/utils/controller/appdeployment/const.go +++ /dev/null @@ -1,17 +0,0 @@ -package appdeployment - -const ( - FinalizerName = "finalizer.appdeployment.devinfra.goms.io" - - // phase types - PhaseEmpty = "" - PhasePending = "Pending" - PhaseDeploying = "Deploying" - PhaseReady = "Ready" - PhaseDeleting = "Deleting" - PhaseDeleted = "Deleted" - - // log keys - LogKeyJobName = "jobName" - LogKeyAppDeploymentName = "appDeploymentName" -) diff --git a/internal/utils/controller/appdeployment_helper.go b/internal/utils/controller/appdeployment_helper.go new file mode 100644 index 0000000..6d2c158 --- /dev/null +++ b/internal/utils/controller/appdeployment_helper.go @@ -0,0 +1,22 @@ +package controller + +import ( + "context" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/Azure/operation-cache-controller/api/v1alpha1" +) + +const ( + // env keys + OperationIDEnvKey = "OPERATION_ID" +) + +type AppDeploymentHelper struct{} + +func NewAppDeploymentHelper() AppDeploymentHelper { return AppDeploymentHelper{} } + +func (ad AppDeploymentHelper) ClearConditions(ctx context.Context, appdeployment *v1alpha1.AppDeployment) { + appdeployment.Status.Conditions = []metav1.Condition{} +} diff --git a/internal/utils/controller/appdeployment/conditions_test.go b/internal/utils/controller/appdeployment_helper_test.go similarity index 83% rename from internal/utils/controller/appdeployment/conditions_test.go rename to internal/utils/controller/appdeployment_helper_test.go index 0d08d67..49b862c 100644 --- a/internal/utils/controller/appdeployment/conditions_test.go +++ b/internal/utils/controller/appdeployment_helper_test.go @@ -1,4 +1,4 @@ -package appdeployment_test +package controller_test import ( "context" @@ -8,9 +8,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/Azure/operation-cache-controller/api/v1alpha1" - "github.com/Azure/operation-cache-controller/internal/utils/controller/appdeployment" + ctrlutils "github.com/Azure/operation-cache-controller/internal/utils/controller" ) +var helper = ctrlutils.NewAppDeploymentHelper() + func TestClearConditions(t *testing.T) { tests := []struct { name string @@ -46,7 +48,7 @@ func TestClearConditions(t *testing.T) { Conditions: tt.existingConditions, }, } - appdeployment.ClearConditions(context.Background(), appDep) + helper.ClearConditions(context.Background(), appDep) assert.Empty(t, appDep.Status.Conditions, "conditions should be cleared") }) } diff --git a/internal/utils/controller/appdeployment/job.go b/internal/utils/controller/appdeployment_job.go similarity index 98% rename from internal/utils/controller/appdeployment/job.go rename to internal/utils/controller/appdeployment_job.go index 4c83082..2569053 100644 --- a/internal/utils/controller/appdeployment/job.go +++ b/internal/utils/controller/appdeployment_job.go @@ -1,4 +1,4 @@ -package appdeployment +package controller import ( "context" @@ -11,8 +11,6 @@ import ( ) const ( - OperationIDEnvKey = "OPERATION_ID" - JobTypeProvision = "provision" JobTypeTeardown = "teardown" ) diff --git a/internal/utils/controller/appdeployment/job_test.go b/internal/utils/controller/appdeployment_job_test.go similarity index 99% rename from internal/utils/controller/appdeployment/job_test.go rename to internal/utils/controller/appdeployment_job_test.go index 5415e49..db5b162 100644 --- a/internal/utils/controller/appdeployment/job_test.go +++ b/internal/utils/controller/appdeployment_job_test.go @@ -1,4 +1,4 @@ -package appdeployment +package controller import ( "context" diff --git a/internal/utils/controller/appdeployment/validate.go b/internal/utils/controller/appdeployment_validate.go similarity index 99% rename from internal/utils/controller/appdeployment/validate.go rename to internal/utils/controller/appdeployment_validate.go index 262225d..2613aea 100644 --- a/internal/utils/controller/appdeployment/validate.go +++ b/internal/utils/controller/appdeployment_validate.go @@ -1,4 +1,4 @@ -package appdeployment +package controller import ( "errors" diff --git a/internal/utils/controller/appdeployment/validate_test.go b/internal/utils/controller/appdeployment_validate_test.go similarity index 99% rename from internal/utils/controller/appdeployment/validate_test.go rename to internal/utils/controller/appdeployment_validate_test.go index eed38be..287cffd 100644 --- a/internal/utils/controller/appdeployment/validate_test.go +++ b/internal/utils/controller/appdeployment_validate_test.go @@ -1,4 +1,4 @@ -package appdeployment +package controller import ( "testing" diff --git a/internal/utils/controller/cache/cache.go b/internal/utils/controller/cache/cache.go deleted file mode 100644 index ac5d1c8..0000000 --- a/internal/utils/controller/cache/cache.go +++ /dev/null @@ -1,21 +0,0 @@ -package cache - -import ( - "math/rand" - "time" - - "github.com/Azure/operation-cache-controller/api/v1alpha1" -) - -func RandomSelectCachedOperation(cache *v1alpha1.Cache) string { - if len(cache.Status.AvailableCaches) == 0 { - return "" - } - // nolint:gosec, G404 // this is expected PRNG usage - return cache.Status.AvailableCaches[rand.Intn(len(cache.Status.AvailableCaches))] -} - -func DefaultCacheExpireTime() string { - // cache expire after 2 hours - return time.Now().Add(2 * time.Hour).Format(time.RFC3339) -} diff --git a/internal/utils/controller/cache/cache_test.go b/internal/utils/controller/cache/cache_test.go deleted file mode 100644 index b90887d..0000000 --- a/internal/utils/controller/cache/cache_test.go +++ /dev/null @@ -1,45 +0,0 @@ -package cache - -import ( - "testing" - - "github.com/stretchr/testify/require" - - "github.com/Azure/operation-cache-controller/api/v1alpha1" -) - -func TestRandomSelectCachedOperation(t *testing.T) { - tests := []struct { - name string - caches []string - expectEmpty bool - }{ - {"empty caches", nil, true}, - {"non-empty caches", []string{"cache1", "cache2", "cache3"}, false}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // ...existing setup code if any... - cacheInstance := &v1alpha1.Cache{ - Status: v1alpha1.CacheStatus{ - AvailableCaches: tt.caches, - }, - } - - result := RandomSelectCachedOperation(cacheInstance) - - if tt.expectEmpty { - require.Equal(t, "", result) - } else { - require.Contains(t, tt.caches, result) - } - // ...existing teardown code if any... - }) - } -} - -func TestDefaultCacheExpireTime(t *testing.T) { - result := DefaultCacheExpireTime() - require.NotEmpty(t, result) -} diff --git a/internal/utils/controller/cachekey.go b/internal/utils/controller/cache_helper.go similarity index 71% rename from internal/utils/controller/cachekey.go rename to internal/utils/controller/cache_helper.go index 80ca973..af06618 100644 --- a/internal/utils/controller/cachekey.go +++ b/internal/utils/controller/cache_helper.go @@ -3,8 +3,10 @@ package controller import ( "crypto/sha256" "encoding/hex" + "math/rand" "sort" "strings" + "time" "github.com/samber/lo" corev1 "k8s.io/api/core/v1" @@ -12,6 +14,22 @@ import ( "github.com/Azure/operation-cache-controller/api/v1alpha1" ) +type CacheHelper struct{} + +func NewCacheHelper() CacheHelper { return CacheHelper{} } + +func (c CacheHelper) RandomSelectCachedOperation(cache *v1alpha1.Cache) string { + if len(cache.Status.AvailableCaches) == 0 { + return "" + } + // nolint:gosec, G404 // this is expected PRNG usage + return cache.Status.AvailableCaches[rand.Intn(len(cache.Status.AvailableCaches))] +} +func (c CacheHelper) DefaultCacheExpireTime() string { + // cache expire after 2 hours + return time.Now().Add(2 * time.Hour).Format(time.RFC3339) +} + type AppCacheField struct { Name string Image string @@ -47,14 +65,14 @@ func (c *AppCacheField) NewCacheKey() string { return hex.EncodeToString(hasher.Sum(nil)) } -func NewCacheKeyFromApplications(apps []v1alpha1.ApplicationSpec) string { +func (c CacheHelper) NewCacheKeyFromApplications(apps []v1alpha1.ApplicationSpec) string { // sort the apps by name to ensure consistent hashing sort.Slice(apps, func(i, j int) bool { return apps[i].Name < apps[j].Name }) srcCacheKeys := lo.Reduce(apps, func(acc []string, app v1alpha1.ApplicationSpec, index int) []string { - return append(acc, AppCacheFieldFromApplicationProvision(app).NewCacheKey()) + return append(acc, c.AppCacheFieldFromApplicationProvision(app).NewCacheKey()) }, []string{}) // get the cache id for the source @@ -65,7 +83,7 @@ func NewCacheKeyFromApplications(apps []v1alpha1.ApplicationSpec) string { return hex.EncodeToString(hasher.Sum(nil)) } -func AppCacheFieldFromApplicationProvision(app v1alpha1.ApplicationSpec) *AppCacheField { +func (c CacheHelper) AppCacheFieldFromApplicationProvision(app v1alpha1.ApplicationSpec) *AppCacheField { return &AppCacheField{ Name: app.Name, Image: app.Provision.Template.Spec.Containers[0].Image, @@ -77,7 +95,7 @@ func AppCacheFieldFromApplicationProvision(app v1alpha1.ApplicationSpec) *AppCac } } -func AppCacheFieldFromApplicationTeardown(app v1alpha1.ApplicationSpec) *AppCacheField { +func (c CacheHelper) AppCacheFieldFromApplicationTeardown(app v1alpha1.ApplicationSpec) *AppCacheField { return &AppCacheField{ Name: app.Name, Image: app.Teardown.Template.Spec.Containers[0].Image, diff --git a/internal/utils/controller/cachekey_test.go b/internal/utils/controller/cache_helper_test.go similarity index 53% rename from internal/utils/controller/cachekey_test.go rename to internal/utils/controller/cache_helper_test.go index 787fdc8..a7e6fbe 100644 --- a/internal/utils/controller/cachekey_test.go +++ b/internal/utils/controller/cache_helper_test.go @@ -4,12 +4,50 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" "github.com/Azure/operation-cache-controller/api/v1alpha1" ) +var cacheHelper = NewCacheHelper() + +func TestDefaultCacheExpireTime(t *testing.T) { + result := cacheHelper.DefaultCacheExpireTime() + require.NotEmpty(t, result) +} + +func TestRandomSelectCachedOperation(t *testing.T) { + tests := []struct { + name string + caches []string + expectEmpty bool + }{ + {"empty caches", nil, true}, + {"non-empty caches", []string{"cache1", "cache2", "cache3"}, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // ...existing setup code if any... + cacheInstance := &v1alpha1.Cache{ + Status: v1alpha1.CacheStatus{ + AvailableCaches: tt.caches, + }, + } + + result := cacheHelper.RandomSelectCachedOperation(cacheInstance) + + if tt.expectEmpty { + require.Equal(t, "", result) + } else { + require.Contains(t, tt.caches, result) + } + // ...existing teardown code if any... + }) + } +} func TestNewCacheKey(t *testing.T) { tests := []struct { name string @@ -265,9 +303,175 @@ func TestNewCacheKeyFromApplications(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - actual := NewCacheKeyFromApplications(tt.source) + actual := cacheHelper.NewCacheKeyFromApplications(tt.source) assert.Equal(t, tt.expected, actual) }) } } + +func TestAppCacheFieldFromApplicationTeardown(t *testing.T) { + tests := []struct { + name string + app v1alpha1.ApplicationSpec + expected AppCacheField + }{ + { + name: "basic teardown", + app: v1alpha1.ApplicationSpec{ + Name: "test-app-1", + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "nginx:cleanup", + Command: []string{"cleanup"}, + Args: []string{"--all"}, + WorkingDir: "/cleanup", + Env: []corev1.EnvVar{ + {Name: "CLEANUP_MODE", Value: "full"}, + {Name: "DEBUG", Value: "true"}, + }, + }, + }, + }, + }, + }, + Dependencies: []string{"dep-1", "dep-2"}, + }, + expected: AppCacheField{ + Name: "test-app-1", + Image: "nginx:cleanup", + Command: []string{"cleanup"}, + Args: []string{"--all"}, + WorkingDir: "/cleanup", + Env: []corev1.EnvVar{{Name: "CLEANUP_MODE", Value: "full"}, {Name: "DEBUG", Value: "true"}}, + Dependencies: []string{"dep-1", "dep-2"}, + }, + }, + { + name: "teardown with empty fields", + app: v1alpha1.ApplicationSpec{ + Name: "test-app-minimal", + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "minimal:latest", + }, + }, + }, + }, + }, + }, + expected: AppCacheField{ + Name: "test-app-minimal", + Image: "minimal:latest", + Command: nil, + Args: nil, + WorkingDir: "", + Env: nil, + Dependencies: nil, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual := cacheHelper.AppCacheFieldFromApplicationTeardown(tt.app) + + assert.Equal(t, tt.expected.Name, actual.Name) + assert.Equal(t, tt.expected.Image, actual.Image) + assert.Equal(t, tt.expected.Command, actual.Command) + assert.Equal(t, tt.expected.Args, actual.Args) + assert.Equal(t, tt.expected.WorkingDir, actual.WorkingDir) + assert.Equal(t, tt.expected.Env, actual.Env) + assert.Equal(t, tt.expected.Dependencies, actual.Dependencies) + }) + } +} + +func TestAppCacheFieldFromApplicationProvision(t *testing.T) { + tests := []struct { + name string + app v1alpha1.ApplicationSpec + expected AppCacheField + }{ + { + name: "basic provision", + app: v1alpha1.ApplicationSpec{ + Name: "test-app-1", + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "nginx:latest", + Command: []string{"nginx"}, + Args: []string{"-g", "daemon off;"}, + WorkingDir: "/app", + Env: []corev1.EnvVar{ + {Name: "ENV_VAR1", Value: "value1"}, + {Name: "ENV_VAR2", Value: "value2"}, + }, + }, + }, + }, + }, + }, + Dependencies: []string{"dep-1", "dep-2"}, + }, + expected: AppCacheField{ + Name: "test-app-1", + Image: "nginx:latest", + Command: []string{"nginx"}, + Args: []string{"-g", "daemon off;"}, + WorkingDir: "/app", + Env: []corev1.EnvVar{{Name: "ENV_VAR1", Value: "value1"}, {Name: "ENV_VAR2", Value: "value2"}}, + Dependencies: []string{"dep-1", "dep-2"}, + }, + }, + { + name: "provision with empty fields", + app: v1alpha1.ApplicationSpec{ + Name: "test-app-minimal", + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Image: "minimal:latest", + }, + }, + }, + }, + }, + }, + expected: AppCacheField{ + Name: "test-app-minimal", + Image: "minimal:latest", + Command: nil, + Args: nil, + WorkingDir: "", + Env: nil, + Dependencies: nil, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual := cacheHelper.AppCacheFieldFromApplicationProvision(tt.app) + + assert.Equal(t, tt.expected.Name, actual.Name) + assert.Equal(t, tt.expected.Image, actual.Image) + assert.Equal(t, tt.expected.Command, actual.Command) + assert.Equal(t, tt.expected.Args, actual.Args) + assert.Equal(t, tt.expected.WorkingDir, actual.WorkingDir) + assert.Equal(t, tt.expected.Env, actual.Env) + assert.Equal(t, tt.expected.Dependencies, actual.Dependencies) + }) + } +} diff --git a/internal/utils/controller/operation/const.go b/internal/utils/controller/operation/const.go deleted file mode 100644 index 206df5c..0000000 --- a/internal/utils/controller/operation/const.go +++ /dev/null @@ -1,17 +0,0 @@ -package operation - -const ( - FinalizerName = "finalizer.operation.controller.azure.com" - AcquiredAnnotationKey = "operation.controller.azure.com/acquired" - - // PhaseEmpty is the phase when the operation is empty. - PhaseEmpty = "" - // PhaseReconciling is the phase when the operation is reconciling. - PhaseReconciling = "Reconciling" - // PhaseReconciled is the phase when the operation is reconciled. - PhaseReconciled = "Reconciled" - // Deletion is the phase when the operation is deleting. - PhaseDeleting = "Deleting" - // PhaseDeleted is the phase when the operation is deleted. - PhaseDeleted = "Deleted" -) diff --git a/internal/utils/controller/operation/operation_test.go b/internal/utils/controller/operation/operation_test.go deleted file mode 100644 index d35b7d9..0000000 --- a/internal/utils/controller/operation/operation_test.go +++ /dev/null @@ -1,141 +0,0 @@ -package operation - -import ( - "testing" - - "github.com/stretchr/testify/assert" - - batchv1 "k8s.io/api/batch/v1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/Azure/operation-cache-controller/api/v1alpha1" -) - -func TestNewOperationId(t *testing.T) { - tests := []struct { - name string - }{ - {name: "basic"}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - id := NewOperationId() - assert.NotEmpty(t, id) - }) - } -} - -func TestDiffAppDeployments(t *testing.T) { - tests := []struct { - name string - expected []v1alpha1.AppDeployment - actual []v1alpha1.AppDeployment - equals func(a, b v1alpha1.AppDeployment) bool - wantAdded int - wantRemoved int - wantUpdated int - }{ - { - name: "basic example", - expected: []v1alpha1.AppDeployment{{ObjectMeta: metav1.ObjectMeta{Name: "app1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "app2"}}}, - actual: []v1alpha1.AppDeployment{{ObjectMeta: metav1.ObjectMeta{Name: "app2"}}, {ObjectMeta: metav1.ObjectMeta{Name: "app3"}}}, - equals: func(a, b v1alpha1.AppDeployment) bool { return a.Name == b.Name }, - wantAdded: 1, wantRemoved: 1, wantUpdated: 0, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - added, removed, updated := DiffAppDeployments(tt.expected, tt.actual, tt.equals) - assert.Equal(t, tt.wantAdded, len(added)) - assert.Equal(t, tt.wantRemoved, len(removed)) - assert.Equal(t, tt.wantUpdated, len(updated)) - }) - } -} - -func TestCompareProvisionJobs(t *testing.T) { - tests := []struct { - name string - a v1alpha1.AppDeployment - b v1alpha1.AppDeployment - want bool - }{ - { - name: "different images", - a: v1alpha1.AppDeployment{ - Spec: v1alpha1.AppDeploymentSpec{ - Provision: batchv1.JobSpec{ - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{{Image: "image1"}}, - }, - }, - }, - Dependencies: []string{"dep1", "dep2"}, - }, - }, - b: v1alpha1.AppDeployment{ - Spec: v1alpha1.AppDeploymentSpec{ - Provision: batchv1.JobSpec{ - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{{Image: "image2"}}, - }, - }, - }, - Dependencies: []string{"dep2", "dep1"}, - }, - }, - want: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := CompareProvisionJobs(tt.a, tt.b) - assert.Equal(t, tt.want, got) - }) - } -} - -func TestCompareTeardownJobs(t *testing.T) { - tests := []struct { - name string - a v1alpha1.AppDeployment - b v1alpha1.AppDeployment - want bool - }{ - { - name: "different teardown images", - a: v1alpha1.AppDeployment{ - Spec: v1alpha1.AppDeploymentSpec{ - Teardown: batchv1.JobSpec{ - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{{Image: "image1"}}, - }, - }, - }, - }, - }, - b: v1alpha1.AppDeployment{ - Spec: v1alpha1.AppDeploymentSpec{ - Teardown: batchv1.JobSpec{ - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{{Image: "image2"}}, - }, - }, - }, - }, - }, - want: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := CompareTeardownJobs(tt.a, tt.b) - assert.Equal(t, tt.want, got) - }) - } -} diff --git a/internal/utils/controller/operation/status.go b/internal/utils/controller/operation/status.go deleted file mode 100644 index 34de847..0000000 --- a/internal/utils/controller/operation/status.go +++ /dev/null @@ -1,11 +0,0 @@ -package operation - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/Azure/operation-cache-controller/api/v1alpha1" -) - -func ClearConditions(operation *v1alpha1.Operation) { - operation.Status.Conditions = []metav1.Condition{} -} diff --git a/internal/utils/controller/operation/status_test.go b/internal/utils/controller/operation/status_test.go deleted file mode 100644 index 670ab08..0000000 --- a/internal/utils/controller/operation/status_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package operation - -import ( - "testing" - - "github.com/stretchr/testify/assert" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/Azure/operation-cache-controller/api/v1alpha1" -) - -func TestConditions(t *testing.T) { - operation := &v1alpha1.Operation{} - operation.Status.Conditions = []metav1.Condition{ - { - Type: "test", - Status: "test", - Reason: "test", - Message: "test", - }, - } - ClearConditions(operation) - assert.Equal(t, 0, len(operation.Status.Conditions)) -} diff --git a/internal/utils/controller/operation/operation.go b/internal/utils/controller/operation_helper.go similarity index 62% rename from internal/utils/controller/operation/operation.go rename to internal/utils/controller/operation_helper.go index 48852ac..9672fcd 100644 --- a/internal/utils/controller/operation/operation.go +++ b/internal/utils/controller/operation_helper.go @@ -1,4 +1,4 @@ -package operation +package controller import ( "sort" @@ -7,17 +7,33 @@ import ( "github.com/google/uuid" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/Azure/operation-cache-controller/api/v1alpha1" ) +type OperationHelper struct{} + +func NewOperationHelper() OperationHelper { return OperationHelper{} } + +func (ou OperationHelper) IsOperationReady(operation *v1alpha1.Operation) bool { + if operation == nil { + return false + } + return operation.Status.Phase == v1alpha1.OperationPhaseReconciled +} + +func (ou OperationHelper) ClearConditions(operation *v1alpha1.Operation) { + operation.Status.Conditions = []metav1.Condition{} +} + // NewOperationId generates a new operation id which is an UUID. -func NewOperationId() string { +func (ou OperationHelper) NewOperationId() string { return strings.Replace(uuid.New().String(), "-", "", -1) } // DiffAppDeployments returns the difference between two slices of AppDeployment. -func DiffAppDeployments(expected, actual []v1alpha1.AppDeployment, +func (ou OperationHelper) DiffAppDeployments(expected, actual []v1alpha1.AppDeployment, equals func(a, b v1alpha1.AppDeployment) bool) (added, removed, updated []v1alpha1.AppDeployment) { // Find added and updated AppDeployments. for _, e := range expected { @@ -53,7 +69,7 @@ func DiffAppDeployments(expected, actual []v1alpha1.AppDeployment, return added, removed, updated } -func isJobResultIdentical(a, b batchv1.JobSpec) bool { +func (ou OperationHelper) isJobResultIdentical(a, b batchv1.JobSpec) bool { if len(a.Template.Spec.Containers) == 0 && len(b.Template.Spec.Containers) == 0 { return true } @@ -72,14 +88,14 @@ func isJobResultIdentical(a, b batchv1.JobSpec) bool { if a.Template.Spec.Containers[0].WorkingDir != b.Template.Spec.Containers[0].WorkingDir { return false } - if !isEnvVarsIdentical(a.Template.Spec.Containers[0].Env, b.Template.Spec.Containers[0].Env) { + if !ou.isEnvVarsIdentical(a.Template.Spec.Containers[0].Env, b.Template.Spec.Containers[0].Env) { return false } return true } -func isEnvVarsIdentical(a, b []corev1.EnvVar) bool { +func (ou OperationHelper) isEnvVarsIdentical(a, b []corev1.EnvVar) bool { if len(a) != len(b) { return false } @@ -112,24 +128,17 @@ func isStringSliceIdentical(a, b []string) bool { return true } -func CompareProvisionJobs(a, b v1alpha1.AppDeployment) bool { - if sameProvisionJob := isJobResultIdentical(a.Spec.Provision, b.Spec.Provision); !sameProvisionJob { +func (ou OperationHelper) CompareProvisionJobs(a, b v1alpha1.AppDeployment) bool { + if sameProvisionJob := ou.isJobResultIdentical(a.Spec.Provision, b.Spec.Provision); !sameProvisionJob { return false } - // sort dependencies to ensure consistent hashing - sort.Strings(a.Spec.Dependencies) - sort.Strings(b.Spec.Dependencies) - for i, dep := range a.Spec.Dependencies { - if dep != b.Spec.Dependencies[i] { - return false - } - } - return true + // Check if the dependencies are the same using the helper function + return isStringSliceIdentical(a.Spec.Dependencies, b.Spec.Dependencies) } -func CompareTeardownJobs(a, b v1alpha1.AppDeployment) bool { - if sameTeardownJob := isJobResultIdentical(a.Spec.Teardown, b.Spec.Teardown); !sameTeardownJob { +func (ou OperationHelper) CompareTeardownJobs(a, b v1alpha1.AppDeployment) bool { + if sameTeardownJob := ou.isJobResultIdentical(a.Spec.Teardown, b.Spec.Teardown); !sameTeardownJob { return false } return true diff --git a/internal/utils/controller/operation_helper_test.go b/internal/utils/controller/operation_helper_test.go new file mode 100644 index 0000000..b50eef0 --- /dev/null +++ b/internal/utils/controller/operation_helper_test.go @@ -0,0 +1,1122 @@ +package controller + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/Azure/operation-cache-controller/api/v1alpha1" +) + +var helper = NewOperationHelper() + +func TestNewOperationId(t *testing.T) { + tests := []struct { + name string + }{ + {name: "basic"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + id := helper.NewOperationId() + assert.NotEmpty(t, id) + }) + } +} + +func TestDiffAppDeployments(t *testing.T) { + tests := []struct { + name string + expected []v1alpha1.AppDeployment + actual []v1alpha1.AppDeployment + equals func(a, b v1alpha1.AppDeployment) bool + wantAdded int + wantRemoved int + wantUpdated int + }{ + { + name: "basic example", + expected: []v1alpha1.AppDeployment{{ObjectMeta: metav1.ObjectMeta{Name: "app1"}}, {ObjectMeta: metav1.ObjectMeta{Name: "app2"}}}, + actual: []v1alpha1.AppDeployment{{ObjectMeta: metav1.ObjectMeta{Name: "app2"}}, {ObjectMeta: metav1.ObjectMeta{Name: "app3"}}}, + equals: func(a, b v1alpha1.AppDeployment) bool { return a.Name == b.Name }, + wantAdded: 1, wantRemoved: 1, wantUpdated: 0, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + added, removed, updated := helper.DiffAppDeployments(tt.expected, tt.actual, tt.equals) + assert.Equal(t, tt.wantAdded, len(added)) + assert.Equal(t, tt.wantRemoved, len(removed)) + assert.Equal(t, tt.wantUpdated, len(updated)) + }) + } +} + +func TestCompareProvisionJobs(t *testing.T) { + tests := []struct { + name string + a v1alpha1.AppDeployment + b v1alpha1.AppDeployment + want bool + }{ + { + name: "different images", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{"dep1", "dep2"}, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image2"}}, + }, + }, + }, + Dependencies: []string{"dep2", "dep1"}, + }, + }, + want: false, + }, + { + name: "identical provision jobs with same dependencies", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{"dep1", "dep2"}, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{"dep2", "dep1"}, + }, + }, + want: true, + }, + { + name: "identical provision jobs with different dependencies", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{"dep1", "dep2"}, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{"dep1", "dep3"}, + }, + }, + want: false, + }, + { + name: "identical provision jobs with different number of dependencies", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{"dep1", "dep2", "dep3"}, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{"dep1", "dep2"}, + }, + }, + want: false, + }, + { + name: "identical provision jobs with empty dependencies", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{}, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{}, + }, + }, + want: true, + }, + { + name: "identical provision jobs with nil and empty dependencies", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: nil, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{}, + }, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := helper.CompareProvisionJobs(tt.a, tt.b) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestCompareProvisionJobsWithDependencies(t *testing.T) { + tests := []struct { + name string + a v1alpha1.AppDeployment + b v1alpha1.AppDeployment + want bool + }{ + { + name: "different images", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{"dep1", "dep2"}, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image2"}}, + }, + }, + }, + Dependencies: []string{"dep2", "dep1"}, + }, + }, + want: false, + }, + { + name: "identical provision jobs with same dependencies in different order", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{"dep1", "dep2"}, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{"dep2", "dep1"}, + }, + }, + want: true, + }, + { + name: "identical provision jobs with different dependencies", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{"dep1", "dep2"}, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{"dep1", "dep3"}, + }, + }, + want: false, + }, + { + name: "identical provision jobs with different number of dependencies", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{"dep1", "dep2", "dep3"}, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{"dep1", "dep2"}, + }, + }, + want: false, + }, + { + name: "identical provision jobs with empty dependencies", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{}, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{}, + }, + }, + want: true, + }, + { + name: "identical provision jobs with nil and empty dependencies", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: nil, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Provision: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + Dependencies: []string{}, + }, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := helper.CompareProvisionJobs(tt.a, tt.b) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestCompareTeardownJobs(t *testing.T) { + tests := []struct { + name string + a v1alpha1.AppDeployment + b v1alpha1.AppDeployment + want bool + }{ + { + name: "different teardown images", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image1"}}, + }, + }, + }, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image2"}}, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "identical teardown jobs", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "same-image"}}, + }, + }, + }, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "same-image"}}, + }, + }, + }, + }, + }, + want: true, + }, + { + name: "both empty teardown containers", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{}, + }, + }, + }, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{}, + }, + }, + }, + }, + }, + want: true, + }, + { + name: "one empty teardown container", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{}, + }, + }, + }, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Image: "image"}}, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "different commands in teardown", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "same-image", + Command: []string{"cmd1", "cmd2"}, + }}, + }, + }, + }, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "same-image", + Command: []string{"cmd3", "cmd4"}, + }}, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "different args in teardown", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "same-image", + Args: []string{"arg1", "arg2"}, + }}, + }, + }, + }, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "same-image", + Args: []string{"arg3", "arg4"}, + }}, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "different working dir in teardown", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "same-image", + WorkingDir: "/dir1", + }}, + }, + }, + }, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "same-image", + WorkingDir: "/dir2", + }}, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "different env vars in teardown", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "same-image", + Env: []corev1.EnvVar{ + {Name: "VAR1", Value: "value1"}, + }, + }}, + }, + }, + }, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "same-image", + Env: []corev1.EnvVar{ + {Name: "VAR1", Value: "different"}, + }, + }}, + }, + }, + }, + }, + }, + want: false, + }, + { + name: "complex identical teardown jobs", + a: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "same-image", + Command: []string{"cmd1", "cmd2"}, + Args: []string{"arg1", "arg2"}, + WorkingDir: "/workdir", + Env: []corev1.EnvVar{ + {Name: "VAR1", Value: "value1"}, + {Name: "VAR2", Value: "value2"}, + }, + }}, + }, + }, + }, + }, + }, + b: v1alpha1.AppDeployment{ + Spec: v1alpha1.AppDeploymentSpec{ + Teardown: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "same-image", + Command: []string{"cmd1", "cmd2"}, + Args: []string{"arg1", "arg2"}, + WorkingDir: "/workdir", + Env: []corev1.EnvVar{ + {Name: "VAR2", Value: "value2"}, + {Name: "VAR1", Value: "value1"}, + }, + }}, + }, + }, + }, + }, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := helper.CompareTeardownJobs(tt.a, tt.b) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestIsOperationReady(t *testing.T) { + tests := []struct { + name string + operation *v1alpha1.Operation + want bool + }{ + { + name: "nil operation", + operation: nil, + want: false, + }, + { + name: "operation not ready", + operation: &v1alpha1.Operation{ + Status: v1alpha1.OperationStatus{ + Phase: v1alpha1.OperationPhaseReconciling, + }, + }, + want: false, + }, + { + name: "operation ready", + operation: &v1alpha1.Operation{ + Status: v1alpha1.OperationStatus{ + Phase: v1alpha1.OperationPhaseReconciled, + }, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := helper.IsOperationReady(tt.operation) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestClearOperationConditions(t *testing.T) { + t.Run("clear conditions", func(t *testing.T) { + operation := &v1alpha1.Operation{ + Status: v1alpha1.OperationStatus{ + Conditions: []metav1.Condition{ + { + Type: "Test", + Status: "True", + Reason: "Testing", + Message: "Test message", + }, + }, + }, + } + + helper.ClearConditions(operation) + assert.Empty(t, operation.Status.Conditions) + }) +} + +func TestIsStringSliceIdentical(t *testing.T) { + tests := []struct { + name string + a []string + b []string + want bool + }{ + { + name: "different length", + a: []string{"a", "b"}, + b: []string{"a"}, + want: false, + }, + { + name: "same elements different order", + a: []string{"a", "b", "c"}, + b: []string{"c", "a", "b"}, + want: true, + }, + { + name: "same elements same order", + a: []string{"a", "b", "c"}, + b: []string{"a", "b", "c"}, + want: true, + }, + { + name: "different elements", + a: []string{"a", "b", "c"}, + b: []string{"a", "b", "d"}, + want: false, + }, + { + name: "empty slices", + a: []string{}, + b: []string{}, + want: true, + }, + { + name: "nil slices", + a: nil, + b: nil, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create copies to verify original slices aren't modified + aCopy := make([]string, len(tt.a)) + bCopy := make([]string, len(tt.b)) + if len(tt.a) > 0 { + copy(aCopy, tt.a) + } + if len(tt.b) > 0 { + copy(bCopy, tt.b) + } + + got := isStringSliceIdentical(aCopy, bCopy) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestIsEnvVarsIdentical(t *testing.T) { + tests := []struct { + name string + a []corev1.EnvVar + b []corev1.EnvVar + want bool + }{ + { + name: "different length", + a: []corev1.EnvVar{ + {Name: "VAR1", Value: "value1"}, + {Name: "VAR2", Value: "value2"}, + }, + b: []corev1.EnvVar{ + {Name: "VAR1", Value: "value1"}, + }, + want: false, + }, + { + name: "same vars different order", + a: []corev1.EnvVar{ + {Name: "VAR1", Value: "value1"}, + {Name: "VAR2", Value: "value2"}, + }, + b: []corev1.EnvVar{ + {Name: "VAR2", Value: "value2"}, + {Name: "VAR1", Value: "value1"}, + }, + want: true, + }, + { + name: "different values", + a: []corev1.EnvVar{ + {Name: "VAR1", Value: "value1"}, + {Name: "VAR2", Value: "value2"}, + }, + b: []corev1.EnvVar{ + {Name: "VAR1", Value: "value1"}, + {Name: "VAR2", Value: "different"}, + }, + want: false, + }, + { + name: "different names", + a: []corev1.EnvVar{ + {Name: "VAR1", Value: "value1"}, + {Name: "VAR2", Value: "value2"}, + }, + b: []corev1.EnvVar{ + {Name: "VAR1", Value: "value1"}, + {Name: "DIFFERENT", Value: "value2"}, + }, + want: false, + }, + { + name: "empty slices", + a: []corev1.EnvVar{}, + b: []corev1.EnvVar{}, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := helper.isEnvVarsIdentical(tt.a, tt.b) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestIsJobResultIdentical(t *testing.T) { + tests := []struct { + name string + a batchv1.JobSpec + b batchv1.JobSpec + want bool + }{ + { + name: "both empty containers", + a: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{}, + }, + }, + }, + b: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{}, + }, + }, + }, + want: true, + }, + { + name: "one empty containers", + a: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{}, + }, + }, + }, + b: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "test-image", + }}, + }, + }, + }, + want: false, + }, + { + name: "different images", + a: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "test-image1", + }}, + }, + }, + }, + b: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "test-image2", + }}, + }, + }, + }, + want: false, + }, + { + name: "different commands", + a: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "test-image", + Command: []string{"cmd1", "cmd2"}, + }}, + }, + }, + }, + b: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "test-image", + Command: []string{"cmd1", "cmd3"}, + }}, + }, + }, + }, + want: false, + }, + { + name: "different args", + a: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "test-image", + Args: []string{"arg1", "arg2"}, + }}, + }, + }, + }, + b: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "test-image", + Args: []string{"arg1", "arg3"}, + }}, + }, + }, + }, + want: false, + }, + { + name: "different working directory", + a: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "test-image", + WorkingDir: "/dir1", + }}, + }, + }, + }, + b: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "test-image", + WorkingDir: "/dir2", + }}, + }, + }, + }, + want: false, + }, + { + name: "different env vars", + a: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "test-image", + Env: []corev1.EnvVar{ + {Name: "VAR1", Value: "value1"}, + }, + }}, + }, + }, + }, + b: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "test-image", + Env: []corev1.EnvVar{ + {Name: "VAR1", Value: "different"}, + }, + }}, + }, + }, + }, + want: false, + }, + { + name: "identical jobs", + a: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "test-image", + Command: []string{"cmd1", "cmd2"}, + Args: []string{"arg1", "arg2"}, + WorkingDir: "/workdir", + Env: []corev1.EnvVar{ + {Name: "VAR1", Value: "value1"}, + {Name: "VAR2", Value: "value2"}, + }, + }}, + }, + }, + }, + b: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "test-image", + Command: []string{"cmd1", "cmd2"}, + Args: []string{"arg1", "arg2"}, + WorkingDir: "/workdir", + Env: []corev1.EnvVar{ + {Name: "VAR2", Value: "value2"}, + {Name: "VAR1", Value: "value1"}, + }, + }}, + }, + }, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := helper.isJobResultIdentical(tt.a, tt.b) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/internal/utils/controller/requirement/const.go b/internal/utils/controller/requirement/const.go deleted file mode 100644 index 2e0b754..0000000 --- a/internal/utils/controller/requirement/const.go +++ /dev/null @@ -1,27 +0,0 @@ -package requirement - -const ( - FinalizerName = "finalizer.requirement.devinfra.goms.io" - - // RequirementInitialized is the condition type for Requirement initialized - ConditionRequirementInitialized = "RequirementInitialized" - // Cache CRD for Requirement is existed - ConditionCacheResourceFound = "CacheCRFound" - // no available Operation CRD for Requirement in cache, create one - ConditionCachedOperationAcquired = "CachedOpAcquired" - // Operation CRD for Requirement is ready - ConditionOperationReady = "OperationReady" - - ConditionReasonNoOperationAvailable = "NoOperationAvailable" - ConditionReasonCacheCRNotFound = "CacheCRNotFound" - ConditionReasonCacheCRFound = "CacheCRFound" - ConditionReasonCacheHit = "CacheHit" - ConditionReasonCacheMiss = "CacheMiss" - - PhaseEmpty = "" - PhaseCacheChecking = "CacheChecking" - PhaseOperating = "Operating" - PhaseReady = "Ready" - PhaseDeleted = "Deleted" - PhaseDeleting = "Deleting" -) diff --git a/internal/utils/controller/requirement/status.go b/internal/utils/controller/requirement_helper.go similarity index 65% rename from internal/utils/controller/requirement/status.go rename to internal/utils/controller/requirement_helper.go index 9966168..aa0819c 100644 --- a/internal/utils/controller/requirement/status.go +++ b/internal/utils/controller/requirement_helper.go @@ -1,4 +1,4 @@ -package requirement +package controller import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -6,13 +6,16 @@ import ( "github.com/Azure/operation-cache-controller/api/v1alpha1" ) -func ClearConditions(r *v1alpha1.Requirement) { +type RequirementHelper struct{} + +func NewRequirementHelper() RequirementHelper { return RequirementHelper{} } +func (rh RequirementHelper) ClearConditions(r *v1alpha1.Requirement) { if r.Status.Conditions == nil { r.Status.Conditions = []metav1.Condition{} } } -func getCondition(r *v1alpha1.Requirement, conditionType string) (int, *metav1.Condition) { +func (rh RequirementHelper) getCondition(r *v1alpha1.Requirement, conditionType string) (int, *metav1.Condition) { for i, condition := range r.Status.Conditions { if condition.Type == conditionType { return i, &condition @@ -21,12 +24,12 @@ func getCondition(r *v1alpha1.Requirement, conditionType string) (int, *metav1.C return -1, nil } -func IsCacheMissed(r *v1alpha1.Requirement) bool { - _, condition := getCondition(r, ConditionCachedOperationAcquired) +func (rh RequirementHelper) IsCacheMissed(r *v1alpha1.Requirement) bool { + _, condition := rh.getCondition(r, v1alpha1.RequirementConditionCachedOperationAcquired) return condition == nil || condition.Status == metav1.ConditionFalse } -func UpdateCondition(r *v1alpha1.Requirement, conditionType string, conditionStatus metav1.ConditionStatus, reason, message string) bool { +func (rh RequirementHelper) UpdateCondition(r *v1alpha1.Requirement, conditionType string, conditionStatus metav1.ConditionStatus, reason, message string) bool { condition := &metav1.Condition{ Type: conditionType, Status: conditionStatus, @@ -35,7 +38,7 @@ func UpdateCondition(r *v1alpha1.Requirement, conditionType string, conditionSta Message: message, } // Try to find this pod condition. - idx, oldCondition := getCondition(r, condition.Type) + idx, oldCondition := rh.getCondition(r, condition.Type) if oldCondition == nil { // We are adding new pod condition. diff --git a/internal/utils/controller/requirement/status_test.go b/internal/utils/controller/requirement_helper_test.go similarity index 90% rename from internal/utils/controller/requirement/status_test.go rename to internal/utils/controller/requirement_helper_test.go index 6ac1b94..d4c5120 100644 --- a/internal/utils/controller/requirement/status_test.go +++ b/internal/utils/controller/requirement_helper_test.go @@ -1,4 +1,4 @@ -package requirement +package controller import ( "testing" @@ -10,6 +10,8 @@ import ( "github.com/Azure/operation-cache-controller/api/v1alpha1" ) +var reqHelper = NewRequirementHelper() + // --- Test ClearConditions --- func TestClearConditions(t *testing.T) { // Requirement with nil Conditions. @@ -18,7 +20,7 @@ func TestClearConditions(t *testing.T) { // Conditions nil for test. }, } - ClearConditions(req) + reqHelper.ClearConditions(req) require.NotNil(t, req.Status.Conditions) require.Equal(t, 0, len(req.Status.Conditions)) } @@ -39,7 +41,7 @@ func TestIsCacheMissed(t *testing.T) { name: "condition exists - false status", conditions: []metav1.Condition{ { - Type: ConditionCachedOperationAcquired, + Type: v1alpha1.RequirementConditionCachedOperationAcquired, Status: metav1.ConditionFalse, }, }, @@ -49,7 +51,7 @@ func TestIsCacheMissed(t *testing.T) { name: "condition exists - true status", conditions: []metav1.Condition{ { - Type: ConditionCachedOperationAcquired, + Type: v1alpha1.RequirementConditionCachedOperationAcquired, Status: metav1.ConditionTrue, }, }, @@ -63,7 +65,7 @@ func TestIsCacheMissed(t *testing.T) { Conditions: tt.conditions, }, } - got := IsCacheMissed(req) + got := reqHelper.IsCacheMissed(req) require.Equal(t, tt.wantMissed, got) }) } @@ -138,7 +140,7 @@ func TestUpdateCondition(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { req := newReq(tt.initialConds) - changed := UpdateCondition(req, tt.conditionType, tt.conditionStatus, tt.reason, tt.message) + changed := reqHelper.UpdateCondition(req, tt.conditionType, tt.conditionStatus, tt.reason, tt.message) require.Equal(t, tt.wantChanged, changed) // Verify the condition is updated/added. diff --git a/internal/mocks/generate.go b/internal/utils/mocks/generate.go similarity index 100% rename from internal/mocks/generate.go rename to internal/utils/mocks/generate.go diff --git a/internal/mocks/mock_cr_client.go b/internal/utils/mocks/mock_cr_client.go similarity index 100% rename from internal/mocks/mock_cr_client.go rename to internal/utils/mocks/mock_cr_client.go diff --git a/internal/mocks/mock_cr_recorder.go b/internal/utils/mocks/mock_cr_recorder.go similarity index 100% rename from internal/mocks/mock_cr_recorder.go rename to internal/utils/mocks/mock_cr_recorder.go diff --git a/internal/mocks/mock_cr_status_writer.go b/internal/utils/mocks/mock_cr_status_writer.go similarity index 100% rename from internal/mocks/mock_cr_status_writer.go rename to internal/utils/mocks/mock_cr_status_writer.go diff --git a/internal/utils/controller/rand_string.go b/internal/utils/rand/rand_string.go similarity index 94% rename from internal/utils/controller/rand_string.go rename to internal/utils/rand/rand_string.go index 2c1e3ff..9e922cb 100644 --- a/internal/utils/controller/rand_string.go +++ b/internal/utils/rand/rand_string.go @@ -1,4 +1,4 @@ -package controller +package rand import ( "math/rand" diff --git a/internal/utils/controller/rand_string_test.go b/internal/utils/rand/rand_string_test.go similarity index 98% rename from internal/utils/controller/rand_string_test.go rename to internal/utils/rand/rand_string_test.go index 9a12a27..f5654be 100644 --- a/internal/utils/controller/rand_string_test.go +++ b/internal/utils/rand/rand_string_test.go @@ -1,4 +1,4 @@ -package controller +package rand import ( "testing" diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go index bb15cc1..7914560 100644 --- a/test/integration/integration_test.go +++ b/test/integration/integration_test.go @@ -12,7 +12,6 @@ import ( "sigs.k8s.io/e2e-framework/pkg/features" "github.com/Azure/operation-cache-controller/api/v1alpha1" - rqutils "github.com/Azure/operation-cache-controller/internal/utils/controller/requirement" "github.com/Azure/operation-cache-controller/test/utils" ) @@ -46,7 +45,7 @@ var SimpleRequirementFeature = features.New("Simple Requirements"). if err := cfg.Client().Resources().Get(ctx, testRequirementName, utils.TestNamespace, requirement); err != nil { return false, err } - if requirement.Status.Phase != rqutils.PhaseReady { + if requirement.Status.Phase != v1alpha1.RequirementPhaseReady { return false, nil } return true, nil @@ -91,7 +90,7 @@ var CachedRequirementFeature = features.New("Cached Requirements"). if err := cfg.Client().Resources().Get(ctx, cachedRequirementName, utils.TestNamespace, requirement); err != nil { return false, err } - if requirement.Status.Phase != rqutils.PhaseReady { + if requirement.Status.Phase != v1alpha1.RequirementPhaseReady { return false, nil } return true, nil @@ -172,7 +171,7 @@ var CachedRequirementFeature = features.New("Cached Requirements"). if err := cfg.Client().Resources().Get(ctx, newCachedRequirementName, utils.TestNamespace, newRequirement); err != nil { return false, err } - if newRequirement.Status.Phase != rqutils.PhaseReady { + if newRequirement.Status.Phase != v1alpha1.RequirementPhaseReady { return false, nil } return true, nil @@ -183,7 +182,7 @@ var CachedRequirementFeature = features.New("Cached Requirements"). // cache should be hit cacheHit := false for _, condition := range newRequirement.Status.Conditions { - if condition.Type == rqutils.ConditionOperationReady { + if condition.Type == v1alpha1.RequirementConditionOperationReady { cacheHit = true break }