Skip to content

Commit ce898ec

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

File tree

7 files changed

+182
-28
lines changed

7 files changed

+182
-28
lines changed

‎go.mod‎

Lines changed: 2 additions & 2 deletions
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.32.0
21-
github.com/operator-framework/helm-operator-plugins v0.5.0
21+
github.com/operator-framework/helm-operator-plugins v0.6.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
@@ -179,7 +179,7 @@ require (
179179
github.com/pkg/errors v0.9.1 // indirect
180180
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
181181
github.com/proglottis/gpgme v0.1.3 // indirect
182-
github.com/prometheus/client_golang v1.20.4 // indirect
182+
github.com/prometheus/client_golang v1.20.5 // indirect
183183
github.com/prometheus/client_model v0.6.1 // indirect
184184
github.com/prometheus/common v0.55.0 // indirect
185185
github.com/prometheus/procfs v0.15.1 // indirect

‎go.sum‎

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

‎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: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,7 @@ 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+
setInstalledStatusConditionUnknown(ext, err.Error())
211210
setStatusProgressing(ext, err)
212211
return ctrl.Result{}, err
213212
}
@@ -217,7 +216,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
217216
resolvedBundle, resolvedBundleVersion, resolvedDeprecation, err := r.Resolver.Resolve(ctx, ext, installedBundle)
218217
if err != nil {
219218
// Note: We don't distinguish between resolution-specific errors and generic errors
220-
setInstallStatus(ext, nil)
219+
setInstalledStatusFromBundle(ext, installedBundle)
221220
setStatusProgressing(ext, err)
222221
ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonFailed, err.Error())
223222
return ctrl.Result{}, err
@@ -254,6 +253,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
254253
// Wrap the error passed to this with the resolution information until we have successfully
255254
// installed since we intend for the progressing condition to replace the resolved condition
256255
// and will be removing the .status.resolution field from the ClusterExtension status API
256+
setInstalledStatusFromBundle(ext, installedBundle)
257257
setStatusProgressing(ext, wrapErrorWithResolutionInfo(resolvedBundleMetadata, err))
258258
return ctrl.Result{}, err
259259
}
@@ -286,10 +286,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
286286
managedObjs, _, err := r.Applier.Apply(ctx, unpackResult.Bundle, ext, objLbls, storeLbls)
287287
if err != nil {
288288
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-
}
289+
setInstalledStatusFromBundle(ext, installedBundle)
293290
return ctrl.Result{}, err
294291
}
295292

@@ -472,26 +469,24 @@ func (d *DefaultInstalledBundleGetter) GetInstalledBundle(ctx context.Context, e
472469
return nil, err
473470
}
474471

475-
rel, err := cl.Get(ext.GetName())
472+
relhis, err := cl.History(ext.GetName())
476473
if err != nil && !errors.Is(err, driver.ErrReleaseNotFound) {
477474
return nil, err
478475
}
479-
if rel == nil {
476+
if len(relhis) == 0 {
480477
return nil, nil
481478
}
482479

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)
480+
// TODO: relhis[0].Info.Status is the status of the highest semver install attempt.
481+
// This might be useful informaton if it's not release.StatusDeployed, in telling us
482+
// the status of an upgrade attempt.
483+
for _, rel := range relhis {
484+
if rel.Info != nil && rel.Info.Status == release.StatusDeployed {
485+
return &ocv1alpha1.BundleMetadata{
486+
Name: rel.Labels[labels.BundleNameKey],
487+
Version: rel.Labels[labels.BundleVersionKey],
488+
}, nil
489+
}
491490
}
492-
493-
return &ocv1alpha1.BundleMetadata{
494-
Name: rel.Labels[labels.BundleNameKey],
495-
Version: rel.Labels[labels.BundleVersionKey],
496-
}, nil
491+
return nil, nil
497492
}

‎internal/controllers/clusterextension_controller_test.go‎

Lines changed: 108 additions & 0 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
)
@@ -1400,3 +1405,106 @@ func TestSetDeprecationStatus(t *testing.T) {
14001405
})
14011406
}
14021407
}
1408+
1409+
type MockActionGetter struct {
1410+
rels []*release.Release
1411+
err error
1412+
}
1413+
1414+
func (mag *MockActionGetter) ActionClientFor(ctx context.Context, obj client.Object) (helmclient.ActionInterface, error) {
1415+
return mag, nil
1416+
}
1417+
1418+
func (mag *MockActionGetter) Get(name string, opts ...helmclient.GetOption) (*release.Release, error) {
1419+
return nil, nil
1420+
}
1421+
1422+
// This is the function we are really testing
1423+
func (mag *MockActionGetter) History(name string, opts ...helmclient.HistoryOption) ([]*release.Release, error) {
1424+
return mag.rels, mag.err
1425+
}
1426+
1427+
func (mag *MockActionGetter) Install(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...helmclient.InstallOption) (*release.Release, error) {
1428+
return nil, nil
1429+
}
1430+
1431+
func (mag *MockActionGetter) Upgrade(name, namespace string, chrt *chart.Chart, vals map[string]interface{}, opts ...helmclient.UpgradeOption) (*release.Release, error) {
1432+
return nil, nil
1433+
}
1434+
1435+
func (mag *MockActionGetter) Uninstall(name string, opts ...helmclient.UninstallOption) (*release.UninstallReleaseResponse, error) {
1436+
return nil, nil
1437+
}
1438+
1439+
func (mag *MockActionGetter) Reconcile(rel *release.Release) error {
1440+
return nil
1441+
}
1442+
1443+
func TestGetInstalledBundleHistory(t *testing.T) {
1444+
mag := MockActionGetter{}
1445+
1446+
getter := controllers.DefaultInstalledBundleGetter{
1447+
ActionClientGetter: &mag,
1448+
}
1449+
1450+
ext := ocv1alpha1.ClusterExtension{
1451+
ObjectMeta: metav1.ObjectMeta{
1452+
Name: "test-ext",
1453+
},
1454+
}
1455+
1456+
md, err := getter.GetInstalledBundle(context.Background(), &ext)
1457+
require.NoError(t, err)
1458+
require.Nil(t, md)
1459+
1460+
mag.err = driver.ErrReleaseNotFound
1461+
md, err = getter.GetInstalledBundle(context.Background(), &ext)
1462+
require.NoError(t, err)
1463+
require.Nil(t, md)
1464+
1465+
mag.err = nil
1466+
mag.rels = []*release.Release{
1467+
{
1468+
Name: "test-ext",
1469+
Info: &release.Info{
1470+
Status: release.StatusDeployed,
1471+
},
1472+
Labels: map[string]string{
1473+
labels.BundleNameKey: "text-ext",
1474+
labels.BundleVersionKey: "1.0",
1475+
},
1476+
},
1477+
}
1478+
1479+
md, err = getter.GetInstalledBundle(context.Background(), &ext)
1480+
require.NoError(t, err)
1481+
require.NotNil(t, md)
1482+
require.Equal(t, &ocv1alpha1.BundleMetadata{Name: "text-ext", Version: "1.0"}, md)
1483+
1484+
mag.rels = []*release.Release{
1485+
{
1486+
Name: "test-ext",
1487+
Info: &release.Info{
1488+
Status: release.StatusFailed,
1489+
},
1490+
Labels: map[string]string{
1491+
labels.BundleNameKey: "text-ext",
1492+
labels.BundleVersionKey: "2.0",
1493+
},
1494+
},
1495+
{
1496+
Name: "test-ext",
1497+
Info: &release.Info{
1498+
Status: release.StatusDeployed,
1499+
},
1500+
Labels: map[string]string{
1501+
labels.BundleNameKey: "text-ext",
1502+
labels.BundleVersionKey: "1.0",
1503+
},
1504+
},
1505+
}
1506+
md, err = getter.GetInstalledBundle(context.Background(), &ext)
1507+
require.NoError(t, err)
1508+
require.NotNil(t, md)
1509+
require.Equal(t, &ocv1alpha1.BundleMetadata{Name: "text-ext", Version: "1.0"}, md)
1510+
}

‎internal/controllers/common_controller.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,24 @@ import (
2626
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
2727
)
2828

29+
// setInstalledStatusFromBundle sets the installed status based on the given installedBundle.
30+
func setInstalledStatusFromBundle(ext *ocv1alpha1.ClusterExtension, installedBundle *ocv1alpha1.BundleMetadata) {
31+
if installedBundle != nil {
32+
installStatus := &ocv1alpha1.ClusterExtensionInstallStatus{
33+
Bundle: *installedBundle,
34+
}
35+
setInstallStatus(ext, installStatus)
36+
// We don't want to change the Transition time if we don't have to
37+
if !apimeta.IsStatusConditionTrue(ext.Status.Conditions, ocv1alpha1.TypeInstalled) {
38+
setInstalledStatusConditionSuccess(ext, "Installed extension successfully")
39+
}
40+
return
41+
}
42+
// If bundle is not already installed, set Installed status condition to False
43+
setInstallStatus(ext, nil)
44+
setInstalledStatusConditionFailed(ext, "no installed bundle found")
45+
}
46+
2947
// setInstalledStatusConditionSuccess sets the installed status condition to success.
3048
func setInstalledStatusConditionSuccess(ext *ocv1alpha1.ClusterExtension, message string) {
3149
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
@@ -48,6 +66,17 @@ func setInstalledStatusConditionFailed(ext *ocv1alpha1.ClusterExtension, message
4866
})
4967
}
5068

69+
// setInstalledStatusConditionUnknown sets the installed status condition to unknown.
70+
func setInstalledStatusConditionUnknown(ext *ocv1alpha1.ClusterExtension, message string) {
71+
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
72+
Type: ocv1alpha1.TypeInstalled,
73+
Status: metav1.ConditionUnknown,
74+
Reason: ocv1alpha1.ReasonFailed,
75+
Message: message,
76+
ObservedGeneration: ext.GetGeneration(),
77+
})
78+
}
79+
5180
func setInstallStatus(ext *ocv1alpha1.ClusterExtension, installStatus *ocv1alpha1.ClusterExtensionInstallStatus) {
5281
ext.Status.Install = installStatus
5382
}

0 commit comments

Comments
 (0)