Use Helm List operator to determine Deployed status

Signed-off-by: Todd Short <tshort@redhat.com>

Author: tmshort

go.mod

@@ -18,7 +18,7 @@ require (
 	github.com/opencontainers/go-digest v1.0.0
 	github.com/operator-framework/api v0.27.0
 	github.com/operator-framework/catalogd v0.35.0
-	github.com/operator-framework/helm-operator-plugins v0.5.0
+	github.com/operator-framework/helm-operator-plugins v0.7.0
 	github.com/operator-framework/operator-registry v1.47.0
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.9.0

go.sum

@@ -537,8 +537,8 @@ github.com/operator-framework/api v0.27.0 h1:OrVaGKZJvbZo58HTv2guz7aURkhVKYhFqZ/
 github.com/operator-framework/api v0.27.0/go.mod h1:lg2Xx+S8NQWGYlEOvFwQvH46E5EK5IrAIL7HWfAhciM=
 github.com/operator-framework/catalogd v0.35.0 h1:2lQPyHIzEfcciUjQ/fo4i/GE/sX2LBzd8S+nYxlvEHU=
 github.com/operator-framework/catalogd v0.35.0/go.mod h1:anZurjcFMBvbkuyqlJ98v9z+yjniPKqmhlyitk9DuBQ=
-github.com/operator-framework/helm-operator-plugins v0.5.0 h1:qph2OoECcI9mpuUBtOsWOMgvpx52mPTTSvzVxICsT04=
-github.com/operator-framework/helm-operator-plugins v0.5.0/go.mod h1:yVncrZ/FJNqedMil+055fk6sw8aMKRrget/AqGM0ig0=
+github.com/operator-framework/helm-operator-plugins v0.7.0 h1:YmtIWFc9BaNaDc5mk/dkG0P2BqPZOqpDvjWih5Fczuk=
+github.com/operator-framework/helm-operator-plugins v0.7.0/go.mod h1:fUUCJR3bWtMBZ1qdDhbwjacsBHi9uT576tF4u/DwOgQ=
 github.com/operator-framework/operator-lib v0.15.0 h1:0QeRM4PMtThqINpcFGCEBnIV3Z8u7/8fYLEx6mUtdcM=
 github.com/operator-framework/operator-lib v0.15.0/go.mod h1:ZxLvFuQ7bRWiTNBOqodbuNvcsy/Iq0kOygdxhlbNdI0=
 github.com/operator-framework/operator-registry v1.47.0 h1:Imr7X/W6FmXczwpIOXfnX8d6Snr1dzwWxkMG+lLAfhg=

internal/action/helm.go

@@ -75,6 +75,12 @@ func (a ActionClient) Get(name string, opts ...actionclient.GetOption) (*release
 	return resp, err
 }
 
+func (a ActionClient) History(name string, opts ...actionclient.HistoryOption) ([]*release.Release, error) {
+	resp, err := a.ActionInterface.History(name, opts...)
+	err = a.actionClientErrorTranslator(err)
+	return resp, err
+}
+
 func (a ActionClient) Reconcile(rel *release.Release) error {
 	return a.actionClientErrorTranslator(a.ActionInterface.Reconcile(rel))
 }

internal/action/helm_test.go

@@ -30,6 +30,17 @@ func (m *mockActionClient) Get(name string, opts ...actionclient.GetOption) (*re
 	return args.Get(0).(*release.Release), args.Error(1)
 }
 
+func (m *mockActionClient) History(name string, opts ...actionclient.HistoryOption) ([]*release.Release, error) {
+	args := m.Called(name, opts)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	rel := []*release.Release{
+		args.Get(0).(*release.Release),
+	}
+	return rel, args.Error(1)
+}
+
 func (m *mockActionClient) Install(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...actionclient.InstallOption) (*release.Release, error) {
 	args := m.Called(name, namespace, chrt, vals, opts)
 	if args.Get(0) == nil {
@@ -82,6 +93,7 @@ func TestActionClientErrorTranslation(t *testing.T) {
 
 	ac := new(mockActionClient)
 	ac.On("Get", mock.Anything, mock.Anything).Return(nil, originalError)
+	ac.On("History", mock.Anything, mock.Anything).Return(nil, originalError)
 	ac.On("Install", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, originalError)
 	ac.On("Uninstall", mock.Anything, mock.Anything).Return(nil, originalError)
 	ac.On("Upgrade", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, originalError)
@@ -93,6 +105,10 @@ func TestActionClientErrorTranslation(t *testing.T) {
 	_, err := wrappedAc.Get("something")
 	assert.Equal(t, expectedErr, err, "expected Get() to return translated error")
 
+	// History
+	_, err = wrappedAc.History("something")
+	assert.Equal(t, expectedErr, err, "expected History() to return translated error")
+
 	// Install
 	_, err = wrappedAc.Install("something", "somethingelse", nil, nil)
 	assert.Equal(t, expectedErr, err, "expected Install() to return translated error")

internal/controllers/clusterextension_controller.go

@@ -84,7 +84,7 @@ type Applier interface {
 }
 
 type InstalledBundleGetter interface {
-	GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*ocv1alpha1.BundleMetadata, error)
+	GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*InstalledBundle, error)
 }
 
 //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions,verbs=get;list;watch;update;patch
@@ -206,19 +206,22 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
 	installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, ext)
 	if err != nil {
 		setInstallStatus(ext, nil)
-		// TODO: use Installed=Unknown
-		setInstalledStatusConditionFailed(ext, err.Error())
-		setStatusProgressing(ext, err)
+		setInstalledStatusConditionUnknown(ext, err.Error())
+		setStatusProgressing(ext, errors.New("retrying to get installed bundle"))
 		return ctrl.Result{}, err
 	}
 
 	// run resolution
 	l.Info("resolving bundle")
-	resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolver.Resolve(ctx, ext, installedBundle)
+	var bm *ocv1alpha1.BundleMetadata
+	if installedBundle != nil {
+		bm = &installedBundle.BundleMetadata
+	}
+	resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolver.Resolve(ctx, ext, bm)
 	if err != nil {
 		// Note: We don't distinguish between resolution-specific errors and generic errors
-		setInstallStatus(ext, nil)
 		setStatusProgressing(ext, err)
+		setInstalledStatusFromBundle(ext, installedBundle, nil)
 		ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonFailed, err.Error())
 		return ctrl.Result{}, err
 	}
@@ -255,6 +258,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
 		// installed since we intend for the progressing condition to replace the resolved condition
 		// and will be removing the .status.resolution field from the ClusterExtension status API
 		setStatusProgressing(ext, wrapErrorWithResolutionInfo(resolvedBundleMetadata, err))
+		setInstalledStatusFromBundle(ext, installedBundle, nil)
 		return ctrl.Result{}, err
 	}
 
@@ -268,9 +272,10 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
 	}
 
 	storeLbls := map[string]string{
-		labels.BundleNameKey:    resolvedBundle.Name,
-		labels.PackageNameKey:   resolvedBundle.Package,
-		labels.BundleVersionKey: resolvedBundleVersion.String(),
+		labels.BundleNameKey:      resolvedBundle.Name,
+		labels.PackageNameKey:     resolvedBundle.Package,
+		labels.BundleVersionKey:   resolvedBundleVersion.String(),
+		labels.BundleReferenceKey: resolvedBundle.Image,
 	}
 
 	l.Info("applying bundle contents")
@@ -286,18 +291,17 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
 	managedObjs, _, err := r.Applier.Apply(ctx, unpackResult.Bundle, ext, objLbls, storeLbls)
 	if err != nil {
 		setStatusProgressing(ext, wrapErrorWithResolutionInfo(resolvedBundleMetadata, err))
-		// If bundle is not already installed, set Installed status condition to False
-		if installedBundle == nil {
-			setInstalledStatusConditionFailed(ext, err.Error())
-		}
+		// Now that we're actually trying to install, use the error
+		setInstalledStatusFromBundle(ext, installedBundle, err)
 		return ctrl.Result{}, err
 	}
 
-	installStatus := &ocv1alpha1.ClusterExtensionInstallStatus{
-		Bundle: resolvedBundleMetadata,
+	newInstalledBundle := &InstalledBundle{
+		BundleMetadata: resolvedBundleMetadata,
+		Image:          resolvedBundle.Image,
 	}
-	setInstallStatus(ext, installStatus)
-	setInstalledStatusConditionSuccess(ext, fmt.Sprintf("Installed bundle %s successfully", resolvedBundle.Image))
+	// Successful install
+	setInstalledStatusFromBundle(ext, newInstalledBundle, nil)
 
 	l.Info("watching managed objects")
 	cache, err := r.Manager.Get(ctx, ext)
@@ -466,32 +470,46 @@ type DefaultInstalledBundleGetter struct {
 	helmclient.ActionClientGetter
 }
 
-func (d *DefaultInstalledBundleGetter) GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*ocv1alpha1.BundleMetadata, error) {
+type InstalledBundle struct {
+	ocv1alpha1.BundleMetadata
+	Image string
+}
+
+func (d *DefaultInstalledBundleGetter) GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*InstalledBundle, error) {
 	cl, err := d.ActionClientFor(ctx, ext)
 	if err != nil {
 		return nil, err
 	}
 
-	rel, err := cl.Get(ext.GetName())
+	relhis, err := cl.History(ext.GetName())
 	if err != nil && !errors.Is(err, driver.ErrReleaseNotFound) {
 		return nil, err
 	}
-	if rel == nil {
+	if len(relhis) == 0 {
 		return nil, nil
 	}
 
-	switch rel.Info.Status {
-	case release.StatusUnknown:
-		return nil, fmt.Errorf("installation status is unknown")
-	case release.StatusDeployed, release.StatusUninstalled, release.StatusSuperseded, release.StatusFailed:
-	case release.StatusUninstalling, release.StatusPendingInstall, release.StatusPendingRollback, release.StatusPendingUpgrade:
-		return nil, fmt.Errorf("installation is still pending: %s", rel.Info.Status)
-	default:
-		return nil, fmt.Errorf("unknown installation status: %s", rel.Info.Status)
+	// TODO: relhis[0].Info.Status is the status of the most recent install attempt.
+	// This might be useful informaton if it's not release.StatusDeployed, in telling us
+	// the status of an upgrade attempt.
+	for _, rel := range relhis {
+		if rel.Info != nil && rel.Info.Status == release.StatusDeployed {
+			// If there are blank values, we should consider this as not installed
+			if n, ok := rel.Labels[labels.BundleNameKey]; !ok || n == "" {
+				return nil, nil
+			}
+			if v, ok := rel.Labels[labels.BundleVersionKey]; !ok || v == "" {
+				return nil, nil
+			}
+			// Not checking BundleReferenceKey, as it's new; upgrade test would fail
+			return &InstalledBundle{
+				BundleMetadata: ocv1alpha1.BundleMetadata{
+					Name:    rel.Labels[labels.BundleNameKey],
+					Version: rel.Labels[labels.BundleVersionKey],
+				},
+				Image: rel.Labels[labels.BundleReferenceKey],
+			}, nil
+		}
 	}
-
-	return &ocv1alpha1.BundleMetadata{
-		Name:    rel.Labels[labels.BundleNameKey],
-		Version: rel.Labels[labels.BundleVersionKey],
-	}, nil
+	return nil, nil
 }