Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 83
add support for handling plain+v0 bundle types#242
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
40022dbe8773ee4a9adbc040d8a58abcc14814cb19f0a02bcFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -156,9 +156,19 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha | ||
| op.Status.ResolvedBundleResource = bundleImage | ||
| setResolvedStatusConditionSuccess(&op.Status.Conditions, fmt.Sprintf("resolved to %q", bundleImage), op.GetGeneration()) | ||
| mediaType, err := bundleEntity.MediaType() | ||
| if err != nil { | ||
| setInstalledStatusConditionFailed(&op.Status.Conditions, err.Error(), op.GetGeneration()) | ||
| return ctrl.Result{}, err | ||
| } | ||
| bundleProvisioner, err := mapBundleMediaTypeToBundleProvisioner(mediaType) | ||
| if err != nil { | ||
| setInstalledStatusConditionFailed(&op.Status.Conditions, err.Error(), op.GetGeneration()) | ||
| return ctrl.Result{}, err | ||
| } | ||
| // Ensure a BundleDeployment exists with its bundle source from the bundle | ||
| // image we just looked up in the solution. | ||
| dep := r.generateExpectedBundleDeployment(*op, bundleImage) | ||
| dep := r.generateExpectedBundleDeployment(*op, bundleImage, bundleProvisioner) | ||
| if err := r.ensureBundleDeployment(ctx, dep); err != nil { | ||
| // originally Reason: operatorsv1alpha1.ReasonInstallationFailed | ||
| op.Status.InstalledBundleResource = "" | ||
| @@ -244,12 +254,13 @@ func (r *OperatorReconciler) getBundleEntityFromSolution(solution *solver.Soluti | ||
| return nil, fmt.Errorf("entity for package %q not found in solution", packageName) | ||
| } | ||
| func (r *OperatorReconciler) generateExpectedBundleDeployment(o operatorsv1alpha1.Operator, bundlePath string) *unstructured.Unstructured { | ||
| func (r *OperatorReconciler) generateExpectedBundleDeployment(o operatorsv1alpha1.Operator, bundlePath string, bundleProvisioner string) *unstructured.Unstructured { | ||
| // We use unstructured here to avoid problems of serializing default values when sending patches to the apiserver. | ||
| // If you use a typed object, any default values from that struct get serialized into the JSON patch, which could | ||
| // cause unrelated fields to be patched back to the default value even though that isn't the intention. Using an | ||
| // unstructured ensures that the patch contains only what is specified. Using unstructured like this is basically | ||
| // identical to "kubectl apply -f" | ||
| bd := &unstructured.Unstructured{Object: map[string]interface{}{ | ||
| "apiVersion": rukpakv1alpha1.GroupVersion.String(), | ||
| "kind": rukpakv1alpha1.BundleDeploymentKind, | ||
| @@ -261,8 +272,7 @@ func (r *OperatorReconciler) generateExpectedBundleDeployment(o operatorsv1alpha | ||
| "provisionerClassName": "core-rukpak-io-plain", | ||
| "template": map[string]interface{}{ | ||
| "spec": map[string]interface{}{ | ||
| // TODO: Don't assume registry provisioner | ||
| "provisionerClassName": "core-rukpak-io-registry", | ||
| "provisionerClassName": bundleProvisioner, | ||
| "source": map[string]interface{}{ | ||
| // TODO: Don't assume image type | ||
| "type": string(rukpakv1alpha1.SourceTypeImage), | ||
| @@ -363,6 +373,23 @@ func isBundleDepStale(bd *rukpakv1alpha1.BundleDeployment) bool { | ||
| return bd != nil && bd.Status.ObservedGeneration != bd.GetGeneration() | ||
| } | ||
| // 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) { | ||
joelanford marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| switch mediaType { | ||
| case entity.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 entity.MediaTypeRegistry, "": | ||
| return "core-rukpak-io-registry", nil | ||
everettraven marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| default: | ||
| return "", fmt.Errorf("unknown bundle mediatype: %s", mediaType) | ||
| } | ||
| } | ||
| // setResolvedStatusConditionSuccess sets the resolved status condition to success. | ||
| func setResolvedStatusConditionSuccess(conditions *[]metav1.Condition, message string, generation int64) { | ||
| apimeta.SetStatusCondition(conditions, metav1.Condition{ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -7,6 +7,7 @@ import ( | ||
| "github.com/operator-framework/deppy/pkg/deppy" | ||
| "github.com/operator-framework/deppy/pkg/deppy/input" | ||
| "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/entity" | ||
| "github.com/operator-framework/operator-registry/alpha/property" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| @@ -80,12 +81,16 @@ func getEntities(ctx context.Context, client client.Client) (input.EntityList, e | ||
| for _, bundle := range bundleMetadatas.Items { | ||
| props := map[string]string{} | ||
| // TODO: We should make sure all properties are forwarded | ||
| // through and avoid a lossy translation from FBC --> entity | ||
| for _, prop := range bundle.Spec.Properties { | ||
| switch prop.Type { | ||
| case property.TypePackage: | ||
| // this is already a json marshalled object, so it doesn't need to be marshalled | ||
| // like the other ones | ||
| props[property.TypePackage] = string(prop.Value) | ||
| case entity.PropertyBundleMediaType: | ||
| props[entity.PropertyBundleMediaType] = string(prop.Value) | ||
everettraven marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -12,6 +12,19 @@ import ( | ||
| const PropertyBundlePath = "olm.bundle.path" | ||
| // TODO: Is this the right place for these? | ||
| // ---- | ||
| const PropertyBundleMediaType = "olm.bundle.mediatype" | ||
| type MediaType string | ||
| const ( | ||
| MediaTypePlain = "plain+v0" | ||
| MediaTypeRegistry = "registry+v1" | ||
| ) | ||
everettraven marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // ---- | ||
| type ChannelProperties struct { | ||
| property.Channel | ||
| Replaces string `json:"replaces,omitempty"` | ||
| @@ -58,6 +71,7 @@ type BundleEntity struct { | ||
| channelProperties *ChannelProperties | ||
| semVersion *semver.Version | ||
| bundlePath string | ||
| mediaType string | ||
| mu sync.RWMutex | ||
| } | ||
| @@ -124,6 +138,27 @@ func (b *BundleEntity) BundlePath() (string, error) { | ||
| return b.bundlePath, nil | ||
| } | ||
| func (b *BundleEntity) MediaType() (string, error) { | ||
| if err := b.loadMediaType(); err != nil { | ||
| return "", err | ||
| } | ||
| return b.mediaType, nil | ||
| } | ||
| func (b *BundleEntity) loadMediaType() error { | ||
| b.mu.Lock() | ||
| defer b.mu.Unlock() | ||
| if b.mediaType == "" { | ||
| mediaType, err := loadFromEntity[string](b.Entity, PropertyBundleMediaType, optional) | ||
| if err != nil { | ||
| return fmt.Errorf("error determining bundle mediatype for entity '%s': %w", b.ID, err) | ||
| } | ||
| b.mediaType = mediaType | ||
| } | ||
| return nil | ||
| } | ||
| func (b *BundleEntity) loadPackage() error { | ||
| b.mu.Lock() | ||
| defer b.mu.Unlock() | ||
Uh oh!
There was an error while loading. Please reload this page.
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.
This isn't new code for this PR but ... why not use SSA?
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.
Looks like it does here:
operator-controller/internal/controllers/operator_controller.go
Line 324 in 22afecf
generateExpectedBundleDeploymentis generating anunstructured.Unstructuredto use for that patch request. I'd be curious to see if it is worth it to create applyconfigurations for the rukpak APIs to improve the creation of the patch information hereThere 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.
We updated the generators upstream so you should get those free with newer codegen tools. I do think the apply you linked will be SSA - and the defaults worries from this block should be obviated.
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.
Watching this closely :) kubernetes-sigs/controller-tools#818