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

Implement user-specified target definitions #472

Merged
merged 3 commits into from
May 4, 2021

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Apr 28, 2021

Initial backend support for #468.

Perhaps there should also be some metadata added to ServiceRefs to mark their source, ex. platform detection vs manually added, but that is currently not implemented.

Here we have two new V2 API handlers, for POST /api/v2/targets and DELETE /api/v2/targets/:targetId.

The POST request checks if there are any existing targets with an identical service URL and fails if so. If not, then a new ServiceRef is created using the service URL and alias provided (by POST form attributes) and tracked by the new CustomTargetPlatformClient.

The DELETE request simply removes any ServiceRef from the CustomTargetPlatformClient which matches the specified targetId path parameter.

The CustomTargetPlatformClient simply maintains an internal sorted set of ServiceRefs with add/remove methods to mutate this state. It also persists the state to disk on each write operation, and loads this state from disk when start()ed. This allows the user's custom target definitions to survive service restarts (assuming the configuration directory is mounted somewhere that will survive a restart - this is not the case by default in run.sh/smoketest.sh).

Results from the CustomTargetPlatformClient are combined with the pre-existing platform-specific client by the new MergingPlatformClient, which is pretty self-explanatory.

Frontend support for this feature will come later.

To test, use websocat and curl or httpie. The following steps use httpie.

Start container.
CRYOSTAT_IMAGE=quay.io/andrewazores/cryostat:custom-targets

Connect to notification channel.
$ websocat -kE wss://0.0.0.0:8181/api/v1/command

Create a new custom target definition.
$ http --verify=false -f POST https://0.0.0.0:8181/api/v2/targets connectUrl="service:jmx:rmi:///jndi/rmi://cryostat:9099/jmxrmi" alias="Fake"

Observe that this target is FOUND in the notification channel stream.
Check that it is reflected in the API results - it should be listed first.
$ http --verify=false https://0.0.0.0:8181/api/v1/targets

Delete the custom target definition.
$ http --verify=false DELETE https://0.0.0.0:8181/api/v2/targets/service%3Ajmx%3Armi%3A%2F%2F%2Fjndi%2Frmi%3A%2F%2Fcryostat%3A9099%2Fjmxrmi

Again observe that the target is LOST in the notification channel stream.
Check that it is no longer reflected in the API result.
$ http --verify=false https://0.0.0.0:8181/api/v1/targets

The web-client UI already has support for the notification events emitted since they reuse the existing platform discovery notification category and type information, so the POST/DELETE effects can also be seen in the graphical target list.

@andrewazores andrewazores marked this pull request as ready for review April 29, 2021 14:12
@andrewazores
Copy link
Member Author

@Alexjsenn @Josh-Matsuoka any takers for a review?

@Alexjsenn
Copy link
Contributor

I can do it!

Copy link
Contributor

@Alexjsenn Alexjsenn left a comment

Choose a reason for hiding this comment

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

Looks good!

@andrewazores andrewazores merged commit 34bf763 into cryostatio:main May 4, 2021
@andrewazores andrewazores deleted the manual-target branch May 4, 2021 13:28
aali309 pushed a commit to aali309/cryostat-legacy that referenced this pull request Jul 22, 2024
* chore(smoketest): move service dependency declarations into correct files

* test(smoketest): add configuration for no configured S3 provider
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.

2 participants