From daf6d7ebe56a96b50403d9962cc8a6742f46cdde Mon Sep 17 00:00:00 2001 From: Luca Burgazzoli Date: Tue, 4 Feb 2025 11:46:24 +0100 Subject: [PATCH] unit-tests --- .../actions/deploy/action_deploy.go | 3 ++- .../actions/deploy/action_deploy_test.go | 2 +- pkg/controller/manager/manager.go | 14 ++++------- pkg/controller/reconciler/reconciler.go | 21 +++++++--------- pkg/controller/reconciler/reconciler_test.go | 24 +++++++++---------- 5 files changed, 28 insertions(+), 36 deletions(-) diff --git a/pkg/controller/actions/deploy/action_deploy.go b/pkg/controller/actions/deploy/action_deploy.go index 8993b570dd9..59155ae4aa6 100644 --- a/pkg/controller/actions/deploy/action_deploy.go +++ b/pkg/controller/actions/deploy/action_deploy.go @@ -118,6 +118,7 @@ func (a *Action) run(ctx context.Context, rr *odhTypes.ReconciliationRequest) er } controllerName := strings.ToLower(kind) + igvk := rr.Instance.GetObjectKind().GroupVersionKind() for i := range rr.Resources { res := rr.Resources[i] @@ -134,7 +135,7 @@ func (a *Action) run(ctx context.Context, rr *odhTypes.ReconciliationRequest) er default: // Remove the previous owner reference if set, This is required during the // transition from the old to the new operator. - if err := resources.RemoveOwnerReferences(ctx, rr.Client, current, ownedTypeIsNot(rr.Manager.GetOwnerType())); err != nil { + if err := resources.RemoveOwnerReferences(ctx, rr.Client, current, ownedTypeIsNot(&igvk)); err != nil { return err } diff --git a/pkg/controller/actions/deploy/action_deploy_test.go b/pkg/controller/actions/deploy/action_deploy_test.go index ea81359f09c..1928ee5e98c 100644 --- a/pkg/controller/actions/deploy/action_deploy_test.go +++ b/pkg/controller/actions/deploy/action_deploy_test.go @@ -604,7 +604,7 @@ func TestDeployOwnerRef(t *testing.T) { Version: version.OperatorVersion{Version: semver.Version{ Major: 1, Minor: 2, Patch: 3, }}}, - Manager: manager.New(nil, nil), + Manager: manager.New(nil), } rr.Manager.AddGVK(gvk.ConfigMap, true) diff --git a/pkg/controller/manager/manager.go b/pkg/controller/manager/manager.go index 73c9989e2c7..b52ad12724b 100644 --- a/pkg/controller/manager/manager.go +++ b/pkg/controller/manager/manager.go @@ -9,23 +9,17 @@ type gvkInfo struct { owned bool } -func New(manager ctrl.Manager, ownerType *schema.GroupVersionKind) *Manager { +func New(manager ctrl.Manager) *Manager { return &Manager{ - m: manager, - ownerType: ownerType, - gvks: map[schema.GroupVersionKind]gvkInfo{}, + m: manager, + gvks: map[schema.GroupVersionKind]gvkInfo{}, } } type Manager struct { m ctrl.Manager - ownerType *schema.GroupVersionKind - gvks map[schema.GroupVersionKind]gvkInfo -} - -func (m *Manager) GetOwnerType() *schema.GroupVersionKind { - return m.ownerType + gvks map[schema.GroupVersionKind]gvkInfo } func (m *Manager) AddGVK(gvk schema.GroupVersionKind, owned bool) { diff --git a/pkg/controller/reconciler/reconciler.go b/pkg/controller/reconciler/reconciler.go index 60165e61da9..76fa83b1dbd 100644 --- a/pkg/controller/reconciler/reconciler.go +++ b/pkg/controller/reconciler/reconciler.go @@ -64,11 +64,6 @@ func NewReconciler[T common.PlatformObject](mgr manager.Manager, name string, ob return nil, err } - gvk, err := resources.GetGroupVersionKindForObject(mgr.GetScheme(), object) - if err != nil { - return nil, err - } - cc := Reconciler{ Client: oc, Scheme: mgr.GetScheme(), @@ -76,7 +71,7 @@ func NewReconciler[T common.PlatformObject](mgr manager.Manager, name string, ob Recorder: mgr.GetEventRecorderFor(name), Release: cluster.GetRelease(), name: name, - m: odhManager.New(mgr, &gvk), + m: odhManager.New(mgr), instanceFactory: func() (common.PlatformObject, error) { t := reflect.TypeOf(object).Elem() res, ok := reflect.New(t).Interface().(T) @@ -281,12 +276,14 @@ func (r *Reconciler) apply(ctx context.Context, res common.PlatformObject) error } if provisionErr != nil { - r.Recorder.Event( - res, - corev1.EventTypeWarning, - "ProvisioningError", - provisionErr.Error(), - ) + if r.Recorder != nil { + r.Recorder.Event( + res, + corev1.EventTypeWarning, + "ProvisioningError", + provisionErr.Error(), + ) + } return fmt.Errorf("provisioning failed: %w", provisionErr) } diff --git a/pkg/controller/reconciler/reconciler_test.go b/pkg/controller/reconciler/reconciler_test.go index 76bb090c1db..dd963a056cf 100644 --- a/pkg/controller/reconciler/reconciler_test.go +++ b/pkg/controller/reconciler/reconciler_test.go @@ -8,7 +8,6 @@ import ( "testing" gomegaTypes "github.com/onsi/gomega/types" - conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" "github.com/rs/xid" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -30,6 +29,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster/gvk" odherrors "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/errors" odhClient "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/client" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/conditions" odhtype "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/resources" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/utils/test/matchers/jq" @@ -82,10 +82,13 @@ func createReconciler(cli *odhClient.Client) *Reconciler { return i, nil }, + conditionsManagerFactory: func(accessor common.ConditionsAccessor) *conditions.Manager { + return conditions.NewManager(accessor, status.ConditionTypeReady) + }, } } -func TestAvailableCondition(t *testing.T) { +func TestConditions(t *testing.T) { ctx := context.Background() g := NewWithT(t) @@ -133,30 +136,27 @@ func TestAvailableCondition(t *testing.T) { matcher gomegaTypes.GomegaMatcher }{ { - name: "available", + name: "ready", err: nil, matcher: And( - jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, conditionsv1.ConditionAvailable, metav1.ConditionTrue), - jq.Match(`.status.conditions[] | select(.type == "%s") | .reason == "%s"`, conditionsv1.ConditionAvailable, conditionsv1.ConditionAvailable), - jq.Match(`.status.conditions[] | select(.type == "%s") | .message == "%s"`, conditionsv1.ConditionAvailable, conditionsv1.ConditionAvailable), + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionTypeReady, metav1.ConditionTrue), + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionTypeProvisioningSucceeded, metav1.ConditionTrue), ), }, { name: "stop", err: odherrors.NewStopError("stop"), matcher: And( - jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, conditionsv1.ConditionAvailable, metav1.ConditionFalse), - jq.Match(`.status.conditions[] | select(.type == "%s") | .reason == "%s"`, conditionsv1.ConditionAvailable, "Degraded"), - jq.Match(`.status.conditions[] | select(.type == "%s") | .message == "%s"`, conditionsv1.ConditionAvailable, "stop"), + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionTypeReady, metav1.ConditionFalse), + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionTypeProvisioningSucceeded, metav1.ConditionFalse), ), }, { name: "failure", err: errors.New("failure"), matcher: And( - jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, conditionsv1.ConditionAvailable, metav1.ConditionFalse), - jq.Match(`.status.conditions[] | select(.type == "%s") | .reason == "%s"`, conditionsv1.ConditionAvailable, status.ReconcileFailed), - jq.Match(`.status.conditions[] | select(.type == "%s") | .message == "%s"`, conditionsv1.ConditionAvailable, "failure"), + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionTypeReady, metav1.ConditionFalse), + jq.Match(`.status.conditions[] | select(.type == "%s") | .status == "%s"`, status.ConditionTypeProvisioningSucceeded, metav1.ConditionFalse), ), }, }