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

chore(db): use Flyway migration to set up discovery plugins #614

Merged

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Aug 21, 2024

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 using a GPG signature

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main


See #590
Fixes #549

Description of the change:

Removes the Quarkus-level startup check and lazy initialization of builtin discovery plugins, replacing these with eager initialization within the database migration scripts handled by Flyway.

Motivation for the change:

This ensures that discovery nodes and plugins creation is correctly tied to the database schema, and that the expected nodes and plugins are seeded in the database prior to application startup. This allows for cleaner application restart on an existing database instance, since the node creation/removal logic is no longer tied to the application lifecycle - no need for a startup check for node records existing, and if not to lazily initialize them.

This also opens up a nice unit testing possibility, where unit tests can @Inject Flyway flyway and then @AfterEach void cleanup() { flyway.clean(); }, which has the effect of resetting the database after each test. Without this change, this hook won't work as expected because the discovery nodes/plugins will be wiped from the database too, leaving the application in a bad state for the next tests.

Testing

  1. Check out PR
  2. quarkus dev, then in another terminal http :8080/api/v3/discovery and http :8080/api/v3/discovery_plugins
  3. Stop the devserver
  4. ./mvnw clean package ; podman image prune -f
  5. ./smoketest.bash -Okt
  6. Wait a while, then in another terminal repeat the http commands above

@andrewazores andrewazores requested a review from a team August 21, 2024 20:13
@andrewazores andrewazores added chore Refactor, rename, cleanup, etc. safe-to-test labels Aug 21, 2024
@andrewazores
Copy link
Member Author

/build_test

Copy link

Workflow started at 8/21/2024, 4:14:10 PM. View Actions Run.

Copy link

No OpenAPI schema changes detected.

Copy link

No GraphQL schema changes detected.

Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat/actions/runs/10496802557

@andrewazores
Copy link
Member Author

/build_test

Copy link

Workflow started at 8/21/2024, 4:40:11 PM. View Actions Run.

Copy link

No OpenAPI schema changes detected.

Copy link

No GraphQL schema changes detected.

Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat/actions/runs/10497154549

@andrewazores
Copy link
Member Author

/build_test

Copy link

Workflow started at 8/22/2024, 4:14:01 PM. View Actions Run.

Copy link

No GraphQL schema changes detected.

Copy link

No OpenAPI schema changes detected.

Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat/actions/runs/10514957799

@andrewazores andrewazores force-pushed the flyway-builtin-discovery-plugins branch from e2c6ca0 to 43ad1fa Compare August 26, 2024 16:47
@andrewazores
Copy link
Member Author

/build_test

Copy link

Workflow started at 8/26/2024, 12:50:21 PM. View Actions Run.

Copy link

No GraphQL schema changes detected.

Copy link

No OpenAPI schema changes detected.

Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat/actions/runs/10563792595

Copy link
Member

@mwangggg mwangggg 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 to me

@andrewazores andrewazores merged commit b45e8a6 into cryostatio:main Aug 26, 2024
9 checks passed
@andrewazores andrewazores deleted the flyway-builtin-discovery-plugins branch August 26, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Refactor, rename, cleanup, etc. safe-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Request] Database initialization and migration
2 participants