Skip to content
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

feat(consoleplugin): operator installs and configures ConsolePlugin #1023

Merged
merged 11 commits into from
Jan 28, 2025

Conversation

andrewazores
Copy link
Member

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits: git commit -S -m "YOUR_COMMIT_MESSAGE"

Fixes: #
Related to cryostatio/cryostat-openshift-console-plugin#3

Description of the change:

This change adds allows the users to provide...

Motivation for the change:

This change is helpful because users may want to...

How to manually test:

  1. Insert steps here...
  2. ...

@andrewazores andrewazores added feat New feature or request safe-to-test labels Jan 21, 2025
@andrewazores andrewazores force-pushed the console-plugin branch 2 times, most recently from b06afe1 to 9ae5035 Compare January 24, 2025 17:54
@ebaron ebaron changed the title feat(consoleplugin): add scrubbed Helm-rendered plugin resource templates feat(consoleplugin): operator installs and configures ConsolePlugin Jan 25, 2025
Copy link
Member Author

@andrewazores andrewazores left a comment

Choose a reason for hiding this comment

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

I think we can remove some more pieces here since we aren't using the patcher Job anymore.

@andrewazores
Copy link
Member Author

2025-01-27T18:53:00Z INFO setup detected OpenShift environment
2025-01-27T18:53:00Z INFO setup found cert-manager installation
&{{0xc00014a750 0xc000693ac0} {0xc00014a750 {}} {0xc000689740 0xc0004299f0} 0xc000280e00 0xc0004299f0 0xc000687e30 map[] false}
2025-01-27T18:53:00Z ERROR setup failed to install OpenShift Console Plugin {"error": "the cache is not started, can not read objects"}
main.main
/opt/app-root/src/internal/main.go:188
runtime.main
/usr/lib/golang/src/runtime/proc.go:271

It seems like this happens because the console plugin installation tries to use the Kubernetes client to create resources (like the ConsolePlugin CR), but since this is being done before mgr.Start() things aren't actually initialized yet. It looks like we can either drop down to using a "raw" HTTP client to install the plugin, or else the ConsolePlugin CR should be handled by a controller as part of the reconcile loop rather than as a one-shot operation before the manager starts?

@ebaron
Copy link
Member

ebaron commented Jan 27, 2025

2025-01-27T18:53:00Z INFO setup detected OpenShift environment
2025-01-27T18:53:00Z INFO setup found cert-manager installation
&{{0xc00014a750 0xc000693ac0} {0xc00014a750 {}} {0xc000689740 0xc0004299f0} 0xc000280e00 0xc0004299f0 0xc000687e30 map[] false}
2025-01-27T18:53:00Z ERROR setup failed to install OpenShift Console Plugin {"error": "the cache is not started, can not read objects"}
main.main
/opt/app-root/src/internal/main.go:188
runtime.main
/usr/lib/golang/src/runtime/proc.go:271

It seems like this happens because the console plugin installation tries to use the Kubernetes client to create resources (like the ConsolePlugin CR), but since this is being done before mgr.Start() things aren't actually initialized yet. It looks like we can either drop down to using a "raw" HTTP client to install the plugin, or else the ConsolePlugin CR should be handled by a controller as part of the reconcile loop rather than as a one-shot operation before the manager starts?

I'll try using the direct (non-cached) Kubernetes client and see if that works.

@ebaron ebaron marked this pull request as ready for review January 28, 2025 00:33
@ebaron
Copy link
Member

ebaron commented Jan 28, 2025

That last commit converts the plugin installer to a Runnable, which gets added to the manager in the same way controllers do. This ensures it doesn't interact with the API server until the manager's cache is started.

For the separate issue of OLM naming the ClusterRoleBinding's unpredictably, I found a shared set of OLM-managed labels between the operator deployment and the ClusterRoleBinding. I use this to find the ClusterRoleBindings belonging to our operator and then iterate until we find the one with the proper Service Account subject.

These two things seemed to work well when tested on OpenShift 4.17.

@ebaron
Copy link
Member

ebaron commented Jan 28, 2025

/build_test

Copy link

/build_test completed successfully ✅.
View Actions Run.

@andrewazores
Copy link
Member Author

GitHub won't let me approve this PR because I'm still technically its author 😁

Looks good and appears to work great. Maintainer override coming up.

@andrewazores andrewazores merged commit 031607e into cryostatio:main Jan 28, 2025
7 checks passed
@andrewazores andrewazores deleted the console-plugin branch January 28, 2025 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request safe-to-test
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants