diff --git a/api/v1alpha1/clusterextension_types.go b/api/v1alpha1/clusterextension_types.go index 97578bbf8..c69803614 100644 --- a/api/v1alpha1/clusterextension_types.go +++ b/api/v1alpha1/clusterextension_types.go @@ -92,6 +92,7 @@ const ( TypePackageDeprecated = "PackageDeprecated" TypeChannelDeprecated = "ChannelDeprecated" TypeBundleDeprecated = "BundleDeprecated" + TypeUnpacked = "Unpacked" ReasonErrorGettingClient = "ErrorGettingClient" ReasonBundleLoadFailed = "BundleLoadFailed" @@ -101,9 +102,11 @@ const ( ReasonInstallationSucceeded = "InstallationSucceeded" ReasonResolutionFailed = "ResolutionFailed" - ReasonSuccess = "Success" - ReasonDeprecated = "Deprecated" - ReasonUpgradeFailed = "UpgradeFailed" + ReasonSuccess = "Success" + ReasonDeprecated = "Deprecated" + ReasonUpgradeFailed = "UpgradeFailed" + ReasonHasValidBundleUnknown = "HasValidBundleUnknown" + ReasonUnpackPending = "UnpackPending" ) func init() { @@ -116,6 +119,7 @@ func init() { TypePackageDeprecated, TypeChannelDeprecated, TypeBundleDeprecated, + TypeUnpacked, ) // TODO(user): add Reasons from above conditionsets.ConditionReasons = append(conditionsets.ConditionReasons, @@ -128,6 +132,8 @@ func init() { ReasonBundleLoadFailed, ReasonErrorGettingClient, ReasonInstallationStatusUnknown, + ReasonHasValidBundleUnknown, + ReasonUnpackPending, ) } diff --git a/go.mod b/go.mod index fd378c54b..c740e962d 100644 --- a/go.mod +++ b/go.mod @@ -165,6 +165,7 @@ require ( github.com/skeema/knownhosts v1.2.2 // indirect github.com/spf13/cast v1.5.0 // indirect github.com/stoewer/go-strcase v1.3.0 // indirect + github.com/stretchr/objx v0.5.2 // indirect github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635 // indirect github.com/ulikunitz/xz v0.5.11 // indirect github.com/vbatts/tar-split v0.11.5 // indirect diff --git a/internal/controllers/clusterextension_controller.go b/internal/controllers/clusterextension_controller.go index 80a682eb4..f92430005 100644 --- a/internal/controllers/clusterextension_controller.go +++ b/internal/controllers/clusterextension_controller.go @@ -235,6 +235,8 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration()) return ctrl.Result{}, err } + // set deprecation status after _successful_ resolution + SetDeprecationStatus(ext, bundle) bundleVersion, err := bundle.Version() if err != nil { @@ -259,12 +261,17 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp switch unpackResult.State { case rukpaksource.StatePending: - updateStatusUnpackPending(&ext.Status, unpackResult) + updateStatusUnpackPending(&ext.Status, unpackResult, ext.GetGeneration()) // There must be a limit to number of entries if status is stuck at // unpack pending. + setHasValidBundleUnknown(&ext.Status.Conditions, "unpack pending", ext.GetGeneration()) + setInstalledStatusConditionUnknown(&ext.Status.Conditions, "installation has not been attempted as unpack is pending", ext.GetGeneration()) + return ctrl.Result{}, nil case rukpaksource.StateUnpacking: updateStatusUnpacking(&ext.Status, unpackResult) + setHasValidBundleUnknown(&ext.Status.Conditions, "unpack pending", ext.GetGeneration()) + setInstalledStatusConditionUnknown(&ext.Status.Conditions, "installation has not been attempted as unpack is pending", ext.GetGeneration()) return ctrl.Result{}, nil case rukpaksource.StateUnpacked: // TODO: Add finalizer to clean the stored bundles, after https://github.com/operator-framework/rukpak/pull/897 @@ -409,7 +416,7 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1 channelName := ext.Spec.Channel versionRange := ext.Spec.Version - installedBundle, err := r.installedBundle(ctx, allBundles, &ext) + installedBundle, err := GetInstalledbundle(ctx, r.ActionClientGetter, allBundles, &ext) if err != nil { return nil, err } @@ -669,8 +676,41 @@ func clusterExtensionRequestsForCatalog(c client.Reader, logger logr.Logger) crh } } -func (r *ClusterExtensionReconciler) installedBundle(ctx context.Context, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { - cl, err := r.ActionClientGetter.ActionClientFor(ctx, ext) +type releaseState string + +const ( + stateNeedsInstall releaseState = "NeedsInstall" + stateNeedsUpgrade releaseState = "NeedsUpgrade" + stateUnchanged releaseState = "Unchanged" + stateError releaseState = "Error" +) + +func (r *ClusterExtensionReconciler) getReleaseState(cl helmclient.ActionInterface, obj metav1.Object, chrt *chart.Chart, values chartutil.Values, post *postrenderer) (*release.Release, releaseState, error) { + currentRelease, err := cl.Get(obj.GetName()) + if err != nil && !errors.Is(err, driver.ErrReleaseNotFound) { + return nil, stateError, err + } + if errors.Is(err, driver.ErrReleaseNotFound) { + return nil, stateNeedsInstall, nil + } + + desiredRelease, err := cl.Upgrade(obj.GetName(), r.ReleaseNamespace, chrt, values, func(upgrade *action.Upgrade) error { + upgrade.DryRun = true + return nil + }, helmclient.AppendUpgradePostRenderer(post)) + if err != nil { + return currentRelease, stateError, err + } + if desiredRelease.Manifest != currentRelease.Manifest || + currentRelease.Info.Status == release.StatusFailed || + currentRelease.Info.Status == release.StatusSuperseded { + return currentRelease, stateNeedsUpgrade, nil + } + return currentRelease, stateUnchanged, nil +} + +var GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { + cl, err := acg.ActionClientFor(ctx, ext) if err != nil { return nil, err } @@ -706,39 +746,6 @@ func (r *ClusterExtensionReconciler) installedBundle(ctx context.Context, allBun return resultSet[0], nil } -type releaseState string - -const ( - stateNeedsInstall releaseState = "NeedsInstall" - stateNeedsUpgrade releaseState = "NeedsUpgrade" - stateUnchanged releaseState = "Unchanged" - stateError releaseState = "Error" -) - -func (r *ClusterExtensionReconciler) getReleaseState(cl helmclient.ActionInterface, obj metav1.Object, chrt *chart.Chart, values chartutil.Values, post *postrenderer) (*release.Release, releaseState, error) { - currentRelease, err := cl.Get(obj.GetName()) - if err != nil && !errors.Is(err, driver.ErrReleaseNotFound) { - return nil, stateError, err - } - if errors.Is(err, driver.ErrReleaseNotFound) { - return nil, stateNeedsInstall, nil - } - - desiredRelease, err := cl.Upgrade(obj.GetName(), r.ReleaseNamespace, chrt, values, func(upgrade *action.Upgrade) error { - upgrade.DryRun = true - return nil - }, helmclient.AppendUpgradePostRenderer(post)) - if err != nil { - return currentRelease, stateError, err - } - if desiredRelease.Manifest != currentRelease.Manifest || - currentRelease.Info.Status == release.StatusFailed || - currentRelease.Info.Status == release.StatusSuperseded { - return currentRelease, stateNeedsUpgrade, nil - } - return currentRelease, stateUnchanged, nil -} - type errRequiredResourceNotFound struct { error } diff --git a/internal/controllers/clusterextension_controller_test.go b/internal/controllers/clusterextension_controller_test.go index b0b121188..5dcd2ea6a 100644 --- a/internal/controllers/clusterextension_controller_test.go +++ b/internal/controllers/clusterextension_controller_test.go @@ -9,6 +9,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -18,9 +19,11 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2" + "github.com/operator-framework/rukpak/pkg/source" ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/catalogmetadata" @@ -126,68 +129,14 @@ func TestClusterExtensionNonExistentVersion(t *testing.T) { require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) } -func TestClusterExtensionBundleDeploymentDoesNotExist(t *testing.T) { - t.Skip("Skip and replace with e2e for BundleDeployment") - cl, reconciler := newClientAndReconciler(t) - ctx := context.Background() - extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} - const pkgName = "prometheus" - installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8)) - - t.Log("When the cluster extension specifies a valid available package") - t.Log("By initializing cluster state") - clusterExtension := &ocv1alpha1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{Name: extKey.Name}, - Spec: ocv1alpha1.ClusterExtensionSpec{ - PackageName: pkgName, - InstallNamespace: installNamespace, - }, - } - require.NoError(t, cl.Create(ctx, clusterExtension)) - - t.Log("When the BundleDeployment does not exist") - t.Log("By running reconcile") - res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) - require.Equal(t, ctrl.Result{}, res) - require.NoError(t, err) - - t.Log("By fetching updated cluster extension after reconcile") - require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) - - t.Log("It results in the expected BundleDeployment") - bd := &rukpakv1alpha2.BundleDeployment{} - require.NoError(t, cl.Get(ctx, types.NamespacedName{Name: extKey.Name}, bd)) - require.Equal(t, "core-rukpak-io-registry", bd.Spec.ProvisionerClassName) - require.Equal(t, installNamespace, bd.Spec.InstallNamespace) - require.NotNil(t, bd.Spec.Source.Image) - require.Equal(t, "quay.io/operatorhubio/prometheus@fake2.0.0", bd.Spec.Source.Image.Ref) - - t.Log("It sets the ResolvedBundle status field") - require.Equal(t, &ocv1alpha1.BundleMetadata{Name: "operatorhub/prometheus/beta/2.0.0", Version: "2.0.0"}, clusterExtension.Status.ResolvedBundle) - - t.Log("It sets the InstalledBundle status field") - require.Empty(t, clusterExtension.Status.InstalledBundle) - - t.Log("It sets the status on the cluster extension") - cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1alpha1.ReasonSuccess, cond.Reason) - require.Equal(t, "resolved to \"quay.io/operatorhubio/prometheus@fake2.0.0\"", cond.Message) - - cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionUnknown, cond.Status) - require.Equal(t, ocv1alpha1.ReasonInstallationStatusUnknown, cond.Reason) - require.Equal(t, "bundledeployment status is unknown", cond.Message) - - verifyInvariants(ctx, t, reconciler.Client, clusterExtension) - require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) -} - func TestClusterExtensionChannelVersionExists(t *testing.T) { - t.Skip("Skip and replace with e2e for BundleDeployment") cl, reconciler := newClientAndReconciler(t) + mockUnpacker := unpacker.(*MockUnpacker) + // Set up the Unpack method to return a result with StateUnpacked + mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*v1alpha2.BundleDeployment")).Return(&source.Result{ + State: source.StatePending, + }, nil) + ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -224,33 +173,29 @@ func TestClusterExtensionChannelVersionExists(t *testing.T) { require.Empty(t, clusterExtension.Status.InstalledBundle) t.Log("By checking the expected conditions") - cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1alpha1.ReasonSuccess, cond.Reason) - require.Equal(t, "resolved to \"quay.io/operatorhubio/prometheus@fake1.0.0\"", cond.Message) - cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionUnknown, cond.Status) - require.Equal(t, ocv1alpha1.ReasonInstallationStatusUnknown, cond.Reason) - require.Equal(t, "bundledeployment status is unknown", cond.Message) - - t.Log("By fetching the bundled deployment") - bd := &rukpakv1alpha2.BundleDeployment{} - require.NoError(t, cl.Get(ctx, types.NamespacedName{Name: extKey.Name}, bd)) - require.Equal(t, "core-rukpak-io-registry", bd.Spec.ProvisionerClassName) - require.Equal(t, installNamespace, bd.Spec.InstallNamespace) - require.Equal(t, rukpakv1alpha2.SourceTypeImage, bd.Spec.Source.Type) - require.NotNil(t, bd.Spec.Source.Image) - require.Equal(t, "quay.io/operatorhubio/prometheus@fake1.0.0", bd.Spec.Source.Image.Ref) + resolvedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) + require.NotNil(t, resolvedCond) + require.Equal(t, metav1.ConditionTrue, resolvedCond.Status) + require.Equal(t, ocv1alpha1.ReasonSuccess, resolvedCond.Reason) + require.Equal(t, "resolved to \"quay.io/operatorhubio/prometheus@fake1.0.0\"", resolvedCond.Message) + + t.Log("By checking the expected unpacked conditions") + unpackedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, rukpakv1alpha2.TypeUnpacked) + require.NotNil(t, unpackedCond) + require.Equal(t, metav1.ConditionFalse, unpackedCond.Status) + require.Equal(t, rukpakv1alpha2.ReasonUnpackPending, unpackedCond.Reason) - verifyInvariants(ctx, t, reconciler.Client, clusterExtension) require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) } func TestClusterExtensionChannelExistsNoVersion(t *testing.T) { - t.Skip("Skip and replace with e2e for BundleDeployment") cl, reconciler := newClientAndReconciler(t) + mockUnpacker := unpacker.(*MockUnpacker) + // Set up the Unpack method to return a result with StateUnpacked + mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*v1alpha2.BundleDeployment")).Return(&source.Result{ + State: source.StatePending, + }, nil) + ctx := context.Background() extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))} @@ -269,13 +214,15 @@ func TestClusterExtensionChannelExistsNoVersion(t *testing.T) { InstallNamespace: installNamespace, }, } - require.NoError(t, cl.Create(ctx, clusterExtension)) + err := cl.Create(ctx, clusterExtension) + require.NoError(t, err) t.Log("It sets resolution success status") t.Log("By running reconcile") res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Equal(t, ctrl.Result{}, res) require.NoError(t, err) + t.Log("By fetching updated cluster extension after reconcile") require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) @@ -284,25 +231,17 @@ func TestClusterExtensionChannelExistsNoVersion(t *testing.T) { require.Empty(t, clusterExtension.Status.InstalledBundle) t.Log("By checking the expected conditions") - cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionTrue, cond.Status) - require.Equal(t, ocv1alpha1.ReasonSuccess, cond.Reason) - require.Equal(t, "resolved to \"quay.io/operatorhubio/prometheus@fake2.0.0\"", cond.Message) - cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled) - require.NotNil(t, cond) - require.Equal(t, metav1.ConditionUnknown, cond.Status) - require.Equal(t, ocv1alpha1.ReasonInstallationStatusUnknown, cond.Reason) - require.Equal(t, "bundledeployment status is unknown", cond.Message) - - t.Log("By fetching the bundledeployment") - bd := &rukpakv1alpha2.BundleDeployment{} - require.NoError(t, cl.Get(ctx, types.NamespacedName{Name: extKey.Name}, bd)) - require.Equal(t, "core-rukpak-io-registry", bd.Spec.ProvisionerClassName) - require.Equal(t, installNamespace, bd.Spec.InstallNamespace) - require.Equal(t, rukpakv1alpha2.SourceTypeImage, bd.Spec.Source.Type) - require.NotNil(t, bd.Spec.Source.Image) - require.Equal(t, "quay.io/operatorhubio/prometheus@fake2.0.0", bd.Spec.Source.Image.Ref) + resolvedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) + require.NotNil(t, resolvedCond) + require.Equal(t, metav1.ConditionTrue, resolvedCond.Status) + require.Equal(t, ocv1alpha1.ReasonSuccess, resolvedCond.Reason) + require.Equal(t, "resolved to \"quay.io/operatorhubio/prometheus@fake2.0.0\"", resolvedCond.Message) + + t.Log("By checking the expected unpacked conditions") + unpackedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, rukpakv1alpha2.TypeUnpacked) + require.NotNil(t, unpackedCond) + require.Equal(t, metav1.ConditionFalse, unpackedCond.Status) + require.Equal(t, rukpakv1alpha2.ReasonUnpackPending, unpackedCond.Reason) verifyInvariants(ctx, t, reconciler.Client, clusterExtension) require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) @@ -478,10 +417,19 @@ func verifyConditionsInvariants(t *testing.T, ext *ocv1alpha1.ClusterExtension) func TestClusterExtensionUpgrade(t *testing.T) { cl, reconciler := newClientAndReconciler(t) + mockUnpacker := unpacker.(*MockUnpacker) + // Set up the Unpack method to return a result with StateUnpackPending + mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*v1alpha2.BundleDeployment")).Return(&source.Result{ + State: source.StatePending, + }, nil) ctx := context.Background() + defer func() { + controllers.GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { + return nil, nil + } + }() t.Run("semver upgrade constraints enforcement of upgrades within major version", func(t *testing.T) { - t.Skip("Skip and replace with e2e for BundleDeployment") defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, true)() defer func() { require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) @@ -529,6 +477,22 @@ func TestClusterExtensionUpgrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) + controllers.GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { + return &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/beta/1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, + }, + }, + CatalogName: "fake-catalog", + InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, + }, nil + } + // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Error(t, err) @@ -547,7 +511,7 @@ func TestClusterExtensionUpgrade(t *testing.T) { require.NotNil(t, cond) assert.Equal(t, metav1.ConditionFalse, cond.Status) assert.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason) - assert.Equal(t, "error upgrading from currently installed version \"1.0.0\": no package \"prometheus\" matching version \"2.0.0\" found in channel \"beta\"", cond.Message) + assert.Equal(t, "error upgrading from currently installed version \"1.0.0\": no package \"prometheus\" matching version \"2.0.0\" in channel \"beta\" found", cond.Message) // Valid update skipping one version clusterExtension.Spec.Version = "1.2.0" @@ -575,7 +539,6 @@ func TestClusterExtensionUpgrade(t *testing.T) { }) t.Run("legacy semantics upgrade constraints enforcement", func(t *testing.T) { - t.Skip("Skip and replace with e2e for BundleDeployment") defer featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.ForceSemverUpgradeConstraints, false)() defer func() { require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) @@ -623,6 +586,22 @@ func TestClusterExtensionUpgrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) + controllers.GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { + return &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/beta/1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, + }, + }, + CatalogName: "fake-catalog", + InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, + }, nil + } + // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Error(t, err) @@ -641,7 +620,7 @@ func TestClusterExtensionUpgrade(t *testing.T) { require.NotNil(t, cond) assert.Equal(t, metav1.ConditionFalse, cond.Status) assert.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason) - assert.Equal(t, "error upgrading from currently installed version \"1.0.0\": no package \"prometheus\" matching version \"1.2.0\" found in channel \"beta\"", cond.Message) + assert.Equal(t, "error upgrading from currently installed version \"1.0.0\": no package \"prometheus\" matching version \"1.2.0\" in channel \"beta\" found", cond.Message) // Valid update skipping one version clusterExtension.Spec.Version = "1.0.1" @@ -669,7 +648,6 @@ func TestClusterExtensionUpgrade(t *testing.T) { }) t.Run("ignore upgrade constraints", func(t *testing.T) { - t.Skip("Skip and replace with e2e for BundleDeployment") for _, tt := range []struct { name string flagState bool @@ -731,6 +709,22 @@ func TestClusterExtensionUpgrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) + controllers.GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { + return &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/beta/1.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, + }, + }, + CatalogName: "fake-catalog", + InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, + }, nil + } + // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.NoError(t, err) @@ -756,10 +750,19 @@ func TestClusterExtensionUpgrade(t *testing.T) { func TestClusterExtensionDowngrade(t *testing.T) { cl, reconciler := newClientAndReconciler(t) + mockUnpacker := unpacker.(*MockUnpacker) + // Set up the Unpack method to return a result with StateUnpacked + mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*v1alpha2.BundleDeployment")).Return(&source.Result{ + State: source.StatePending, + }, nil) ctx := context.Background() + defer func() { + controllers.GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { + return nil, nil + } + }() t.Run("enforce upgrade constraints", func(t *testing.T) { - t.Skip("Skip and replace with e2e for BundleDeployment") for _, tt := range []struct { name string flagState bool @@ -818,6 +821,22 @@ func TestClusterExtensionDowngrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) + controllers.GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { + return &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/beta/1.0.1", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake1.0.1", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"1.0.1"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, + }, + }, + CatalogName: "fake-catalog", + InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, + }, nil + } + // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Error(t, err) @@ -836,13 +855,12 @@ func TestClusterExtensionDowngrade(t *testing.T) { require.NotNil(t, cond) assert.Equal(t, metav1.ConditionFalse, cond.Status) assert.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason) - assert.Equal(t, "error upgrading from currently installed version \"1.0.1\": no package \"prometheus\" matching version \"1.0.0\" found in channel \"beta\"", cond.Message) + assert.Equal(t, "error upgrading from currently installed version \"1.0.1\": no package \"prometheus\" matching version \"1.0.0\" in channel \"beta\" found", cond.Message) }) } }) t.Run("ignore upgrade constraints", func(t *testing.T) { - t.Skip("Skip and replace with e2e for BundleDeployment") for _, tt := range []struct { name string flagState bool @@ -902,6 +920,22 @@ func TestClusterExtensionDowngrade(t *testing.T) { err = cl.Update(ctx, clusterExtension) require.NoError(t, err) + controllers.GetInstalledbundle = func(ctx context.Context, acg helmclient.ActionClientGetter, allBundles []*catalogmetadata.Bundle, ext *ocv1alpha1.ClusterExtension) (*catalogmetadata.Bundle, error) { + return &catalogmetadata.Bundle{ + Bundle: declcfg.Bundle{ + Name: "operatorhub/prometheus/beta/2.0.0", + Package: "prometheus", + Image: "quay.io/operatorhubio/prometheus@fake2.0.0", + Properties: []property.Property{ + {Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"prometheus","version":"2.0.0"}`)}, + {Type: property.TypeGVK, Value: json.RawMessage(`[]`)}, + }, + }, + CatalogName: "fake-catalog", + InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel}, + }, nil + } + // Run reconcile again res, err = reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.NoError(t, err) diff --git a/internal/controllers/clusterextension_registryv1_validation_test.go b/internal/controllers/clusterextension_registryv1_validation_test.go index 3d14b3ac4..a289d65ba 100644 --- a/internal/controllers/clusterextension_registryv1_validation_test.go +++ b/internal/controllers/clusterextension_registryv1_validation_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -16,6 +17,7 @@ import ( "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" + "github.com/operator-framework/rukpak/pkg/source" ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/catalogmetadata" @@ -31,7 +33,6 @@ func TestClusterExtensionRegistryV1DisallowDependencies(t *testing.T) { name string bundle *catalogmetadata.Bundle wantErr string - skip bool }{ { name: "package with no dependencies", @@ -46,9 +47,6 @@ func TestClusterExtensionRegistryV1DisallowDependencies(t *testing.T) { }, CatalogName: "fake-catalog", }, - // Skipping the happy path, since it requires us to mock unpacker, store and the - // entire installation. This should be handled in an e2e instead. - skip: true, }, { name: "package with olm.package.required property", @@ -103,16 +101,20 @@ func TestClusterExtensionRegistryV1DisallowDependencies(t *testing.T) { defer func() { require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{})) }() - - if tt.skip { - return - } - fakeCatalogClient := testutil.NewFakeCatalogClient([]*catalogmetadata.Bundle{tt.bundle}) + mockUnpacker := unpacker.(*MockUnpacker) + // Verify if mockUnpacker is correctly set + t.Log("MockUnpacker set to:", mockUnpacker) + // Set up the Unpack method to return a result with StatePending + mockUnpacker.On("Unpack", mock.Anything, mock.AnythingOfType("*v1alpha2.BundleDeployment")).Return(&source.Result{ + State: source.StatePending, + }, nil) + reconciler := &controllers.ClusterExtensionReconciler{ Client: cl, BundleProvider: &fakeCatalogClient, - ActionClientGetter: acg, + ActionClientGetter: helmClientGetter, + Unpacker: unpacker, } installNamespace := fmt.Sprintf("test-ns-%s", rand.String(8)) diff --git a/internal/controllers/clusterextension_status.go b/internal/controllers/clusterextension_status.go deleted file mode 100644 index ba5de76cd..000000000 --- a/internal/controllers/clusterextension_status.go +++ /dev/null @@ -1,72 +0,0 @@ -/* -Copyright 2023. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package controllers - -import ( - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2" - "github.com/operator-framework/rukpak/pkg/source" - - ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" -) - -func updateStatusUnpackFailing(status *ocv1alpha1.ClusterExtensionStatus, err error) error { - status.ResolvedBundle = nil - status.InstalledBundle = nil - meta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: rukpakv1alpha2.TypeUnpacked, - Status: metav1.ConditionFalse, - Reason: rukpakv1alpha2.ReasonUnpackFailed, - Message: err.Error(), - }) - return err -} - -// TODO: verify if we need to update the installBundle status or leave it as is. -func updateStatusUnpackPending(status *ocv1alpha1.ClusterExtensionStatus, result *source.Result) { - status.ResolvedBundle = nil - status.InstalledBundle = nil - meta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: rukpakv1alpha2.TypeUnpacked, - Status: metav1.ConditionFalse, - Reason: rukpakv1alpha2.ReasonUnpackPending, - Message: result.Message, - }) -} - -// TODO: verify if we need to update the installBundle status or leave it as is. -func updateStatusUnpacking(status *ocv1alpha1.ClusterExtensionStatus, result *source.Result) { - status.ResolvedBundle = nil - status.InstalledBundle = nil - meta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: rukpakv1alpha2.TypeUnpacked, - Status: metav1.ConditionFalse, - Reason: rukpakv1alpha2.ReasonUnpacking, - Message: result.Message, - }) -} - -func updateStatusUnpacked(status *ocv1alpha1.ClusterExtensionStatus, result *source.Result) { - meta.SetStatusCondition(&status.Conditions, metav1.Condition{ - Type: rukpakv1alpha2.TypeUnpacked, - Status: metav1.ConditionTrue, - Reason: rukpakv1alpha2.ReasonUnpackSuccessful, - Message: result.Message, - }) -} diff --git a/internal/controllers/common_controller.go b/internal/controllers/common_controller.go index a927e6d2f..7031d3a8f 100644 --- a/internal/controllers/common_controller.go +++ b/internal/controllers/common_controller.go @@ -22,6 +22,9 @@ import ( apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + rukpakv1alpha2 "github.com/operator-framework/rukpak/api/v1alpha2" + "github.com/operator-framework/rukpak/pkg/source" + ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/catalogmetadata" ) @@ -43,6 +46,28 @@ func setResolvedStatusConditionSuccess(conditions *[]metav1.Condition, message s }) } +// setInstalledStatusConditionUnknown sets the installed status condition to unknown. +func setInstalledStatusConditionUnknown(conditions *[]metav1.Condition, message string, generation int64) { + apimeta.SetStatusCondition(conditions, metav1.Condition{ + Type: ocv1alpha1.TypeInstalled, + Status: metav1.ConditionUnknown, + Reason: ocv1alpha1.ReasonInstallationStatusUnknown, + Message: message, + ObservedGeneration: generation, + }) +} + +// setHasValidBundleUnknown sets the installed status condition to unknown. +func setHasValidBundleUnknown(conditions *[]metav1.Condition, message string, generation int64) { + apimeta.SetStatusCondition(conditions, metav1.Condition{ + Type: ocv1alpha1.TypeHasValidBundle, + Status: metav1.ConditionUnknown, + Reason: ocv1alpha1.ReasonHasValidBundleUnknown, + Message: message, + ObservedGeneration: generation, + }) +} + // setResolvedStatusConditionFailed sets the resolved status condition to failed. func setResolvedStatusConditionFailed(conditions *[]metav1.Condition, message string, generation int64) { apimeta.SetStatusCondition(conditions, metav1.Condition{ @@ -95,3 +120,46 @@ func setDeprecationStatusesUnknown(conditions *[]metav1.Condition, message strin }) } } + +func updateStatusUnpackFailing(status *ocv1alpha1.ClusterExtensionStatus, err error) error { + status.InstalledBundle = nil + apimeta.SetStatusCondition(&status.Conditions, metav1.Condition{ + Type: rukpakv1alpha2.TypeUnpacked, + Status: metav1.ConditionFalse, + Reason: rukpakv1alpha2.ReasonUnpackFailed, + Message: err.Error(), + }) + return err +} + +// TODO: verify if we need to update the installBundle status or leave it as is. +func updateStatusUnpackPending(status *ocv1alpha1.ClusterExtensionStatus, result *source.Result, generation int64) { + status.InstalledBundle = nil + apimeta.SetStatusCondition(&status.Conditions, metav1.Condition{ + Type: rukpakv1alpha2.TypeUnpacked, + Status: metav1.ConditionFalse, + Reason: rukpakv1alpha2.ReasonUnpackPending, + Message: result.Message, + ObservedGeneration: generation, + }) +} + +// TODO: verify if we need to update the installBundle status or leave it as is. +func updateStatusUnpacking(status *ocv1alpha1.ClusterExtensionStatus, result *source.Result) { + status.InstalledBundle = nil + apimeta.SetStatusCondition(&status.Conditions, metav1.Condition{ + Type: rukpakv1alpha2.TypeUnpacked, + Status: metav1.ConditionFalse, + Reason: rukpakv1alpha2.ReasonUnpacking, + Message: result.Message, + }) +} + +func updateStatusUnpacked(status *ocv1alpha1.ClusterExtensionStatus, result *source.Result) { + apimeta.SetStatusCondition(&status.Conditions, metav1.Condition{ + Type: rukpakv1alpha2.TypeUnpacked, + Status: metav1.ConditionTrue, + Reason: rukpakv1alpha2.ReasonUnpackSuccessful, + Message: result.Message, + }) +} diff --git a/internal/controllers/suite_test.go b/internal/controllers/suite_test.go index 9f4bf8cde..a46e1bc6d 100644 --- a/internal/controllers/suite_test.go +++ b/internal/controllers/suite_test.go @@ -17,31 +17,78 @@ limitations under the License. package controllers_test import ( - "crypto/x509" + "context" + "io/fs" "log" + "net/http" "os" "path/filepath" "testing" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/api/meta" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" - "sigs.k8s.io/controller-runtime/pkg/manager" helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client" + "github.com/operator-framework/rukpak/api/v1alpha2" "github.com/operator-framework/rukpak/pkg/source" - "github.com/operator-framework/rukpak/pkg/util" + "github.com/operator-framework/rukpak/pkg/storage" "github.com/operator-framework/operator-controller/internal/controllers" "github.com/operator-framework/operator-controller/pkg/scheme" testutil "github.com/operator-framework/operator-controller/test/util" ) +// MockUnpacker is a mock of Unpacker interface +type MockUnpacker struct { + mock.Mock +} + +// Unpack mocks the Unpack method +func (m *MockUnpacker) Unpack(ctx context.Context, bd *v1alpha2.BundleDeployment) (*source.Result, error) { + args := m.Called(ctx, bd) + return args.Get(0).(*source.Result), args.Error(1) +} + +// MockStorage is a mock of Storage interface +type MockStorage struct { + mock.Mock +} + +func (m *MockStorage) Load(ctx context.Context, owner client.Object) (fs.FS, error) { + args := m.Called(ctx, owner) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(fs.FS), args.Error(1) +} + +func (m *MockStorage) Delete(ctx context.Context, owner client.Object) error { + //TODO implement me + panic("implement me") +} + +func (m *MockStorage) ServeHTTP(writer http.ResponseWriter, request *http.Request) { + //TODO implement me + panic("implement me") +} + +func (m *MockStorage) URLFor(ctx context.Context, owner client.Object) (string, error) { + //TODO implement me + panic("implement me") +} + +func (m *MockStorage) Store(ctx context.Context, owner client.Object, bundle fs.FS) error { + args := m.Called(ctx, owner, bundle) + return args.Error(0) +} + func newClient(t *testing.T) client.Client { - cl, err := client.New(cfg, client.Options{Scheme: scheme.Scheme}) + cl, err := client.New(config, client.Options{Scheme: scheme.Scheme}) require.NoError(t, err) require.NotNil(t, cl) return cl @@ -53,16 +100,18 @@ func newClientAndReconciler(t *testing.T) (client.Client, *controllers.ClusterEx reconciler := &controllers.ClusterExtensionReconciler{ Client: cl, BundleProvider: &fakeCatalogClient, - ActionClientGetter: acg, - Unpacker: unp, + ActionClientGetter: helmClientGetter, + Unpacker: unpacker, + Storage: store, } return cl, reconciler } var ( - cfg *rest.Config - acg helmclient.ActionClientGetter - unp source.Unpacker + config *rest.Config + helmClientGetter helmclient.ActionClientGetter + unpacker source.Unpacker // Interface, will be initialized as a mock in TestMain + store storage.Storage ) func TestMain(m *testing.M) { @@ -73,22 +122,20 @@ func TestMain(m *testing.M) { } var err error - cfg, err = testEnv.Start() + config, err = testEnv.Start() utilruntime.Must(err) - if cfg == nil { + if config == nil { log.Panic("expected cfg to not be nil") } rm := meta.NewDefaultRESTMapper(nil) - cfgGetter, err := helmclient.NewActionConfigGetter(cfg, rm) + cfgGetter, err := helmclient.NewActionConfigGetter(config, rm) utilruntime.Must(err) - acg, err = helmclient.NewActionClientGetter(cfgGetter) + helmClientGetter, err = helmclient.NewActionClientGetter(cfgGetter) utilruntime.Must(err) - mgr, err := manager.New(cfg, manager.Options{}) - utilruntime.Must(err) - unp, err = source.NewDefaultUnpacker(mgr, util.DefaultSystemNamespace, util.DefaultUnpackImage, (*x509.CertPool)(nil)) - utilruntime.Must(err) + unpacker = new(MockUnpacker) + store = new(MockStorage) code := m.Run() utilruntime.Must(testEnv.Stop()) diff --git a/test/e2e/cluster_extension_install_test.go b/test/e2e/cluster_extension_install_test.go index a3efe884e..721a0f716 100644 --- a/test/e2e/cluster_extension_install_test.go +++ b/test/e2e/cluster_extension_install_test.go @@ -197,7 +197,7 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) { InstallNamespace: "default", } require.NoError(t, c.Create(context.Background(), clusterExtension)) - t.Log("By eventually reporting a successful resolution") + t.Log("By eventually reporting a successful installation") require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved) @@ -207,6 +207,7 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) { assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) assert.Contains(ct, cond.Message, "resolved to") assert.Equal(ct, &ocv1alpha1.BundleMetadata{Name: "prometheus-operator.1.0.0", Version: "1.0.0"}, clusterExtension.Status.ResolvedBundle) + assert.Equal(ct, &ocv1alpha1.BundleMetadata{Name: "prometheus-operator.1.0.0", Version: "1.0.0"}, clusterExtension.Status.InstalledBundle) }, pollDuration, pollInterval) t.Log("It does not allow to upgrade the ClusterExtension to a non-successor version")