V8-archive: Add "needs migration" flag to the API

Created on 12 Jul 2019  ·  25Comments  ·  Source: directus/v8-archive

When the API got updated but the migrations haven't been run yet, the user can face errors or other unwanted behavior in the app, as the database got outdated compared to the API version. In order to prevent this, the API should return a flag either in the / endpoint or in every API response so the application can handle this state by showing the user a "API needs updating" modal or notification.

@directus/team what would be a good format for this? We currently have meta data and error as top level object properties in the JSON output, should we add a fourth for this? Or maybe add it into meta?

Alternatively, we could return the migration information into the / endpoint. That way the app can check if latest !== current and use that instead.

GET /

{
    "data": {
        "api": {
            "version": "2.2.2",
            "database": "mysql",
            "project_name": "Directus",
            "project_logo": ""
        },
        "server": {
            "max_upload_size": 2097152,
            "general": {
                "php_version": "7.3.7-1+ubuntu16.04.1+deb.sury.org+1",
                "php_api": "fpm-fcgi"
            }
        },
        "migrations": {
            "latest": "201907121024_add_2fa",
            "current": "201805011529_update_field"
        }
    }
}
enhancement

Most helpful comment

In a high volume api installation several requests can come in parallel. How we'd manage concurrent access on long running migration process and avoid running multiple instances of it? File locks won't work on docker for example (or any distributed environment).

Sorry not entering the discussion early, but IMHO this should not be an automated process. As mentioned before, I think putting the api into a needs migration state and denying any kind of requests besides a secure migration process would be more likely to work on most environments.

Also in case of failure, how do we rollback?

All 25 comments

To achieve better clarity/visibility, we are now tracking feature requests within the Feature Request project board.

This issue being closed does not mean it's not being considered.

From @benhaynes


If we're redirecting, then it might be good to have this App page completed:

directus/app#1370

That way they can get more information (beyond just clicking a button). Also, I worry about small migrations causing API downtime — especially for small migrations that wouldn't normally affect the API responses (like removing a field from users). Could we still return API responses like normal, but still have the error message let the user/app know what needs to happen?

Or maybe I'm overthinking it and we should just force them. 🤔

Hey @rijkvanzanten

we could return the migration information into the / endpoint. That way the app can check if latest !== current and use that instead.

This seems okay to me. But it will resolve the issue at APP side only.

What should the other endpoints of API return when some migration need to be executed? Cause we are giving the support of standalone API too. So there must be maintenance mode as a response when unexecuted migration exists in the system.

You may also want to consider to clone the repo automatically if any updates are available using the Git API.

As the upgrade is all about

  • Migrate the code
  • Migrate the DB

So let me know currently what should be considered for development and what should need to consider as a feature request.

The main question now: is this warning important enough to show in every request, or just when the user asks for it?

I don't necessarily agree that having the info in / only helps the app. It's a way to retrieve that information from the API, which can be used by anyone at anytime.

So there must be maintenance mode as a response when unexecuted migration exists in the system.

This is tricky, I'm not sure if we can / should lock the API completely whenever there is an unfinished migration.. Thoughts @directus/team?

I don't necessarily agree that having the info in / only helps the app. It's a way to retrieve that information from the API, which can be used by anyone at anytime.

I have one quick suggestion here. Instead of responding latest and current migration, we can execute the unexecuted migrations from the / endpoint.

This is tricky,

Agree. It's not necessary to lock the API for now. Will do that if we need it in the future.

I don't know if we should execute the migrations automatically. Thoughts on that @benhaynes?

The issue is that we can potentially wreck someone's database by a shady migration. Right now the migration is a manually activated feature, that has a warning to backup your database before running it.

That being said, the API has the potential of being bricked without running the migrations, and we can add this 'backup first' warning to the docs too (eg make sure to backup your database before updating the API).

Running the migrations automatically on any request if it hasn't run yet is the easiest fix for all of this as the API will 'just work' the moment you download the update. I'm just not too sure if we can do that from an ethical perspective.

Yeah, it makes me too nervous to run the DB migrations automatically (even with a note in the docs). The admin should always have an explicit warning before the database is updated so they can backup beforehand. Unless...

One potential option would be to:

  1. Update codebase
  2. Someone initiates any API call
  3. Automatically backup database to local file
  4. Run migrations

That way we handle the backup and can conditionally run the DB migration only if the backup was created without errors. It would also be nice to have this built in backup feature 😉.

I also don't think we should block the API if there is a pending migration required (unless the migration needs to be updated to return the data). So maybe offering a maintenance mode is the safest bet?

@benhaynes could we assume that when someone updates the api, they will have downtime? That way, we could block the api until the person runs the db migrations.
Would it make sense anyway as someone that runs directus to update the api out of the blue?

could we assume that when someone updates the api, they will have downtime?

That's currently not the case


Generally speaking, the cleanest way to make sure everyone is on the latest database migration is to.. migrate the database on update. Most other CMSes actually do this (you hit update in settings, it downloads the latest code, runs db migrations, and done). The main difference is that Directus doesn't have a way to update from the suite itself yet.

OK, I might have been convinced. Perhaps we should make it very clear in the Docs (and anywhere that you might initiate an upgrade) that when upgrading your version of Directus, the database upgrade will be performed _automatically_. Therefore, the "database back" should be performed by the admin _before_ they start the upgrade process.

If you think of it as one step (which it should be), then initiating an upgrade should prepare the Admin for database changes.

Down to discuss, anyone completely against this approach?

Especially if this is an industry standard, I think we should go with it. After all, that means that a migration should be expected by admins

@benhaynes

Based on the discussion; I would like to summerise the flow. Let me know your thoughts here.

If we are going to provide the upgrade functionality then it should reflect at 3 places.

  1. Upgrade Functionality in admin panel ( APP )

    We have to introduce another functionality of Upgrade System in the Admin Settings of the admin panel, which will contain below points.

    (i) Upgrade Codebase
    (ii) Automatically backup database to the local file
    (iii) Run unexecuted migrations

  2. APP side

    After login APP should call an another API which will migrate this stuff. So every time when the user logged in to the system they will able to find the latest code as well as latest DB along with the back up of their DB.

  3. API side

    We need to update the doc with the info that "User need to sync their DB with repo; if they clone/update their codebase manually". If someone takes a clone and not update the DB; API will throw an error (Only if any breakable changes exist). So I believe that we must introduce a maintenance mode here otherwise the 500 will be thrown automatically. I am suggesting this to handle 500 status code only.

    It will be also helpful for those who only use standalone API. It will be work as a reminder of the upgradation of DB for them.

I'm not sure about it. What does "when upgrading your version of Directus, the database upgrade will be performed automatically" mean in a point in time?

I think it is a pretty fragile approach to automatically update when the api version changed without asking. I still prefer a manual step.

I just imagine running directus with docker and I upgrade the version but forgot to mount the volume of the api where the backup is made on the host machine it could lead to a loss of the backup even if on directus side everything worked fine ... it may be also complicated when the database is huge or similar things ... so I would always like to have a option to do the migration manually if needed.

I like how craftcms is doing it: When the schema needs to be updated after updating craftcms, you will be routed to a blank screen asking for a database migration before you can keep on working within the app. So instead of having a migrate now button you can't do anything in the app till the migration is done.

Of course, I understand it is maybe tricky to keep the api up and running without errors when the migration is not done yet. But normally I'm aware of that when I can't do anything within the app anymore before I migrated I will understand the api may be broken too.

  1. Is going to be very difficult: we can't update the app reliably from the API. We can only update the API, but that leaves the app in a weird place.

  2. Should work, except for when the API migrations resolve breaking changes, as is the case with the 2fa PR. In case of the 2FA one, we won't make it past the login stage, as that's where it breaks already.

@mimamuh The problem right now is that @OscBacon implemented an amazing feature that is a breaking change in the API if you haven't run migrations yet..

I like how craftcms is doing it: When the schema needs to be updated after updating craftcms, you will be routed to a blank screen asking for a database migration before you can keep on working within the app. So instead of having a migrate now button you can't do anything in the app till the migration is done.

I think this is generally speaking the best approach, but it does leave the application in a stale state. We need to figure out a way where the API can update both the API and the application, or neither. If the API gets updated without the application reflecting this update as well, a lot can (/will) break

I think adding the migrations key in / endpoint is a good approach. (Only applicable for admin role)

When the migration is available the other endpoints work normally. In the edge case scenario, the other endpoint return 500 if there is some breaking changes in code which need the migration must run before proceeding.

Additionally, we can add one setting for auto-update migration in the admin setting. It this setting is true API will run migration when any new migration found.

@rijkvanzanten

  1. Should work, except for when the API migrations resolve breaking changes, as is the case with the 2fa PR. In case of the 2FA one, we won't make it past the login stage, as that's where it breaks already.

Additionally, we can add the migrations in the response of the /login endpoint too.

@hemratna by adding migrations in the response, do you mean throwing a 500 and adding the need for a migration in the error ?
Or a 400 and saying to upgrade the app ?

@OscBacon API informs the APP to run the migration.

@hemratna the problem still persists though:

/login is the endpoint that's broken (500), and you need to be logged in as admin to retrieve that info on /, therefore, you can't actually access it and you're locked out of the system..

I think we finally decided on the DB migrations after upgrade: we're thinking they automatically run the first time you access any API endpoint. And we'll update the docs to inform admins that they should perform a database backup before starting the upgrade process.

@directus/api-team — any ideas how long this would take to implement?

In a high volume api installation several requests can come in parallel. How we'd manage concurrent access on long running migration process and avoid running multiple instances of it? File locks won't work on docker for example (or any distributed environment).

Sorry not entering the discussion early, but IMHO this should not be an automated process. As mentioned before, I think putting the api into a needs migration state and denying any kind of requests besides a secure migration process would be more likely to work on most environments.

Also in case of failure, how do we rollback?

Putting the api into a needs migration state and denying any kind of requests besides a secure migration process would be more likely to work on most environments.

This came up before too, the only problem there is that the user needs to manually activate the migration flow. Keeping Directus Cloud ☁️ in mind, that would mean that we completely brick your project every other week or so because we rolled out an update that requires you to manually run the DB migration 🤔

But in that case we'd be responsible for "batch running" the migrations for all clients whenever something updates dont we? We're the ones keeping the database. In a sharded environment we can do it in small batches and in a scheduled maintenance window.

The other problem in this is all is the multi-tenant nature of the API. The DB migrations are per-project, but the change in the API code might require the updated DB..

Should/can migrations be run API wide?

Yeah, that will be a problem whenever we want to just update one tenant instead of all the configured ones. We'd need to separate into different installations to keep an old and a new version.

Should/can migrations be run API wide?

You mean all the available tenants?

You mean all the available tenants?

Yes exactly.

Lets use the 2FA PR as an example: the code will now check for the existence of a token in a OTP column, if this column doesn't exist (migrations haven't run) the API returns a 500 error.

You update the API as a whole, but run migrations _per project_, as each project has it's own database.

Was this page helpful?
0 / 5 - 0 ratings