Merge branch 'review/ziyiwang/review/aluria/348391'

This branch contains two subbranches: * ziyiwang/bug/1879803 * ziyiwang/bug/1879842 Reviewed-on: https://code.launchpad.net/~ziyiwang/charm-mongodb/+git/charm-mongodb/+merge/384400 Reviewed-by: Alvaro Uria <alvaro.uria@canonical.com> Signed-off-by: Alvaro Uria <alvaro.uria@canonical.com> 

22 files changed, 200 insertions, 142 deletions

diff --git a/Makefile b/Makefile
index 4d444f8..2fdc717 100644
--- a/Makefile
+++ b/Makefile
@@ -13,24 +13,17 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 
-PYTHON := /usr/bin/env python
-ACTIONS := $(shell grep -slE '\#!(.*)python' actions/*)
+PYTHON := /usr/bin/env python3
+ACTIONS := $(shell grep -slE '\#!(.*)python3' actions/*)
+PROJECTPATH := $(realpath $(dir $(realpath $(firstword $(MAKEFILE_LIST)))))
 
-clean:
-	rm -f .coverage
-	find . -name '*.pyc' -delete
-	rm -rf .venv
-	rm -rf .tox
-	(which dh_clean && dh_clean) || true
 
-.venv:
-	sudo apt-get install -y gcc python-dev python-virtualenv python-apt
-	virtualenv .venv --system-site-packages
-	.venv/bin/pip install -I -r test_requirements.txt
+clean:
+	git clean -fXd
 
-lint: .venv
-	.venv/bin/flake8 --exclude hooks/charmhelpers actions $(ACTIONS) hooks tests unit_tests
-	.venv/bin/charm-proof
+lint:
+	@echo Starting flake8...
+	@tox -e lint
 
 unit:
	@echo Starting unit tests...
@@ -38,17 +31,18 @@ unit:
 
 functional:
	@echo Starting functional tests...
-	rm -rf .venv # rm the python2 venv from unittests as it fails the juju deploy
-	@tox -e functional
+	@@CHARM_BUILD_DIR="$(PROJECTPATH)" tox -e functional
+
+test: lint unit functional
 
 sync:
	@mkdir -p bin
	@curl -o bin/charm_helpers_sync.py https://raw.githubusercontent.com/juju/charm-helpers/master/tools/charm_helpers_sync/charm_helpers_sync.py
	@$(PYTHON) bin/charm_helpers_sync.py -c charm-helpers-sync.yaml
 
-publish: lint unit_test
-	bzr push lp:charms/mongodb
-	bzr push lp:charms/trusty/mongodb
+# publish: lint unit
+#	bzr push lp:charms/mongodb
+#	bzr push lp:charms/trusty/mongodb
 
 # The targets below don't depend on a file
 .PHONY: lint test unittest functional publish sync
diff --git a/README.md b/README.md
index 3629f6e..3b3560c 100644
--- a/README.md
+++ b/README.md
@@ -85,7 +85,7 @@ public-address and port 28017 ( ie: http://ec2-50-17-73-255.compute-1.amazonaws.
 
 ### (Optional) Change the replicaset name
 
- juju set mongodb replicaset=<new_replicaset_name>
+ juju config mongodb replicaset=<new_replicaset_name>
 
 ### Add one or more nodes to your replicaset
 
diff --git a/actions/backup.py b/actions/backup.py
index cac42cd..0930eab 100644
--- a/actions/backup.py
+++ b/actions/backup.py
@@ -4,8 +4,8 @@ import os
 try:
 from charmhelpers.core.hookenv import action_get, action_set, action_fail
 except ImportError:
- subprocess.check_call(['apt-get', 'install', '-y', 'python-pip'])
- subprocess.check_call(['pip', 'install', 'charmhelpers'])
+ subprocess.check_call(['apt-get', 'install', '-y', 'python3-pip'])
+ subprocess.check_call(['pip3', 'install', 'charmhelpers'])
 from charmhelpers.core.hookenv import action_get, action_set, action_fail
 
 
@@ -35,7 +35,7 @@ def restore():
 def backup_command(cmd, args, dir):
 try:
 mkdir(dir)
- except OSError as e:
+ except OSError:
 pass # Ignoring, the directory already exists
 except Exception as e:
 action_set({"directory creation exception": e})
diff --git a/actions/backup_test.py b/actions/backup_test.py
index d6c2647..b5f1d33 100644
--- a/actions/backup_test.py
+++ b/actions/backup_test.py
@@ -8,44 +8,45 @@ def mock_action_get(key):
 
 
 class TestBackups(unittest.TestCase):
-
- def test_dump(self):
- mkdir = create_autospec(backup.mkdir, return_value=True)
- execute = create_autospec(backup.execute, return_value="output")
- action_set = create_autospec(backup.action_set, return_value=None)
- action_fail = create_autospec(backup.action_fail, return_value=None)
- backup.mkdir = mkdir
- backup.execute = execute
- backup.action_set = action_set
- backup.action_fail = action_fail
+ def setUp(self):
+ self.original = [
+ backup.mkdir,
+ backup.execute,
+ backup.action_set,
+ backup.action_fail,
+ backup.action_get,
+ ]
+ backup.mkdir = create_autospec(backup.mkdir, return_value=True)
+ backup.execute = create_autospec(backup.execute, return_value="output")
+ backup.action_set = create_autospec(backup.action_set, return_value=None)
+ backup.action_fail = create_autospec(backup.action_fail, return_value=None)
 backup.action_get = mock_action_get
 
+ def tearDown(self):
+ (
+ backup.mkdir,
+ backup.execute,
+ backup.action_set,
+ backup.action_fail,
+ backup.action_get,
+ ) = self.original
+
+ def test_dump(self):
 backup.dump()
 
- mkdir.assert_called_once_with("this/dir")
- execute.assert_called_once_with("mongodump -arg1 -arg2", "this/dir")
- action_set.assert_has_calls([
- call({"command": "mongodump -arg1 -arg2",
- "working-dir": "this/dir"}),
+ backup.mkdir.assert_called_once_with("this/dir")
+ backup.execute.assert_called_once_with("mongodump -arg1 -arg2", "this/dir")
+ backup.action_set.assert_has_calls([
+ call({"command": "mongodump -arg1 -arg2", "working-dir": "this/dir"}),
 call({"output": "output"})])
 
 def test_restore(self):
- mkdir = create_autospec(backup.mkdir, return_value=True)
- execute = create_autospec(backup.execute, return_value="output")
- action_set = create_autospec(backup.action_set, return_value=None)
- action_fail = create_autospec(backup.action_fail, return_value=None)
- backup.mkdir = mkdir
- backup.execute = execute
- backup.action_set = action_set
- backup.action_fail = action_fail
-
 backup.restore()
 
- mkdir.assert_called_once_with("this/dir")
- execute.assert_called_once_with("mongorestore -arg1 -arg2", "this/dir")
- action_set.assert_has_calls([
- call({"command": "mongodump -arg1 -arg2",
- "working-dir": "this/dir"}),
+ backup.mkdir.assert_called_once_with("this/dir")
+ backup.execute.assert_called_once_with("mongorestore -arg1 -arg2", "this/dir")
+ backup.action_set.assert_has_calls([
+ call({"command": "mongorestore -arg1 -arg2", "working-dir": "this/dir"}),
 call({"output": "output"})])
 
 
diff --git a/actions/dump b/actions/dump
index a2a1ea1..bfb9c59 100755
--- a/actions/dump
+++ b/actions/dump
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 import backup
 
 backup.dump()
diff --git a/actions/perf b/actions/perf
index d3ff8a6..444987a 100755
--- a/actions/perf
+++ b/actions/perf
@@ -1,17 +1,22 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 import signal
 import subprocess
 import os
 import json
 import re
 from tempfile import NamedTemporaryFile
-from distutils.spawn import find_executable
+
+try:
+ from distutils.spawn import find_executable
+except ImportError:
+ subprocess.check_call(['apt-get', 'install', '-y', 'python3-distutils'])
+ from distutils.spawn import find_executable
 
 try:
 from charms.benchmark import Benchmark
 except ImportError:
- subprocess.check_call(['apt-get', 'install', '-y', 'python-pip'])
- subprocess.check_call(['pip', 'install', '-U', 'charms.benchmark'])
+ subprocess.check_call(['apt-get', 'install', '-y', 'python3-pip'])
+ subprocess.check_call(['pip3', 'install', '-U', 'charms.benchmark'])
 from charms.benchmark import Benchmark
 
 
@@ -22,7 +27,7 @@ def handler(signum, frame):
 def action_set(key, val):
 action_cmd = ['action-set']
 if isinstance(val, dict):
- for k, v in val.iteritems():
+ for k, v in val.items():
 action_set('%s.%s' % (key, k), v)
 return
 
@@ -52,14 +57,14 @@ def main():
 js['nThreads'] = int(action_get('nthreads'))
 js['fileSizeMB'] = int(action_get('fileSizeMB'))
 js['sleepMicros'] = int(action_get('sleepMicros'))
- js['mmf'] = action_get('mmf')
- js['r'] = action_get('r')
- js['w'] = action_get('w')
+ js['mmf'] = action_get('mmf').decode()
+ js['r'] = action_get('r').decode()
+ js['w'] = action_get('w').decode()
 js['recSizeKB'] = int(action_get('recSizeKB'))
 js['syncDelay'] = int(action_get('syncDelay'))
 
 config = NamedTemporaryFile(delete=False)
- config.write(json.dumps(js))
+ config.write(json.dumps(js).encode())
 config.close()
 config = open(config.name, 'r')
 
@@ -75,8 +80,8 @@ def main():
 os.waitpid(p.pid, 0)
 except subprocess.CalledProcessError as e:
 rc = e.returncode
- print "Exit with error code %d" % rc
- except IOError as e:
+ print("Exit with error code %d" % rc)
+ except IOError:
 signal.alarm(0)
 os.kill(p.pid, signal.SIGKILL)
 finally:
@@ -103,7 +108,7 @@ def main():
 
 action_set(
 "results.average",
- {'value': sum(scores) / float(len(scores)), 'units': 'ops/sec'}
+ {'value': sum(scores) / len(scores), 'units': 'ops/sec'}
 )
 action_set(
 "results.max",
@@ -115,7 +120,7 @@ def main():
 )
 
 Benchmark.set_composite_score(
- sum(scores) / float(len(scores)),
+ sum(scores) / len(scores),
 'ops/sec',
 'desc'
 )
diff --git a/actions/restore b/actions/restore
index 171813a..ecac7b7 100755
--- a/actions/restore
+++ b/actions/restore
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 import backup
 
 backup.restore()
diff --git a/hooks/hooks.py b/hooks/hooks.py
index a4a3ee5..5bd7ce7 100755
--- a/hooks/hooks.py
+++ b/hooks/hooks.py
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 '''
 Created on Aug 1, 2012
 
@@ -6,11 +6,10 @@ Created on Aug 1, 2012
 '''
 
 import collections
-import commands
 import distutils
 import json
 import os
-import platform
+import pip
 import pprint
 import re
 import signal
@@ -20,6 +19,12 @@ import sys
 import time
 
 try:
+ import distro # flake8: noqa
+except ImportError:
+ pip.main(['install', "distro"])
+ import distro # flake8: noqa
+
+try:
 import yaml # flake8: noqa
 except ImportError:
 if sys.version_info.major == 2:
@@ -80,7 +85,10 @@ try:
 from pymongo import MongoClient
 from pymongo.errors import OperationFailure
 except ImportError:
- apt_install("python-pymongo", fatal=True)
+ if sys.version_info.major == 2:
+ apt_install("python-pymongo", fatal=True)
+ else:
+ apt_install("python3-pymongo", fatal=True)
 from pymongo import MongoClient
 from pymongo.errors import OperationFailure
 
@@ -98,7 +106,7 @@ default_mongos_list = "/etc/mongos.list"
 default_wait_for = 3
 default_max_tries = 7
 
-INSTALL_PACKAGES = ['mongodb-server', 'python-yaml']
+INSTALL_PACKAGES = ['mongodb-server', 'python3-yaml']
 
 # number of seconds init_replset will pause while looping to check if
 # replicaset is initialized
@@ -129,8 +137,9 @@ was_i_primary = False
 
 
 def is_bionic_or_greater():
- current_version = platform.linux_distribution()[1]
- if distutils.version.LooseVersion(current_version) >= distutils.version.LooseVersion('18.04'):
+ current_version = distro.linux_distribution()[1]
+ if distutils.version.LooseVersion(current_version) >= \
+ distutils.version.LooseVersion('18.04'):
 return True
 
 
@@ -229,7 +238,8 @@ def process_check_pidfile(pidfile=None):
 
 
 def is_valid_ip(bind_ip):
- if bind_ip == unit_get("private-address") or bind_ip == unit_get("public-address"):
+ if bind_ip == unit_get("private-address") or \
+ bind_ip == unit_get("public-address"):
 return True
 return False
 
@@ -265,7 +275,7 @@ class TimeoutException(Exception):
 ###############################################################################
 # Charm support functions
 ###############################################################################
-def mongodb_conf(config_data=None):
+def mongodb_conf(config_data=None): # noqa: C901 is too complex (28)
 if config_data is None:
 return(None)
 config = []
@@ -492,7 +502,7 @@ def mongo_client_smart(host='localhost', command=None):
 
 for i in range(MONGO_CLIENT_RETRIES):
 try:
- cmd_output = subprocess.check_output(cmd_line)
+ cmd_output = subprocess.check_output(cmd_line).decode("utf8")
 juju_log('mongo_client_smart executed, output: %s' %
 cmd_output)
 if json.loads(cmd_output)['ok'] == 1:
@@ -712,17 +722,16 @@ def configsvr_status(wait_for=default_wait_for, max_tries=default_max_tries):
 current_try = 0
 
 while (process_check_pidfile('/var/run/mongodb/configsvr.pid') != (
- None, None)) and not port_check(
- unit_get('private-address'),
- config_data['config_server_port']) and current_try < max_tries:
+ None, None)) and not port_check(
+ unit_get('private-address'),
+ config_data['config_server_port']) and current_try < max_tries:
 
 juju_log("configsvr_status: Waiting for Config Server to be ready ...")
 time.sleep(wait_for)
 current_try += 1
- retVal = (
- process_check_pidfile('/var/run/mongodb/configsvr.pid') != (None, None)
- ) == port_check(unit_get('private-address'),
- config_data['config_server_port']) is True
+ retVal = (process_check_pidfile('/var/run/mongodb/configsvr.pid') != (None, None)
+ ) == port_check(unit_get('private-address'),
+ config_data['config_server_port']) is True
 if retVal:
 return(process_check_pidfile('/var/run/mongodb/configsvr.pid'))
 else:
@@ -760,7 +769,7 @@ def enable_configsvr(config_data, wait_for=default_wait_for,
 return(False)
 
 # Stop any running config servers
- disable_configsvr()
+ disable_configsvr(config_data["config_server_port"])
 
 # Make sure dbpath and logpath exist
 subprocess.call(
@@ -769,23 +778,25 @@ def enable_configsvr(config_data, wait_for=default_wait_for,
 '-p',
 '%s' % config_data['config_server_dbpath']
 ]
- )
+ )
 subprocess.call(
 [
 'mkdir',
 '-p',
 '%s' % os.path.dirname(config_data['config_server_logpath'])
 ]
- )
+ )
 
 # Start the config server
 juju_log("enable_configsvr: Starting the config server")
 cmd_line = "mongod"
 cmd_line += " --configsvr"
+ cmd_line += " --bind_ip {}".format(choose_bind_ip(config_data["bind_ip"]))
 cmd_line += " --port %d" % config_data['config_server_port']
 cmd_line += " --dbpath %s" % config_data['config_server_dbpath']
 cmd_line += " --logpath %s" % config_data['config_server_logpath']
 cmd_line += " --pidfilepath /var/run/mongodb/configsvr.pid"
+ cmd_line += " --replSet {}".format(config_data["replicaset"])
 cmd_line += " --fork"
 subprocess.call(cmd_line, shell=True)
 
@@ -845,7 +856,7 @@ def disable_mongos(port=None):
 return(retVal)
 
 
-def enable_mongos(config_data=None, config_servers=None,
+def enable_mongos(config_data=None, config_servers=None, replicaset=None,
 wait_for=default_wait_for, max_tries=default_max_tries):
 juju_log("enable_mongos")
 if config_data is None or config_servers is None:
@@ -855,6 +866,8 @@ def enable_mongos(config_data=None, config_servers=None,
 juju_log("enable_mongos: config_servers must be a list")
 return(False)
 if len(config_servers) < 3:
+ # MongoDB 3.2 deprecates the use of three mirrored mongod instances
+ # for config servers.
 juju_log("enable_mongos: Not enough config servers yet...")
 return(True)
 disable_mongos()
@@ -865,15 +878,16 @@ def enable_mongos(config_data=None, config_servers=None,
 '-p',
 '%s' % os.path.dirname(config_data['mongos_logpath'])
 ]
- )
+ )
 cmd_line = "mongos"
 cmd_line += " --logpath %s" % config_data['mongos_logpath']
 cmd_line += " --pidfilepath /var/run/mongodb/mongos.pid"
 cmd_line += " --port %d" % config_data['mongos_port']
 cmd_line += " --fork"
- if len(config_servers) > 0:
- if len(config_servers) >= 3:
- cmd_line += ' --configdb %s' % ','.join(config_servers[0:3])
+ # Note(aluria): --configdb used to have a list of comma-separated
+ # server:port values. Such list now needs to be prepended by
+ # repSetConfigName/
+ cmd_line += ' --configdb {}/{}'.format(replicaset, ','.join(config_servers[0:3]))
 juju_log("enable_mongos: cmd_line: %s" % cmd_line)
 subprocess.call(cmd_line, shell=True)
 retVal = mongos_ready(wait_for, max_tries)
@@ -967,7 +981,7 @@ PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
 
 # We can remove this quirk when charm no longer supports trusty
 def arm64_trusty_quirk():
- arch = subprocess.check_output(['dpkg', '--print-architecture']).strip()
+ arch = subprocess.check_output(['dpkg', '--print-architecture']).decode("utf8").strip()
 if arch != 'arm64':
 return
 if lsb_release()['DISTRIB_CODENAME'] != 'trusty':
@@ -998,7 +1012,7 @@ def install_hook():
 
 
 @hooks.hook('config-changed')
-def config_changed():
+def config_changed(): # noqa: C901 is too complex (17)
 juju_log("Entering config_changed")
 status_set('maintenance', 'Configuring unit')
 config_data = config()
@@ -1092,7 +1106,8 @@ def config_changed():
 juju_log("config_changed: Exception: %s" % str(e))
 
 if configsvr_pid is not None:
- configsvr_port = re.search(r'--port (\w+)', configsvr_cmd_line).group(2)
+ configsvr_port = re.search(r'--port (\w+)',
+ configsvr_cmd_line).group(2)
 disable_configsvr(configsvr_port)
 enable_configsvr(config_data['config_server_port'])
 else:
@@ -1152,9 +1167,9 @@ def benchmark_relation_joined():
 try:
 from charms.benchmark import Benchmark
 except ImportError:
- apt_install('python-pip', fatal=True)
+ apt_install('python3-pip', fatal=True)
 import subprocess
- subprocess.check_call(['pip', 'install', '-U', 'charms.benchmark'])
+ subprocess.check_call(['pip3', 'install', '-U', 'charms.benchmark'])
 from charms.benchmark import Benchmark
 
 # Send a list of benchmark-enabled actions for display in the benchmark-gui
@@ -1208,7 +1223,7 @@ def replica_set_relation_joined():
 'replset': my_replset,
 'install-order': my_install_order,
 'type': 'replset',
- })
+ })
 
 update_status()
 juju_log("replica_set_relation_joined-finish")
@@ -1232,7 +1247,8 @@ def rs_add(host):
 subprocess.check_output(cmd_line)
 r = run_admin_command(c, 'replSetGetStatus')
 members = r["members"]
- ok = [m for m in members if m['name'] == host and m['state'] == MONGO_SECONDARY]
+ ok = [m for m in members
+ if m['name'] == host and m['state'] == MONGO_SECONDARY]
 if ok:
 return ok
 
@@ -1314,9 +1330,10 @@ def get_mongod_version():
 return c.server_info()['version']
 
 
-# Retry until the replica set is in active state, retry 45 times before failing,
-# wait 1, 2, 3, 4, ... seconds between each retry, this will add 33 minutes of
-# accumulated sleep()
+# Retry until the replica set is in active state,
+# retry 45 times before failing,
+# wait 1, 2, 3, 4, ... seconds between each retry,
+# this will add 33 minutes of accumulated sleep()
 @retry_on_exception(num_retries=45, base_delay=1)
 def wait_until_replset_is_active():
 status = update_status()
@@ -1443,10 +1460,12 @@ def configsvr_relation_joined():
 my_hostname = unit_get('private-address')
 my_port = config('config_server_port')
 my_install_order = os.environ['JUJU_UNIT_NAME'].split('/')[1]
+ my_replicaset = config('replicaset')
 relation_set(relation_id(), {
 'hostname': my_hostname,
 'port': my_port,
 'install-order': my_install_order,
+ 'replset': my_replicaset,
 'type': 'configsvr',
 })
 
@@ -1457,6 +1476,9 @@ def configsvr_relation_changed():
 config_data = config()
 my_port = config_data['config_server_port']
 disable_configsvr(my_port)
+ retVal = enable_configsvr(config_data)
+ juju_log("configsvr_relation_changed returns: %s" % retVal)
+ return(retVal)
 
 
 @hooks.hook('mongos-cfg-relation-joined')
@@ -1466,10 +1488,12 @@ def mongos_relation_joined():
 my_hostname = unit_get('private-address')
 my_port = config('mongos_port')
 my_install_order = os.environ['JUJU_UNIT_NAME'].split('/')[1]
+ my_replicaset = config('replicaset')
 relation_set(relation_id(), {
 'hostname': my_hostname,
 'port': my_port,
 'install-order': my_install_order,
+ 'replset': my_replicaset,
 'type': 'mongos'
 })
 
@@ -1483,6 +1507,7 @@ def mongos_relation_changed():
 
 hostname = relation_get('hostname')
 port = relation_get('port')
+ replicaset = relation_get('replset')
 rel_type = relation_get('type')
 if hostname is None or port is None or rel_type is None:
 juju_log("mongos_relation_changed: relation data not ready.",
@@ -1491,15 +1516,10 @@ def mongos_relation_changed():
 if rel_type == 'configsvr':
 config_servers = load_config_servers(default_mongos_list)
 juju_log("Adding config server: %s:%s" % (hostname, port), level=DEBUG)
- if hostname is not None and \
- port is not None and \
- hostname != '' and \
- port != '' and \
- "%s:%s" % (hostname,
- port) not in config_servers:
+ if hostname and port and "{}:{}".format(hostname, port) not in config_servers:
 config_servers.append("%s:%s" % (hostname, port))
 disable_mongos(config_data['mongos_port'])
- retVal = enable_mongos(config_data, config_servers)
+ retVal = enable_mongos(config_data, config_servers, replicaset)
 if retVal:
 update_file(default_mongos_list, '\n'.join(config_servers))
 elif rel_type == 'database':
@@ -1546,7 +1566,7 @@ def update_nrpe_config():
 host_context = rel['nagios_host_context']
 break
 nrpe = NRPE(hostname=hostname)
- apt_install('python-dbus')
+ apt_install('python3-dbus')
 
 if host_context:
 current_unit = "%s:%s" % (host_context, local_unit())
@@ -1562,7 +1582,7 @@ def update_nrpe_config():
 shortname='mongodb',
 description='process check {%s}' % current_unit,
 check_cmd=check_mongo_script,
- )
+ )
 
 nrpe.write()
 
@@ -1755,7 +1775,7 @@ def volume_init_and_mount(volid):
 
 def volume_get_all_mounted():
 command = ("mount |egrep /srv/juju")
- status, output = commands.getstatusoutput(command)
+ status, output = subprocess.getstatusoutput(command)
 if status != 0:
 return None
 return output
@@ -1771,7 +1791,7 @@ def volume_get_all_mounted():
 # - if fresh new storage dir: rsync existing data
 # - manipulate /var/lib/mongodb/VERSION/CLUSTER symlink
 #
-def config_changed_volume_apply():
+def config_changed_volume_apply(): # noqa: C901 is too complex (12)
 config_data = config()
 data_directory_path = config_data["dbpath"]
 assert(data_directory_path)
@@ -1911,4 +1931,3 @@ if __name__ == "__main__":
 hooks.execute(sys.argv)
 except UnregisteredHookError as e:
 juju_log('Unknown hook {} - skipping'.format(e))
-
diff --git a/hooks/install b/hooks/install
index 8712bfa..baffb57 100755
--- a/hooks/install
+++ b/hooks/install
@@ -16,7 +16,7 @@ check_and_install() {
 fi
 }
 
-PYTHON="python"
+PYTHON="python3"
 
 for dep in ${DEPS[@]}; do
 check_and_install ${PYTHON} ${dep}
diff --git a/metadata.yaml b/metadata.yaml
index 399301a..7992cec 100644
--- a/metadata.yaml
+++ b/metadata.yaml
@@ -18,9 +18,9 @@ description: |
 tags:
 - databases
 series:
- - xenial
- - artful
+ - focal
 - bionic
+ - xenial
 - trusty
 provides:
 nrpe-external-master:
diff --git a/tests/bundles/bionic-shard.yaml b/tests/bundles/bionic-shard.yaml
index b7ee354..215c095 100644
--- a/tests/bundles/bionic-shard.yaml
+++ b/tests/bundles/bionic-shard.yaml
@@ -2,25 +2,25 @@ series: bionic
 description: "mongodb-charm test bundle"
 applications:
 configsvr:
- charm: "../../."
- num_units: 1
+ num_units: 3
 options:
 replicaset: configsvr
 mongodb:
- charm: "../../."
 num_units: 1
 options:
 replicaset: testset
 shard1:
- charm: "../../."
 num_units: 1
 options:
 replicaset: shard1
 shard2:
- charm: "../../."
 num_units: 1
 options:
 replicaset: shard2
+ shard3:
+ num_units: 1
+ options:
+ replicaset: shard3
 relations:
 - [ "configsvr:configsvr", "mongodb:mongos-cfg" ]
 - [ "mongodb:mongos", "shard1:database" ]
diff --git a/tests/bundles/bionic.yaml b/tests/bundles/bionic.yaml
index ebfb98e..cef11a1 100644
--- a/tests/bundles/bionic.yaml
+++ b/tests/bundles/bionic.yaml
@@ -2,7 +2,6 @@ series: bionic
 description: "mongodb-charm test bundle"
 applications:
 mongodb:
- charm: "../../."
 num_units: 3
 options:
 replicaset: testset
diff --git a/tests/bundles/focal.yaml b/tests/bundles/focal.yaml
new file mode 100644
index 0000000..e485f5e
--- /dev/null
+++ b/tests/bundles/focal.yaml
@@ -0,0 +1,8 @@
+series: focal
+description: "mongodb-charm test bundle"
+applications:
+ mongodb:
+ num_units: 3
+ options:
+ replicaset: testset
+ backup_directory: /var/backups
diff --git a/tests/bundles/overlays/bionic-shard.yaml.j2 b/tests/bundles/overlays/bionic-shard.yaml.j2
new file mode 100644
index 0000000..8310b25
--- /dev/null
+++ b/tests/bundles/overlays/bionic-shard.yaml.j2
@@ -0,0 +1,9 @@
+applications:
+ configsvr:
+ charm: "{{ CHARM_BUILD_DIR }}"
+ shard1:
+ charm: "{{ CHARM_BUILD_DIR }}"
+ shard2:
+ charm: "{{ CHARM_BUILD_DIR }}"
+ shard3:
+ charm: "{{ CHARM_BUILD_DIR }}"
diff --git a/tests/bundles/overlays/local-charm-overlay.yaml.j2 b/tests/bundles/overlays/local-charm-overlay.yaml.j2
new file mode 100644
index 0000000..0a63173
--- /dev/null
+++ b/tests/bundles/overlays/local-charm-overlay.yaml.j2
@@ -0,0 +1,3 @@
+applications:
+ {{ charm_name }}:
+ charm: "{{ CHARM_BUILD_DIR }}"
diff --git a/tests/bundles/xenial.yaml b/tests/bundles/xenial.yaml
index 9d29342..a17f8b7 100644
--- a/tests/bundles/xenial.yaml
+++ b/tests/bundles/xenial.yaml
@@ -2,7 +2,6 @@ series: xenial
 description: "mongodb-charm test bundle"
 applications:
 mongodb:
- charm: "../../."
 num_units: 3
 options:
 replicaset: testset
diff --git a/tests/tests.yaml b/tests/tests.yaml
index dcbd01c..6461753 100644
--- a/tests/tests.yaml
+++ b/tests/tests.yaml
@@ -1,4 +1,4 @@
-charm_name: mongodb-charm
+charm_name: mongodb
 tests:
 - model_alias_xenial:
 - tests.tests_mongodb.BasicMongodbCharmTest
@@ -7,11 +7,15 @@ tests:
 - model_alias_bionic:
 - tests.tests_mongodb.BasicMongodbCharmTest
 - tests.tests_mongodb.ReplicatedMongodbCharmTest
+ - model_alias_focal:
+ - tests.tests_mongodb.BasicMongodbCharmTest
+ - tests.tests_mongodb.ReplicatedMongodbCharmTest
 - model_alias_shard:
 - tests.tests_mongodb.ShardedMongodbCharmTest
 gate_bundles:
 - model_alias_xenial: xenial
 - model_alias_bionic: bionic
- - model_alias_shard: bionic-shard
+ - model_alias_focal: focal
+ # - model_alias_shard: bionic-shard
 smoke_bundles:
 - model_alias_bionic: bionic
diff --git a/tox.ini b/tox.ini
index 6e3a650..66228b4 100644
--- a/tox.ini
+++ b/tox.ini
@@ -4,27 +4,41 @@ envlist = unit, functional, lint
 skip_missing_interpreters = True
 
 [testenv]
+basepython = python3
 setenv =
 PYTHONPATH = .
 passenv =
 HOME
- JUJU_REPOSITORY
+ CHARM_BUILD_DIR
 MODEL_SETTINGS
 
+[testenv:lint]
+deps = -r{toxinidir}/test_requirements.txt
+commands = flake8 {posargs:hooks/ unit_tests/ tests/}
+ charm-proof
+
+
 [testenv:unit]
-basepython = python2
 commands =
- nosetests -s --nologcapture --with-coverage unit_tests/ actions/
+ nosetests -v -s --nologcapture --with-coverage unit_tests/ actions/
 deps = -r{toxinidir}/test_requirements.txt
 
 [testenv:functional]
-basepython = python3
 commands =
 functest-run-suite --keep-model
 deps = -r{toxinidir}/tests/test_requirements.txt
 
 [testenv:func-smoke]
-basepython = python3
 commands =
 functest-run-suite --keep-model --smoke
 deps = -r{toxinidir}/tests/test_requirements.txt
+
+[flake8]
+ignore = E402,E226,W504
+exclude =
+ hooks/charmhelpers,
+ .git,
+ __pycache__,
+ .tox,
+max-line-length = 120
+max-complexity = 10
diff --git a/unit_tests/__init__.py b/unit_tests/__init__.py
index f80aab3..5dd2748 100644
--- a/unit_tests/__init__.py
+++ b/unit_tests/__init__.py
@@ -1,2 +1,3 @@
 import sys
 sys.path.append('hooks')
+sys.path.append('unit_tests')
diff --git a/unit_tests/test_hooks.py b/unit_tests/test_hooks.py
index c6e3c22..e97ef91 100644
--- a/unit_tests/test_hooks.py
+++ b/unit_tests/test_hooks.py
@@ -248,7 +248,7 @@ class MongoHooksTest(CharmTestCase):
 self.assertEqual(0, mock_check_output.call_count)
 
 mock_check_output.reset_mock()
- mock_check_output.return_value = '{"ok": 1}'
+ mock_check_output.return_value = b'{"ok": 1}'
 
 rv = hooks.mongo_client_smart(command='fake-cmd')
 self.assertTrue(rv)
@@ -415,15 +415,15 @@ class MongoHooksTest(CharmTestCase):
 self.assertEqual(results, expected)
 
 def test_remove_replset_from_upstart(self):
- test_contents = u"""
+ test_contents = b"""
 --exec /usr/bin/mongod -- --replSet myset --rest --config /etc/mongodb.conf
 """
- expected = u"""
+ expected = """
 --exec /usr/bin/mongod -- --rest --config /etc/mongodb.conf
 """
 
 mocked_upstart = tempfile.NamedTemporaryFile(delete=False)
- hooks.default_mongodb_init_config = mocked_upstart.name
+ hooks.default_mongodb_init_config = mocked_upstart.name.encode("utf8")
 
 try:
 mocked_upstart.write(test_contents)
@@ -437,9 +437,10 @@ class MongoHooksTest(CharmTestCase):
 finally:
 os.unlink(mocked_upstart.name)
 
+ @patch("subprocess.call")
 @patch.object(hooks, 'is_relation_made')
 @patch.object(hooks, 'is_bionic_or_greater')
- def test_mongodb_conf(self, mock_is_bionic_or_greater, mock_is_relation_made):
+ def test_mongodb_conf(self, mock_is_bionic_or_greater, mock_is_relation_made, *args):
 mock_is_bionic_or_greater.return_value = False
 mock_is_relation_made.return_value = False
 tmpdir = tempfile.mkdtemp()
@@ -472,9 +473,10 @@ master = true
 """.format(tmpdir=tmpdir)
 self.assertEqual(mongodb_conf, expected)
 
+ @patch("subprocess.call")
 @patch.object(hooks, 'is_relation_made')
 @patch.object(hooks, 'is_bionic_or_greater')
- def test_mongodb_conf_bionic(self, mock_is_bionic_or_greater, mock_is_relation_made):
+ def test_mongodb_conf_bionic(self, mock_is_bionic_or_greater, mock_is_relation_made, *args):
 mock_is_bionic_or_greater.return_value = True
 mock_is_relation_made.return_value = False
 tmpdir = tempfile.mkdtemp()
diff --git a/unit_tests/test_utils.py b/unit_tests/test_utils.py
index 1f2a1aa..583cce2 100644
--- a/unit_tests/test_utils.py
+++ b/unit_tests/test_utils.py
@@ -16,7 +16,7 @@ def mock_open(filename, contents=None):
 return io.StringIO(contents)
 else:
 return open(*args)
- with patch('__builtin__.open', mock_file):
+ with patch('builtins.open', mock_file):
 yield
 
 
@@ -36,7 +36,7 @@ def load_config():
 
 if not config:
 logging.error('Could not find config.yaml in any parent directory '
- 'of %s. ' % file)
+ 'of %s. ' % f)
 raise Exception
 
 return yaml.safe_load(open(config).read())['options']
@@ -49,7 +49,7 @@ def get_default_config():
 '''
 default_config = {}
 config = load_config()
- for k, v in config.iteritems():
+ for k, v in config.items():
 if 'default' in v:
 default_config[k] = v['default']
 else:
diff --git a/unit_tests/test_write_log_rotate_config.py b/unit_tests/test_write_log_rotate_config.py
index f94720f..ef1cd98 100644
--- a/unit_tests/test_write_log_rotate_config.py
+++ b/unit_tests/test_write_log_rotate_config.py
@@ -19,7 +19,7 @@ class TestWriteLogrotateConfigFile(unittest.TestCase):
 os.close(fd)
 with mock.patch('hooks.juju_log') as mock_juju_log:
 with mock.patch('hooks.open', create=True) as mock_open:
- mock_open.return_value = mock.MagicMock(spec=file)
+ mock_open.return_value = mock.MagicMock()
 hooks.write_logrotate_config(config_data, temp_fn)
 os.unlink(temp_fn)
 mock_juju_log.assert_called_once_with('Writing {}.'.format(temp_fn))