Skip to content

Commit

Permalink
[WIP] Remove the Resolved status condition (#1312)
Browse files Browse the repository at this point in the history
Signed-off-by: yashoza19 <[email protected]>
  • Loading branch information
yashoza19 authored Sep 26, 2024
1 parent d9cf2ed commit 8699d25
Show file tree
Hide file tree
Showing 5 changed files with 10 additions and 171 deletions.
2 changes: 0 additions & 2 deletions api/v1alpha1/clusterextension_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,6 @@ type CRDUpgradeSafetyPreflightConfig struct {
const (
// TODO(user): add more Types, here and into init()
TypeInstalled = "Installed"
TypeResolved = "Resolved"
TypeProgressing = "Progressing"

// TypeDeprecated is a rollup condition that is present when
Expand All @@ -440,7 +439,6 @@ func init() {
// TODO(user): add Types from above
conditionsets.ConditionTypes = append(conditionsets.ConditionTypes,
TypeInstalled,
TypeResolved,
TypeDeprecated,
TypePackageDeprecated,
TypeChannelDeprecated,
Expand Down
3 changes: 0 additions & 3 deletions internal/controllers/clusterextension_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
// it is properly labeled with its observed generation.
setInstallStatus(ext, nil)
setResolutionStatus(ext, nil)
setResolvedStatusConditionFailed(ext, err.Error())
setStatusProgressing(ext, err)
ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonFailed, err.Error())
return ctrl.Result{}, err
Expand Down Expand Up @@ -228,7 +227,6 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
// Note: We don't distinguish between resolution-specific errors and generic errors
setInstallStatus(ext, nil)
setResolutionStatus(ext, nil)
setResolvedStatusConditionFailed(ext, err.Error())
setStatusProgressing(ext, err)
ensureAllConditionsWithReason(ext, ocv1alpha1.ReasonFailed, err.Error())
return ctrl.Result{}, err
Expand All @@ -255,7 +253,6 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
Bundle: resolvedBundleMetadata,
}
setResolutionStatus(ext, resStatus)
setResolvedStatusConditionSuccess(ext, fmt.Sprintf("resolved to %q", resolvedBundle.Image))

bundleSource := &rukpaksource.BundleSource{
Name: ext.GetName(),
Expand Down
42 changes: 1 addition & 41 deletions internal/controllers/clusterextension_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,7 @@ func TestClusterExtensionResolutionFails(t *testing.T) {
require.Empty(t, clusterExtension.Status.Install)

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

cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
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)
Expand Down Expand Up @@ -161,12 +155,6 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) {
require.Empty(t, clusterExtension.Status.Install)

t.Log("By checking the expected conditions")
resolvedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
require.NotNil(t, resolvedCond)
require.Equal(t, metav1.ConditionTrue, resolvedCond.Status)
require.Equal(t, ocv1alpha1.ReasonSuccess, resolvedCond.Reason)
require.Equal(t, "resolved to \"quay.io/operatorhubio/[email protected]\"", resolvedCond.Message)

progressingCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
require.NotNil(t, progressingCond)
require.Equal(t, metav1.ConditionTrue, progressingCond.Status)
Expand Down Expand Up @@ -302,13 +290,6 @@ func TestClusterExtensionResolutionAndUnpackSuccessfulApplierFails(t *testing.T)
require.Equal(t, expectedBundleMetadata, clusterExtension.Status.Resolution.Bundle)
require.Empty(t, clusterExtension.Status.Install)

t.Log("By checking the expected resolution conditions")
resolvedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
require.NotNil(t, resolvedCond)
require.Equal(t, metav1.ConditionTrue, resolvedCond.Status)
require.Equal(t, ocv1alpha1.ReasonSuccess, resolvedCond.Reason)
require.Equal(t, "resolved to \"quay.io/operatorhubio/[email protected]\"", resolvedCond.Message)

t.Log("By checking the expected installed conditions")
installedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
require.NotNil(t, installedCond)
Expand Down Expand Up @@ -394,13 +375,6 @@ func TestClusterExtensionManagerFailed(t *testing.T) {
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 resolution conditions")
resolvedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
require.NotNil(t, resolvedCond)
require.Equal(t, metav1.ConditionTrue, resolvedCond.Status)
require.Equal(t, ocv1alpha1.ReasonSuccess, resolvedCond.Reason)
require.Equal(t, "resolved to \"quay.io/operatorhubio/[email protected]\"", resolvedCond.Message)

t.Log("By checking the expected installed conditions")
installedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
require.NotNil(t, installedCond)
Expand Down Expand Up @@ -488,13 +462,6 @@ func TestClusterExtensionManagedContentCacheWatchFail(t *testing.T) {
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 resolution conditions")
resolvedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
require.NotNil(t, resolvedCond)
require.Equal(t, metav1.ConditionTrue, resolvedCond.Status)
require.Equal(t, ocv1alpha1.ReasonSuccess, resolvedCond.Reason)
require.Equal(t, "resolved to \"quay.io/operatorhubio/[email protected]\"", resolvedCond.Message)

t.Log("By checking the expected installed conditions")
installedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
require.NotNil(t, installedCond)
Expand Down Expand Up @@ -579,13 +546,6 @@ func TestClusterExtensionInstallationSucceeds(t *testing.T) {
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 resolution conditions")
resolvedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
require.NotNil(t, resolvedCond)
require.Equal(t, metav1.ConditionTrue, resolvedCond.Status)
require.Equal(t, ocv1alpha1.ReasonSuccess, resolvedCond.Reason)
require.Equal(t, "resolved to \"quay.io/operatorhubio/[email protected]\"", resolvedCond.Message)

t.Log("By checking the expected installed conditions")
installedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
require.NotNil(t, installedCond)
Expand Down
22 changes: 0 additions & 22 deletions internal/controllers/common_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,28 +26,6 @@ import (
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
)

// setResolvedStatusConditionSuccess sets the resolved status condition to success.
func setResolvedStatusConditionSuccess(ext *ocv1alpha1.ClusterExtension, message string) {
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Type: ocv1alpha1.TypeResolved,
Status: metav1.ConditionTrue,
Reason: ocv1alpha1.ReasonSuccess,
Message: message,
ObservedGeneration: ext.GetGeneration(),
})
}

// setResolvedStatusConditionFailed sets the resolved status condition to failed.
func setResolvedStatusConditionFailed(ext *ocv1alpha1.ClusterExtension, message string) {
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Type: ocv1alpha1.TypeResolved,
Status: metav1.ConditionFalse,
Reason: ocv1alpha1.ReasonFailed,
Message: message,
ObservedGeneration: ext.GetGeneration(),
})
}

// setInstalledStatusConditionSuccess sets the installed status condition to success.
func setInstalledStatusConditionSuccess(ext *ocv1alpha1.ClusterExtension, message string) {
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Expand Down
112 changes: 9 additions & 103 deletions test/e2e/cluster_extension_install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,13 +268,6 @@ func TestClusterExtensionInstallRegistry(t *testing.T) {
t.Log("By eventually reporting a successful resolution and bundle path")
require.EventuallyWithT(t, func(ct *assert.CollectT) {
assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
if !assert.NotNil(ct, cond) {
return
}
assert.Equal(ct, metav1.ConditionTrue, cond.Status)
assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
assert.Contains(ct, cond.Message, "resolved to")
assert.Equal(ct,
&ocv1alpha1.ClusterExtensionResolutionStatus{Bundle: ocv1alpha1.BundleMetadata{
Name: fmt.Sprintf("%s-operator.1.2.0", tc.packageName),
Expand Down Expand Up @@ -339,13 +332,6 @@ func TestClusterExtensionInstallRegistryMultipleBundles(t *testing.T) {
t.Log("By eventually reporting a failed resolution with multiple bundles")
require.EventuallyWithT(t, func(ct *assert.CollectT) {
assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
if !assert.NotNil(ct, cond) {
return
}
assert.Equal(ct, metav1.ConditionFalse, cond.Status)
assert.Equal(ct, ocv1alpha1.ReasonFailed, cond.Reason)
assert.Contains(ct, cond.Message, "in multiple catalogs with the same priority [operatorhubio test-catalog]")
assert.Nil(ct, clusterExtension.Status.Resolution)
}, pollDuration, pollInterval)

Expand Down Expand Up @@ -391,13 +377,6 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) {
t.Log("By eventually reporting a successful installation")
require.EventuallyWithT(t, func(ct *assert.CollectT) {
assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
if !assert.NotNil(ct, cond) {
return
}
assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
assert.Contains(ct, cond.Message, "resolved to")

assert.Equal(ct,
&ocv1alpha1.ClusterExtensionResolutionStatus{Bundle: ocv1alpha1.BundleMetadata{
Name: "prometheus-operator.1.0.0",
Expand All @@ -413,7 +392,7 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) {
clusterExtension.Status.Install,
)

cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
if !assert.NotNil(ct, cond) {
return
}
Expand All @@ -429,12 +408,6 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) {
t.Log("By eventually reporting an unsatisfiable resolution")
require.EventuallyWithT(t, func(ct *assert.CollectT) {
assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
if !assert.NotNil(ct, cond) {
return
}
assert.Equal(ct, ocv1alpha1.ReasonFailed, cond.Reason)
assert.Equal(ct, "error upgrading from currently installed version \"1.0.0\": no bundles found for package \"prometheus\" matching version \"1.2.0\"", cond.Message)
assert.Empty(ct, clusterExtension.Status.Resolution)
}, pollDuration, pollInterval)

Expand Down Expand Up @@ -478,14 +451,7 @@ func TestClusterExtensionForceInstallNonSuccessorVersion(t *testing.T) {
t.Log("By eventually reporting a successful resolution")
require.EventuallyWithT(t, func(ct *assert.CollectT) {
assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
if !assert.NotNil(ct, cond) {
return
}
assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
assert.Contains(ct, cond.Message, "resolved to")

cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
if !assert.NotNil(ct, cond) {
return
}
Expand All @@ -510,14 +476,7 @@ func TestClusterExtensionForceInstallNonSuccessorVersion(t *testing.T) {
t.Log("By eventually reporting a satisfiable resolution")
require.EventuallyWithT(t, func(ct *assert.CollectT) {
assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
if !assert.NotNil(ct, cond) {
return
}
assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
assert.Contains(ct, cond.Message, "resolved to")

cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
if !assert.NotNil(ct, cond) {
return
}
Expand Down Expand Up @@ -561,14 +520,7 @@ func TestClusterExtensionInstallSuccessorVersion(t *testing.T) {
t.Log("By eventually reporting a successful resolution")
require.EventuallyWithT(t, func(ct *assert.CollectT) {
assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
if !assert.NotNil(ct, cond) {
return
}
assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
assert.Contains(ct, cond.Message, "resolved to")

cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
if !assert.NotNil(ct, cond) {
return
}
Expand All @@ -592,14 +544,7 @@ func TestClusterExtensionInstallSuccessorVersion(t *testing.T) {
t.Log("By eventually reporting a successful resolution and bundle path")
require.EventuallyWithT(t, func(ct *assert.CollectT) {
assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
if !assert.NotNil(ct, cond) {
return
}
assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
assert.Contains(ct, cond.Message, "resolved to")

cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
if !assert.NotNil(ct, cond) {
return
}
Expand Down Expand Up @@ -653,15 +598,7 @@ func TestClusterExtensionInstallReResolvesWhenCatalogIsPatched(t *testing.T) {
t.Log("By reporting a successful resolution and bundle path")
require.EventuallyWithT(t, func(ct *assert.CollectT) {
assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
if !assert.NotNil(ct, cond) {
return
}
assert.Equal(ct, metav1.ConditionTrue, cond.Status)
assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
assert.Contains(ct, cond.Message, "resolved to")

cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
if !assert.NotNil(ct, cond) {
return
}
Expand Down Expand Up @@ -695,15 +632,7 @@ func TestClusterExtensionInstallReResolvesWhenCatalogIsPatched(t *testing.T) {
t.Log("By eventually reporting a successful resolution and bundle path")
require.EventuallyWithT(t, func(ct *assert.CollectT) {
assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
if !assert.NotNil(ct, cond) {
return
}
assert.Equal(ct, metav1.ConditionTrue, cond.Status)
assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
assert.Contains(ct, cond.Message, "resolved to")

cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
if !assert.NotNil(ct, cond) {
return
}
Expand Down Expand Up @@ -769,15 +698,7 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) {
t.Log("By reporting a successful resolution and bundle path")
require.EventuallyWithT(t, func(ct *assert.CollectT) {
assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
if !assert.NotNil(ct, cond) {
return
}
assert.Equal(ct, metav1.ConditionTrue, cond.Status)
assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
assert.Contains(ct, cond.Message, "resolved to")

cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
if !assert.NotNil(ct, cond) {
return
}
Expand Down Expand Up @@ -811,15 +732,7 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) {
t.Log("By eventually reporting a successful resolution and bundle path")
require.EventuallyWithT(t, func(ct *assert.CollectT) {
assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
if !assert.NotNil(ct, cond) {
return
}
assert.Equal(ct, metav1.ConditionTrue, cond.Status)
assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
assert.Contains(ct, cond.Message, "resolved to")

cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing)
if !assert.NotNil(ct, cond) {
return
}
Expand Down Expand Up @@ -940,13 +853,6 @@ func TestClusterExtensionRecoversFromInitialInstallFailedWhenFailureFixed(t *tes
t.Log("By eventually reporting a successful resolution and bundle path")
require.EventuallyWithT(t, func(ct *assert.CollectT) {
assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension))
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
if !assert.NotNil(ct, cond) {
return
}
assert.Equal(ct, metav1.ConditionTrue, cond.Status)
assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason)
assert.Contains(ct, cond.Message, "resolved to")
assert.Equal(ct,
&ocv1alpha1.ClusterExtensionResolutionStatus{Bundle: ocv1alpha1.BundleMetadata{
Name: "prometheus-operator.1.2.0",
Expand Down

0 comments on commit 8699d25

Please sign in to comment.