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

Move database to docker volume instead of in ~/.ddev, fixes #714 #987

Merged
merged 73 commits into from
Aug 11, 2018

Conversation

rfay
Copy link
Member

@rfay rfay commented Jul 11, 2018

The Problem/Issue/Bug:

OP #714 - We've had no end of trouble bind-mounting the database onto the host on various platforms, especially Windows.

How this PR Solves The Problem:

  • Switch to using docker volume
  • Provide database snapshot technique, including snapshots on ddev rm -R

TODO:

  • Command to create a snapshot
  • Command to restore a snapshot
  • Option on --remove-data to create snapshot (or omit it)
  • Figure out how to migrate existing databases into a volume.
  • Test scripts should delete all ddev volumes before starting.
  • Too much text output when running mariabackup (creating snapshot)
  • mariadb container tests need to use volume, not bind mount.
  • Update minimum docker version to 18.06
  • Docs required
    • Update explanation of where db is stored
    • Update remove docs

Manual Testing Instructions:

  • Before testing, please docker pull drud/ddev-dbserver:20180801_volume
  • **Before testing: Any docker-compose.*.yaml files need to be changed to version: '3.6', since we were forced to do a version bump here. This is an important thing to point out in release notes and announcements.
  • Sample d7 databases are provided at d7tester_test1 (one node, site name "d7tester 1") and d7tester_test2 (two nodes, site name "d7tester 2")
  • Create a project, load a db
  • Verify that the docker volume has been created with docker volume ls
  • Use ddev remove --remove-data to remove the project, you should see a snapshot created and the volume destroyed.
  • Test ddev snapshot, verify that the snapshot gets created.
  • Load a different site (like d7tester_test2)
  • Test ddev revert-to-snapshot and you should lose the change you made.
  • Run ddev rm (with or without --remove-data) and you should see a snapshot created.
  • Test migration. Create a project with ddev v1.0.0 and then do a ddev start with it using this version. Verify that the migration happens successfully and that you have the project you expected.

This may be sensitive to different OSs; although we have tests running on all supported OSs, it will be good to get manual testing experience with this on Windows and Linux (and WIndows 10 Home).

Automated Testing Overview:

  • TestDbMigration
  • TestDdevRestoreSnapshot

And of course every database-related action tests the behavior.

Required tests:

  • snapshot creation
  • snapshot loading/reverting
  • Full test suite needs to be run at least manually on macOS with docker edge.

Related Issue Link(s):

OP #714
Mariadb upgrade to 10.2 #947

Release/Deployment notes:

Questions for discussion

  • Currently, ddev rm always creates a snapshot unless you do ddev remove --create-snapshot=false. Think that's the right behavior?
  • It may be worth figuring out how to automatically update people's docker-compose.*.yaml to version: '3.6', but I'm afraid this would be a fair bit of dead code for a few people's short-term need.

Copy link
Contributor

@rickmanelius rickmanelius left a comment

Choose a reason for hiding this comment

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

Preparation

DDEV Binary

Site

  • Download Drupal 7.59
  • Unzip
  • ddev config --projectname drupal-7.59 --projecttype drupal7 --docroot .
  • ddev start
  • ddev exec drush site-install --db-url=mysql://db:db@db/db -y
  • ddev exec drush user-login
  • Now I have a starting point with a DB.

Testing

  • Verify Docker Volume Exists
    • docker volume ls shows a volume name "drupal-7.59-mariadb"
    • Added a test WP site. Confirm the volume wasn't there during config, but it is there right after ddev start.
    • CD /Users/rmanelius/Downloads/wordpress/.ddev/db_snapshots/wordpress_20180712150625 and saw the entire mysql directory.
    • Question, since we are in fact "snapshoting" and I would think it would be better to give someone a recoverable .sql file that we could then pipe back into ddev via import-db versus the folders from the OS. That would at least give me (or anyone) a better shot of manually reverting. HOWEVER. That was what was being done before in the global .ddev folder and no one complained so there's that.
    • Verifyied that a ddev rm --remove-data does remove the volume.
  • Verify Snapshot Generation
    • In my Drupal 7 site, I created a test node.
    • Running ddev snapshot
      • Shows a warning that it's snapshotting before destroying: "Creating database snapshot before destroying the database."
      • I would figure that it's saving versus saving + destroy.
      • I verified the DB and volume is still there.
    • Running a 2nd snapshot
      • I see a 2nd snapshot in /Users/rmanelius/git/drupal-7.59/.ddev/db_snapshots/
  • Testing Revert
    • Added another node
    • Run ddev revert-to-snapshot
    • I didn't seem to get a revert to the 1 node version.
    • Added a 3rd node
    • Run ddev revert-to-snapshot
    • See 3 nodes. Hmm.
    • Run ddev rm --remove-data (which creates a snapshot)
    • ddev start shows a blank DB
    • run ddev revert-to-snapshot
    • I'm not seeing the DB revert :(

So I'm seeing the snapshotting and volume create/destroys working, but it's not a usable backup at this point. I could also be using this incorrectly, but I tried to follow the directions as closely as possible.

@rfay
Copy link
Member Author

rfay commented Jul 12, 2018

Something has been regularly killing off docker on macOS, completely fatal. I'm wondering if it's this PR, and wondering if it's the volume usage (for the first time) that's doing it. There was also too much output from the snapshot save and load, but I think that's calmed down now.

@rickmanelius
Copy link
Contributor

So another thought regarding snapshots. I really, really like this overall idea. I know it's somewhat pressured to come in as a result of docker volume usage, so I'm hoping my feedback doesn't add too much scope. However, in an "ideal" use case I could see something akin to...

  1. Prompting to Save a Snapshot During Removal
    When running ddev rm --remove-data, then prompt the user with a question of whether or not they want to save a snapshot. Not all cases (particularly with testing) warrants saving snapshots that are not really valuable/necessary/wanted. Adding a flag "-f" would force this and bypass this prompt. And the default to make a snapshot would be a "y" so a quick hit of enter would allow them to save the snapshot. That might be overloading things a bit, but the default of always a snapshot seems like it could become overrun with backups that don't mean much.

  2. Snapshot Generation & Restore
    I can see this being something of great value, particularly when stepping through database changes as a site builder. I would see this more as 3 components:

A. Generation of backups
B. Query/list of backups.
C. Restore backups (default to last backup and, if running a query, pick one).

This could look something like...

  • ddev snapshot => backups_dir/projectname_timestamp1
  • ddev snapshot => backups_dir/projectname_timestamp2
  • ddev snapshot --list => shows list.
  • ddev restore-snapshot => restores last snapshot
  • ddev restore-snapshot timestamp => restores specific snapshot

It's worth noting that if someone ran the list command, they could be prompted to enter a number to restore a particular backup or 0/n to not restore.

A complication is the fact that we also have snapshots from provider plugins (currently, only Pantheon), which stores in yet another archive format of an archive per type (db and files). This is where I was pushing to have a consistent file format that we saved archives in to somewhat marry the two versions. Alas, we can still have them work as independent items for now. And the ability to query and revert to another backup is not needed out of the gates. But if we are storing multiple backups per project, we need a way to actually use that...

@rfay
Copy link
Member Author

rfay commented Jul 13, 2018

Thanks so much for your thoughts @rickmanelius, I definitely think this can improve, both right now and longer-term. We may not want to do it all right now, since the purpose was actually just to get into docker volume (and not lose our ability to get things onto the fileystem). Also, we may want to have some feedback from actual users.

  • I added instructions, including sample databases to import for testing. These are used in the test that's under development.
  • There is currently an arg on ddev remove to skip the snapshot. I don't particularly like the choice (ddev remove -h) and we can fiddle for sure.
  • We could certainly change to not snapshot except on --remove-data, that was my original plan and then I thought "well maybe". We could also just not do it for people, and only do it with the snapshot command.
  • Snapshots are currently pretty accessible in project .ddev/db_snapshots. My testing technique has been to change the name of important ones in testing, because you get a lot in current situation.

I think we can make some choices on this one and then let it mature with user feedback.

Hoping I can get you to try it again :)

@rickmanelius
Copy link
Contributor

Hi @rfay. I'm very excited about this feature, so of course I'll give it another test :)

Agreed that we can improve long term. My comments were more what/how I see this feature growing into and not something that had to be done right now. So yes, once we can validate a working snapshot revert (which I wasn't able to do my last go around), I vote to approve and we can file a subsequent wishlist item.

FWIW, here was one area where snapshots (particularly in the UI) was a conceived feature https://github.com/drud/ddev-ui/wiki/Roadmap#v050-workflow-improvements. This particularly true for the WP community, which tends to have more site building and the ability to "save state" and test some new changes with the ability to revert would be super valuable.

@rickmanelius
Copy link
Contributor

Thanks for the improved testing instructions in the issue summary!

@rickmanelius
Copy link
Contributor

ion

DDEV Binary

Site

  • Clean Up
    • Remove previous Drupal 7.59 site (ddev rm --remove-data && rm -rf)
  • Download Drupal 7.59
  • Unzip
  • ddev config --projectname drupal-7.59 --projecttype drupal7 --docroot .
  • ddev start

Testing

  • Verify Docker Volume Exists
    • docker volume ls shows a volume name "drupal-7.59-mariadb"
  • Verify Snapshot Generation
    • Import db1 with ddev import-db --src ~/Downloads/d7tester_test_1.sql.gz
    • Verified 1 node with sitename "d7 tester test 1"
    • ddev remove --remove-data creates a snapshot in .ddev/db_snapshots/
    • Attempt to run ddev snapshot while in a stopped state.
      • I see lots of warnings and errors here. A simple "containers need to be operational or healthy" with a prompt to run ddev start would probably be better.
      • Despite the error, I did see another snapshot attempt based on the folder. The folder is in fact empty, but it's there.
  • Verify a revert
    • From nothing, re-run ddev start to bring up the containers.
    • Import db 2 from ddev import-db --src ~/Downloads/d7tester_test_2.sql.gz
    • Verified two nodes and site name "d7 tester test 2"
    • Ran ddev revert-to-snapshot
    • I still see the 2nd database...
    • Running another ddev rm --remove-data and verified another snapshot created
    • Run ddev start and verified no db
    • Re-run ddev revert-to-snapshot
    • Still no database...

Looks like I'm not getting the revert working...

@rfay
Copy link
Member Author

rfay commented Jul 14, 2018

Well, I never would have thought of anybody trying to snapshot when it's not running, but that's why it's nice to have multiple eyes on these things. I

  • Made snapshot work even if stopped or rm'd
  • Made import-db work when stopped or rm'd, since that's the same and has always been an annoyance ("Why doesn't it just start itself?").

You should know that revert is reverting to the entire mariadb environment (mysql database, empty db database). So if you snapshot a beginner db, and then revert to it... you'll have a db ready to be installed. It's not just snapshotting the 'db' database, the the entire system. It wasn't clear from your writeup whether maybe that happened to you (snapshot and restore an empty database, with just the mysql db installed).

I'd love to walk through this with you, to try to completely understand the issue that's happening. Great to have multiple eyes on something like this!

@rfay rfay modified the milestones: v1.0.0, v1.1.0 Jul 14, 2018
@rfay
Copy link
Member Author

rfay commented Jul 14, 2018

I have good news and bad news.

  • Good news: upstream/master does not seem to destroy docker on macOS. I've run it manually 4 times and am starting a 20-cycle run of it.
  • Bad news: Testing this PR (just the buildkite test.sh) does destroy docker on macOS. I'm able to recreate complete docker failure after one or two runs on macOS. You get a dialog box that says "Fatal error: com.docker.supervisor failed to start. Exit code 1 "

So the bad news is

  • I don't think this can be a candidate for v1.0.0, because we have to have confidence that this behavior is stopped.
  • Unfortunately there are a number of fixes and features in here that will be sad to leave out (or move in another way). Not just snapshotting (which should work without docker volumes, assuming docker volumes are the fragility here). But significant improvements to tests (including the ability to test content on a running ddev project) and test script improvements. Sad.

Changing the tag to v1.1.0, we'll see what happens..

@rfay rfay removed the request for review from andrewfrench July 14, 2018 18:19
@rickmanelius
Copy link
Contributor

Hi @rfay! Wow, thank you for the dedication on this one. 20 cycles to catch an error state? That's dedication! I certainly won't be doing that during my manual, end-user testing :)

In all seriousness, I think it's useful to jump on a video chat tomorrow because I believe I fully understand what you are saying, notably that the snapshot is effectively backing up MariaDB for the given project and not just the imported *.sql file. Still, my expectation of what should revert is not being met because I do have working databases post install and with nodes. So I'm either misunderstanding this or I found another fun edge case :) I don't know what your schedule is tomorrow, but I put an event on the calendar around 10:00 and hopefully we can sort it out.

As for the instability, that is unfortunate because this is something that is very exciting and it would be awesome to release... but not if it's unstable :(

@rfay
Copy link
Member Author

rfay commented Jul 16, 2018

The problem with this PR is that destroys docker, whether in general use or running tests. I've removed it from the upcoming release, and it will be a significant effort to chase what's happening. But what it does, if you test it (on macOS at least) is it brings down docker on the testbots, meaning that all our testing is completely destabilized and confusing.

To continue chasing I'll have to manually run tests against an early, early commit that has only the docker volume stuff; I'm pretty sure there was a clean one running using the volume technique. I can put the test script in a loop and run that hard on a disabled macOS testbot. It's certainly possible that the volume approach is OK but something else here stresses Docker.

I'm fine with meeting and testing it tomorrow, but not sure it's worth your time, as this is not ready for release, and may not ever make it as is. It's passing automated tests that do everything I specified to do manually, so I think we'll be successful walking through those. So it's up to you.

I just can't imagine the pain if we break every mac user's docker :(

@rfay
Copy link
Member Author

rfay commented Jul 16, 2018

Rick and I walked through this today and figured out what was happening! He was using ddev revert-to-snapshot and it accepted that! And did nothing... We decided on these outcomes:

  1. ddev revert-to-snapshot needs to require the arg of which snapshot!
  2. revert-to-snapshot might also (if no arg) display some snapshots for attention and say how to use it.
  3. The args to ddev rm should be improved. Probably --create-snapshot and --omit-snapshot, with no binary arg like currently done.
  4. ddev rm --remove-data should do a snapshot by default.
  5. Regular ddev rm show not do the snapshot by default.

Rick, I'll edit if I missed something there.

@rickmanelius
Copy link
Contributor

Hi @rfay. Looks good! Thanks for summarizing. I think there was one more item, and I had an additional idea after our meeting.

  • --remove-data => --remove-db to indicate that this isn't about "data" but rather specifically the database.
  • idea: revert-to-snapshot is a bit long. What about "restore"? I agree "revert-to-snapshot" is more precise and we can always alias laster.
  • I believe this summarizes the syntax if these suggestions work:
    • ddev rm --remove-db -O (ommits snapshot)
    • ddev rm --remove-db -S (creates snapshot)
    • ddev restore --snapshot-name=snapshot

@rfay rfay changed the title Try making the database stored on docker volume instead of in ~/.ddev, fixes #714 Move database to docker volume instead of in ~/.ddev, fixes #714 Aug 8, 2018
@rickmanelius
Copy link
Contributor

I'm starting to test this now. Right off the bat I had an older config.yml (probably just v1.0.0) where re-running ddev config and then a ddev start resulted in:

Starting drupal-7.59...
Migrating bind-mounted database in ~/.ddev to docker-volume mounted database
Failed to start drupal-7.59: Failed to migrate db from bind-mounted db: failed to run migrate_file_to_volume.sh, err=failed to create/start docker container ({drupal-7.59_migrate_volume 0xc4202005a0 0xc42023a400 <nil> <nil>}):no such image output=

I'm going to blow everything out, but toggling between an established working v1.0.0 project and back will be useful to test the upgrade path. This one already feels like it's going to be a doozy... notably on existing projects.

@rickmanelius
Copy link
Contributor

Also, looks like (based on the config.yml generated from a ddev config) that the manual testing instructions should be docker pull drud/ddev-dbserver:20180801_volume. Note the change from "mariadb" to "dbserver" in the string.

@rickmanelius
Copy link
Contributor

Sorry for 3 quick comments in a row. There were two other factors that existed in this repo that might have caused the previous issue. I had a docker-compose.memcached.yaml file as well as pre-existing .ddev/db_snapshots. Either of those two (from previous tests) could be problematic. I'm going to tear down to a clean D7 repo and start over.

@rickmanelius rickmanelius self-requested a review August 10, 2018 15:07
Copy link
Contributor

@rickmanelius rickmanelius left a comment

Choose a reason for hiding this comment

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

DDEV Binary

Site

  • Clean Up
    • Remove previous Drupal 7.59 site (ddev rm --remove-data && rm -rf)
  • Download Drupal 7.59
  • Unzip
  • ddev config --projectname drupal-7.59 --projecttype drupal7 --docroot .
  • ddev start

Testing

  • Initial Setup Using Existing DB from v1.0.0
    • Observed the following: "Migrating bind-mounted database in ~/.ddev to docker-volume mounted database"
    • This command did not report any errors, but I need to run a full test starting with commerce kickstart on v1.0.0 and then verifying the DB comes with it on the next release.
    • I see a snapshot in .ddev/drupal-7.59_volume_migration_snapshot_20180810084455
  • Verify Docker Volume Exists
    • docker volume ls shows a volume name "drupal-7.59-mariadb"
  • Verify Snapshot Generation
    • Import db1 with ddev import-db --src ~/Downloads/d7tester_test_1.sql.gz
    • Verified 1 node with sitename "d7 tester test 1"
    • ddev remove --remove-data creates a snapshot in the projects .ddev folder in .ddev/db_snapshots/
    • Attempt to run ddev snapshot while in a stopped state.
      • This triggers a ddev start and then a ddev snapshot
      • Re running ddev snapshot saves another one very quickly.
  • Verify a revert
    • I ran a series of imports and snapshots. See
➜  drupal-7.59 ddev import-db --src ~/Downloads/d7tester_test_2.sql.gz
Ensuring write permissions for drupal-7.59
Existing settings.php file includes settings.ddev.php
Successfully imported database for drupal-7.59
➜  drupal-7.59 ddev snapshot
Creating database snapshot drupal-7.59_20180810085634
Created database snapshot drupal-7.59_20180810085634 in /Users/rmanelius/git/drupal-7.59/.ddev/db_snapshots/drupal-7.59_20180810085634
Created snapshot drupal-7.59_20180810085634
➜  drupal-7.59 ddev import-db --src ~/Downloads/d7tester_test_1.sql.gz
Ensuring write permissions for drupal-7.59
Existing settings.php file includes settings.ddev.php
Successfully imported database for drupal-7.59
➜  drupal-7.59 ddev snapshot
Creating database snapshot drupal-7.59_20180810085721
Created database snapshot drupal-7.59_20180810085721 in /Users/rmanelius/git/drupal-7.59/.ddev/db_snapshots/drupal-7.59_20180810085721
Created snapshot drupal-7.59_20180810085721
  • Testing a revert to the d7tester_test_2.sql.gz database.
    • ddev restore-snapshot
      • This brings up the help screen (e.g. ddev restore-snapshot --help)
    • Added a specific snapshot to and re-run.
      • Observed ddev stop and start and then an import.
  • Testing a revert to the d7tester_test_2.sql.gz snapshot.
  • LOVE the --name flag to name them!
  • Verifying v1.0.0 to v1.1.0 Migration
    • Rolling back to v1.0.0 binary
    • Configured a fresh site and created a node
    • Upgraded to current ddev binary w/commit
    • Run ddev start
      • Note the prompt to update configuration
      • Containers stopped and starting.
      • I see an error: "Failed to start drupal-7.59: Failed to migrate db from bind-mounted db: Unable to rename /Users/rmanelius/.ddev/drupal-7.59/mysql to /Users/rmanelius/.ddev/drupal-7.59/mysql_migrated.bak; you can remove the directory manually if that's ok: rename /Users/rmanelius/.ddev/drupal-7.59/mysql /Users/rmanelius/.ddev/drupal-7.59/mysql_migrated.bak: file exists"
      • I already had a /Users/rmanelius/.ddev/drupal-7.59/mysql_migrated.bak, so I named it something different.
      • Re-running ddev start must have picked up the old db.
  • Verifying v1.0.0 to v1.1.0 Migration: 2nd Attempt
    • Cleaning out global .ddev/drupal-7.59 folder and full drupal-7.59 project (repo, db, etc)
    • Set up with v1.0.0
    • proceed through site install and make 1 node
    • Confirm docker volume ls doesn't show the mount.
    • Confirm /Users/rmanelius/.ddev/drupal-7.59/mysql exists
    • Run ddev start
      • Observed
Your /Users/rmanelius/git/drupal-7.59/.ddev/config.yaml version is v1.0.0, but ddev is version v1.0.0-87-g7e9fc408.
Please run 'ddev config' to update your config.yaml.
ddev may not operate correctly until you do.
Migrating bind-mounted database in ~/.ddev to docker-volume mounted database
The DDEV_DATADIR variable is not set. Defaulting to a blank string.
* Noted the docker volume is there now.
* Noded the global .ddev folder now containes drupal-7.59/mysql_migrated.bak
  • So my mistake before was toggling back and forth too many times. I should have just deleted the .bak file of the previous test and I should have been fine during the transition.
  • CONFIRMED!

Recommendations

  • Any reason we fully ddev rm and ddev start before running a snapshot? From a speed perspective, it would seem the speed would be far faster to jump back and forth between snapshots. I realize this may be more error prone.
  • When running ddev restore-snapshot without a parameter, it would be helpful to run a ls -la of the snapshots directory to quickly get access to them.
    • Future wishlist item would be to get the list of the 5 last ones and pick one. Combined with not requiring a full shut down (if stability isn't compromised) would result in a fast experience togglign between older databases.

Copy link
Contributor

@andrewfrench andrewfrench left a comment

Choose a reason for hiding this comment

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

There are lots of database updates here that I don't fully understand, but the Golang stuff is all reasonable and the functional tests look good. 👍

@rfay
Copy link
Member Author

rfay commented Aug 11, 2018

@rickmanelius it doesn't run rm/start when creating a snapshot.

It does run rm/start when restoring a snapshot, and the reason is the mechanism: At startup time we either use the existing db or create one from scratch. In the past, we always created from scratch using the db baked into the image (which sped up startup times quite a lot). Now, with this PR, we use the exact same code, but if we have a snapshot specified it creates from the snapshot, so the basic code remains the same. I'm sure there's more than one way to do this.

@rfay rfay merged commit 54950bd into ddev:master Aug 11, 2018
@rfay rfay deleted the 20180711_volume branch December 13, 2019 14:26
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.

None yet

3 participants