Skip to content

Commit

Permalink
Remove status.resolution
Browse files Browse the repository at this point in the history
Fixes #1306

Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
  • Loading branch information
LalatenduMohanty committed Sep 27, 2024
1 parent 8699d25 commit 26bdc55
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 276 deletions.
12 changes: 0 additions & 12 deletions api/v1alpha1/clusterextension_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,8 +470,6 @@ type BundleMetadata struct {
type ClusterExtensionStatus struct {
Install *ClusterExtensionInstallStatus `json:"install,omitempty"`

Resolution *ClusterExtensionResolutionStatus `json:"resolution,omitempty"`

// conditions is a representation of the current state for this ClusterExtension.
// The status is represented by a set of "conditions".
//
Expand Down Expand Up @@ -512,16 +510,6 @@ type ClusterExtensionInstallStatus struct {
Bundle BundleMetadata `json:"bundle"`
}

type ClusterExtensionResolutionStatus struct {
// bundle is a representation of the bundle that was identified during
// resolution to meet all installation/upgrade constraints and is slated to be
// installed or upgraded to.
//
// A "bundle" is a versioned set of content that represents the resources that
// need to be applied to a cluster to install a package.
Bundle BundleMetadata `json:"bundle"`
}

//+kubebuilder:object:root=true
//+kubebuilder:resource:scope=Cluster
//+kubebuilder:subresource:status
Expand Down
21 changes: 0 additions & 21 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -572,34 +572,6 @@ spec:
required:
- bundle
type: object
resolution:
properties:
bundle:
description: |-
bundle is a representation of the bundle that was identified during
resolution to meet all installation/upgrade constraints and is slated to be
installed or upgraded to.
A "bundle" is a versioned set of content that represents the resources that
need to be applied to a cluster to install a package.
properties:
name:
description: |-
name is a required field and is a reference
to the name of a bundle
type: string
version:
description: |-
version is a required field and is a reference
to the version that this bundle represents
type: string
required:
- name
- version
type: object
required:
- bundle
type: object
type: object
type: object
served: true
Expand Down
18 changes: 0 additions & 18 deletions docs/refs/api/operator-controller-api-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ Package v1alpha1 contains API Schema definitions for the olm v1alpha1 API group

_Appears in:_
- [ClusterExtensionInstallStatus](#clusterextensioninstallstatus)
- [ClusterExtensionResolutionStatus](#clusterextensionresolutionstatus)

| Field | Description | Default | Validation |
| --- | --- | --- | --- |
Expand Down Expand Up @@ -162,22 +161,6 @@ ClusterExtensionList contains a list of ClusterExtension
| `items` _[ClusterExtension](#clusterextension) array_ | | | |


#### ClusterExtensionResolutionStatus







_Appears in:_
- [ClusterExtensionStatus](#clusterextensionstatus)

| Field | Description | Default | Validation |
| --- | --- | --- | --- |
| `bundle` _[BundleMetadata](#bundlemetadata)_ | bundle is a representation of the bundle that was identified during<br />resolution to meet all installation/upgrade constraints and is slated to be<br />installed or upgraded to.<br /><br />A "bundle" is a versioned set of content that represents the resources that<br />need to be applied to a cluster to install a package. | | |


#### ClusterExtensionSpec


Expand Down Expand Up @@ -209,7 +192,6 @@ _Appears in:_
| Field | Description | Default | Validation |
| --- | --- | --- | --- |
| `install` _[ClusterExtensionInstallStatus](#clusterextensioninstallstatus)_ | | | |
| `resolution` _[ClusterExtensionResolutionStatus](#clusterextensionresolutionstatus)_ | | | |
| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | conditions is a representation of the current state for this ClusterExtension.<br />The status is represented by a set of "conditions".<br /><br />Each condition is generally structured in the following format:<br /> - Type: a string representation of the condition type. More or less the condition "name".<br /> - Status: a string representation of the state of the condition. Can be one of ["True", "False", "Unknown"].<br /> - Reason: a string representation of the reason for the current state of the condition. Typically useful for building automation around particular Type+Reason combinations.<br /> - Message: a human readable message that further elaborates on the state of the condition<br /><br />The global set of condition types are:<br /> - "Installed", represents whether or not the a bundle has been installed for this ClusterExtension<br /> - "Resolved", represents whether or not a bundle was found that satisfies the selection criteria outlined in the spec<br /> - "Unpacked", represents whether or not the bundle contents have been successfully unpacked<br /><br />When the ClusterExtension is sourced from a catalog, the following conditions are also possible:<br /> - "Deprecated", represents an aggregation of the PackageDeprecated, ChannelDeprecated, and BundleDeprecated condition types<br /> - "PackageDeprecated", represents whether or not the package specified in the spec.source.catalog.packageName field has been deprecated<br /> - "ChannelDeprecated", represents whether or not any channel specified in spec.source.catalog.channels has been deprecated<br /> - "BundleDeprecated", represents whether or not the installed bundle is deprecated<br /><br />The current set of reasons are:<br /> - "Success", this reason is set on the "Unpacked", "Resolved" and "Installed" conditions when unpacking a bundle's content, resolution and installation/upgrading is successful<br /> - "Failed", this reason is set on the "Unpacked", "Resolved" and "Installed" conditions when an error has occurred while unpacking the contents of a bundle, during resolution or installation. | | |


Expand Down
7 changes: 0 additions & 7 deletions internal/controllers/clusterextension_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
// as much status as possible before returning, or at least keeps previous state if
// it is properly labeled with its observed generation.
setInstallStatus(ext, nil)
setResolutionStatus(ext, nil)
setStatusProgressing(ext, err)
ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonFailed, err.Error())
return ctrl.Result{}, err
Expand All @@ -226,7 +225,6 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
if err != nil {
// Note: We don't distinguish between resolution-specific errors and generic errors
setInstallStatus(ext, nil)
setResolutionStatus(ext, nil)
setStatusProgressing(ext, err)
ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonFailed, err.Error())
return ctrl.Result{}, err
Expand All @@ -249,11 +247,6 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
SetDeprecationStatus(ext, resolvedBundle.Name, resolvedDeprecation)

resolvedBundleMetadata := bundleutil.MetadataFor(resolvedBundle.Name, *resolvedBundleVersion)
resStatus := &ocv1alpha1.ClusterExtensionResolutionStatus{
Bundle: resolvedBundleMetadata,
}
setResolutionStatus(ext, resStatus)

bundleSource := &rukpaksource.BundleSource{
Name: ext.GetName(),
Type: rukpaksource.SourceTypeImage,
Expand Down
92 changes: 2 additions & 90 deletions internal/controllers/clusterextension_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"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/resolve"
"github.com/operator-framework/operator-controller/internal/rukpak/source"
Expand All @@ -39,61 +38,7 @@ func TestClusterExtensionDoesNotExist(t *testing.T) {
require.NoError(t, err)
}

func TestClusterExtensionResolutionFails(t *testing.T) {
pkgName := fmt.Sprintf("non-existent-%s", rand.String(6))
cl, reconciler := newClientAndReconciler(t)
reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1alpha1.ClusterExtension, _ *ocv1alpha1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) {
return nil, nil, nil, fmt.Errorf("no package %q found", pkgName)
})
ctx := context.Background()
extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}

t.Log("When the cluster extension specifies a non-existent package")
t.Log("By initializing cluster state")
clusterExtension := &ocv1alpha1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
Spec: ocv1alpha1.ClusterExtensionSpec{
Source: ocv1alpha1.SourceConfig{
SourceType: "Catalog",
Catalog: &ocv1alpha1.CatalogSource{
PackageName: pkgName,
},
},
Install: ocv1alpha1.ClusterExtensionInstallConfig{
Namespace: "default",
ServiceAccount: ocv1alpha1.ServiceAccountReference{
Name: "default",
},
},
},
}
require.NoError(t, cl.Create(ctx, clusterExtension))

t.Log("It sets resolution failure status")
t.Log("By running reconcile")
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
require.Equal(t, ctrl.Result{}, res)
require.EqualError(t, err, fmt.Sprintf("no package %q found", pkgName))

t.Log("By fetching updated cluster extension after reconcile")
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))

t.Log("By checking the status fields")
require.Empty(t, clusterExtension.Status.Resolution)
require.Empty(t, clusterExtension.Status.Install)

t.Log("By checking the expected conditions")
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionTrue, cond.Status)
require.Equal(t, ocv1alpha1.ReasonRetrying, cond.Reason)
require.Equal(t, fmt.Sprintf("no package %q found", pkgName), cond.Message)

verifyInvariants(ctx, t, reconciler.Client, clusterExtension)
require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
}

func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) {
func TestClusterExtensionUnpackFails(t *testing.T) {
cl, reconciler := newClientAndReconciler(t)
reconciler.Unpacker = &MockUnpacker{
err: errors.New("unpack failure"),
Expand Down Expand Up @@ -132,7 +77,6 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) {
err := cl.Create(ctx, clusterExtension)
require.NoError(t, err)

t.Log("It sets resolution success status")
t.Log("By running reconcile")
reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1alpha1.ClusterExtension, _ *ocv1alpha1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) {
v := bsemver.MustParse("1.0.0")
Expand All @@ -151,7 +95,6 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) {

t.Log("By checking the status fields")
expectedBundleMetadata := ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}
require.Equal(t, expectedBundleMetadata, clusterExtension.Status.Resolution.Bundle)
require.Empty(t, clusterExtension.Status.Install)

t.Log("By checking the expected conditions")
Expand Down Expand Up @@ -205,7 +148,6 @@ func TestClusterExtensionUnpackUnexpectedState(t *testing.T) {
err := cl.Create(ctx, clusterExtension)
require.NoError(t, err)

t.Log("It sets resolution success status")
t.Log("By running reconcile")
reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1alpha1.ClusterExtension, _ *ocv1alpha1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) {
v := bsemver.MustParse("1.0.0")
Expand All @@ -223,7 +165,7 @@ func TestClusterExtensionUnpackUnexpectedState(t *testing.T) {
require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
}

func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T) {
func TestClusterExtensionUnpackSuccessfulApplierFails(t *testing.T) {
cl, reconciler := newClientAndReconciler(t)
reconciler.Unpacker = &MockUnpacker{
result: &source.Result{
Expand Down Expand Up @@ -265,7 +207,6 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T)
err := cl.Create(ctx, clusterExtension)
require.NoError(t, err)

t.Log("It sets resolution success status")
t.Log("By running reconcile")
reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1alpha1.ClusterExtension, _ *ocv1alpha1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) {
v := bsemver.MustParse("1.0.0")
Expand All @@ -287,7 +228,6 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T)

t.Log("By checking the status fields")
expectedBundleMetadata := ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}
require.Equal(t, expectedBundleMetadata, clusterExtension.Status.Resolution.Bundle)
require.Empty(t, clusterExtension.Status.Install)

t.Log("By checking the expected installed conditions")
Expand Down Expand Up @@ -348,7 +288,6 @@ func TestClusterExtensionManagerFailed(t *testing.T) {
err := cl.Create(ctx, clusterExtension)
require.NoError(t, err)

t.Log("It sets resolution success status")
t.Log("By running reconcile")
reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1alpha1.ClusterExtension, _ *ocv1alpha1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) {
v := bsemver.MustParse("1.0.0")
Expand All @@ -372,7 +311,6 @@ func TestClusterExtensionManagerFailed(t *testing.T) {
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))

t.Log("By checking the status fields")
require.Equal(t, ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.Resolution.Bundle)
require.Equal(t, ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.Install.Bundle)

t.Log("By checking the expected installed conditions")
Expand Down Expand Up @@ -433,7 +371,6 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) {
err := cl.Create(ctx, clusterExtension)
require.NoError(t, err)

t.Log("It sets resolution success status")
t.Log("By running reconcile")
reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1alpha1.ClusterExtension, _ *ocv1alpha1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) {
v := bsemver.MustParse("1.0.0")
Expand All @@ -459,7 +396,6 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) {
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))

t.Log("By checking the status fields")
require.Equal(t, ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.Resolution.Bundle)
require.Equal(t, ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.Install.Bundle)

t.Log("By checking the expected installed conditions")
Expand Down Expand Up @@ -519,7 +455,6 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) {
err := cl.Create(ctx, clusterExtension)
require.NoError(t, err)

t.Log("It sets resolution success status")
t.Log("By running reconcile")
reconciler.Resolver = resolve.Func(func(_ context.Context, _ *ocv1alpha1.ClusterExtension, _ *ocv1alpha1.BundleMetadata) (*declcfg.Bundle, *bsemver.Version, *declcfg.Deprecation, error) {
v := bsemver.MustParse("1.0.0")
Expand All @@ -543,7 +478,6 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) {
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))

t.Log("By checking the status fields")
require.Equal(t, ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.Resolution.Bundle)
require.Equal(t, ocv1alpha1.BundleMetadata{Name: "prometheus.v1.0.0", Version: "1.0.0"}, clusterExtension.Status.Install.Bundle)

t.Log("By checking the expected installed conditions")
Expand All @@ -561,28 +495,6 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) {
require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
}

func verifyInvariants(ctx context.Context, t *testing.T, c client.Client, ext *ocv1alpha1.ClusterExtension) {
key := client.ObjectKeyFromObject(ext)
require.NoError(t, c.Get(ctx, key, ext))

verifyConditionsInvariants(t, ext)
}

func verifyConditionsInvariants(t *testing.T, ext *ocv1alpha1.ClusterExtension) {
// Expect that the cluster extension's set of conditions contains all defined
// condition types for the ClusterExtension API. Every reconcile should always
// ensure every condition type's status/reason/message reflects the state
// read during _this_ reconcile call.
require.Len(t, ext.Status.Conditions, len(conditionsets.ConditionTypes))
for _, tt := range conditionsets.ConditionTypes {
cond := apimeta.FindStatusCondition(ext.Status.Conditions, tt)
require.NotNil(t, cond)
require.NotEmpty(t, cond.Status)
require.Contains(t, conditionsets.ConditionReasons, cond.Reason)
require.Equal(t, ext.GetGeneration(), cond.ObservedGeneration)
}
}

func TestSetDeprecationStatus(t *testing.T) {
for _, tc := range []struct {
name string
Expand Down
4 changes: 0 additions & 4 deletions internal/controllers/common_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,6 @@ func setInstalledStatusConditionFailed(ext *ocv1alpha1.ClusterExtension, message
})
}

func setResolutionStatus(ext *ocv1alpha1.ClusterExtension, resStatus *ocv1alpha1.ClusterExtensionResolutionStatus) {
ext.Status.Resolution = resStatus
}

func setInstallStatus(ext *ocv1alpha1.ClusterExtension, installStatus *ocv1alpha1.ClusterExtensionInstallStatus) {
ext.Status.Install = installStatus
}
Expand Down
Loading

0 comments on commit 26bdc55

Please sign in to comment.