Skip to content

Commit

Permalink
Fixes replace based upgrades
Browse files Browse the repository at this point in the history 
Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
  • Loading branch information
@m1kola
m1kola committed Aug 10, 2023
1 parent dea70eb commit 28da6bd
Show file tree
Hide file tree
Showing 11 changed files with 107 additions and 96 deletions.
47 changes: 26 additions & 21 deletions internal/controllers/operator_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1082,34 +1082,39 @@ func verifyConditionsInvariants(op *operatorsv1alpha1.Operator) {

var testEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{
"operatorhub/prometheus/0.37.0": *input.NewEntity("operatorhub/prometheus/0.37.0", map[string]string{
"olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"`,
"olm.channel": `{"channelName":"beta","priority":0}`,
"olm.package": `{"packageName":"prometheus","version":"0.37.0"}`,
"olm.gvk": `[]`,
"olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"`,
"olm.bundle.channelEntry": `{"name":"prometheus.0.37.0"}`,
"olm.channel": `{"channelName":"beta","priority":0}`,
"olm.package": `{"packageName":"prometheus","version":"0.37.0"}`,
"olm.gvk": `[]`,
}),
"operatorhub/prometheus/0.47.0": *input.NewEntity("operatorhub/prometheus/0.47.0", map[string]string{
"olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed"`,
"olm.channel": `{"channelName":"beta","priority":0,"replaces":"prometheusoperator.0.37.0"}`,
"olm.package": `{"packageName":"prometheus","version":"0.47.0"}`,
"olm.gvk": `[]`,
"olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed"`,
"olm.bundle.channelEntry": `{"name":"prometheus.0.47.0"}`,
"olm.channel": `{"channelName":"beta","priority":0,"replaces":"prometheusoperator.0.37.0"}`,
"olm.package": `{"packageName":"prometheus","version":"0.47.0"}`,
"olm.gvk": `[]`,
}),
"operatorhub/badimage/0.1.0": *input.NewEntity("operatorhub/badimage/0.1.0", map[string]string{
"olm.bundle.path": `{"name": "quay.io/operatorhubio/badimage:v0.1.0"}`,
"olm.package": `{"packageName":"badimage","version":"0.1.0"}`,
"olm.gvk": `[]`,
"olm.bundle.path": `{"name": "quay.io/operatorhubio/badimage:v0.1.0"}`,
"olm.bundle.channelEntry": `{"name":"badimage.0.1.0"}`,
"olm.package": `{"packageName":"badimage","version":"0.1.0"}`,
"olm.gvk": `[]`,
}),
"operatorhub/plain/0.1.0": *input.NewEntity("operatorhub/plain/0.1.0", map[string]string{
"olm.bundle.path": `"quay.io/operatorhub/plain@sha256:plain"`,
"olm.channel": `{"channelName":"beta","priority":0}`,
"olm.package": `{"packageName":"plain","version":"0.1.0"}`,
"olm.gvk": `[]`,
"olm.bundle.mediatype": `"plain+v0"`,
"olm.bundle.path": `"quay.io/operatorhub/plain@sha256:plain"`,
"olm.bundle.channelEntry": `{"name":"plain.0.1.0"}`,
"olm.channel": `{"channelName":"beta","priority":0}`,
"olm.package": `{"packageName":"plain","version":"0.1.0"}`,
"olm.gvk": `[]`,
"olm.bundle.mediatype": `"plain+v0"`,
}),
"operatorhub/badmedia/0.1.0": *input.NewEntity("operatorhub/badmedia/0.1.0", map[string]string{
"olm.bundle.path": `"quay.io/operatorhub/badmedia@sha256:badmedia"`,
"olm.channel": `{"channelName":"beta","priority":0}`,
"olm.package": `{"packageName":"badmedia","version":"0.1.0"}`,
"olm.gvk": `[]`,
"olm.bundle.mediatype": `"badmedia+v1"`,
"olm.bundle.path": `"quay.io/operatorhub/badmedia@sha256:badmedia"`,
"olm.bundle.channelEntry": `{"name":"badmedia.0.1.0"}`,
"olm.channel": `{"channelName":"beta","priority":0}`,
"olm.package": `{"packageName":"badmedia","version":"0.1.0"}`,
"olm.gvk": `[]`,
"olm.bundle.mediatype": `"badmedia+v1"`,
}),
})
28 changes: 14 additions & 14 deletions internal/resolution/entities/bundle_entity.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ import (
)

const PropertyBundlePath = "olm.bundle.path"

// TODO: Is this the right place for these?
// ----
const PropertyBundleChannelEntry = "olm.bundle.channelEntry"
const PropertyBundleMediaType = "olm.bundle.mediatype"

type MediaType string
Expand All @@ -39,8 +37,10 @@ type PackageRequired struct {

type GVK property.GVK

type Replaces struct {
type ChannelEntry struct {
Name string `json:"name"`
Replaces string `json:"replaces"`
// Skips and skipRange will probably go here as well
}

func (g GVK) String() string {
Expand All @@ -66,7 +66,7 @@ type BundleEntity struct {
requiredGVKs []GVKRequired
requiredPackages []PackageRequired
channel *property.Channel
replaces *Replaces
channelEntry *ChannelEntry
semVersion *semver.Version
bundlePath string
mediaType string
Expand Down Expand Up @@ -129,22 +129,22 @@ func (b *BundleEntity) Channel() (*property.Channel, error) {
return b.channel, nil
}

func (b *BundleEntity) Replaces() (string, error) {
if err := b.loadReplaces(); err != nil {
return "", err
func (b *BundleEntity) BundleChannelEntry() (*ChannelEntry, error) {
if err := b.loadBundleChannelEntry(); err != nil {
return nil, err
}
return b.replaces.Replaces, nil
return b.channelEntry, nil
}

func (b *BundleEntity) loadReplaces() error {
func (b *BundleEntity) loadBundleChannelEntry() error {
b.mu.Lock()
defer b.mu.Unlock()
if b.replaces == nil {
replaces, err := loadFromEntity[Replaces](b.Entity, "olm.replaces", optional)
if err != nil {
if b.channelEntry == nil {
channelEntry, err := loadFromEntity[*ChannelEntry](b.Entity, PropertyBundleChannelEntry, optional)
if err != nil || channelEntry == nil {
return fmt.Errorf("error determining replaces for entity '%s': %w", b.ID, err)
}
b.replaces = &replaces
b.channelEntry = channelEntry
}
return nil
}
Expand Down
16 changes: 8 additions & 8 deletions internal/resolution/entities/bundle_entity_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,24 +265,24 @@ var _ = Describe("BundleEntity", func() {
})
})

Describe("Replaces", func() {
It("should return the replaces property if present", func() {
Describe("ChannelEntry", func() {
It("should return the channel entry property if present", func() {
entity := input.NewEntity("test", map[string]string{
"olm.replaces": `{"replaces": "test.v0.2.0"}`,
"olm.bundle.channelEntry": `{"name":"test.v0.3.0","replaces": "test.v0.2.0"}`,
})
bundleEntity := olmentity.NewBundleEntity(entity)
replaces, err := bundleEntity.Replaces()
channelEntry, err := bundleEntity.BundleChannelEntry()
Expect(err).ToNot(HaveOccurred())
Expect(replaces).To(Equal("test.v0.2.0"))
Expect(channelEntry).To(Equal(&olmentity.ChannelEntry{Name: "test.v0.3.0", Replaces: "test.v0.2.0"}))
})
It("should not return an error if the property is not found", func() {
entity := input.NewEntity("test", map[string]string{
"olm.thingy": `{"whatever":"this"}`,
})
bundleEntity := olmentity.NewBundleEntity(entity)
replaces, err := bundleEntity.Replaces()
Expect(replaces).To(BeEmpty())
Expect(err).NotTo(HaveOccurred())
channelEntry, err := bundleEntity.BundleChannelEntry()
Expect(channelEntry).To(BeNil())
Expect(err).To(HaveOccurred())
})
})

Expand Down
12 changes: 5 additions & 7 deletions internal/resolution/entitysources/catalogdsource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ func (es *CatalogdEntitySource) Iterate(ctx context.Context, fn input.IteratorFu
return nil
}

type replacesProperty struct {
Replaces string `json:"replaces"`
}

func getEntities(ctx context.Context, client client.Client) (input.EntityList, error) {
entityList := input.EntityList{}
bundleMetadatas, packageMetdatas, err := fetchMetadata(ctx, client)
Expand Down Expand Up @@ -110,9 +106,11 @@ func getEntities(ctx context.Context, client client.Client) (input.EntityList, e
if catalogScopedEntryName == bundle.Name {
channelValue, _ := json.Marshal(property.Channel{ChannelName: ch.Name, Priority: 0})
props[property.TypeChannel] = string(channelValue)
// TODO(jmprusi): Add the proper PropertyType for this
replacesValue, _ := json.Marshal(replacesProperty{Replaces: b.Replaces})
props["olm.replaces"] = string(replacesValue)
replacesValue, _ := json.Marshal(entities.ChannelEntry{
Name: b.Name,
Replaces: b.Replaces,
})
props[entities.PropertyBundleChannelEntry] = string(replacesValue)
entity := input.Entity{
ID: deppy.IdentifierFromString(fmt.Sprintf("%s%s%s", bundle.Name, bundle.Spec.Package, ch.Name)),
Properties: props,
Expand Down
4 changes: 2 additions & 2 deletions internal/resolution/util/predicates/predicates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ func WithBundleImage(bundleImage string) input.Predicate {
func Replaces(bundleID string) input.Predicate {
return func(entity *input.Entity) bool {
bundleEntity := olmentity.NewBundleEntity(entity)
replaces, err := bundleEntity.Replaces()
replaces, err := bundleEntity.BundleChannelEntry()
if err != nil {
return false
}
return replaces == bundleID
return replaces.Replaces == bundleID
}
}
4 changes: 2 additions & 2 deletions internal/resolution/util/predicates/predicates_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ var _ = Describe("Predicates", func() {
Describe("Replaces", func() {
It("should return true when the entity provides the replaces property", func() {
entity := input.NewEntity("test", map[string]string{
"olm.replaces": `{"replaces": "test.v0.2.0"}`,
"olm.bundle.channelEntry": `{"replaces": "test.v0.2.0"}`,
})
Expect(predicates.Replaces("test.v0.2.0")(entity)).To(BeTrue())
Expect(predicates.Replaces("test.v0.1.0")(entity)).To(BeFalse())
Expand All @@ -118,7 +118,7 @@ var _ = Describe("Predicates", func() {
})
It("should return false when the replace value is not a valid json", func() {
entity := input.NewEntity("test", map[string]string{
"olm.replaces": `{"replaces"}`,
"olm.bundle.channelEntry": `{"replaces"}`,
})
Expect(predicates.Replaces("test.v0.2.0")(entity)).To(BeFalse())
})
Expand Down
6 changes: 4 additions & 2 deletions internal/resolution/variables/installed_package.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
olmentity "github.com/operator-framework/operator-controller/internal/resolution/entities"
)

var _ deppy.Variable = &InstalledPackageVariable{}

type InstalledPackageVariable struct {
*input.SimpleVariable
bundleEntities []*olmentity.BundleEntity
Expand All @@ -19,8 +21,8 @@ func (r *InstalledPackageVariable) BundleEntities() []*olmentity.BundleEntity {
return r.bundleEntities
}

func NewInstalledPackageVariable(bundleImage string, bundleEntities []*olmentity.BundleEntity) *InstalledPackageVariable {
id := deppy.IdentifierFromString(fmt.Sprintf("installed package %s", bundleImage))
func NewInstalledPackageVariable(packageName string, bundleEntities []*olmentity.BundleEntity) *InstalledPackageVariable {
id := deppy.IdentifierFromString(fmt.Sprintf("installed package %s", packageName))
entityIDs := make([]deppy.Identifier, 0, len(bundleEntities))
for _, bundle := range bundleEntities {
entityIDs = append(entityIDs, bundle.ID)
Expand Down
32 changes: 18 additions & 14 deletions internal/resolution/variablesources/bundle_deployment_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,23 +49,25 @@ func bundleDeployment(name, image string) *rukpakv1alpha1.BundleDeployment {

var BundleDeploymentTestEntityCache = map[deppy.Identifier]input.Entity{
"operatorhub/prometheus/0.37.0": *input.NewEntity("operatorhub/prometheus/0.37.0", map[string]string{
"olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"`,
"olm.channel": "{\"channelName\":\"beta\",\"priority\":0,\"replaces\":\"prometheusoperator.0.32.0\"}",
"olm.gvk": "[{\"group\":\"monitoring.coreos.com\",\"kind\":\"Alertmanager\",\"version\":\"v1\"}, {\"group\":\"monitoring.coreos.com\",\"kind\":\"Prometheus\",\"version\":\"v1\"}]",
"olm.package": "{\"packageName\":\"prometheus\",\"version\":\"0.37.0\"}",
"olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:3e281e587de3d03011440685fc4fb782672beab044c1ebadc42788ce05a21c35"`,
"olm.bundle.channelEntry": "{\"name\":\"prometheus.0.37.0\"}",
"olm.channel": "{\"channelName\":\"beta\",\"priority\":0,\"replaces\":\"prometheusoperator.0.32.0\"}",
"olm.gvk": "[{\"group\":\"monitoring.coreos.com\",\"kind\":\"Alertmanager\",\"version\":\"v1\"}, {\"group\":\"monitoring.coreos.com\",\"kind\":\"Prometheus\",\"version\":\"v1\"}]",
"olm.package": "{\"packageName\":\"prometheus\",\"version\":\"0.37.0\"}",
}),
"operatorhub/prometheus/0.47.0": *input.NewEntity("operatorhub/prometheus/0.47.0", map[string]string{
"olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed"`,
"olm.channel": "{\"channelName\":\"beta\",\"priority\":0,\"replaces\":\"prometheusoperator.0.37.0\"}",
"olm.gvk": "[{\"group\":\"monitoring.coreos.com\",\"kind\":\"Alertmanager\",\"version\":\"v1\"}, {\"group\":\"monitoring.coreos.com\",\"kind\":\"Prometheus\",\"version\":\"v1alpha1\"}]",
"olm.package": "{\"packageName\":\"prometheus\",\"version\":\"0.47.0\"}",
"olm.replaces": "{\"replaces\":\"prometheus.0.37.0\"}",
"olm.bundle.path": `"quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed"`,
"olm.bundle.channelEntry": "{\"replaces\":\"prometheus.0.37.0\", \"name\":\"prometheus.0.47.0\"}",
"olm.channel": "{\"channelName\":\"beta\",\"priority\":0,\"replaces\":\"prometheusoperator.0.37.0\"}",
"olm.gvk": "[{\"group\":\"monitoring.coreos.com\",\"kind\":\"Alertmanager\",\"version\":\"v1\"}, {\"group\":\"monitoring.coreos.com\",\"kind\":\"Prometheus\",\"version\":\"v1alpha1\"}]",
"olm.package": "{\"packageName\":\"prometheus\",\"version\":\"0.47.0\"}",
}),
"operatorhub/packageA/2.0.0": *input.NewEntity("operatorhub/packageA/2.0.0", map[string]string{
"olm.bundle.path": `"foo.io/packageA/packageA:v2.0.0"`,
"olm.channel": "{\"channelName\":\"stable\",\"priority\":0}",
"olm.gvk": "[{\"group\":\"foo.io\",\"kind\":\"Foo\",\"version\":\"v1\"}]",
"olm.package": "{\"packageName\":\"packageA\",\"version\":\"2.0.0\"}",
"olm.bundle.path": `"foo.io/packageA/packageA:v2.0.0"`,
"olm.bundle.channelEntry": "{\"name\":\"packageA.2.0.0\"}",
"olm.channel": "{\"channelName\":\"stable\",\"priority\":0}",
"olm.gvk": "[{\"group\":\"foo.io\",\"kind\":\"Foo\",\"version\":\"v1\"}]",
"olm.package": "{\"packageName\":\"packageA\",\"version\":\"2.0.0\"}",
}),
}

Expand All @@ -92,7 +94,9 @@ var _ = Describe("BundleDeploymentVariableSource", func() {
}
return out
}, Equal(map[deppy.Identifier]int{
deppy.IdentifierFromString("installed package prometheus.v0.37.0"): 1,
// Underlying `InstalledPackageVariableSource` returns current installed package
// as a possible upgrade edge
deppy.IdentifierFromString("installed package prometheus"): 2,
})))
})
It("should return an error if the bundleDeployment image doesn't match any operator resource", func() {
Expand Down
9 changes: 5 additions & 4 deletions internal/resolution/variablesources/installed_package.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,13 @@ func (r *InstalledPackageVariableSource) GetVariables(ctx context.Context, entit
if err != nil {
return nil, err
}
version, err := installedBundle.Version()

channelEntry, err := installedBundle.BundleChannelEntry()
if err != nil {
return nil, err
}
bundleID := fmt.Sprintf("%s.v%s", packageName, version.String())
resultSet, err = entitySource.Filter(ctx, predicates.Replaces(bundleID))

resultSet, err = entitySource.Filter(ctx, predicates.Replaces(channelEntry.Name))
if err != nil {
return nil, err
}
Expand All @@ -68,7 +69,7 @@ func (r *InstalledPackageVariableSource) GetVariables(ctx context.Context, entit
// you can always upgrade to yourself, i.e. not upgrade
upgradeEdges = append(upgradeEdges, installedBundle)
return []deppy.Variable{
variables.NewInstalledPackageVariable(bundleID, upgradeEdges),
variables.NewInstalledPackageVariable(packageName, upgradeEdges),
}, nil
}

Expand Down
41 changes: 21 additions & 20 deletions internal/resolution/variablesources/installed_package_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,33 +29,34 @@ var _ = Describe("InstalledPackageVariableSource", func() {

mockEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{
"test-package.v1.0.0": *input.NewEntity("test-package.v1.0.0test-packagestable", map[string]string{
property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`,
olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v1.0.0"`,
property.TypeChannel: `{"channelName":"stable","priority":0}`,
property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`,
property.TypeChannel: `{"channelName":"stable","priority":0}`,
olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v1.0.0"`,
olmentity.PropertyBundleChannelEntry: `{"name": "test-package.v1.0.0"}`,
}),
"test-package.v3.0.0": *input.NewEntity("test-package.v3.0.0test-packagestable", map[string]string{
property.TypePackage: `{"packageName": "test-package", "version": "3.0.0"}`,
property.TypeChannel: `{"channelName":"stable","priority":0}`,
olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v3.0.0"`,
"olm.replaces": `{"replaces": "test-package.v2.0.0"}`,
property.TypePackage: `{"packageName": "test-package", "version": "3.0.0"}`,
property.TypeChannel: `{"channelName":"stable","priority":0}`,
olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v3.0.0"`,
olmentity.PropertyBundleChannelEntry: `{"name": "test-package.v3.0.0","replaces": "test-package.v2.0.0"}`,
}),
"test-package.v2.0.0": *input.NewEntity("test-package.v2.0.0test-packagestable", map[string]string{
property.TypePackage: `{"packageName": "test-package", "version": "2.0.0"}`,
property.TypeChannel: `{"channelName":"stable","priority":0}`,
olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v2.0.0"`,
"olm.replaces": `{"replaces": "test-package.v1.0.0"}`,
property.TypePackage: `{"packageName": "test-package", "version": "2.0.0"}`,
property.TypeChannel: `{"channelName":"stable","priority":0}`,
olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v2.0.0"`,
olmentity.PropertyBundleChannelEntry: `{"name": "test-package.v2.0.0","replaces": "test-package.v1.0.0"}`,
}),
"test-package.4.0.0": *input.NewEntity("test-package.v4.0.0test-packagestable", map[string]string{
property.TypePackage: `{"packageName": "test-package", "version": "4.0.0"}`,
property.TypeChannel: `{"channelName":"stable","priority":0}`,
olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v4.0.0"`,
"olm.replaces": `{"replaces": "test-package.v3.0.0"}`,
property.TypePackage: `{"packageName": "test-package", "version": "4.0.0"}`,
property.TypeChannel: `{"channelName":"stable","priority":0}`,
olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v4.0.0"`,
olmentity.PropertyBundleChannelEntry: `{"name": "test-package.v4.0.0","replaces": "test-package.v3.0.0"}`,
}),
"test-package.5.0.0": *input.NewEntity("test-package.v5.0.0test-packagestable", map[string]string{
property.TypePackage: `{"packageName": "test-package", "version": "5-00"}`,
property.TypeChannel: `{"channelName":"stable","priority":0}`,
olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v5.0.0"`,
"olm.replaces": `{"replaces": "test-package.v4.0.0"}`,
property.TypePackage: `{"packageName": "test-package", "version": "5-00"}`,
property.TypeChannel: `{"channelName":"stable","priority":0}`,
olmentity.PropertyBundlePath: `"registry.io/repo/test-package@v5.0.0"`,
olmentity.PropertyBundleChannelEntry: `{"name": "test-package.v5.0.0","replaces": "test-package.v4.0.0"}`,
}),
})
})
Expand All @@ -66,7 +67,7 @@ var _ = Describe("InstalledPackageVariableSource", func() {
Expect(variables).To(HaveLen(1))
reqPackageVar, ok := variables[0].(*olmvariables.InstalledPackageVariable)
Expect(ok).To(BeTrue())
Expect(reqPackageVar.Identifier()).To(Equal(deppy.IdentifierFromString("installed package test-package.v2.0.0")))
Expect(reqPackageVar.Identifier()).To(Equal(deppy.IdentifierFromString("installed package test-package")))

// ensure bundle entities are in version order (high to low)
Expect(reqPackageVar.BundleEntities()[0].ID).To(Equal(deppy.IdentifierFromString("test-package.v3.0.0test-packagestable")))
Expand Down
Loading

0 comments on commit 28da6bd

Please sign in to comment.