Use Helm List operator to determine Deployed status

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

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.32.0
-	github.com/operator-framework/helm-operator-plugins v0.5.0
+	github.com/operator-framework/helm-operator-plugins v0.6.0
 	github.com/operator-framework/operator-registry v1.47.0
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.9.0
@@ -179,7 +179,7 @@ require (
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/proglottis/gpgme v0.1.3 // indirect
-	github.com/prometheus/client_golang v1.20.4 // indirect
+	github.com/prometheus/client_golang v1.20.5 // indirect
 	github.com/prometheus/client_model v0.6.1 // indirect
 	github.com/prometheus/common v0.55.0 // indirect
 	github.com/prometheus/procfs v0.15.1 // indirect

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.32.0 h1:VKD+7wfEF6CnJgR4aUYyT85KP2Te7zjhaPvgvWy25Uw=
 github.com/operator-framework/catalogd v0.32.0/go.mod h1:FrFSCwRXr4aPslcXIv48dan5AdM37k/B9tK/RpdvZCU=
-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.6.0 h1:MAb1oKsqlsb2gVTFJkytIySK+mzT5sdICLM9HfYF49A=
+github.com/operator-framework/helm-operator-plugins v0.6.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=
@@ -569,8 +569,8 @@ github.com/prometheus/client_golang v0.9.1/go.mod h1:7SWBe2y4D6OKWSNQJUaRYU/AaXP
 github.com/prometheus/client_golang v0.9.3/go.mod h1:/TN21ttK/J9q6uSwhBd54HahCDft0ttaMvbicHlPoso=
 github.com/prometheus/client_golang v1.0.0/go.mod h1:db9x61etRT2tGnBNRi70OPL5FsnadC4Ky3P0J6CfImo=
 github.com/prometheus/client_golang v1.1.0/go.mod h1:I1FGZT9+L76gKKOs5djB6ezCbFQP1xR9D75/vuwEF3g=
-github.com/prometheus/client_golang v1.20.4 h1:Tgh3Yr67PaOv/uTqloMsCEdeuFTatm5zIq5+qNN23vI=
-github.com/prometheus/client_golang v1.20.4/go.mod h1:PIEt8X02hGcP8JWbeHyeZ53Y/jReSnHgO035n//V5WE=
+github.com/prometheus/client_golang v1.20.5 h1:cxppBPuYhUnsO6yo/aoRol4L7q7UFfdm+bR9r+8l63Y=
+github.com/prometheus/client_golang v1.20.5/go.mod h1:PIEt8X02hGcP8JWbeHyeZ53Y/jReSnHgO035n//V5WE=
 github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo=
 github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
 github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=

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

@@ -206,8 +206,7 @@ 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())
+		setInstalledStatusConditionUnknown(ext, "retrying get installed bundle")
 		setStatusProgressing(ext, err)
 		return ctrl.Result{}, err
 	}
@@ -217,7 +216,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
 	resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolver.Resolve(ctx, ext, installedBundle)
 	if err != nil {
 		// Note: We don't distinguish between resolution-specific errors and generic errors
-		setInstallStatus(ext, nil)
+		setInstalledStatusFromBundle(ext, installedBundle)
 		setStatusProgressing(ext, err)
 		ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonFailed, err.Error())
 		return ctrl.Result{}, err
@@ -254,6 +253,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
 		// Wrap the error passed to this with the resolution information until we have successfully
 		// 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
+		setInstalledStatusFromBundle(ext, installedBundle)
 		setStatusProgressing(ext, wrapErrorWithResolutionInfo(resolvedBundleMetadata, err))
 		return ctrl.Result{}, err
 	}
@@ -286,10 +286,7 @@ 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())
-		}
+		setInstalledStatusFromBundle(ext, installedBundle)
 		return ctrl.Result{}, err
 	}
 
@@ -472,26 +469,24 @@ func (d *DefaultInstalledBundleGetter) GetInstalledBundle(ctx context.Context, e
 		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 highest semver 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 {
+			return &ocv1alpha1.BundleMetadata{
+				Name:    rel.Labels[labels.BundleNameKey],
+				Version: rel.Labels[labels.BundleVersionKey],
+			}, nil
+		}
 	}
-
-	return &ocv1alpha1.BundleMetadata{
-		Name:    rel.Labels[labels.BundleNameKey],
-		Version: rel.Labels[labels.BundleVersionKey],
-	}, nil
+	return nil, nil
 }

internal/controllers/clusterextension_controller_test.go

@@ -12,6 +12,9 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"helm.sh/helm/v3/pkg/chart"
+	"helm.sh/helm/v3/pkg/release"
+	"helm.sh/helm/v3/pkg/storage/driver"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -21,12 +24,14 @@ import (
 	crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
+	helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 
 	ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
 	"github.com/operator-framework/operator-controller/internal/conditionsets"
 	"github.com/operator-framework/operator-controller/internal/controllers"
 	"github.com/operator-framework/operator-controller/internal/finalizers"
+	"github.com/operator-framework/operator-controller/internal/labels"
 	"github.com/operator-framework/operator-controller/internal/resolve"
 	"github.com/operator-framework/operator-controller/internal/rukpak/source"
 )
@@ -1400,3 +1405,106 @@ func TestSetDeprecationStatus(t *testing.T) {
 		})
 	}
 }
+
+type MockActionGetter struct {
+	rels []*release.Release
+	err  error
+}
+
+func (mag *MockActionGetter) ActionClientFor(ctx context.Context, obj client.Object) (helmclient.ActionInterface, error) {
+	return mag, nil
+}
+
+func (mag *MockActionGetter) Get(name string, opts ...helmclient.GetOption) (*release.Release, error) {
+	return nil, nil
+}
+
+// This is the function we are really testing
+func (mag *MockActionGetter) History(name string, opts ...helmclient.HistoryOption) ([]*release.Release, error) {
+	return mag.rels, mag.err
+}
+
+func (mag *MockActionGetter) Install(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...helmclient.InstallOption) (*release.Release, error) {
+	return nil, nil
+}
+
+func (mag *MockActionGetter) Upgrade(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...helmclient.UpgradeOption) (*release.Release, error) {
+	return nil, nil
+}
+
+func (mag *MockActionGetter) Uninstall(name string, opts ...helmclient.UninstallOption) (*release.UninstallReleaseResponse, error) {
+	return nil, nil
+}
+
+func (mag *MockActionGetter) Reconcile(rel *release.Release) error {
+	return nil
+}
+
+func TestGetInstalledBundleHistory(t *testing.T) {
+	mag := MockActionGetter{}
+
+	getter := controllers.DefaultInstalledBundleGetter{
+		ActionClientGetter: &mag,
+	}
+
+	ext := ocv1alpha1.ClusterExtension{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-ext",
+		},
+	}
+
+	md, err := getter.GetInstalledBundle(context.Background(), &ext)
+	require.NoError(t, err)
+	require.Nil(t, md)
+
+	mag.err = driver.ErrReleaseNotFound
+	md, err = getter.GetInstalledBundle(context.Background(), &ext)
+	require.NoError(t, err)
+	require.Nil(t, md)
+
+	mag.err = nil
+	mag.rels = []*release.Release{
+		{
+			Name: "test-ext",
+			Info: &release.Info{
+				Status: release.StatusDeployed,
+			},
+			Labels: map[string]string{
+				labels.BundleNameKey:    "text-ext",
+				labels.BundleVersionKey: "1.0",
+			},
+		},
+	}
+
+	md, err = getter.GetInstalledBundle(context.Background(), &ext)
+	require.NoError(t, err)
+	require.NotNil(t, md)
+	require.Equal(t, &ocv1alpha1.BundleMetadata{Name: "text-ext", Version: "1.0"}, md)
+
+	mag.rels = []*release.Release{
+		{
+			Name: "test-ext",
+			Info: &release.Info{
+				Status: release.StatusFailed,
+			},
+			Labels: map[string]string{
+				labels.BundleNameKey:    "text-ext",
+				labels.BundleVersionKey: "2.0",
+			},
+		},
+		{
+			Name: "test-ext",
+			Info: &release.Info{
+				Status: release.StatusDeployed,
+			},
+			Labels: map[string]string{
+				labels.BundleNameKey:    "text-ext",
+				labels.BundleVersionKey: "1.0",
+			},
+		},
+	}
+	md, err = getter.GetInstalledBundle(context.Background(), &ext)
+	require.NoError(t, err)
+	require.NotNil(t, md)
+	require.Equal(t, &ocv1alpha1.BundleMetadata{Name: "text-ext", Version: "1.0"}, md)
+}

internal/controllers/common_controller.go

@@ -26,6 +26,24 @@ import (
 	ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
 )
 
+// setInstalledStatusFromBundle sets the installed status based on the given installedBundle.
+func setInstalledStatusFromBundle(ext *ocv1alpha1.ClusterExtension, installedBundle *ocv1alpha1.BundleMetadata) {
+	if installedBundle != nil {
+		installStatus := &ocv1alpha1.ClusterExtensionInstallStatus{
+			Bundle: *installedBundle,
+		}
+		setInstallStatus(ext, installStatus)
+		// We don't want to change the Transition time if we don't have to
+		if !apimeta.IsStatusConditionTrue(ext.Status.Conditions, ocv1alpha1.TypeInstalled) {
+			setInstalledStatusConditionSuccess(ext, "Installed extension successfully")
+		}
+		return
+	}
+	// If bundle is not already installed, set Installed status condition to False
+	setInstallStatus(ext, nil)
+	setInstalledStatusConditionFailed(ext, "no installed bundle found")
+}
+
 // setInstalledStatusConditionSuccess sets the installed status condition to success.
 func setInstalledStatusConditionSuccess(ext *ocv1alpha1.ClusterExtension, message string) {
 	apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
@@ -48,6 +66,17 @@ func setInstalledStatusConditionFailed(ext *ocv1alpha1.ClusterExtension, message
 	})
 }
 
+// setInstalledStatusConditionUnknown sets the installed status condition to unknown.
+func setInstalledStatusConditionUnknown(ext *ocv1alpha1.ClusterExtension, message string) {
+	apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+		Type:               ocv1alpha1.TypeInstalled,
+		Status:             metav1.ConditionUnknown,
+		Reason:             ocv1alpha1.ReasonFailed,
+		Message:            message,
+		ObservedGeneration: ext.GetGeneration(),
+	})
+}
+
 func setInstallStatus(ext *ocv1alpha1.ClusterExtension, installStatus *ocv1alpha1.ClusterExtensionInstallStatus) {
 	ext.Status.Install = installStatus
 }