Merge lp:~verterok/charms/trusty/mongodb/fix-data_directory-migration into lp:charms/trusty/mongodb

[Guillermo Gonzalez]

Add flag to signal when the new_mongo_dir was just created and migrate the existing data directory.

Add flag to signal when the new_mongo_dir was just created and migrate the existing data directory.

Current code in trunk, will never migrate the data into the new_mongo_dir as the conditional will skip that code path.

In order to reproduce this, you need a non-local provider. I used this juju-deployer config: http://paste.ubuntu.com/11392167/

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #4844 mongodb for verterok mp260290
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/4844/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #4524 mongodb for verterok mp260290
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/4524/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #4335 mongodb for verterok mp260290
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/4335/

[Adam Israel (aisrael)]

Hi Guillermo,

Thanks for your work on improving the mongodb charm! I had a chance to this merge proposal today and wanted to provide you with some feedback.

The changes you've made look to be straightforward, and I suspect that they work as intended, but I'd prefer to see a simple amulet test added to verify the migration works as expected. With that in place, I'd have no problem giving this my approval.

Thanks!

review: Needs Fixing

[Guillermo Gonzalez (verterok)]

Hi Adam,

Thanks for the prompt reply.
I tried to get an amulet test for this, but I cannot find a way to test this without making a really complex test case, e.g: adding 2 more charms (storage and block-storage-provider) plus handling credentials for the cloud that's used to run tests.

Also tried to use the local prodiver (for the storage charm), but found a new bug in the mongo charm, it fails if the provider is local (just a directory) as it tried to find the device...probably a mix of old volume-map support code and data relation handling

Is there a simpler way to test this with amulet?

As an alternative, I could try to write a unittest for the fix in config_changed_volume_apply function.

Cheers

[Guillermo Gonzalez (verterok)]

Hi Adam,

Thanks for the prompt reply.
I tried to get an amulet test for this, but I cannot find a way to test this without making a really complex test case, e.g: adding 2 more charms (storage and block-storage-provider) plus handling credentials for the cloud that's used to run tests.

Also tried to use the local prodiver (for the storage charm), but found a new bug in the mongo charm, it fails if the provider is local (just a directory) as it tried to find the device...probably a mix of old volume-map support code and data relation handling

Is there a simpler way to test this with amulet?

As an alternative, I could try to write a unittest for the fix in config_changed_volume_apply function.

Cheers

[lp:~verterok/charms/trusty/mongodb/fix-data_directory-migration]

72. By Guillermo Gonzalez on 2015-05-29

fix local storage support

73. By Guillermo Gonzalez on 2015-06-02

add amulet test

[Guillermo Gonzalez (verterok)]

Hi Adam,

Fixed the local provider issue and added the amulet test.

Cheers

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #4622 mongodb for verterok mp260290
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/4622/

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #4942 mongodb for verterok mp260290
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/4942/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #4393 mongodb for verterok mp260290
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/4393/

[Adam Israel (aisrael)]

Hey Guillermo,

I'm running into an error while testing, against both local and amazon providers. It looks like this might be the root cause:

ERROR: volume_init_and_mount: coult not find an unused device for regexp: /dev/vd[b-z]

Any ideas?

$ ./04_deploy_with_storage.py
2015-06-04 17:25:37 Starting deployment of amazon
2015-06-04 17:25:39 Deploying services...
2015-06-04 17:25:41 Deploying service mongodb using cs:trusty/mongodb-21
2015-06-04 17:26:00 Deploying service storage using cs:~chris-gondolin/trusty/storage-5
2015-06-04 17:26:19 Config specifies num units for subordinate: storage
2015-06-04 17:31:42 Adding relations...
2015-06-04 17:31:43 Exposing service 'mongodb'
2015-06-04 17:31:43 Deployment complete in 366.42 seconds
Adding storage relation, and sleeping for 2 min.
Traceback (most recent call last):
  File "./04_deploy_with_storage.py", line 86, in <module>
    validate_status()
  File "./04_deploy_with_storage.py", line 52, in validate_status
    d.sentry.wait_for_status(d.juju_env, ['mongodb'])
  File "/usr/lib/python3/dist-packages/amulet/sentry.py", line 260, in wait_for_status
    unit, unit_dict.get('agent-state-info')))
Exception: Error on unit mongodb/0: hook failed: "data-relation-changed" for storage:data

nged" for storage:data
(venv)vagrant@vagrant-ubuntu-trusty-64:/charms/trusty/mongodb/tests$ juju pprint
- mongodb/0: 52.1.122.247 (error) 27017/tcp, 27019/tcp, 27021/tcp, 28017/tcp
  - storage/0: 52.1.122.247 (started)
- mongodb/1: 54.152.170.105 (error) 27017/tcp, 27019/tcp, 27021/tcp, 28017/tcp
  - storage/1: 54.152.170.105 (started)

Here are pastebins of the logs:

mongodb/0:
http://paste.ubuntu.com/11571075/

storage/0:
http://paste.ubuntu.com/11571095/

mongodb/1:
http://paste.ubuntu.com/11571093/

storage/1:
http://paste.ubuntu.com/11571098/

review: Needs Fixing

[Guillermo Gonzalez (verterok)]

Hi Adam,

It looks like it's using the current trunk of the charm instead of the local one.
That's the data-relation-changed bug I had to fix in order to use the storage subordinate with the local provider in the amulet test.

Please check if the charm code in the units matches the one in the branch or trunk.

Thanks.

[Ryan Beisner (1chb1n)]

FYI - amulet output: http://paste.ubuntu.com/11587737/

[Tim Van Steenburgh (tvansteenburgh)]

I'm +1 on this as it passes the OSCI tests.

review: Approve

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2015-04-22 13:24:09 +0000
+++ hooks/hooks.py	2015-06-02 05:33:28 +0000
@@ -1547,7 +1547,8 @@
     assert(data_directory_path)
     volid = volume_get_volume_id()
     if volid:
-        if volume_is_permanent(volid):
+        volid_from_subordinate = volume_get_id_for_storage_subordinate()
+        if volume_is_permanent(volid) and not volid_from_subordinate:
             if not volume_init_and_mount(volid):
                 juju_log(
                     "volume_init_and_mount failed, not applying changes")
@@ -1581,9 +1582,11 @@
 
         # Create a directory structure below "new" mount_point
         curr_dir_stat = os.stat(data_directory_path)
+        new_mongo_dir_just_created = False
         if not os.path.isdir(new_mongo_dir):
             juju_log("mkdir %s" % new_mongo_dir)
             os.mkdir(new_mongo_dir)
+            new_mongo_dir_just_created = True
             # copy permissions from current data_directory_path
             os.chown(new_mongo_dir, curr_dir_stat.st_uid, curr_dir_stat.st_gid)
             os.chmod(new_mongo_dir, curr_dir_stat.st_mode)
@@ -1595,7 +1598,7 @@
         if not stop_hook():
             juju_log("stop_hook() failed - can't migrate data.")
             return False
-        if not os.path.exists(new_mongo_dir):
+        if not os.path.exists(new_mongo_dir) or new_mongo_dir_just_created:
             juju_log("migrating mongo data {}/ -> {}/".format(
                 data_directory_path, new_mongo_dir))
             # void copying PID file to perm storage (shouldn't be any...)

=== added file 'tests/04_deploy_with_storage.py'
--- tests/04_deploy_with_storage.py	1970-01-01 00:00:00 +0000
+++ tests/04_deploy_with_storage.py	2015-06-02 05:33:28 +0000
@@ -0,0 +1,87 @@
+#!/usr/bin/env python3
+
+import amulet
+import time
+from pymongo import MongoClient
+from collections import Counter
+
+
+#########################################################
+# Test Quick Config
+#########################################################
+scale = 2
+seconds = 1800
+
+# amount of time to wait before testing for replicaset
+# status
+wait_for_replicaset = 30
+# amount of time to wait for the data relation
+wait_for_relation = 60*2
+
+#########################################################
+# 3shard cluster configuration
+#########################################################
+d = amulet.Deployment(series='trusty')
+
+d.add('mongodb', units=scale, series='trusty')
+d.add('storage', charm='cs:~chris-gondolin/trusty/storage-5', series='trusty')
+d.configure('storage', {'provider': 'local'})
+
+d.expose('mongodb')
+
+# Perform the setup for the deployment.
+try:
+    d.setup(seconds)
+    d.sentry.wait(seconds)
+except amulet.helpers.TimeoutError:
+    message = 'The environment did not setup in %d seconds.', seconds
+    amulet.raise_status(amulet.SKIP, msg=message)
+except:
+    raise
+
+sentry_dict = {
+    'mongodb0-sentry': d.sentry.unit['mongodb/0'],
+    'mongodb1-sentry': d.sentry.unit['mongodb/1'],
+}
+
+
+#############################################################
+# Check agent status
+#############################################################
+def validate_status():
+    d.sentry.wait_for_status(d.juju_env, ['mongodb'])
+
+
+#############################################################
+# Validate proper replicaset setup
+#############################################################
+def validate_replicaset_setup():
+
+    time.sleep(wait_for_replicaset)
+
+    unit_status = []
+
+    for service in sentry_dict:
+        client = MongoClient(sentry_dict[service].info['public-address'])
+        r = client.admin.command('replSetGetStatus')
+        unit_status.append(r['myState'])
+        client.close()
+
+    primaries = Counter(unit_status)[1]
+    if primaries != 1:
+        message = "Only one PRIMARY unit allowed! Found: %s" % (primaries)
+        amulet.raise_status(amulet.FAIL, message)
+
+    secondrs = Counter(unit_status)[2]
+    if secondrs != 1:
+        message = "Only one SECONDARY unit allowed! (Found %s)" % (secondrs)
+        amulet.raise_status(amulet.FAIL, message)
+
+
+validate_status()
+validate_replicaset_setup()
+print("Adding storage relation, and sleeping for 2 min.")
+d.relate('mongodb:data', 'storage:data')
+time.sleep(wait_for_relation)
+validate_status()
+validate_replicaset_setup()