diff --git a/internal/cmd/internal/olmv1/catalog_create.go b/internal/cmd/internal/olmv1/catalog_create.go index d27c01ea..df76c871 100644 --- a/internal/cmd/internal/olmv1/catalog_create.go +++ b/internal/cmd/internal/olmv1/catalog_create.go @@ -13,7 +13,7 @@ import ( // NewCatalogCreateCmd allows creating a new catalog func NewCatalogCreateCmd(cfg *action.Configuration) *cobra.Command { - i := v1action.NewCatalogCreate(cfg.Client) + i := v1action.NewCatalogCreate(cfg) i.Logf = log.Printf cmd := &cobra.Command{ diff --git a/internal/cmd/internal/olmv1/printing.go b/internal/cmd/internal/olmv1/printing.go index c69d27c0..4da607c2 100644 --- a/internal/cmd/internal/olmv1/printing.go +++ b/internal/cmd/internal/olmv1/printing.go @@ -21,11 +21,16 @@ func printFormattedOperators(extensions ...olmv1.ClusterExtension) { sortOperators(extensions) for _, ext := range extensions { + var bundleName, bundleVersion string + if ext.Status.Install != nil { + bundleName = ext.Status.Install.Bundle.Name + bundleVersion = ext.Status.Install.Bundle.Version + } age := time.Since(ext.CreationTimestamp.Time) _, _ = fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\t%s\t%s\n", ext.Name, - ext.Status.Install.Bundle.Name, - ext.Status.Install.Bundle.Version, + bundleName, + bundleVersion, ext.Spec.Source.SourceType, status(ext.Status.Conditions, olmv1.TypeInstalled), status(ext.Status.Conditions, olmv1.TypeProgressing), @@ -41,13 +46,16 @@ func printFormattedCatalogs(catalogs ...olmv1.ClusterCatalog) { sortCatalogs(catalogs) for _, cat := range catalogs { + var lastUnpacked string + if cat.Status.LastUnpacked != nil { + duration.HumanDuration(time.Since(cat.Status.LastUnpacked.Time)) + } age := time.Since(cat.CreationTimestamp.Time) - lastUnpacked := time.Since(cat.Status.LastUnpacked.Time) _, _ = fmt.Fprintf(tw, "%s\t%s\t%d\t%s\t%s\t%s\n", cat.Name, string(cat.Spec.AvailabilityMode), cat.Spec.Priority, - duration.HumanDuration(lastUnpacked), + lastUnpacked, status(cat.Status.Conditions, olmv1.TypeServing), duration.HumanDuration(age), ) @@ -59,6 +67,9 @@ func printFormattedCatalogs(catalogs ...olmv1.ClusterCatalog) { // name (asc), version (desc) func sortOperators(extensions []olmv1.ClusterExtension) { slices.SortFunc(extensions, func(a, b olmv1.ClusterExtension) int { + if a.Status.Install == nil || b.Status.Install == nil { + return cmp.Compare(a.Name, b.Name) + } return cmp.Or( cmp.Compare(a.Name, b.Name), -semver.MustParse(a.Status.Install.Bundle.Version).Compare(semver.MustParse(b.Status.Install.Bundle.Version)), diff --git a/internal/pkg/v1/action/action_suite_test.go b/internal/pkg/v1/action/action_suite_test.go index e01046b2..6b25714e 100644 --- a/internal/pkg/v1/action/action_suite_test.go +++ b/internal/pkg/v1/action/action_suite_test.go @@ -12,8 +12,16 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" olmv1 "github.com/operator-framework/operator-controller/api/v1" + + "github.com/operator-framework/kubectl-operator/pkg/action" +) + +const ( + verbCreate = "create" ) func TestCommand(t *testing.T) { @@ -21,34 +29,68 @@ func TestCommand(t *testing.T) { RunSpecs(t, "Internal v1 action Suite") } -type mockCreator struct { - createErr error - createCalled int -} - -func (mc *mockCreator) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { - mc.createCalled++ - return mc.createErr -} +type fakeClient struct { + // Expected errors for create/delete/get. + createErr error + deleteErr error + getErr error -type mockDeleter struct { - deleteErr error + // counters for number of create/delete/get calls seen. + createCalled int deleteCalled int -} + getCalled int -func (md *mockDeleter) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { - md.deleteCalled++ - return md.deleteErr + // transformer functions for applying changes to an object + // matching the objectKey prior to an operation of the + // type `verb` (get/create/delete), where the operation is + // not set to error fail with a corresponding error (getErr/createErr/deleteErr). + transformers []objectTransformer + client.Client } -type mockGetter struct { - getErr error - getCalled int +type objectTransformer struct { + verb string + objectKey client.ObjectKey + transformFunc func(obj *client.Object) } -func (mg *mockGetter) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { - mg.getCalled++ - return mg.getErr +func (c *fakeClient) Initialize() error { + scheme, err := action.NewScheme() + if err != nil { + return err + } + clientBuilder := fake.NewClientBuilder().WithInterceptorFuncs(interceptor.Funcs{ + Create: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error { + c.createCalled++ + if c.createErr != nil { + return c.createErr + } + objKey := types.NamespacedName{Name: obj.GetName(), Namespace: obj.GetNamespace()} + for _, t := range c.transformers { + if t.verb == verbCreate && objKey == t.objectKey && t.transformFunc != nil { + t.transformFunc(&obj) + } + } + // make sure to plumb request through to underlying client + return client.Create(ctx, obj, opts...) + }, + Delete: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.DeleteOption) error { + c.deleteCalled++ + if c.deleteErr != nil { + return c.deleteErr + } + return client.Delete(ctx, obj, opts...) + }, + Get: func(ctx context.Context, client client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + c.getCalled++ + if c.getErr != nil { + return c.getErr + } + return client.Get(ctx, key, obj, opts...) + }, + }).WithScheme(scheme) + c.Client = clientBuilder.Build() + return nil } func setupTestCatalogs(n int) []client.Object { diff --git a/internal/pkg/v1/action/catalog_create.go b/internal/pkg/v1/action/catalog_create.go index 7a206862..e8196a01 100644 --- a/internal/pkg/v1/action/catalog_create.go +++ b/internal/pkg/v1/action/catalog_create.go @@ -7,16 +7,12 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" olmv1 "github.com/operator-framework/operator-controller/api/v1" -) -type createClient interface { - creator - deleter - getter -} + "github.com/operator-framework/kubectl-operator/pkg/action" +) type CatalogCreate struct { - client createClient + config *action.Configuration CatalogName string ImageSourceRef string @@ -29,28 +25,28 @@ type CatalogCreate struct { Logf func(string, ...interface{}) } -func NewCatalogCreate(client createClient) *CatalogCreate { +func NewCatalogCreate(config *action.Configuration) *CatalogCreate { return &CatalogCreate{ - client: client, + config: config, Logf: func(string, ...interface{}) {}, } } func (i *CatalogCreate) Run(ctx context.Context) error { catalog := i.buildCatalog() - if err := i.client.Create(ctx, &catalog); err != nil { + if err := i.config.Client.Create(ctx, &catalog); err != nil { return err } var err error if i.Available { - err = waitUntilCatalogStatusCondition(ctx, i.client, &catalog, olmv1.TypeServing, metav1.ConditionTrue) + err = waitUntilCatalogStatusCondition(ctx, i.config.Client, &catalog, olmv1.TypeServing, metav1.ConditionTrue) } else { - err = waitUntilCatalogStatusCondition(ctx, i.client, &catalog, olmv1.TypeServing, metav1.ConditionFalse) + err = waitUntilCatalogStatusCondition(ctx, i.config.Client, &catalog, olmv1.TypeServing, metav1.ConditionFalse) } if err != nil { - if cleanupErr := deleteWithTimeout(i.client, &catalog, i.CleanupTimeout); cleanupErr != nil { + if cleanupErr := deleteWithTimeout(i.config.Client, &catalog, i.CleanupTimeout); cleanupErr != nil { i.Logf("cleaning up failed catalog: %v", cleanupErr) } return err diff --git a/internal/pkg/v1/action/catalog_create_test.go b/internal/pkg/v1/action/catalog_create_test.go index 9e7a52c1..e8729460 100644 --- a/internal/pkg/v1/action/catalog_create_test.go +++ b/internal/pkg/v1/action/catalog_create_test.go @@ -8,30 +8,21 @@ import ( . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" olmv1 "github.com/operator-framework/operator-controller/api/v1" internalaction "github.com/operator-framework/kubectl-operator/internal/pkg/v1/action" + "github.com/operator-framework/kubectl-operator/pkg/action" ) -type mockCreateClient struct { - *mockCreator - *mockGetter - *mockDeleter - createCatalog *olmv1.ClusterCatalog -} - -func (mcc *mockCreateClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { - mcc.createCatalog = obj.(*olmv1.ClusterCatalog) - return mcc.mockCreator.Create(ctx, obj, opts...) -} - var _ = Describe("CatalogCreate", func() { + catalogName := "testcatalog" pollInterval := 20 expectedCatalog := olmv1.ClusterCatalog{ ObjectMeta: metav1.ObjectMeta{ - Name: "testcatalog", + Name: catalogName, Labels: map[string]string{"a": "b"}, }, Spec: olmv1.ClusterCatalogSpec{ @@ -49,9 +40,10 @@ var _ = Describe("CatalogCreate", func() { It("fails creating catalog", func() { expectedErr := errors.New("create failed") - mockClient := &mockCreateClient{&mockCreator{createErr: expectedErr}, nil, nil, &expectedCatalog} + testClient := fakeClient{createErr: expectedErr} + Expect(testClient.Initialize()).To(Succeed()) - creator := internalaction.NewCatalogCreate(mockClient) + creator := internalaction.NewCatalogCreate(&action.Configuration{Client: testClient}) creator.Available = true creator.CatalogName = expectedCatalog.Name creator.ImageSourceRef = expectedCatalog.Spec.Source.Image.Ref @@ -62,41 +54,78 @@ var _ = Describe("CatalogCreate", func() { Expect(err).NotTo(BeNil()) Expect(err).To(MatchError(expectedErr)) - Expect(mockClient.createCalled).To(Equal(1)) - - // there is no way of testing a happy path in unit tests because we have no way to - // set/mock the catalog status condition we're waiting for in waitUntilCatalogStatusCondition - // but we can still at least verify that CR would have been created with expected attribute values - validateCreateCatalog(mockClient.createCatalog, &expectedCatalog) + Expect(testClient.createCalled).To(Equal(1)) }) It("fails waiting for created catalog status, successfully cleans up", func() { expectedErr := errors.New("get failed") - mockClient := &mockCreateClient{&mockCreator{}, &mockGetter{getErr: expectedErr}, &mockDeleter{}, nil} + testClient := fakeClient{getErr: expectedErr} + Expect(testClient.Initialize()).To(Succeed()) - creator := internalaction.NewCatalogCreate(mockClient) + creator := internalaction.NewCatalogCreate(&action.Configuration{Client: testClient}) + // fakeClient requires at least the catalogName to be set to run + creator.CatalogName = expectedCatalog.Name err := creator.Run(context.TODO()) Expect(err).NotTo(BeNil()) Expect(err).To(MatchError(expectedErr)) - Expect(mockClient.createCalled).To(Equal(1)) - Expect(mockClient.getCalled).To(Equal(1)) - Expect(mockClient.deleteCalled).To(Equal(1)) + Expect(testClient.createCalled).To(Equal(1)) + Expect(testClient.getCalled).To(Equal(1)) + Expect(testClient.deleteCalled).To(Equal(1)) }) It("fails waiting for created catalog status, fails clean up", func() { getErr := errors.New("get failed") deleteErr := errors.New("delete failed") - mockClient := &mockCreateClient{&mockCreator{}, &mockGetter{getErr: getErr}, &mockDeleter{deleteErr: deleteErr}, nil} + testClient := fakeClient{deleteErr: deleteErr, getErr: getErr} + Expect(testClient.Initialize()).To(Succeed()) - creator := internalaction.NewCatalogCreate(mockClient) + creator := internalaction.NewCatalogCreate(&action.Configuration{Client: testClient}) + // fakeClient requires at least the catalogName to be set to run + creator.CatalogName = expectedCatalog.Name err := creator.Run(context.TODO()) Expect(err).NotTo(BeNil()) Expect(err).To(MatchError(getErr)) - Expect(mockClient.createCalled).To(Equal(1)) - Expect(mockClient.getCalled).To(Equal(1)) - Expect(mockClient.deleteCalled).To(Equal(1)) + Expect(testClient.createCalled).To(Equal(1)) + Expect(testClient.getCalled).To(Equal(1)) + Expect(testClient.deleteCalled).To(Equal(1)) + }) + It("succeeds creating catalog", func() { + testClient := fakeClient{ + transformers: []objectTransformer{ + { + verb: verbCreate, + objectKey: types.NamespacedName{Name: catalogName}, + transformFunc: func(obj *client.Object) { + if obj == nil { + return + } + catalogObj, ok := (*obj).(*olmv1.ClusterCatalog) + if !ok { + return + } + catalogObj.Status.Conditions = []metav1.Condition{{Type: olmv1.TypeServing, Status: metav1.ConditionTrue}} + }, + }, + }, + } + Expect(testClient.Initialize()).To(Succeed()) + + creator := internalaction.NewCatalogCreate(&action.Configuration{Client: testClient}) + creator.Available = true + creator.CatalogName = expectedCatalog.Name + creator.ImageSourceRef = expectedCatalog.Spec.Source.Image.Ref + creator.Priority = expectedCatalog.Spec.Priority + creator.Labels = expectedCatalog.Labels + creator.PollIntervalMinutes = *expectedCatalog.Spec.Source.Image.PollIntervalMinutes + Expect(creator.Run(context.TODO())).To(Succeed()) + + Expect(testClient.createCalled).To(Equal(1)) + + actualCatalog := &olmv1.ClusterCatalog{TypeMeta: metav1.TypeMeta{Kind: "ClusterCatalog", APIVersion: "olm.operatorframework.io/v1"}} + Expect(testClient.Client.Get(context.TODO(), types.NamespacedName{Name: catalogName}, actualCatalog)).To(Succeed()) + validateCreateCatalog(actualCatalog, &expectedCatalog) }) }) diff --git a/internal/pkg/v1/action/helpers.go b/internal/pkg/v1/action/helpers.go index e3002300..69edd750 100644 --- a/internal/pkg/v1/action/helpers.go +++ b/internal/pkg/v1/action/helpers.go @@ -80,9 +80,8 @@ func deleteWithTimeout(cl deleter, obj client.Object, timeout time.Duration) err return nil } -func waitForDeletion(ctx context.Context, cl client.Client, objs ...client.Object) error { +func waitForDeletion(ctx context.Context, cl getter, objs ...client.Object) error { for _, obj := range objs { - obj := obj lowerKind := strings.ToLower(obj.GetObjectKind().GroupVersionKind().Kind) key := objectKeyForObject(obj) if err := wait.PollUntilContextCancel(ctx, pollInterval, true, func(conditionCtx context.Context) (bool, error) { diff --git a/internal/pkg/v1/action/interfaces.go b/internal/pkg/v1/action/interfaces.go index cdc17ae0..1acf1366 100644 --- a/internal/pkg/v1/action/interfaces.go +++ b/internal/pkg/v1/action/interfaces.go @@ -6,10 +6,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -type creator interface { - Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error -} - type deleter interface { Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error } diff --git a/internal/pkg/v1/action/operator_update_test.go b/internal/pkg/v1/action/operator_update_test.go index f4459f7b..79743635 100644 --- a/internal/pkg/v1/action/operator_update_test.go +++ b/internal/pkg/v1/action/operator_update_test.go @@ -145,8 +145,9 @@ var _ = Describe("OperatorUpdate", func() { cfg := setupEnv(testExt, buildExtension("test2"), buildExtension("test3")) go func() { - Eventually(updateOperatorConditionStatus("test", cfg.Client, olmv1.TypeInstalled, metav1.ConditionTrue)). - WithTimeout(5 * time.Second).WithPolling(200 * time.Second). + Eventually(updateOperatorConditionStatus). + WithArguments("test", cfg.Client, olmv1.TypeInstalled, metav1.ConditionTrue). + WithTimeout(5 * time.Second).WithPolling(200 * time.Millisecond). Should(Succeed()) }()