🌱 Proposed way to unskip some tests (#874)

* Proposed way to unskip some tests TestClusterExtensionChannelVersionExists mostly restored here Signed-off-by: Brett Tofel <btofel@redhat.com> * Refactors global variable names Signed-off-by: Brett Tofel <btofel@redhat.com> * Removes BundleDeployment related checking In just one test, for now. Signed-off-by: Brett Tofel <btofel@redhat.com> * fix unit test - TestClusterExtensionChannelVersionExists * Unskip all - failing tests (up|down)grade should err Unskips all in clusterextension_controller_test.go Signed-off-by: Brett Tofel <btofel@redhat.com> * Unskip all clusterextension_registryv1_validation_test.go Still failing tests (up|down)grade should err b/c we don't have an installedBundle to check against Signed-off-by: Brett Tofel <btofel@redhat.com> * debugging Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> * Fixes and additions pending & unpack path Signed-off-by: Brett Tofel <btofel@redhat.com> * Fix linter, remove debug logs and extraneous file Signed-off-by: dtfranz <dfranz@redhat.com> * Fix e2e failure due to race condition Signed-off-by: dtfranz <dfranz@redhat.com> --------- Signed-off-by: Brett Tofel <btofel@redhat.com> Signed-off-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> Signed-off-by: dtfranz <dfranz@redhat.com> Co-authored-by: Varsha Prasad Narsing <varshaprasad96@gmail.com> Co-authored-by: dtfranz <dfranz@redhat.com>
operator-framework · May 29, 2024 · 04df6fb · 04df6fb
1 parent b993945
commit 04df6fb
Show file tree

Hide file tree

Showing 9 changed files with 343 additions and 249 deletions.
diff --git 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
@@ -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
@@ -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
 }