Skip to content

✨ Set up right watches and all labels to postrenderer #763

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

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

varshaprasad96
Copy link
Member

I don't think there is need to list helm releases, to find metadata. The helm install code, in the getReleaseState method does comparison of the templated and existing chart and finds if changes are needed.

Additionally, based on the documentation, helm by default creates and manages secrets for each release, and if we want pass any additional sensitive data to the chart, we need to do so while templating. Which means, we can use the default release which the helm creates and store bundle version and name through annotations in the chart.

Description

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@varshaprasad96 varshaprasad96 requested a review from a team as a code owner April 17, 2024 23:46
Comment on lines +4 to +9
CoreOwnerKindKey = "core.clusterextension.io/owner-kind"
CoreOwnerNameKey = "core.clusterextension.io/owner-name"
ResolvedBundleCatalogID = "core.clusterextension.io/catalog-id"
ResolvedbundlePackageName = "core.clusterextension.io/package-name"
ResolvedbundleName = "core.clusterextension.io/bundle-name"
ResolvedbundleVersion = "core.clusterextension.io/bundle-version"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe we "own" the clusterextension.io domain, but this is a PoC...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 these should definitely be updated before 1.0.0

I'd suggest olm.operatorframework.io/*

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also a few nits on naming:

  1. CamelCase words in the identifier (e.g. Resolvedbundle should be ResolvedBundle)
  2. Consistently use Key suffix to all of the identifiers to clarify that these are to be used for label/annotation keys.
  3. Drop Core prefix.

Overall, align the identifier name with the value of the constant. e.g.

  • "core.clusterextension.io/catalog-id" would map to CatalogIDKey
  • "core.clusterextension.io/package-name" would map to PackageNameKey
  • etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a PoC branch, so it's not going into main as-is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -460,6 +471,58 @@ func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager) error {
return nil
}

func mapOwneeToOwnerHandler(cl client.Client, log logr.Logger, owner client.Object) crhandler.EventHandler {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took me a second to "get" this name. Is "ownee" even a word? :)

@tmshort tmshort merged commit 3ad299f into operator-framework:helm-poc Apr 18, 2024
Comment on lines +236 to +238
util.ResolvedbundleName: bundle.Name,
util.ResolvedbundlePackageName: bundle.Package,
util.ResolvedbundleVersion: bundleVersion.String(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No catalogID here? Can we drop that constant?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bentito pushed a commit to bentito/operator-controller that referenced this pull request May 3, 2024
tmshort added a commit that referenced this pull request May 3, 2024
plug in resolver

Deal with removal of HigherBundleVersion

Removed in e079129

Signed-off-by: Todd Short <[email protected]>

:sparkles: Starting Helm POC - pull in some rukpak (#756)

* Fix suite_test.go

Signed-off-by: Todd Short <[email protected]>

* Copy over some rukpak code and replace BundleDeployment

Replace BundleDeployment in the Unpack APIs with a combination of
BundleSource and ClusterExtension.

It builds...

Signed-off-by: Todd Short <[email protected]>

* fixup! Copy over some rukpak code and replace BundleDeployment

---------

Signed-off-by: Todd Short <[email protected]>

Plugin unpacker, add Handler (#757)

Signed-off-by: Varsha Prasad Narsing <[email protected]>
Co-authored-by: [email protected] <[email protected]>

A bit of cleanup (#761)

Signed-off-by: Todd Short <[email protected]>

:warning: Install the helm chart (#762)

* Install the helm chart

Signed-off-by: Todd Short <[email protected]>

* fixup! Install the helm chart

Signed-off-by: Todd Short <[email protected]>

---------

Signed-off-by: Todd Short <[email protected]>

Set up right watches and all labels to postrenderer (#763)

Co-authored-by: [email protected] <[email protected]>

:warning: Update owner keys (#765)

* Update owner keys

Signed-off-by: Todd Short <[email protected]>

* fixup! Update owner keys

Signed-off-by: Todd Short <[email protected]>

---------

Signed-off-by: Todd Short <[email protected]>

No more panics (#767)

Signed-off-by: Todd Short <[email protected]>

Add relevant RBAC to enable controller to watch resources (#776)

Co-authored-by: [email protected] <[email protected]>

Changes required for ClusterExtension to install an operator  (#789)

* Add relevant RBAC to enable controller to watch resources

* Debugging iteration - one

* ClusterExtension installing the operator - working

---------

Co-authored-by: [email protected] <[email protected]>

Fix some lints (#793)

Signed-off-by: Todd Short <[email protected]>

:sparkles: Getting cluster extension running (#795)

* Getting cluster extension running

* Specify namespace to create secret

Set resolved and installed versions (#806)

Remove install references to rukpak (#805)

Signed-off-by: Todd Short <[email protected]>

Consolidate error message generation (#807)

Signed-off-by: Todd Short <[email protected]>

Add make kind-redeploy (#808)

Signed-off-by: Todd Short <[email protected]>

Use rukpak as a library (#821)

Signed-off-by: Todd Short <[email protected]>

Improve caching and fix constant reconciles (#825)

Improve performance by caching objects that only have ClusterExtension
as owners.

Signed-off-by: Varsha Prasad Narsing <[email protected]>

Fix linter (#826)

Signed-off-by: Varsha Prasad Narsing <[email protected]>

Move helm-operator-plugin (#828)

Signed-off-by: Todd Short <[email protected]>

Really fix linter (#833)

Signed-off-by: Todd Short <[email protected]>
@tmshort tmshort mentioned this pull request May 3, 2024
4 tasks
tmshort added a commit that referenced this pull request May 6, 2024
plug in resolver

Deal with removal of HigherBundleVersion

Removed in e079129

Signed-off-by: Todd Short <[email protected]>

:sparkles: Starting Helm POC - pull in some rukpak (#756)

* Fix suite_test.go

Signed-off-by: Todd Short <[email protected]>

* Copy over some rukpak code and replace BundleDeployment

Replace BundleDeployment in the Unpack APIs with a combination of
BundleSource and ClusterExtension.

It builds...

Signed-off-by: Todd Short <[email protected]>

* fixup! Copy over some rukpak code and replace BundleDeployment

---------

Signed-off-by: Todd Short <[email protected]>

Plugin unpacker, add Handler (#757)

Signed-off-by: Varsha Prasad Narsing <[email protected]>
Co-authored-by: [email protected] <[email protected]>

A bit of cleanup (#761)

Signed-off-by: Todd Short <[email protected]>

:warning: Install the helm chart (#762)

* Install the helm chart

Signed-off-by: Todd Short <[email protected]>

* fixup! Install the helm chart

Signed-off-by: Todd Short <[email protected]>

---------

Signed-off-by: Todd Short <[email protected]>

Set up right watches and all labels to postrenderer (#763)

Co-authored-by: [email protected] <[email protected]>

:warning: Update owner keys (#765)

* Update owner keys

Signed-off-by: Todd Short <[email protected]>

* fixup! Update owner keys

Signed-off-by: Todd Short <[email protected]>

---------

Signed-off-by: Todd Short <[email protected]>

No more panics (#767)

Signed-off-by: Todd Short <[email protected]>

Add relevant RBAC to enable controller to watch resources (#776)

Co-authored-by: [email protected] <[email protected]>

Changes required for ClusterExtension to install an operator  (#789)

* Add relevant RBAC to enable controller to watch resources

* Debugging iteration - one

* ClusterExtension installing the operator - working

---------

Co-authored-by: [email protected] <[email protected]>

Fix some lints (#793)

Signed-off-by: Todd Short <[email protected]>

:sparkles: Getting cluster extension running (#795)

* Getting cluster extension running

* Specify namespace to create secret

Set resolved and installed versions (#806)

Remove install references to rukpak (#805)

Signed-off-by: Todd Short <[email protected]>

Consolidate error message generation (#807)

Signed-off-by: Todd Short <[email protected]>

Add make kind-redeploy (#808)

Signed-off-by: Todd Short <[email protected]>

Use rukpak as a library (#821)

Signed-off-by: Todd Short <[email protected]>

Improve caching and fix constant reconciles (#825)

Improve performance by caching objects that only have ClusterExtension
as owners.

Signed-off-by: Varsha Prasad Narsing <[email protected]>

Fix linter (#826)

Signed-off-by: Varsha Prasad Narsing <[email protected]>

Move helm-operator-plugin (#828)

Signed-off-by: Todd Short <[email protected]>

Really fix linter (#833)

Signed-off-by: Todd Short <[email protected]>
varshaprasad96 pushed a commit to varshaprasad96/operator-controller that referenced this pull request May 10, 2024
plug in resolver

Deal with removal of HigherBundleVersion

Removed in e079129

Signed-off-by: Todd Short <[email protected]>

:sparkles: Starting Helm POC - pull in some rukpak (operator-framework#756)

* Fix suite_test.go

Signed-off-by: Todd Short <[email protected]>

* Copy over some rukpak code and replace BundleDeployment

Replace BundleDeployment in the Unpack APIs with a combination of
BundleSource and ClusterExtension.

It builds...

Signed-off-by: Todd Short <[email protected]>

* fixup! Copy over some rukpak code and replace BundleDeployment

---------

Signed-off-by: Todd Short <[email protected]>

Plugin unpacker, add Handler (operator-framework#757)

Signed-off-by: Varsha Prasad Narsing <[email protected]>
Co-authored-by: [email protected] <[email protected]>

A bit of cleanup (operator-framework#761)

Signed-off-by: Todd Short <[email protected]>

:warning: Install the helm chart (operator-framework#762)

* Install the helm chart

Signed-off-by: Todd Short <[email protected]>

* fixup! Install the helm chart

Signed-off-by: Todd Short <[email protected]>

---------

Signed-off-by: Todd Short <[email protected]>

Set up right watches and all labels to postrenderer (operator-framework#763)

Co-authored-by: [email protected] <[email protected]>

:warning: Update owner keys (operator-framework#765)

* Update owner keys

Signed-off-by: Todd Short <[email protected]>

* fixup! Update owner keys

Signed-off-by: Todd Short <[email protected]>

---------

Signed-off-by: Todd Short <[email protected]>

No more panics (operator-framework#767)

Signed-off-by: Todd Short <[email protected]>

Add relevant RBAC to enable controller to watch resources (operator-framework#776)

Co-authored-by: [email protected] <[email protected]>

Changes required for ClusterExtension to install an operator  (operator-framework#789)

* Add relevant RBAC to enable controller to watch resources

* Debugging iteration - one

* ClusterExtension installing the operator - working

---------

Co-authored-by: [email protected] <[email protected]>

Fix some lints (operator-framework#793)

Signed-off-by: Todd Short <[email protected]>

:sparkles: Getting cluster extension running (operator-framework#795)

* Getting cluster extension running

* Specify namespace to create secret

Set resolved and installed versions (operator-framework#806)

Remove install references to rukpak (operator-framework#805)

Signed-off-by: Todd Short <[email protected]>

Consolidate error message generation (operator-framework#807)

Signed-off-by: Todd Short <[email protected]>

Add make kind-redeploy (operator-framework#808)

Signed-off-by: Todd Short <[email protected]>

Use rukpak as a library (operator-framework#821)

Signed-off-by: Todd Short <[email protected]>

Improve caching and fix constant reconciles (operator-framework#825)

Improve performance by caching objects that only have ClusterExtension
as owners.

Signed-off-by: Varsha Prasad Narsing <[email protected]>

Fix linter (operator-framework#826)

Signed-off-by: Varsha Prasad Narsing <[email protected]>

Move helm-operator-plugin (operator-framework#828)

Signed-off-by: Todd Short <[email protected]>

Really fix linter (operator-framework#833)

Signed-off-by: Todd Short <[email protected]>
tmshort added a commit that referenced this pull request May 11, 2024
plug in resolver

Deal with removal of HigherBundleVersion

Removed in e079129

Signed-off-by: Todd Short <[email protected]>

:sparkles: Starting Helm POC - pull in some rukpak (#756)

* Fix suite_test.go

Signed-off-by: Todd Short <[email protected]>

* Copy over some rukpak code and replace BundleDeployment

Replace BundleDeployment in the Unpack APIs with a combination of
BundleSource and ClusterExtension.

It builds...

Signed-off-by: Todd Short <[email protected]>

* fixup! Copy over some rukpak code and replace BundleDeployment

---------

Signed-off-by: Todd Short <[email protected]>

Plugin unpacker, add Handler (#757)

Signed-off-by: Varsha Prasad Narsing <[email protected]>
Co-authored-by: [email protected] <[email protected]>

A bit of cleanup (#761)

Signed-off-by: Todd Short <[email protected]>

:warning: Install the helm chart (#762)

* Install the helm chart

Signed-off-by: Todd Short <[email protected]>

* fixup! Install the helm chart

Signed-off-by: Todd Short <[email protected]>

---------

Signed-off-by: Todd Short <[email protected]>

Set up right watches and all labels to postrenderer (#763)

Co-authored-by: [email protected] <[email protected]>

:warning: Update owner keys (#765)

* Update owner keys

Signed-off-by: Todd Short <[email protected]>

* fixup! Update owner keys

Signed-off-by: Todd Short <[email protected]>

---------

Signed-off-by: Todd Short <[email protected]>

No more panics (#767)

Signed-off-by: Todd Short <[email protected]>

Add relevant RBAC to enable controller to watch resources (#776)

Co-authored-by: [email protected] <[email protected]>

Changes required for ClusterExtension to install an operator  (#789)

* Add relevant RBAC to enable controller to watch resources

* Debugging iteration - one

* ClusterExtension installing the operator - working

---------

Co-authored-by: [email protected] <[email protected]>

Fix some lints (#793)

Signed-off-by: Todd Short <[email protected]>

:sparkles: Getting cluster extension running (#795)

* Getting cluster extension running

* Specify namespace to create secret

Set resolved and installed versions (#806)

Remove install references to rukpak (#805)

Signed-off-by: Todd Short <[email protected]>

Consolidate error message generation (#807)

Signed-off-by: Todd Short <[email protected]>

Add make kind-redeploy (#808)

Signed-off-by: Todd Short <[email protected]>

Use rukpak as a library (#821)

Signed-off-by: Todd Short <[email protected]>

Improve caching and fix constant reconciles (#825)

Improve performance by caching objects that only have ClusterExtension
as owners.

Signed-off-by: Varsha Prasad Narsing <[email protected]>

Fix linter (#826)

Signed-off-by: Varsha Prasad Narsing <[email protected]>

Move helm-operator-plugin (#828)

Signed-off-by: Todd Short <[email protected]>

Really fix linter (#833)

Signed-off-by: Todd Short <[email protected]>
bentito pushed a commit that referenced this pull request May 14, 2024
plug in resolver

Deal with removal of HigherBundleVersion

Removed in e079129

Signed-off-by: Todd Short <[email protected]>

:sparkles: Starting Helm POC - pull in some rukpak (#756)

* Fix suite_test.go

Signed-off-by: Todd Short <[email protected]>

* Copy over some rukpak code and replace BundleDeployment

Replace BundleDeployment in the Unpack APIs with a combination of
BundleSource and ClusterExtension.

It builds...

Signed-off-by: Todd Short <[email protected]>

* fixup! Copy over some rukpak code and replace BundleDeployment

---------

Signed-off-by: Todd Short <[email protected]>

Plugin unpacker, add Handler (#757)

Signed-off-by: Varsha Prasad Narsing <[email protected]>
Co-authored-by: [email protected] <[email protected]>

A bit of cleanup (#761)

Signed-off-by: Todd Short <[email protected]>

:warning: Install the helm chart (#762)

* Install the helm chart

Signed-off-by: Todd Short <[email protected]>

* fixup! Install the helm chart

Signed-off-by: Todd Short <[email protected]>

---------

Signed-off-by: Todd Short <[email protected]>

Set up right watches and all labels to postrenderer (#763)

Co-authored-by: [email protected] <[email protected]>

:warning: Update owner keys (#765)

* Update owner keys

Signed-off-by: Todd Short <[email protected]>

* fixup! Update owner keys

Signed-off-by: Todd Short <[email protected]>

---------

Signed-off-by: Todd Short <[email protected]>

No more panics (#767)

Signed-off-by: Todd Short <[email protected]>

Add relevant RBAC to enable controller to watch resources (#776)

Co-authored-by: [email protected] <[email protected]>

Changes required for ClusterExtension to install an operator  (#789)

* Add relevant RBAC to enable controller to watch resources

* Debugging iteration - one

* ClusterExtension installing the operator - working

---------

Co-authored-by: [email protected] <[email protected]>

Fix some lints (#793)

Signed-off-by: Todd Short <[email protected]>

:sparkles: Getting cluster extension running (#795)

* Getting cluster extension running

* Specify namespace to create secret

Set resolved and installed versions (#806)

Remove install references to rukpak (#805)

Signed-off-by: Todd Short <[email protected]>

Consolidate error message generation (#807)

Signed-off-by: Todd Short <[email protected]>

Add make kind-redeploy (#808)

Signed-off-by: Todd Short <[email protected]>

Use rukpak as a library (#821)

Signed-off-by: Todd Short <[email protected]>

Improve caching and fix constant reconciles (#825)

Improve performance by caching objects that only have ClusterExtension
as owners.

Signed-off-by: Varsha Prasad Narsing <[email protected]>

Fix linter (#826)

Signed-off-by: Varsha Prasad Narsing <[email protected]>

Move helm-operator-plugin (#828)

Signed-off-by: Todd Short <[email protected]>

Really fix linter (#833)

Signed-off-by: Todd Short <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants