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(discovery): pluggable discovery services API #1028

Merged
merged 10 commits into from
Aug 19, 2022

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Jul 28, 2022

@andrewazores andrewazores added the feat New feature or request label Jul 28, 2022
@andrewazores andrewazores linked an issue Jul 28, 2022 that may be closed by this pull request
4 tasks
@andrewazores andrewazores marked this pull request as draft July 28, 2022 19:01
@andrewazores andrewazores force-pushed the 939-story-pluggable-discovery-api branch 4 times, most recently from ee5f7ca to a4799d8 Compare August 2, 2022 17:47
@andrewazores andrewazores force-pushed the 939-story-pluggable-discovery-api branch 3 times, most recently from d4feb8a to 168cd9f Compare August 4, 2022 20:24
@andrewazores andrewazores force-pushed the 939-story-pluggable-discovery-api branch from 168cd9f to 862f627 Compare August 6, 2022 04:28
@andrewazores andrewazores force-pushed the 939-story-pluggable-discovery-api branch 2 times, most recently from 3ff6233 to 580d4a6 Compare August 8, 2022 20:08
@andrewazores andrewazores force-pushed the 939-story-pluggable-discovery-api branch 2 times, most recently from 80096dc to 1c995c9 Compare August 16, 2022 22:01
@andrewazores andrewazores marked this pull request as ready for review August 16, 2022 22:04
@andrewazores
Copy link
Member Author

Ready for review. This is just merging this long-lived upstream feature branch into upstream main. The component PRs of the feature have already been reviewed separately and merged into this branch.

andrewazores and others added 4 commits August 17, 2022 12:15
* Initial rough cut

Adds API endpoints for pluggable discovery.

Also introduces DiscoveryStorage which is either a stand-in for a
database ORM or an abstraction around a later database connection to
come.

BuiltInDiscovery is used as a bridge between existing PlatformClient
implementations and platform-specific discovery behaviours and the
DiscoveryStorage component. This allows existing PlatformClients to
continue performing discovery within the main Cryostat process, but
publish those results into the DiscoveryStorage when changes occur. API
endpoints that query for a list of targets or the discovery tree read
from DiscoveryStorage so that these calls do not cascade out into
further network calls (ex. for KubeApiPlatformClient).

* DiscoveryStorage implements PlatformClient

The new DiscoveryStorage replaces the previous platform detection
mechanism and provided PlatformClient, and the MergingPlatformClient.
The CustomTargetPlatformClient and the platform-detected appropriate
PlatformClient implementation are injected into a `Set<PlatformClient>`
which the BuiltInDiscovery consumes and uses to bridge updates into the
DiscoveryStorage. Since DiscoveryStorage itself implements the
PlatformClient interface, but is able to return results from storage
rather than re-querying the base platform, it is used as a replacement
injection in various existing places that injected the PlatformClient
instance previously (which would have always received the
MergingPlatformClient instance).

* fixup! Initial rough cut

* fixup! fixup! Initial rough cut

* Refactor and cleanup

* add discovery deregistration endpoint

* builtin discovery can be disabled

* add comment

* use UUID string for plugin registration ID, not numeric

* include placeholder auth token in registration response

* handle null children set as empty set

* internally-added callback URIs may be null

non-nullness is enforced on API endpoint. this allows internal cases like BuiltInDiscovery to register themselves through the discovery mechanism as an internal API client, without having to expose some meaningless callback URL since the Cryostat process is the "plugin" process

* experimental fixes

* don't send previous state back to plugin

* fix typos

* implement gson adapter for parsing subtrees provided by discovery plugins

* add quarkus-test sample using pluggable discovery

* builtin discovery completes startup promise

* discovery storage is also a verticle

* user basic auth

* fix image tag

* emit warnings on unexpected object members

clears spotbugs warning for missing 'default' in switch

* fixup webPort for quarkus-test-plugin
* feat(discovery): prepare to implement URL callback handling

persist to disk on shutdown and restore from disk on startup to emulate
database connection persistence. On startup, handle callback URLs
restored and check if discovery services are still known/present.

* don't persist or restore plugins without callbacks

API registration endpoint enforces callback URI presence, so only builtin discovery mechanisms can have null callbacks. These should not be persisted

* POST plugin callback URLs on startup to check presence

* run discovery plugin with restart policy

plugin will exit itself on startup if registration fails

* improve shutdown handling if verticles fail to start up

* refactor startup for cleanliness

* don't register self if builtin discovery disabled

* respond 400 if UUID invalid

* add handler unit tests, minor fixups

* use newer vertx-fib-demo that handles SIGTERM

* auto generate basic auth credentials file
* feat(db): add in-memory h2 database w/ Hibernate

replace discovery file-based storage with in-memory h2

* extract StorageModule

* extract AbstractDao

* make database parameters configurable

* format sql in logs

* use jsonb for subtrees

* use file-based h2 by default

* minor cleanup

* rough setup for postgres db and switching from h2 in-memory, h2 file, and postgres remote

* minor postgres fixes

* remove thread sleep hack for dependent startup

* don't fail startup if a discovery plugin is offline

* extract VerticleDeployer

* refactor

* only update schema in file mode, don't (re)create

* handle target discovery events async

* emit target discovery ws notifications async

* detach entity on save

* use correct exception type

* Revert "emit target discovery ws notifications async"

This reverts commit cd78257.

* fixup! extract VerticleDeployer

* use normalized form of env var names

* avoid addChildNode mutation

* cleanup and add tests

* update plugin sample app version

* fixup! cleanup and add tests

* avoid redundant null check

* remove useless file

* query logging can be disabled

* remove json file persistence from custom targets, update unit tests, add itest

* fixup! remove json file persistence from custom targets, update unit tests, add itest

* builtin discovery revamp

on startup, check if a realm is registered for each built-in discovery mechanism (platform client). register a plugin realm if not, then attach an update listener to the platform client to update that realm. delegate to the platform client to perform on-start load of current known state to replace existing persistent storage contents. Custom Targets overrides this and does not replace existing storage contents, so that the discovery database is the backing storage for custom targets

* configure postgres container to self-init on startup, no need for separate init command

* re-initialize custom targets from database on startup

allows custom targets to be deleted on subsequent runs
@andrewazores andrewazores force-pushed the 939-story-pluggable-discovery-api branch from 1c995c9 to 6b68364 Compare August 17, 2022 16:16
@andrewazores andrewazores force-pushed the 939-story-pluggable-discovery-api branch from c5ca710 to 55b7072 Compare August 17, 2022 20:58
Copy link
Member

@maxcao13 maxcao13 left a comment

Choose a reason for hiding this comment

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

Everything looks good from my view testing with smoketest. I have nothing to add, good work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Story] Pluggable Discovery API [Task] Pluggable Discovery In-Place Model Updates
2 participants