Skip to content

Commit

Permalink
remove olm.bundle.mediatype property and clusterextension support for…
Browse files Browse the repository at this point in the history
… plain+v0

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
  • Loading branch information
joelanford committed May 2, 2024
1 parent 5985cce commit ea020ed
Show file tree
Hide file tree
Showing 10 changed files with 11 additions and 457 deletions.
3 changes: 0 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ e2e: $(SETUP_ENVTEST) #EXHELP Run the e2e tests.
E2E_REGISTRY_NAME := docker-registry
E2E_REGISTRY_NAMESPACE := operator-controller-e2e
export REG_PKG_NAME := registry-operator
export PLAIN_PKG_NAME := plain-operator
export CATALOG_IMG := $(E2E_REGISTRY_NAME).$(E2E_REGISTRY_NAMESPACE).svc:5000/test-catalog:e2e
.PHONY: test-ext-dev-e2e
test-ext-dev-e2e: $(SETUP_ENVTEST) $(OPERATOR_SDK) $(KUSTOMIZE) $(KIND) #HELP Run extension create, upgrade and delete tests.
Expand Down Expand Up @@ -187,12 +186,10 @@ kind-load-test-artifacts: $(KIND) #EXHELP Load the e2e testdata container images
$(CONTAINER_RUNTIME) tag localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.0 localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.1
$(CONTAINER_RUNTIME) tag localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.0 localhost/testdata/bundles/registry-v1/prometheus-operator:v1.2.0
$(CONTAINER_RUNTIME) tag localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.0 localhost/testdata/bundles/registry-v1/prometheus-operator:v2.0.0
$(CONTAINER_RUNTIME) build testdata/bundles/plain-v0/plain.v0.1.0 -t localhost/testdata/bundles/plain-v0/plain:v0.1.0
$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.0 --name $(KIND_CLUSTER_NAME)
$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v1.0.1 --name $(KIND_CLUSTER_NAME)
$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v1.2.0 --name $(KIND_CLUSTER_NAME)
$(KIND) load docker-image localhost/testdata/bundles/registry-v1/prometheus-operator:v2.0.0 --name $(KIND_CLUSTER_NAME)
$(KIND) load docker-image localhost/testdata/bundles/plain-v0/plain:v0.1.0 --name $(KIND_CLUSTER_NAME)


#SECTION Build
Expand Down
28 changes: 0 additions & 28 deletions internal/catalogmetadata/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,6 @@ import (
"github.com/operator-framework/operator-registry/alpha/property"
)

const (
MediaTypePlain = "plain+v0"
MediaTypeRegistry = "registry+v1"
PropertyBundleMediaType = "olm.bundle.mediatype"
)

type Schemas interface {
Package | Bundle | Channel | Deprecation
}
Expand Down Expand Up @@ -50,7 +44,6 @@ type Bundle struct {
bundlePackage *property.Package
semVersion *bsemver.Version
requiredPackages []PackageRequired
mediaType *string
}

func (b *Bundle) Version() (*bsemver.Version, error) {
Expand All @@ -67,14 +60,6 @@ func (b *Bundle) RequiredPackages() ([]PackageRequired, error) {
return b.requiredPackages, nil
}

func (b *Bundle) MediaType() (string, error) {
if err := b.loadMediaType(); err != nil {
return "", err
}

return *b.mediaType, nil
}

func (b *Bundle) loadPackage() error {
b.mu.Lock()
defer b.mu.Unlock()
Expand Down Expand Up @@ -120,19 +105,6 @@ func (b *Bundle) loadRequiredPackages() error {
return nil
}

func (b *Bundle) loadMediaType() error {
b.mu.Lock()
defer b.mu.Unlock()
if b.mediaType == nil {
mediaType, err := loadOneFromProps[string](b, PropertyBundleMediaType, false)
if err != nil {
return fmt.Errorf("error determining bundle mediatype for bundle %q: %s", b.Name, err)
}
b.mediaType = &mediaType
}
return nil
}

func (b *Bundle) propertiesByType(propType string) []*property.Property {
if b.propertiesMap == nil {
b.propertiesMap = make(map[string][]*property.Property)
Expand Down
56 changes: 0 additions & 56 deletions internal/catalogmetadata/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package catalogmetadata_test

import (
"encoding/json"
"fmt"
"testing"

bsemver "github.com/blang/semver/v4"
Expand Down Expand Up @@ -147,61 +146,6 @@ func TestBundleRequiredPackages(t *testing.T) {
}
}

func TestBundleMediaType(t *testing.T) {
for _, tt := range []struct {
name string
bundle *catalogmetadata.Bundle
wantMediaType string
wantErr string
}{
{
name: "valid mediatype property",
bundle: &catalogmetadata.Bundle{Bundle: declcfg.Bundle{
Name: "fake-bundle.v1",
Properties: []property.Property{
{
Type: catalogmetadata.PropertyBundleMediaType,
Value: json.RawMessage(fmt.Sprintf(`"%s"`, catalogmetadata.MediaTypePlain)),
},
},
}},
wantMediaType: catalogmetadata.MediaTypePlain,
},
{
name: "no media type provided",
bundle: &catalogmetadata.Bundle{Bundle: declcfg.Bundle{
Name: "fake-bundle.noMediaType",
Properties: []property.Property{},
}},
wantMediaType: "",
},
{
name: "malformed media type",
bundle: &catalogmetadata.Bundle{Bundle: declcfg.Bundle{
Name: "fake-bundle.badMediaType",
Properties: []property.Property{
{
Type: catalogmetadata.PropertyBundleMediaType,
Value: json.RawMessage("badtype"),
},
},
}},
wantMediaType: "",
wantErr: `error determining bundle mediatype for bundle "fake-bundle.badMediaType": property "olm.bundle.mediatype" with value "badtype" could not be parsed: invalid character 'b' looking for beginning of value`,
},
} {
t.Run(tt.name, func(t *testing.T) {
mediaType, err := tt.bundle.MediaType()
assert.Equal(t, tt.wantMediaType, mediaType)
if tt.wantErr != "" {
assert.EqualError(t, err, tt.wantErr)
} else {
assert.NoError(t, err)
}
})
}
}

func TestBundleHasDeprecation(t *testing.T) {
for _, tt := range []struct {
name string
Expand Down
34 changes: 4 additions & 30 deletions internal/controllers/clusterextension_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,25 +135,16 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alp

// 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
}

if err := r.validateBundle(bundle); 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
}
// Assume all bundles are registry+v1 for now, since that's all we support.
// If the bundle is not a registry+v1 bundle, the conversion will fail.
bundleProvisioner := "core-rukpak-io-registry"

// Ensure a BundleDeployment exists with its bundle source from the bundle
// image we just looked up in the solution.
dep := r.GenerateExpectedBundleDeployment(*ext, bundle.Image, bundleProvisioner)
Expand Down Expand Up @@ -523,23 +514,6 @@ func (r *ClusterExtensionReconciler) existingBundleDeploymentUnstructured(ctx co
return &unstructured.Unstructured{Object: unstrExistingBundleDeploymentObj}, nil
}

// mapBundleMediaTypeToBundleProvisioner maps an olm.bundle.mediatype property to a
// rukpak bundle provisioner class name that is capable of unpacking the bundle type
func mapBundleMediaTypeToBundleProvisioner(mediaType string) (string, error) {
switch mediaType {
case catalogmetadata.MediaTypePlain:
return "core-rukpak-io-plain", nil
// To ensure compatibility with bundles created with OLMv0 where the
// olm.bundle.mediatype property doesn't exist, we assume that if the
// property is empty (i.e doesn't exist) that the bundle is one created
// with OLMv0 and therefore should use the registry provisioner
case catalogmetadata.MediaTypeRegistry, "":
return "core-rukpak-io-registry", nil
default:
return "", fmt.Errorf("unknown bundle mediatype: %s", mediaType)
}
}

// 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
144 changes: 0 additions & 144 deletions internal/controllers/clusterextension_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -909,116 +909,6 @@ func TestClusterExtensionNoVersion(t *testing.T) {
require.NoError(t, cl.DeleteAllOf(ctx, &rukpakv1alpha2.BundleDeployment{}))
}

func TestClusterExtensionPlainV0Bundle(t *testing.T) {
cl, reconciler := newClientAndReconciler(t)
ctx := context.Background()
extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}

t.Log("When the cluster extension specifies a package with a plain+v0 bundle")
t.Log("By initializing cluster state")
pkgName := "plain"
pkgVer := "0.1.0"
pkgChan := "beta"
clusterExtension := &ocv1alpha1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
Spec: ocv1alpha1.ClusterExtensionSpec{
PackageName: pkgName,
Version: pkgVer,
Channel: pkgChan,
InstallNamespace: "default",
},
}
require.NoError(t, cl.Create(ctx, clusterExtension))

t.Log("It sets resolution success status")
t.Log("By running reconcile")
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
require.Equal(t, ctrl.Result{}, res)
require.NoError(t, err)

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.Equal(t, &ocv1alpha1.BundleMetadata{Name: "operatorhub/plain/0.1.0", Version: "0.1.0"}, clusterExtension.Status.ResolvedBundle)
require.Empty(t, clusterExtension.Status.InstalledBundle)
t.Log("By checking the expected conditions")
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionTrue, cond.Status)
require.Equal(t, ocv1alpha1.ReasonSuccess, cond.Reason)
require.Equal(t, "resolved to \"quay.io/operatorhub/plain@sha256:plain\"", cond.Message)
cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionUnknown, cond.Status)
require.Equal(t, ocv1alpha1.ReasonInstallationStatusUnknown, cond.Reason)
require.Equal(t, "bundledeployment status is unknown", cond.Message)

t.Log("By fetching the bundled deployment")
bd := &rukpakv1alpha2.BundleDeployment{}
require.NoError(t, cl.Get(ctx, types.NamespacedName{Name: extKey.Name}, bd))
require.Equal(t, "core-rukpak-io-plain", bd.Spec.ProvisionerClassName)
require.Equal(t, rukpakv1alpha2.SourceTypeImage, bd.Spec.Source.Type)
require.NotNil(t, bd.Spec.Source.Image)
require.Equal(t, "quay.io/operatorhub/plain@sha256:plain", bd.Spec.Source.Image.Ref)

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

func TestClusterExtensionBadBundleMediaType(t *testing.T) {
cl, reconciler := newClientAndReconciler(t)
ctx := context.Background()
extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}

t.Log("When the cluster extension specifies a package with a bad bundle mediatype")
t.Log("By initializing cluster state")
pkgName := "badmedia"
pkgVer := "0.1.0"
pkgChan := "beta"
clusterExtension := &ocv1alpha1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
Spec: ocv1alpha1.ClusterExtensionSpec{
PackageName: pkgName,
Version: pkgVer,
Channel: pkgChan,
InstallNamespace: "default",
},
}
require.NoError(t, cl.Create(ctx, clusterExtension))

t.Log("It sets resolution success status")
t.Log("By running reconcile")
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
require.Equal(t, ctrl.Result{}, res)
require.Error(t, err)
require.ErrorContains(t, err, "unknown bundle mediatype: badmedia+v1")

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.Equal(t, &ocv1alpha1.BundleMetadata{Name: "operatorhub/badmedia/0.1.0", Version: "0.1.0"}, clusterExtension.Status.ResolvedBundle)
require.Empty(t, clusterExtension.Status.InstalledBundle)

t.Log("By checking the expected conditions")
cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeResolved)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionTrue, cond.Status)
require.Equal(t, ocv1alpha1.ReasonSuccess, cond.Reason)
require.Equal(t, "resolved to \"quay.io/operatorhub/badmedia@sha256:badmedia\"", cond.Message)
cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1alpha1.ReasonInstallationFailed, cond.Reason)
require.Equal(t, "unknown bundle mediatype: badmedia+v1", cond.Message)

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

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))
Expand Down Expand Up @@ -2000,12 +1890,6 @@ var (
},
},
}
plainBetaChannel = catalogmetadata.Channel{
Channel: declcfg.Channel{
Name: "beta",
Package: "plain",
},
}
badmediaBetaChannel = catalogmetadata.Channel{

Check failure on line 1893 in internal/controllers/clusterextension_controller_test.go

View workflow job for this annotation

GitHub Actions / lint

var `badmediaBetaChannel` is unused (unused)
Channel: declcfg.Channel{
Name: "beta",
Expand Down Expand Up @@ -2080,32 +1964,4 @@ var testBundleList = []*catalogmetadata.Bundle{
CatalogName: "fake-catalog",
InChannels: []*catalogmetadata.Channel{&prometheusBetaChannel},
},
{
Bundle: declcfg.Bundle{
Name: "operatorhub/plain/0.1.0",
Package: "plain",
Image: "quay.io/operatorhub/plain@sha256:plain",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"plain","version":"0.1.0"}`)},
{Type: property.TypeGVK, Value: json.RawMessage(`[]`)},
{Type: "olm.bundle.mediatype", Value: json.RawMessage(`"plain+v0"`)},
},
},
CatalogName: "fake-catalog",
InChannels: []*catalogmetadata.Channel{&plainBetaChannel},
},
{
Bundle: declcfg.Bundle{
Name: "operatorhub/badmedia/0.1.0",
Package: "badmedia",
Image: "quay.io/operatorhub/badmedia@sha256:badmedia",
Properties: []property.Property{
{Type: property.TypePackage, Value: json.RawMessage(`{"packageName":"badmedia","version":"0.1.0"}`)},
{Type: property.TypeGVK, Value: json.RawMessage(`[]`)},
{Type: "olm.bundle.mediatype", Value: json.RawMessage(`"badmedia+v1"`)},
},
},
CatalogName: "fake-catalog",
InChannels: []*catalogmetadata.Channel{&badmediaBetaChannel},
},
}
26 changes: 1 addition & 25 deletions internal/controllers/extension_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,31 +164,7 @@ func (r *ExtensionReconciler) reconcile(ctx context.Context, ext *ocv1alpha1.Ext
ext.Status.ResolvedBundle = bundleMetadataFor(bundle)
setResolvedStatusConditionSuccess(&ext.Status.Conditions, fmt.Sprintf("resolved to %q", bundle.Image), ext.GetGeneration())

mediaType, err := bundle.MediaType()
if err != nil {
if c := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1alpha1.TypeInstalled); c == nil {
ext.Status.InstalledBundle = nil
setInstalledStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("failed to read bundle mediaType: %v", err), ext.GetGeneration())
setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration())
}
setProgressingStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("failed to read bundle mediaType: %v", err), ext.GetGeneration())
return ctrl.Result{}, err
}

// TODO: this needs to include the registryV1 bundle option. As of this PR, this only supports direct
// installation of a set of manifests.
if mediaType != catalogmetadata.MediaTypePlain {
if c := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1alpha1.TypeInstalled); c == nil {
// Set the TypeInstalled condition to Failed to indicate that the resolution
// hasn't been attempted yet, due to the spec being invalid.
ext.Status.InstalledBundle = nil
setInstalledStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("bundle type %s not supported currently", mediaType), ext.GetGeneration())
setDeprecationStatusesUnknown(&ext.Status.Conditions, "deprecation checks have not been attempted as installation has failed", ext.GetGeneration())
}
setProgressingStatusConditionFailed(&ext.Status.Conditions, fmt.Sprintf("bundle type %s not supported currently", mediaType), ext.GetGeneration())
return ctrl.Result{}, nil
}

// Right now, we just assume that the bundle is a plain+v0 bundle.
app, err := r.GenerateExpectedApp(*ext, bundle)
if err != nil {
if c := apimeta.FindStatusCondition(ext.Status.Conditions, ocv1alpha1.TypeInstalled); c == nil {
Expand Down
Loading

0 comments on commit ea020ed

Please sign in to comment.