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.

Sign up for GitHub

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

Jump to bottom

Flyway and Liquibase are now run as init containers in manifests. #29026

Merged
merged 12 commits into from
Jan 17, 2023
Merged

Flyway and Liquibase are now run as init containers in manifests. #29026

merged 12 commits into from
Jan 17, 2023

Conversation

iocanel
Copy link
Contributor

@iocanel iocanel commented Nov 3, 2022 •
edited
Loading

The pull request introduces an initialization phase where init tasks like Flyway & Liquibase migration take place. It allows users to run those tasks in isolation (without running the whole application).

For Kubernetes environments where it only makes sense for such tasks to run once, the Kubernetes manifest generator exposes them as Jobs and tunes Deployment so that they only run once initialization is complete.

In other words, it enables the scaling of Quarkus applications that use init tasks in Kubernetes.

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 3, 2022

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should not end up with dot
  • title should preferably start with an uppercase character (if it makes sense!)

This message is automatically generated by a bot.

@iocanel iocanel force-pushed the db-migration-init-containers branch 2 times, most recently from 12ba35e to cfe5c17 Compare November 3, 2022 09:17
geoand
geoand requested changes Nov 3, 2022
View reviewed changes
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

I made a suggestion for properly stopping the application

extensions/flyway/runtime/src/main/java/io/quarkus/flyway/runtime/FlywayRecorder.java Outdated Show resolved Hide resolved
extensions/liquibase/runtime/src/main/java/io/quarkus/liquibase/runtime/LiquibaseRecorder.java Outdated Show resolved Hide resolved
@iocanel iocanel force-pushed the db-migration-init-containers branch from cfe5c17 to a8f2aa1 Compare November 3, 2022 13:33
geoand
geoand approved these changes Nov 7, 2022
View reviewed changes
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

This is awesome, thanks!

@geoand
Copy link
Contributor

geoand commented Nov 8, 2022

We should also include documentation in the liquibase and flyway extensions about this

@iocanel iocanel force-pushed the db-migration-init-containers branch from a8f2aa1 to c3f7d7a Compare November 16, 2022 16:03
@loicmathieu
Copy link
Contributor

Liquibase MongoDB is missing, if you didn't have time to include it in this PR can you open an issue to not forget it ?
I didn't had time to look at the PR but this looks like a good idea, I'll read it later when I'll find the time.

@iocanel iocanel force-pushed the db-migration-init-containers branch from c3f7d7a to 3f8b51d Compare November 17, 2022 10:00
@iocanel iocanel force-pushed the db-migration-init-containers branch 2 times, most recently from 7eb7ab0 to 347a01e Compare December 2, 2022 12:38
@iocanel iocanel changed the title [wip] Flyway and Liquibase are now run as init containers in manifests. Flyway and Liquibase are now run as init containers in manifests. Dec 2, 2022
@iocanel
Copy link
Contributor Author

iocanel commented Dec 2, 2022

Added some new changes:

  • Use Jobs to perform init tasks
  • Make Init task definition generic
  • Configure RBAC needed for allowing the Pod to wait for job.
  • Add flag to enable disable behavior per platform (e.g. kubernetes / openshfit).

@iocanel
Copy link
Contributor Author

iocanel commented Dec 2, 2022

Forgot to mention:

  • Test
  • Docs

geoand
geoand requested changes Dec 2, 2022
View reviewed changes
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

This is amazing!

I have a few questions and suggestions

core/deployment/src/main/java/io/quarkus/deployment/builditem/InitTaskBuildItem.java Outdated Show resolved Hide resolved
core/deployment/src/main/java/io/quarkus/deployment/builditem/InitTaskBuildItem.java Outdated Show resolved Hide resolved
core/runtime/src/main/java/io/quarkus/runtime/ApplicationLifecycleManager.java Show resolved Hide resolved
docs/src/main/asciidoc/flyway.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/flyway.adoc Outdated Show resolved Hide resolved
extensions/flyway/runtime/src/main/java/io/quarkus/flyway/runtime/FlywayRuntimeConfig.java Show resolved Hide resolved
...illa/deployment/src/main/java/io/quarkus/kubernetes/deployment/AddJobResourceDecorator2.java Show resolved Hide resolved
...ngodb/runtime/src/main/java/io/quarkus/liquibase/mongodb/runtime/LiquibaseMongodbConfig.java Show resolved Hide resolved
...ngodb/runtime/src/main/java/io/quarkus/liquibase/mongodb/runtime/LiquibaseMongodbConfig.java Outdated Show resolved Hide resolved
...ase/runtime/src/main/java/io/quarkus/liquibase/runtime/LiquibaseDataSourceRuntimeConfig.java Outdated Show resolved Hide resolved
gsmet
gsmet previously requested changes Dec 2, 2022
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

I asked something that I would like to be clarified.

Also probably a good idea to at least squash this commit given it's a fixup (and maybe others, I have no idea if they are semantically separate).

extensions/flyway/runtime/src/main/java/io/quarkus/flyway/runtime/FlywayRecorder.java Show resolved Hide resolved
@geoand
Copy link
Contributor

geoand commented Jan 15, 2023

🎉

I'll have another look tomorrow

geoand
geoand reviewed Jan 16, 2023
View reviewed changes
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

I only added a couple tiny comments, I think this is almost good to go!

@iocanel iocanel force-pushed the db-migration-init-containers branch from db7cf54 to 4e72544 Compare January 16, 2023 11:16
@iocanel iocanel force-pushed the db-migration-init-containers branch from 4e72544 to 4e93bb4 Compare January 16, 2023 11:35
@iocanel
Copy link
Contributor Author

iocanel commented Jan 16, 2023

@geoand: feedback applied, can you please have another look?

@quarkus-bot

This comment has been minimized.

Sign in to view
geoand
geoand approved these changes Jan 17, 2023
View reviewed changes
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

🎉

@gsmet gsmet dismissed their stale review January 17, 2023 12:02

Addressed.

@quarkus-bot
Copy link

quarkus-bot bot commented Jan 17, 2023

Failing Jobs - Building 4e93bb4

Status Name Step Failures Logs Raw logs
✔️ Maven Tests - JDK 11
⌛ Maven Tests - JDK 11 Windows Build ⚠️ Check → Logs Raw logs

@iocanel iocanel merged commit 9c09042 into quarkusio:main Jan 17, 2023
@quarkus-bot quarkus-bot bot added this to the 2.17 - main milestone Jan 17, 2023
@Sgitario Sgitario mentioned this pull request Mar 9, 2023
Closed
This was referenced May 3, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geoand geoand geoand approved these changes

@maxandersen maxandersen Awaiting requested review from maxandersen

@gastaldi gastaldi Awaiting requested review from gastaldi

@gsmet gsmet Awaiting requested review from gsmet

Assignees
No one assigned
Labels
area/arc Issue related to ARC (dependency injection) area/core area/documentation area/flyway area/kubernetes area/liquibase release/noteworthy-feature
Projects
None yet
Milestone
3.0.0.Alpha4
Development

Successfully merging this pull request may close these issues.
6 participants
@iocanel @geoand @loicmathieu @maxandersen @gastaldi @gsmet