Skip to content

Commit 21f4d94

Browse files
tmshorttmshort
committed
Use Helm List operator to determine Deployed status
Signed-off-by: Todd Short <[email protected]>
1 parent 3083879 commit 21f4d94

File tree

9 files changed

+241
-45
lines changed

9 files changed

+241
-45
lines changed

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ require (
1818
github.com/opencontainers/go-digest v1.0.0
1919
github.com/operator-framework/api v0.27.0
2020
github.com/operator-framework/catalogd v0.33.0
21-
github.com/operator-framework/helm-operator-plugins v0.5.0
21+
github.com/operator-framework/helm-operator-plugins v0.7.0
2222
github.com/operator-framework/operator-registry v1.47.0
2323
github.com/spf13/pflag v1.0.5
2424
github.com/stretchr/testify v1.9.0

‎go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -537,8 +537,8 @@ github.com/operator-framework/api v0.27.0 h1:OrVaGKZJvbZo58HTv2guz7aURkhVKYhFqZ/
537537
github.com/operator-framework/api v0.27.0/go.mod h1:lg2Xx+S8NQWGYlEOvFwQvH46E5EK5IrAIL7HWfAhciM=
538538
github.com/operator-framework/catalogd v0.33.0 h1:hnLIFykO1FkjOAUFRPuYRIHQTE0oBF9jkGmWjKhxniQ=
539539
github.com/operator-framework/catalogd v0.33.0/go.mod h1:anZurjcFMBvbkuyqlJ98v9z+yjniPKqmhlyitk9DuBQ=
540-
github.com/operator-framework/helm-operator-plugins v0.5.0 h1:qph2OoECcI9mpuUBtOsWOMgvpx52mPTTSvzVxICsT04=
541-
github.com/operator-framework/helm-operator-plugins v0.5.0/go.mod h1:yVncrZ/FJNqedMil+055fk6sw8aMKRrget/AqGM0ig0=
540+
github.com/operator-framework/helm-operator-plugins v0.7.0 h1:YmtIWFc9BaNaDc5mk/dkG0P2BqPZOqpDvjWih5Fczuk=
541+
github.com/operator-framework/helm-operator-plugins v0.7.0/go.mod h1:fUUCJR3bWtMBZ1qdDhbwjacsBHi9uT576tF4u/DwOgQ=
542542
github.com/operator-framework/operator-lib v0.15.0 h1:0QeRM4PMtThqINpcFGCEBnIV3Z8u7/8fYLEx6mUtdcM=
543543
github.com/operator-framework/operator-lib v0.15.0/go.mod h1:ZxLvFuQ7bRWiTNBOqodbuNvcsy/Iq0kOygdxhlbNdI0=
544544
github.com/operator-framework/operator-registry v1.47.0 h1:Imr7X/W6FmXczwpIOXfnX8d6Snr1dzwWxkMG+lLAfhg=

‎internal/action/helm.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,12 @@ func (a ActionClient) Get(name string, opts ...actionclient.GetOption) (*release
7575
return resp, err
7676
}
7777

78+
func (a ActionClient) History(name string, opts ...actionclient.HistoryOption) ([]*release.Release, error) {
79+
resp, err := a.ActionInterface.History(name, opts...)
80+
err = a.actionClientErrorTranslator(err)
81+
return resp, err
82+
}
83+
7884
func (a ActionClient) Reconcile(rel *release.Release) error {
7985
return a.actionClientErrorTranslator(a.ActionInterface.Reconcile(rel))
8086
}

‎internal/action/helm_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,17 @@ func (m *mockActionClient) Get(name string, opts ...actionclient.GetOption) (*re
3030
return args.Get(0).(*release.Release), args.Error(1)
3131
}
3232

33+
func (m *mockActionClient) History(name string, opts ...actionclient.HistoryOption) ([]*release.Release, error) {
34+
args := m.Called(name, opts)
35+
if args.Get(0) == nil {
36+
return nil, args.Error(1)
37+
}
38+
rel := []*release.Release{
39+
args.Get(0).(*release.Release),
40+
}
41+
return rel, args.Error(1)
42+
}
43+
3344
func (m *mockActionClient) Install(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...actionclient.InstallOption) (*release.Release, error) {
3445
args := m.Called(name, namespace, chrt, vals, opts)
3546
if args.Get(0) == nil {
@@ -82,6 +93,7 @@ func TestActionClientErrorTranslation(t *testing.T) {
8293

8394
ac := new(mockActionClient)
8495
ac.On("Get", mock.Anything, mock.Anything).Return(nil, originalError)
96+
ac.On("History", mock.Anything, mock.Anything).Return(nil, originalError)
8597
ac.On("Install", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, originalError)
8698
ac.On("Uninstall", mock.Anything, mock.Anything).Return(nil, originalError)
8799
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) {
93105
_, err := wrappedAc.Get("something")
94106
assert.Equal(t, expectedErr, err, "expected Get() to return translated error")
95107

108+
// History
109+
_, err = wrappedAc.History("something")
110+
assert.Equal(t, expectedErr, err, "expected History() to return translated error")
111+
96112
// Install
97113
_, err = wrappedAc.Install("something", "somethingelse", nil, nil)
98114
assert.Equal(t, expectedErr, err, "expected Install() to return translated error")

‎internal/controllers/clusterextension_controller.go

Lines changed: 44 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ type Applier interface {
8484
}
8585

8686
type InstalledBundleGetter interface {
87-
GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*ocv1alpha1.BundleMetadata, error)
87+
GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*InstalledBundle, error)
8888
}
8989

9090
//+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensions,verbs=get;list;watch;update;patch
@@ -206,19 +206,24 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
206206
installedBundle, err := r.InstalledBundleGetter.GetInstalledBundle(ctx, ext)
207207
if err != nil {
208208
setInstallStatus(ext, nil)
209-
// TODO: use Installed=Unknown
210-
setInstalledStatusConditionFailed(ext, err.Error())
209+
// The error is put into Progressing
210+
setInstalledStatusConditionUnknown(ext, "retrying get installed bundle")
211211
setStatusProgressing(ext, err)
212212
return ctrl.Result{}, err
213213
}
214214

215215
// run resolution
216216
l.Info("resolving bundle")
217-
resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolver.Resolve(ctx, ext, installedBundle)
217+
var bm *ocv1alpha1.BundleMetadata
218+
if installedBundle != nil {
219+
bm = &installedBundle.BundleMetadata
220+
}
221+
resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolver.Resolve(ctx, ext, bm)
218222
if err != nil {
219223
// Note: We don't distinguish between resolution-specific errors and generic errors
220-
setInstallStatus(ext, nil)
221224
setStatusProgressing(ext, err)
225+
// The error is put into Progressing
226+
setInstalledStatusFromBundle(ext, installedBundle, nil)
222227
ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonFailed, err.Error())
223228
return ctrl.Result{}, err
224229
}
@@ -255,6 +260,8 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
255260
// installed since we intend for the progressing condition to replace the resolved condition
256261
// and will be removing the .status.resolution field from the ClusterExtension status API
257262
setStatusProgressing(ext, wrapErrorWithResolutionInfo(resolvedBundleMetadata, err))
263+
// The error is put into Progressing
264+
setInstalledStatusFromBundle(ext, installedBundle, nil)
258265
return ctrl.Result{}, err
259266
}
260267

@@ -268,9 +275,10 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
268275
}
269276

270277
storeLbls := map[string]string{
271-
labels.BundleNameKey: resolvedBundle.Name,
272-
labels.PackageNameKey: resolvedBundle.Package,
273-
labels.BundleVersionKey: resolvedBundleVersion.String(),
278+
labels.BundleNameKey: resolvedBundle.Name,
279+
labels.PackageNameKey: resolvedBundle.Package,
280+
labels.BundleVersionKey: resolvedBundleVersion.String(),
281+
labels.BundleReferenceKey: resolvedBundle.Image,
274282
}
275283

276284
l.Info("applying bundle contents")
@@ -286,18 +294,16 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
286294
managedObjs, _, err := r.Applier.Apply(ctx, unpackResult.Bundle, ext, objLbls, storeLbls)
287295
if err != nil {
288296
setStatusProgressing(ext, wrapErrorWithResolutionInfo(resolvedBundleMetadata, err))
289-
// If bundle is not already installed, set Installed status condition to False
290-
if installedBundle == nil {
291-
setInstalledStatusConditionFailed(ext, err.Error())
292-
}
297+
// Now that we're actually trying to install, use the error
298+
setInstalledStatusFromBundle(ext, installedBundle, err)
293299
return ctrl.Result{}, err
294300
}
295301

296-
installStatus := &ocv1alpha1.ClusterExtensionInstallStatus{
297-
Bundle: resolvedBundleMetadata,
302+
installedBundle = &InstalledBundle{
303+
BundleMetadata: resolvedBundleMetadata,
304+
Image: resolvedBundle.Image,
298305
}
299-
setInstallStatus(ext, installStatus)
300-
setInstalledStatusConditionSuccess(ext, fmt.Sprintf("Installed bundle %s successfully", resolvedBundle.Image))
306+
setInstalledStatusFromBundle(ext, installedBundle, nil)
301307

302308
l.Info("watching managed objects")
303309
cache, err := r.Manager.Get(ctx, ext)
@@ -466,32 +472,38 @@ type DefaultInstalledBundleGetter struct {
466472
helmclient.ActionClientGetter
467473
}
468474

469-
func (d *DefaultInstalledBundleGetter) GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*ocv1alpha1.BundleMetadata, error) {
475+
type InstalledBundle struct {
476+
ocv1alpha1.BundleMetadata
477+
Image string
478+
}
479+
480+
func (d *DefaultInstalledBundleGetter) GetInstalledBundle(ctx context.Context, ext *ocv1alpha1.ClusterExtension) (*InstalledBundle, error) {
470481
cl, err := d.ActionClientFor(ctx, ext)
471482
if err != nil {
472483
return nil, err
473484
}
474485

475-
rel, err := cl.Get(ext.GetName())
486+
relhis, err := cl.History(ext.GetName())
476487
if err != nil && !errors.Is(err, driver.ErrReleaseNotFound) {
477488
return nil, err
478489
}
479-
if rel == nil {
490+
if len(relhis) == 0 {
480491
return nil, nil
481492
}
482493

483-
switch rel.Info.Status {
484-
case release.StatusUnknown:
485-
return nil, fmt.Errorf("installation status is unknown")
486-
case release.StatusDeployed, release.StatusUninstalled, release.StatusSuperseded, release.StatusFailed:
487-
case release.StatusUninstalling, release.StatusPendingInstall, release.StatusPendingRollback, release.StatusPendingUpgrade:
488-
return nil, fmt.Errorf("installation is still pending: %s", rel.Info.Status)
489-
default:
490-
return nil, fmt.Errorf("unknown installation status: %s", rel.Info.Status)
494+
// TODO: relhis[0].Info.Status is the status of the highest semver install attempt.
495+
// This might be useful informaton if it's not release.StatusDeployed, in telling us
496+
// the status of an upgrade attempt.
497+
for _, rel := range relhis {
498+
if rel.Info != nil && rel.Info.Status == release.StatusDeployed {
499+
return &InstalledBundle{
500+
BundleMetadata: ocv1alpha1.BundleMetadata{
501+
Name: rel.Labels[labels.BundleNameKey],
502+
Version: rel.Labels[labels.BundleVersionKey],
503+
},
504+
Image: rel.Labels[labels.BundleReferenceKey],
505+
}, nil
506+
}
491507
}
492-
493-
return &ocv1alpha1.BundleMetadata{
494-
Name: rel.Labels[labels.BundleNameKey],
495-
Version: rel.Labels[labels.BundleVersionKey],
496-
}, nil
508+
return nil, nil
497509
}

‎internal/controllers/clusterextension_controller_test.go

Lines changed: 132 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ import (
1212
"github.com/google/go-cmp/cmp/cmpopts"
1313
"github.com/stretchr/testify/assert"
1414
"github.com/stretchr/testify/require"
15+
"helm.sh/helm/v3/pkg/chart"
16+
"helm.sh/helm/v3/pkg/release"
17+
"helm.sh/helm/v3/pkg/storage/driver"
1518
apimeta "k8s.io/apimachinery/pkg/api/meta"
1619
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1720
"k8s.io/apimachinery/pkg/types"
@@ -21,12 +24,14 @@ import (
2124
crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer"
2225
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2326

27+
helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
2428
"github.com/operator-framework/operator-registry/alpha/declcfg"
2529

2630
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
2731
"github.com/operator-framework/operator-controller/internal/conditionsets"
2832
"github.com/operator-framework/operator-controller/internal/controllers"
2933
"github.com/operator-framework/operator-controller/internal/finalizers"
34+
"github.com/operator-framework/operator-controller/internal/labels"
3035
"github.com/operator-framework/operator-controller/internal/resolve"
3136
"github.com/operator-framework/operator-controller/internal/rukpak/source"
3237
)
@@ -392,7 +397,10 @@ func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) {
392397
cache: &MockManagedContentCache{},
393398
}
394399
reconciler.InstalledBundleGetter = &MockInstalledBundleGetter{
395-
bundle: &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"},
400+
bundle: &controllers.InstalledBundle{
401+
BundleMetadata: ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"},
402+
Image: "quay.io/operatorhubio/[email protected]",
403+
},
396404
}
397405
reconciler.Applier = &MockApplier{
398406
objs: []client.Object{},
@@ -745,7 +753,10 @@ func TestClusterExtensionDeleteFinalizerFails(t *testing.T) {
745753
cache: &MockManagedContentCache{},
746754
}
747755
reconciler.InstalledBundleGetter = &MockInstalledBundleGetter{
748-
bundle: &ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"},
756+
bundle: &controllers.InstalledBundle{
757+
BundleMetadata: ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"},
758+
Image: "quay.io/operatorhubio/[email protected]",
759+
},
749760
}
750761
err = reconciler.Finalizers.Register(fakeFinalizer, finalizers.FinalizerFunc(func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) {
751762
return crfinalizer.Result{}, errors.New(finalizersMessage)
@@ -1400,3 +1411,122 @@ func TestSetDeprecationStatus(t *testing.T) {
14001411
})
14011412
}
14021413
}
1414+
1415+
type MockActionGetter struct {
1416+
rels []*release.Release
1417+
err error
1418+
}
1419+
1420+
func (mag *MockActionGetter) ActionClientFor(ctx context.Context, obj client.Object) (helmclient.ActionInterface, error) {
1421+
return mag, nil
1422+
}
1423+
1424+
func (mag *MockActionGetter) Get(name string, opts ...helmclient.GetOption) (*release.Release, error) {
1425+
return nil, nil
1426+
}
1427+
1428+
// This is the function we are really testing
1429+
func (mag *MockActionGetter) History(name string, opts ...helmclient.HistoryOption) ([]*release.Release, error) {
1430+
return mag.rels, mag.err
1431+
}
1432+
1433+
func (mag *MockActionGetter) Install(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...helmclient.InstallOption) (*release.Release, error) {
1434+
return nil, nil
1435+
}
1436+
1437+
func (mag *MockActionGetter) Upgrade(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...helmclient.UpgradeOption) (*release.Release, error) {
1438+
return nil, nil
1439+
}
1440+
1441+
func (mag *MockActionGetter) Uninstall(name string, opts ...helmclient.UninstallOption) (*release.UninstallReleaseResponse, error) {
1442+
return nil, nil
1443+
}
1444+
1445+
func (mag *MockActionGetter) Reconcile(rel *release.Release) error {
1446+
return nil
1447+
}
1448+
1449+
func TestGetInstalledBundleHistory(t *testing.T) {
1450+
mag := MockActionGetter{}
1451+
1452+
getter := controllers.DefaultInstalledBundleGetter{
1453+
ActionClientGetter: &mag,
1454+
}
1455+
1456+
ext := ocv1alpha1.ClusterExtension{
1457+
ObjectMeta: metav1.ObjectMeta{
1458+
Name: "test-ext",
1459+
},
1460+
}
1461+
1462+
md, err := getter.GetInstalledBundle(context.Background(), &ext)
1463+
require.NoError(t, err)
1464+
require.Nil(t, md)
1465+
1466+
mag.err = driver.ErrReleaseNotFound
1467+
md, err = getter.GetInstalledBundle(context.Background(), &ext)
1468+
require.NoError(t, err)
1469+
require.Nil(t, md)
1470+
1471+
mag.err = nil
1472+
mag.rels = []*release.Release{
1473+
{
1474+
Name: "test-ext",
1475+
Info: &release.Info{
1476+
Status: release.StatusDeployed,
1477+
},
1478+
Labels: map[string]string{
1479+
labels.BundleNameKey: "test-ext",
1480+
labels.BundleVersionKey: "1.0",
1481+
labels.BundleReferenceKey: "bundle-ref",
1482+
},
1483+
},
1484+
}
1485+
expected := &controllers.InstalledBundle{
1486+
BundleMetadata: ocv1alpha1.BundleMetadata{
1487+
Name: "test-ext",
1488+
Version: "1.0",
1489+
},
1490+
Image: "bundle-ref",
1491+
}
1492+
md, err = getter.GetInstalledBundle(context.Background(), &ext)
1493+
require.NoError(t, err)
1494+
require.NotNil(t, md)
1495+
require.Equal(t, expected, md)
1496+
1497+
mag.rels = []*release.Release{
1498+
{
1499+
Name: "test-ext",
1500+
Info: &release.Info{
1501+
Status: release.StatusFailed,
1502+
},
1503+
Labels: map[string]string{
1504+
labels.BundleNameKey: "test-ext",
1505+
labels.BundleVersionKey: "2.0",
1506+
labels.BundleReferenceKey: "bundle-ref-2",
1507+
},
1508+
},
1509+
{
1510+
Name: "test-ext",
1511+
Info: &release.Info{
1512+
Status: release.StatusDeployed,
1513+
},
1514+
Labels: map[string]string{
1515+
labels.BundleNameKey: "test-ext",
1516+
labels.BundleVersionKey: "1.0",
1517+
labels.BundleReferenceKey: "bundle-ref-1",
1518+
},
1519+
},
1520+
}
1521+
expected = &controllers.InstalledBundle{
1522+
BundleMetadata: ocv1alpha1.BundleMetadata{
1523+
Name: "test-ext",
1524+
Version: "1.0",
1525+
},
1526+
Image: "bundle-ref-1",
1527+
}
1528+
md, err = getter.GetInstalledBundle(context.Background(), &ext)
1529+
require.NoError(t, err)
1530+
require.NotNil(t, md)
1531+
require.Equal(t, expected, md)
1532+
}

0 commit comments

Comments
 (0)