-
Notifications
You must be signed in to change notification settings - Fork 62
Implement upgrade edges support #275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,3 +10,4 @@ metadata: | |
name: operator-sample | ||
spec: | ||
packageName: argocd-operator | ||
version: 0.6.0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -205,28 +205,24 @@ var _ = Describe("BundleEntity", func() { | |
}) | ||
}) | ||
|
||
Describe("ChannelProperties", func() { | ||
Describe("Channel", func() { | ||
It("should return the bundle channel properties if present", func() { | ||
entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ | ||
"olm.channel": `{"channelName":"beta","priority":0, "replaces": "bundle.v1.0.0", "skips": ["bundle.v0.9.0", "bundle.v0.9.6"], "skipRange": ">=0.9.0 <=0.9.6"}`, | ||
}) | ||
bundleEntity := olmentity.NewBundleEntity(entity) | ||
channelProperties, err := bundleEntity.ChannelProperties() | ||
channelProperties, err := bundleEntity.Channel() | ||
Expect(err).ToNot(HaveOccurred()) | ||
Expect(*channelProperties).To(Equal(olmentity.ChannelProperties{ | ||
Channel: property.Channel{ | ||
ChannelName: "beta", | ||
Priority: 0, | ||
}, | ||
Replaces: "bundle.v1.0.0", | ||
Skips: []string{"bundle.v0.9.0", "bundle.v0.9.6"}, | ||
SkipRange: ">=0.9.0 <=0.9.6", | ||
})) | ||
Expect(*channelProperties).To(Equal(property.Channel{ | ||
ChannelName: "beta", | ||
Priority: 0, | ||
}, | ||
)) | ||
}) | ||
It("should return an error if the property is not found", func() { | ||
entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{}) | ||
bundleEntity := olmentity.NewBundleEntity(entity) | ||
channelProperties, err := bundleEntity.ChannelProperties() | ||
channelProperties, err := bundleEntity.Channel() | ||
Expect(channelProperties).To(BeNil()) | ||
Expect(err.Error()).To(Equal("error determining bundle channel properties for entity 'operatorhub/prometheus/0.14.0': required property 'olm.channel' not found")) | ||
}) | ||
|
@@ -235,7 +231,7 @@ var _ = Describe("BundleEntity", func() { | |
"olm.channel": "badChannelPropertiesStructure", | ||
}) | ||
bundleEntity := olmentity.NewBundleEntity(entity) | ||
channelProperties, err := bundleEntity.ChannelProperties() | ||
channelProperties, err := bundleEntity.Channel() | ||
Expect(channelProperties).To(BeNil()) | ||
Expect(err.Error()).To(Equal("error determining bundle channel properties for entity 'operatorhub/prometheus/0.14.0': property 'olm.channel' ('badChannelPropertiesStructure') could not be parsed: invalid character 'b' looking for beginning of value")) | ||
}) | ||
|
@@ -269,6 +265,27 @@ var _ = Describe("BundleEntity", func() { | |
}) | ||
}) | ||
|
||
Describe("Replaces", func() { | ||
It("should return the replaces property if present", func() { | ||
entity := input.NewEntity("test", map[string]string{ | ||
"olm.replaces": `{"replaces": "test.v0.2.0"}`, | ||
}) | ||
bundleEntity := olmentity.NewBundleEntity(entity) | ||
replaces, err := bundleEntity.Replaces() | ||
Expect(err).ToNot(HaveOccurred()) | ||
Expect(replaces).To(Equal("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()) | ||
}) | ||
}) | ||
|
||
Describe("MediaType", func() { | ||
It("should return the bundle mediatype property if present", func() { | ||
entity := input.NewEntity("operatorhub/prometheus/0.14.0", map[string]string{ | ||
|
@@ -296,4 +313,46 @@ var _ = Describe("BundleEntity", func() { | |
Expect(err.Error()).To(Equal("error determining bundle mediatype for entity 'operatorhub/prometheus/0.14.0': property 'olm.bundle.mediatype' ('badtype') could not be parsed: invalid character 'b' looking for beginning of value")) | ||
}) | ||
}) | ||
|
||
// Increase test coverage | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: I would remove this or add a TODO if this is meant to be done in the future, non-blocking though. |
||
Describe("GVKRequired properties", func() { | ||
It("should return the GVKRequired properties", func() { | ||
gvk := olmentity.GVKRequired{ | ||
Group: "foo.io", | ||
Kind: "Foo", | ||
Version: "v1", | ||
} | ||
Expect(gvk.AsGVK().Version).To(Equal("v1")) | ||
Expect(gvk.AsGVK().Group).To(Equal("foo.io")) | ||
Expect(gvk.AsGVK().Kind).To(Equal("Foo")) | ||
}) | ||
It("should return the GVKRequired properties as a string", func() { | ||
gvk := olmentity.GVKRequired{ | ||
Group: "foo.io", | ||
Kind: "Foo", | ||
Version: "v1", | ||
} | ||
Expect(gvk.String()).To(Equal(`group:"foo.io" version:"v1" kind:"Foo"`)) | ||
}) | ||
}) | ||
Describe("GVK properties", func() { | ||
It("should return the gvk properties", func() { | ||
gvk := olmentity.GVK{ | ||
Group: "foo.io", | ||
Kind: "Foo", | ||
Version: "v1", | ||
} | ||
Expect(gvk.Version).To(Equal("v1")) | ||
Expect(gvk.Group).To(Equal("foo.io")) | ||
Expect(gvk.Kind).To(Equal("Foo")) | ||
}) | ||
It("should return the gvk properties as a string", func() { | ||
gvk := olmentity.GVK{ | ||
Group: "foo.io", | ||
Kind: "Foo", | ||
Version: "v1", | ||
} | ||
Expect(gvk.String()).To(Equal(`group:"foo.io" version:"v1" kind:"Foo"`)) | ||
}) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,6 +71,10 @@ 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) | ||
|
@@ -106,6 +110,9 @@ 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we address this todo? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know the following suggestion is a bigger refactor, and I'm not suggesting we do it now, but I think we should refactor things such that there are:
Bundle entities would no longer carry upgrade edge or channel membership information. Rather, channel variables would incorporate bundle membership and upgrade edges. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO, something like my suggestion should happen almost immediately after this PR lands so that we set ourselves up for a more extensible future where different channel implementations can exist and we don't have to keep adding synthetic properties to bundles (which might have undesirable effects if/when we support more arbitrary contraint definitions in the future) |
||
replacesValue, _ := json.Marshal(replacesProperty{Replaces: b.Replaces}) | ||
props["olm.replaces"] = string(replacesValue) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we move this to a constant? (olm.replaces) |
||
entity := input.Entity{ | ||
ID: deppy.IdentifierFromString(fmt.Sprintf("%s%s%s", bundle.Name, bundle.Spec.Package, ch.Name)), | ||
Properties: props, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!