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: manage database migration #812

Closed

Conversation

Ubisoft-potato
Copy link
Collaborator

@Ubisoft-potato Ubisoft-potato commented Feb 9, 2024

Fixes #685

Install atlas

We should install atlas first by running this command:

make local-deps

Migration workflow

  1. Create migration file from the dev database. [1]
    Explanation: This commmand will create migration files by comparing the migration file directory and dev database. For Example: when the migration directory is empty, after execute this command will creste the migration files that creates all tables.

    Run this command will need Docker, beacause atlas need an ephemeral local Docker
    container to create the migration file.

    The migration files directory is migration/mysql.

  2. Check the migration files. [1]
    After create migration files, the directory will looks like the following:

    migration/
    ├── Dockerfile
    └── mysql
     ├── 20240311022556_initialization.sql
     ├── 20240311024017_add_c.sql
     └── atlas.sum

    Suppose we are currently at version 20240311022556, so we will check the next migration file, which is version 20240311024017.
    After executing this command, it will display the migration process for the database.

    make apply-migration USER=bucketeer PASS=bucketeer HOST=localhost PORT=3306 DB=bucketeer
    atlas migrate apply \
                 --dir file://migration/mysql \
                 --url mysql://bucketeer:bucketeer@localhost:3306/bucketeer \
         --dry-run
    Migrating to version 20240311024017 from 20240311022556 (1 migrations in total):
    
    -- migrating version 20240311024017
     -> ALTER DATABASE `bucketeer` CHARSET utf8 COLLATE utf8_general_ci;
     -> ALTER TABLE `bucketeer`.`admin_audit_log` ADD COLUMN `test_c` int NULL;
    -- ok (4.198µs)
    
    -------------------------
    -- 6.229579ms
    -- 1 migration
    -- 2 sql statements
  3. Merge the PR that contains the migration files. [1]

  4. Triger Github Action to build the migration image. [1]


Helm Hook

As we need to do migration before install or upgrade Bucketeer Helm Charts, so we will add a pre-install and pre-upgrade Helm Hook to do migration process using the migration image that we build. [1] [2]

Additionally, we have to set the migration database URL.

If this Helm Hook Job fails, it will block the entire installation until it succeeds.


Other

If we need to use atlas migrattion in our existing Bucketeer cluster, we need to set the current version for atlas, because atlas will apply migration files from the first to last.
Additionally, atlas will save the migration version in the database table: atlas_schema_revisions.

For example, suppose our database version is at 20240311022556, we can use the following command to set the version for atlas:

make atlas-set-version VERSION=20240311022556 USER=bucketeer PASS=bucketeer HOST=localhost PORT=3306 DB=bucketeer
atlas migrate set 20240311022556 \
        --dir file://migration/mysql \
        --url mysql://bucketeer:bucketeer@localhost:3306/bucketeer
Current version is 20240311022556 (1 set):

  + 20240311022556 (initialization)

After set atlas version, we wll find atlas_schema_revisions table in database.

@Ubisoft-potato Ubisoft-potato force-pushed the feature-migration branch 2 times, most recently from f94c263 to 8ee18f2 Compare February 9, 2024 05:19
@Ubisoft-potato Ubisoft-potato marked this pull request as ready for review February 9, 2024 06:46
@Ubisoft-potato Ubisoft-potato force-pushed the feature-migration branch 2 times, most recently from 9022b58 to f1bef92 Compare February 21, 2024 02:41
@kentakozuka
Copy link
Contributor

@Ubisoft-potato migration pod has "helm.sh/hook": post-install, post-upgrade annotation. This means that the migration is executed after all k8s resources are updated, does it?
This causes errors because services of the new version expect the new schema.

IMO, it would be better to run migration before helm upgrades.

@Ubisoft-potato
Copy link
Collaborator Author

@Ubisoft-potato migration pod has "helm.sh/hook": post-install, post-upgrade annotation. This means that the migration is executed after all k8s resources are updated, does it? This causes errors because services of the new version expect the new schema.

IMO, it would be better to run migration before helm upgrades.

@kentakozuka Currently, the database migration API is implemented in the Batch service, so we can only wait for the Batch service to be deployed and then send the migration request to it.So I am using "helm.sh/hook": post-install, post-upgrade.

Let me check if we can make sure Batch service chart could be deployed first when we install or upgrade Bucketeer Helm Charts.

@cre8ivejp
Copy link
Member

cre8ivejp commented Mar 7, 2024

@Ubisoft-potato, as we spoke, we might achieve our goal of upgrading new schemas without causing downtime by running the migration process outside the Bucketeer app. We could use a script or container deployed temporarily and terminated after upgrading.

This guide seems a good solution to our problem.
https://atlasgo.io/guides/deploying/helm#using-helm-lifecycle-hooks
https://atlasgo.io/guides/deploying/k8s-init-container

@Ubisoft-potato
Copy link
Collaborator Author

Ubisoft-potato commented Mar 7, 2024

@Ubisoft-potato, as we spoke, we might achieve our goal of upgrading new schemas without causing downtime by running the migration process outside the Bucketeer app. We could use a script or container deployed temporarily and terminated after upgrading.

This guide seems a good solution to our problem. https://atlasgo.io/guides/deploying/helm#using-helm-lifecycle-hooks

@cre8ivejp Let's eliminate migration in the batch service.

First, I will create our GitHub workflow for creating migration images using atlasgo. Then, I will add a pre-install helm hook for Bucketeer Charts.

IMO, it would be better to run migration before helm upgrades.

@kentakozuka Please check the new solution here.

@kentakozuka
Copy link
Contributor

@Ubisoft-potato

First, I will create our GitHub workflow for creating migration images using atlasgo. Then, I will add a pre-install helm hook for Bucketeer Charts.

Sounds good to me!

Copy link
Contributor

@kentakozuka kentakozuka left a comment

Choose a reason for hiding this comment

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

@Ubisoft-potato Thanks!
How do we rollback a migration if the migration succeeded but we want to revert it?

Makefile Outdated
Comment on lines 271 to 273
--dir file://migration/mysql \
--to mysql://${USER}:${PASS}@${HOST}:${PORT}/${DB} \
--dev-url docker://mysql/8
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace spaces with tabs.

Makefile Outdated
Comment on lines 286 to 287
--dir file://migration/mysql \
--url mysql://${USER}:${PASS}@${HOST}:${PORT}/${DB} \
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace spaces with tabs.

Makefile Outdated
Comment on lines 263 to 265
.PHONY: install-atlas
install-atlas:
curl -sSf https://atlasgo.sh | sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add these lines to local-deps.

@Ubisoft-potato
Copy link
Collaborator Author

Ubisoft-potato commented Apr 5, 2024

@Ubisoft-potato Thanks! How do we rollback a migration if the migration succeeded but we want to revert it?

@kentakozuka This PR does not include the rollback migration implementation. We will address it in a new PR due to the complexity of the atlas down migration.

kentakozuka
kentakozuka previously approved these changes Apr 10, 2024
Copy link
Contributor

@kentakozuka kentakozuka left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@cre8ivejp cre8ivejp marked this pull request as draft June 12, 2024 06:57
@Ubisoft-potato Ubisoft-potato force-pushed the feature-migration branch 2 times, most recently from 43fc76a to bf7fca3 Compare June 19, 2024 04:33
@cre8ivejp cre8ivejp force-pushed the feature-migration branch from 4f9907a to 09d6fa7 Compare June 19, 2024 08:41
@cre8ivejp cre8ivejp force-pushed the feature-migration branch from 09d6fa7 to 7e289b8 Compare June 19, 2024 08:42
@@ -245,7 +267,7 @@ jobs:
token: ${{ secrets.REPO_ACCESS_PAT }}
workflow: dev-deploy.yaml
ref: master
inputs: '{"ref": "${{ needs.setup.outputs.REF }}", "image_tag": "${{ needs.setup.outputs.IMAGE_TAG }}", "committer": "${{ github.event.head_commit.author.name }}", "commit_message": "${{ needs.setup.outputs.COMMIT_MESSAGE }}", "commit_url": "${{ github.event.head_commit.url }}", "skip_e2e": "${{ github.event.inputs.skip_e2e }}", "skip_sdk_e2e": "${{ github.event.inputs.skip_sdk_e2e }}", "skip_prd_pr": "${{ github.event.inputs.skip_prd_pr }}"}'
inputs: '{"ref": "${{ needs.setup.outputs.REF }}", "image_tag": "${{ needs.setup.outputs.IMAGE_TAG }}", "committer": "${{ github.event.head_commit.author.name }}", "commit_message": "${{ needs.setup.outputs.COMMIT_MESSAGE }}", "commit_url": "${{ github.event.head_commit.url }}", "skip_e2e": "${{ github.event.inputs.skip_e2e }}", "skip_sdk_e2e": "${{ github.event.inputs.skip_sdk_e2e }}", "skip_prd_pr": "${{ github.event.inputs.skip_prd_pr }}", "migration_required": "${{ needs.setup.outputs.MIGRATION_REQUIRED }}"}'
Copy link
Member

Choose a reason for hiding this comment

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

When deploying, we will need the migration_required to update the migration container image tag in the dev environment.

@@ -98,13 +106,17 @@ jobs:
IMAGE=`./tools/build/show-image-name.sh $APP`
docker build -f Dockerfile-app-$APP -t bucketeer-$IMAGE:${{ needs.setup.outputs.IMAGE_TAG }} --load .
done
- name: Build and push the Migration image
if: needs.setup.outputs.MIGRATION_REQUIRED == 'true'
Copy link
Member

Choose a reason for hiding this comment

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

It will only build and push the image when the migration directory changes.

migration:
image:
repository: ghcr.io/bucketeer-io/bucketeer-migration
tag: v0.4.5 # x-release-please-version
Copy link
Member

Choose a reason for hiding this comment

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

Set the default tag and the comment so the Release Please can update the tag when releasing.

Comment on lines +65 to +71
- name: Store the migration_required value to output
uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
id: changes
with:
filters: |
migration:
- 'migration/**'
Copy link
Member

Choose a reason for hiding this comment

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

I'm using the dorny/paths-filter action, which is great for checking for changes in directories or files.

@cre8ivejp cre8ivejp force-pushed the feature-migration branch from 0d72467 to 74dc8ad Compare June 19, 2024 10:21
@cre8ivejp
Copy link
Member

Closing this in favor of #1060

@cre8ivejp cre8ivejp closed this Jun 26, 2024
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.

feat: manage the sql migration and schema changes in the main repository
3 participants