Skip to content

Commit

Permalink
do some cleanup
Browse files Browse the repository at this point in the history 
Signed-off-by: everettraven <[email protected]>
  • Loading branch information
@everettraven
everettraven committed Jan 8, 2024
1 parent 6b43b23 commit ef355d0
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 23 deletions.
3 changes: 0 additions & 3 deletions internal/catalogmetadata/client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,6 @@ func PopulateExtraFields(catalogName string, channels []*catalogmetadata.Channel
}
}

// if package is deprecated, add deprecation to bundle
// if channel is deprecated, add deprecation to bundle
// if bundle is deprecated, add deprecation to bundle
for i := range bundles {
for _, dep := range deprecations {
if bundles[i].Package == dep.Package {
Expand Down
83 changes: 63 additions & 20 deletions internal/controllers/clusterextension_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/operator-framework/operator-registry/alpha/declcfg"
rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/meta"
apimeta "k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand All @@ -42,7 +41,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/operator-framework/operator-controller/api/v1alpha1"
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
"github.com/operator-framework/operator-controller/internal/controllers/validators"
Expand Down Expand Up @@ -134,6 +132,8 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
// hasn't been attempted yet, due to the spec being invalid.
ext.Status.ResolvedBundleResource = ""
setResolvedStatusConditionUnknown(&ext.Status.Conditions, "validation has not been attempted as spec is invalid", ext.GetGeneration())

setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as spec is invalid", ext.GetGeneration())
return ctrl.Result{}, nil
}

Expand All @@ -144,6 +144,8 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
setInstalledStatusConditionUnknown(&ext.Status.Conditions, "installation has not been attempted due to failure to gather data for resolution", ext.GetGeneration())
ext.Status.ResolvedBundleResource = ""
setResolvedStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())

setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted due to failure to gather data for resolution", ext.GetGeneration())
return ctrl.Result{}, err
}

Expand All @@ -154,6 +156,8 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
setInstalledStatusConditionUnknown(&ext.Status.Conditions, "installation has not been attempted as resolution failed", ext.GetGeneration())
ext.Status.ResolvedBundleResource = ""
setResolvedStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())

setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as resolution failed", ext.GetGeneration())
return ctrl.Result{}, err
}

Expand All @@ -165,21 +169,27 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
setInstalledStatusConditionUnknown(&ext.Status.Conditions, "installation has not been attempted as resolution failed", ext.GetGeneration())
ext.Status.ResolvedBundleResource = ""
setResolvedStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())

setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as resolution failed", ext.GetGeneration())
return ctrl.Result{}, err
}

// Now we can set the Resolved Condition, and the resolvedBundleSource field to the bundle.Image value.
ext.Status.ResolvedBundleResource = bundle.Image
setResolvedStatusConditionSuccess(&ext.Status.Conditions, fmt.Sprintf("resolved to %q", bundle.Image), ext.GetGeneration())

// TODO: Question - Should we set the deprecation statuses after we have successfully resolved instead of after a successful installation?

mediaType, err := bundle.MediaType()
if err != nil {
setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration())
return ctrl.Result{}, err
}
bundleProvisioner, err := mapBundleMediaTypeToBundleProvisioner(mediaType)
if err != nil {
setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration())
return ctrl.Result{}, err
}
// Ensure a BundleDeployment exists with its bundle source from the bundle
Expand All @@ -189,6 +199,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
// originally Reason: ocv1alpha1.ReasonInstallationFailed
ext.Status.InstalledBundleResource = ""
setInstalledStatusConditionFailed(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration())
return ctrl.Result{}, err
}

Expand All @@ -198,6 +209,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp
// originally Reason: ocv1alpha1.ReasonInstallationStatusUnknown
ext.Status.InstalledBundleResource = ""
setInstalledStatusConditionUnknown(&ext.Status.Conditions, err.Error(), ext.GetGeneration())
setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration())
return ctrl.Result{}, err
}

Expand Down Expand Up @@ -275,12 +287,24 @@ func mapBDStatusToInstalledCondition(existingTypedBundleDeployment *rukpakv1alph

// setDeprecationStatus will set the appropriate deprecation statuses for a ClusterExtension
// based on the provided bundle
func setDeprecationStatus(ext *v1alpha1.ClusterExtension, bundle *catalogmetadata.Bundle) {
// unset conditions
meta.RemoveStatusCondition(&ext.Status.Conditions, v1alpha1.TypePackageDeprecated)
meta.RemoveStatusCondition(&ext.Status.Conditions, v1alpha1.TypeChannelDeprecated)
meta.RemoveStatusCondition(&ext.Status.Conditions, v1alpha1.TypeBundleDeprecated)
meta.RemoveStatusCondition(&ext.Status.Conditions, v1alpha1.TypeDeprecated)
func setDeprecationStatus(ext *ocv1alpha1.ClusterExtension, bundle *catalogmetadata.Bundle) {
// reset conditions to false
conditionTypes := []string{
ocv1alpha1.TypeDeprecated,
ocv1alpha1.TypePackageDeprecated,
ocv1alpha1.TypeChannelDeprecated,
ocv1alpha1.TypeBundleDeprecated,
}

for _, conditionType := range conditionTypes {
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Type: conditionType,
Reason: ocv1alpha1.ReasonDeprecated,
Status: metav1.ConditionFalse,
Message: "",
ObservedGeneration: ext.Generation,
})
}

if !bundle.HasDeprecation() {
return
Expand All @@ -291,9 +315,9 @@ func setDeprecationStatus(ext *v1alpha1.ClusterExtension, bundle *catalogmetadat
for _, deprecation := range bundle.Deprecations {
switch deprecation.Reference.Schema {
case declcfg.SchemaPackage:
meta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Type: v1alpha1.TypePackageDeprecated,
Reason: v1alpha1.ReasonDeprecated,
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Type: ocv1alpha1.TypePackageDeprecated,
Reason: ocv1alpha1.ReasonDeprecated,
Status: metav1.ConditionTrue,
Message: deprecation.Message,
ObservedGeneration: ext.Generation,
Expand All @@ -303,17 +327,17 @@ func setDeprecationStatus(ext *v1alpha1.ClusterExtension, bundle *catalogmetadat
continue
}

meta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Type: v1alpha1.TypeChannelDeprecated,
Reason: v1alpha1.ReasonDeprecated,
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Type: ocv1alpha1.TypeChannelDeprecated,
Reason: ocv1alpha1.ReasonDeprecated,
Status: metav1.ConditionTrue,
Message: deprecation.Message,
ObservedGeneration: ext.Generation,
})
case declcfg.SchemaBundle:
meta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Type: v1alpha1.TypeBundleDeprecated,
Reason: v1alpha1.ReasonDeprecated,
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Type: ocv1alpha1.TypeBundleDeprecated,
Reason: ocv1alpha1.ReasonDeprecated,
Status: metav1.ConditionTrue,
Message: deprecation.Message,
ObservedGeneration: ext.Generation,
Expand All @@ -323,9 +347,9 @@ func setDeprecationStatus(ext *v1alpha1.ClusterExtension, bundle *catalogmetadat
deprecationMessages = append(deprecationMessages, deprecation.Message)
}

meta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Type: v1alpha1.TypeDeprecated,
Reason: v1alpha1.ReasonDeprecated,
apimeta.SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
Type: ocv1alpha1.TypeDeprecated,
Reason: ocv1alpha1.ReasonDeprecated,
Status: metav1.ConditionTrue,
Message: strings.Join(deprecationMessages, ";"),
ObservedGeneration: ext.Generation,
Expand Down Expand Up @@ -522,6 +546,25 @@ func setInstalledStatusConditionUnknown(conditions *[]metav1.Condition, message
})
}

func setDeprecationStatusesUnknown(conditions *[]metav1.Condition, message string, generation int64) {
conditionTypes := []string{
ocv1alpha1.TypeDeprecated,
ocv1alpha1.TypePackageDeprecated,
ocv1alpha1.TypeChannelDeprecated,
ocv1alpha1.TypeBundleDeprecated,
}

for _, conditionType := range conditionTypes {
apimeta.SetStatusCondition(conditions, metav1.Condition{
Type: conditionType,
Reason: ocv1alpha1.ReasonDeprecated,
Status: metav1.ConditionUnknown,
Message: message,
ObservedGeneration: generation,
})
}
}

// Generate reconcile requests for all cluster extensions affected by a catalog change
func clusterExtensionRequestsForCatalog(c client.Reader, logger logr.Logger) handler.MapFunc {
return func(ctx context.Context, _ client.Object) []reconcile.Request {
Expand Down
40 changes: 40 additions & 0 deletions internal/resolution/variablesources/required_package_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,44 @@ func TestMakeRequiredPackageVariables(t *testing.T) {
},
},
},
"test-package.v4.1.0": {
Bundle: declcfg.Bundle{
Name: "test-package.v4.1.0",
Package: "test-package",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "4.1.0"}`)},
},
},
InChannels: []*catalogmetadata.Channel{&stableChannel},
Deprecations: []declcfg.DeprecationEntry{
{
Reference: declcfg.PackageScopedReference{
Schema: declcfg.SchemaBundle,
Name: "test-package.v4.1.0",
},
Message: "test-package.v4.1.0 has been deprecated",
},
},
},
"test-package.v5.0.0": {
Bundle: declcfg.Bundle{
Name: "test-package.v5.0.0",
Package: "test-package",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName": "test-package", "version": "5.0.0"}`)},
},
},
InChannels: []*catalogmetadata.Channel{&stableChannel},
Deprecations: []declcfg.DeprecationEntry{
{
Reference: declcfg.PackageScopedReference{
Schema: declcfg.SchemaBundle,
Name: "test-package.v5.0.0",
},
Message: "test-package.v5.0.0 has been deprecated",
},
},
},

// We need at least one bundle from different package
// to make sure that we are filtering it out.
Expand Down Expand Up @@ -130,6 +168,8 @@ func TestMakeRequiredPackageVariables(t *testing.T) {
bundleSet["test-package.v3.0.0"],
bundleSet["test-package.v2.0.0"],
bundleSet["test-package.v1.0.0"],
bundleSet["test-package.v5.0.0"],
bundleSet["test-package.v4.1.0"],
bundleSet["test-package.v4.0.0"],
}),
},
Expand Down

0 comments on commit ef355d0

Please sign in to comment.