Repository: canonical/operator
Pull request #327 (2020-06-11 13:24:24)

many: move Model.name to be read from _ModelBackend
Unit name was already being read by _ModelBackend, as well as being
read by main(). We wanted to eliminate the duplication. We could either
pass more names down the stack, or we can pull them out of the backend.
This ended up a bit cleaner because we pass the Backend into Model
anyway, so it means having fewer args to the same object.

This also gave me an opportunity to eliminate a bunch of code that was
poking at os.environ just to set JUJU_UNIT_NAME.


Patch:
diff --git a/ops/jujuversion.py b/ops/jujuversion.py
index 4517886..b2b8177 100755
--- a/ops/jujuversion.py
+++ b/ops/jujuversion.py
@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import os
 import re
 from functools import total_ordering
 
@@ -83,3 +84,15 @@ class JujuVersion:
         elif self.build != other.build:
             return self.build < other.build
         return False
+
+    @classmethod
+    def from_environ(cls) -> 'JujuVersion':
+        """Build a JujuVersion from JUJU_VERSION."""
+        v = os.environ.get('JUJU_VERSION')
+        if not v:
+            raise RuntimeError('environ has no JUJU_VERSION')
+        return cls(v)
+
+    def has_app_data(self) -> bool:
+        """Determine whether this juju version knows about app data."""
+        return (self.major, self.minor, self.patch) >= (2, 7, 0)
diff --git a/ops/main.py b/ops/main.py
index 77fa536..2de3f74 100755
--- a/ops/main.py
+++ b/ops/main.py
@@ -290,9 +290,7 @@ def main(charm_class):
 
     metadata, actions_metadata = _load_metadata(charm_dir)
     meta = ops.charm.CharmMeta(metadata, actions_metadata)
-    unit_name = os.environ['JUJU_UNIT_NAME']
-    model_name = os.environ.get('JUJU_MODEL_NAME')
-    model = ops.model.Model(unit_name, meta, model_backend, model_name=model_name)
+    model = ops.model.Model(meta, model_backend)
 
     # TODO: If Juju unit agent crashes after exit(0) from the charm code
     # the framework will commit the snapshot but Juju will not commit its
diff --git a/ops/model.py b/ops/model.py
index 10ae534..b96e891 100644
--- a/ops/model.py
+++ b/ops/model.py
@@ -30,11 +30,15 @@ from pathlib import Path
 from subprocess import run, PIPE, CalledProcessError
 
 import ops
+from ops.jujuversion import JujuVersion
 
 
 class Model:
     """Represents the Juju Model as seen from this unit.
 
+    This should not be instantiated directly by Charmers, but can be accessed as `self.model`
+    from any class that derives from Object.
+
     Attributes:
         unit: A :class:`Unit` that represents the unit that is running this code (eg yourself)
         app: A :class:`Application` that represents the application this unit is a part of.
@@ -49,12 +53,10 @@ class Model:
             for Kubernetes charms.
     """
 
-    def __init__(
-            self, unit_name: str, meta: 'ops.charm.CharmMeta', backend: '_ModelBackend', *,
-            model_name: str = None):
+    def __init__(self, meta: 'ops.charm.CharmMeta', backend: '_ModelBackend'):
         self._cache = _ModelCache(backend)
         self._backend = backend
-        self.unit = self.get_unit(unit_name)
+        self.unit = self.get_unit(self._backend.unit_name)
         self.app = self.unit.app
         self.relations = RelationMapping(meta.relations, self.unit, self._backend, self._cache)
         self.config = ConfigData(self._backend)
@@ -62,7 +64,6 @@ class Model:
         self.pod = Pod(self._backend)
         self.storages = StorageMapping(list(meta.storages), self._backend)
         self._bindings = BindingMapping(self._backend)
-        self._model_name = model_name
 
     @property
     def name(self) -> str:
@@ -70,7 +71,7 @@ class Model:
 
         This is read from the environment variable ``JUJU_MODEL_NAME``.
         """
-        return self._model_name
+        return self._backend.model_name
 
     def get_unit(self, unit_name: str) -> 'Unit':
         """Get an arbitrary unit by name.
@@ -954,8 +955,14 @@ class _ModelBackend:
 
     LEASE_RENEWAL_PERIOD = datetime.timedelta(seconds=30)
 
-    def __init__(self):
-        self.unit_name = os.environ['JUJU_UNIT_NAME']
+    def __init__(self, unit_name=None, model_name=None):
+        if unit_name is None:
+            self.unit_name = os.environ['JUJU_UNIT_NAME']
+        else:
+            self.unit_name = unit_name
+        if model_name is None:
+            model_name = os.environ.get('JUJU_MODEL_NAME')
+        self.model_name = model_name
         self.app_name = self.unit_name.split('/')[0]
 
         self._is_leader = None
@@ -996,10 +1003,18 @@ class _ModelBackend:
         if not isinstance(is_app, bool):
             raise TypeError('is_app parameter to relation_get must be a boolean')
 
+        if is_app:
+            version = JujuVersion.from_environ()
+            if not version.has_app_data():
+                raise RuntimeError(
+                    'getting application data is not supported on Juju version {}'.format(version))
+
+        args = ['relation-get', '-r', str(relation_id), '-', member_name]
+        if is_app:
+            args.append('--app')
+
         try:
-            return self._run('relation-get', '-r', str(relation_id),
-                             '-', member_name, '--app={}'.format(is_app),
-                             return_output=True, use_json=True)
+            return self._run(*args, return_output=True, use_json=True)
         except ModelError as e:
             if 'relation not found' in str(e):
                 raise RelationNotFoundError() from e
@@ -1009,9 +1024,18 @@ class _ModelBackend:
         if not isinstance(is_app, bool):
             raise TypeError('is_app parameter to relation_set must be a boolean')
 
+        if is_app:
+            version = JujuVersion.from_environ()
+            if not version.has_app_data():
+                raise RuntimeError(
+                    'setting application data is not supported on Juju version {}'.format(version))
+
+        args = ['relation-set', '-r', str(relation_id), '{}={}'.format(key, value)]
+        if is_app:
+            args.append('--app')
+
         try:
-            return self._run('relation-set', '-r', str(relation_id),
-                             '{}={}'.format(key, value), '--app={}'.format(is_app))
+            return self._run(*args)
         except ModelError as e:
             if 'relation not found' in str(e):
                 raise RelationNotFoundError() from e


Test patch:
diff --git a/ops/testing.py b/ops/testing.py
index 0d1c0c0..c066f62 100644
--- a/ops/testing.py
+++ b/ops/testing.py
@@ -75,7 +75,7 @@ class Harness:
         self._hooks_enabled = True
         self._relation_id_counter = 0
         self._backend = _TestingModelBackend(self._unit_name, self._meta)
-        self._model = model.Model(self._unit_name, self._meta, self._backend)
+        self._model = model.Model(self._meta, self._backend)
         self._framework = framework.Framework(":memory:", self._charm_dir, self._meta, self._model)
 
     @property
@@ -304,6 +304,16 @@ class Harness:
         """Read the workload version that was set by the unit."""
         return self._backend._workload_version
 
+    def set_model_name(self, name: str) -> None:
+        """Set the name of the Model that this is representing.
+
+        This cannot be called once begin() has been called. But it lets you set the value that
+        will be returned by Model.name.
+        """
+        if self._charm is not None:
+            raise RuntimeError('cannot set the Model name after begin()')
+        self._backend.model_name = name
+
     def update_relation_data(
             self,
             relation_id: int,
@@ -466,6 +476,7 @@ class _TestingModelBackend:
     def __init__(self, unit_name, meta):
         self.unit_name = unit_name
         self.app_name = self.unit_name.split('/')[0]
+        self.model_name = None
         self._calls = []
         self._meta = meta
         self._is_leader = None
diff --git a/test/charms/test_main/actions.yaml b/test/charms/test_main/actions.yaml
new file mode 100644
index 0000000..d16c942
--- /dev/null
+++ b/test/charms/test_main/actions.yaml
@@ -0,0 +1,29 @@
+foo-bar:
+  description: Foos the bar.
+  title: foo-bar
+  params:
+    foo-name:
+      type: string
+      description: A foo name to bar.
+    silent:
+      type: boolean
+      description: ""
+      default: false
+  required:
+    - foo-name
+start:
+  description: Start the unit.
+get-model-name:
+  description: Return the name of the model
+get-status:
+  description: Return the Status of the unit
+log-critical:
+  description: log a message at Critical level
+log-error:
+  description: log a message at Error level
+log-warning:
+  description: log a message at Warning level
+log-info:
+  description: log a message at Info level
+log-debug:
+  description: log a message at Debug level
diff --git a/test/test_charm.py b/test/test_charm.py
index 41bbccb..e1d69f6 100755
--- a/test/test_charm.py
+++ b/test/test_charm.py
@@ -60,7 +60,7 @@ class TestCharm(unittest.TestCase):
         self.addCleanup(cleanup)
 
     def create_framework(self):
-        model = Model('local/0', self.meta, _ModelBackend())
+        model = Model(self.meta, _ModelBackend('local/0'))
         framework = Framework(self.tmpdir / "framework.data", self.tmpdir, self.meta, model)
         self.addCleanup(framework.close)
         return framework
diff --git a/test/test_helpers.py b/test/test_helpers.py
index 3439ca5..cc7ed7c 100644
--- a/test/test_helpers.py
+++ b/test/test_helpers.py
@@ -18,7 +18,6 @@ import subprocess
 import shutil
 import tempfile
 import unittest
-from unittest.mock import patch
 
 from ops.framework import Framework
 from ops.model import Model, _ModelBackend
@@ -109,12 +108,7 @@ class BaseTestCase(unittest.TestCase):
 
     def create_model(self):
         """Create a Model object."""
-        unit_name = 'myapp/0'
-        patcher = patch.dict(os.environ, {'JUJU_UNIT_NAME': unit_name})
-        patcher.start()
-        self.addCleanup(patcher.stop)
-
-        backend = _ModelBackend()
+        backend = _ModelBackend(unit_name='myapp/0')
         meta = CharmMeta()
-        model = Model('myapp/0', meta, backend)
+        model = Model(meta, backend)
         return model
diff --git a/test/test_jujuversion.py b/test/test_jujuversion.py
index d19fd60..1e38210 100755
--- a/test/test_jujuversion.py
+++ b/test/test_jujuversion.py
@@ -1,4 +1,3 @@
-#!/usr/bin/env python3
 # Copyright 2019 Canonical Ltd.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -13,7 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import os
 import unittest
+import unittest.mock  # in this file, importing just 'patch' would be confusing
 
 from ops.jujuversion import JujuVersion
 
@@ -41,6 +42,24 @@ class TestJujuVersion(unittest.TestCase):
             self.assertEqual(v.patch, patch)
             self.assertEqual(v.build, build)
 
+    @unittest.mock.patch('os.environ', new={})
+    def test_from_environ(self):
+        with self.assertRaisesRegex(RuntimeError, 'environ has no JUJU_VERSION'):
+            JujuVersion.from_environ()
+
+        os.environ['JUJU_VERSION'] = 'no'
+        with self.assertRaisesRegex(RuntimeError, 'not a valid Juju version'):
+            JujuVersion.from_environ()
+
+        os.environ['JUJU_VERSION'] = '2.8.0'
+        v = JujuVersion.from_environ()
+        self.assertEqual(v, JujuVersion('2.8.0'))
+
+    def test_has_app_data(self):
+        self.assertTrue(JujuVersion('2.8.0').has_app_data())
+        self.assertTrue(JujuVersion('2.7.0').has_app_data())
+        self.assertFalse(JujuVersion('2.6.9').has_app_data())
+
     def test_parsing_errors(self):
         invalid_versions = [
             "xyz",
@@ -124,7 +143,3 @@ class TestJujuVersion(unittest.TestCase):
             self.assertEqual(JujuVersion(a) <= b, expected_weak)
             self.assertEqual(b > JujuVersion(a), expected_strict)
             self.assertEqual(b >= JujuVersion(a), expected_weak)
-
-
-if __name__ == "__main__":
-    unittest.main()
diff --git a/test/test_main.py b/test/test_main.py
index 76bbb63..81b6de4 100755
--- a/test/test_main.py
+++ b/test/test_main.py
@@ -131,7 +131,7 @@ class CharmInitTestCase(unittest.TestCase):
         self.assertFalse(warn_cm)
 
 
-class TestMain(abc.ABC):
+class _TestMain(abc.ABC):
 
     @abc.abstractmethod
     def _setup_entry_point(self, directory, entry_point):
@@ -160,9 +160,8 @@ class TestMain(abc.ABC):
     def setUp(self):
         self._setup_charm_dir()
 
-        _, tmp_file = tempfile.mkstemp()
+        _, tmp_file = tempfile.mkstemp(dir=str(self._tmpdir))
         self._state_file = Path(tmp_file)
-        self.addCleanup(self._state_file.unlink)
 
         # Relations events are defined dynamically and modify the class attributes.
         # We use a subclass temporarily to prevent these side effects from leaking.
@@ -171,7 +170,6 @@ class TestMain(abc.ABC):
         CharmBase.on = TestCharmEvents()
 
         def cleanup():
-            shutil.rmtree(str(self.JUJU_CHARM_DIR))
             CharmBase.on = CharmEvents()
         self.addCleanup(cleanup)
 
@@ -182,7 +180,9 @@ class TestMain(abc.ABC):
         self.stderr = None
 
     def _setup_charm_dir(self):
-        self.JUJU_CHARM_DIR = Path(tempfile.mkdtemp()) / 'test_main'
+        self._tmpdir = Path(tempfile.mkdtemp(prefix='tmp-ops-test-'))
+        self.addCleanup(shutil.rmtree, str(self._tmpdir))
+        self.JUJU_CHARM_DIR = self._tmpdir / 'test_main'
         self.hooks_dir = self.JUJU_CHARM_DIR / 'hooks'
         charm_path = str(self.JUJU_CHARM_DIR / 'src/charm.py')
         self.charm_exec_path = os.path.relpath(charm_path,
@@ -202,32 +202,9 @@ class TestMain(abc.ABC):
             self._setup_entry_point(self.hooks_dir, hook)
 
     def _prepare_actions(self):
-        actions_meta = '''
-foo-bar:
-  description: Foos the bar.
-  title: foo-bar
-  params:
-    foo-name:
-      type: string
-      description: A foo name to bar.
-    silent:
-      type: boolean
-      description:
-      default: false
-  required:
-    - foo-name
-start:
-    description: Start the unit.
-get-model-name:
-    description: Return the name of the model
-get-status:
-    description: Return the Status of the unit
-'''
+        # TODO: jam 2020-06-16 this same work could be done just triggering the 'install' event
+        #  of the charm, it might be cleaner to not set up entry points directly here.
         actions_dir_name = 'actions'
-        actions_meta_file = 'actions.yaml'
-
-        with (self.JUJU_CHARM_DIR / actions_meta_file).open('w+t') as f:
-            f.write(actions_meta)
         actions_dir = self.JUJU_CHARM_DIR / actions_dir_name
         actions_dir.mkdir()
         for action_name in ('start', 'foo-bar', 'get-model-name', 'get-status'):
@@ -522,15 +499,6 @@ get-status:
             'USE_LOG_ACTIONS': True,
         }))
         fake_script(self, 'action-get', "echo '{}'")
-        actions_yaml = self.JUJU_CHARM_DIR / 'actions.yaml'
-        actions_yaml.write_text(
-            '''
-log_critical: {}
-log_error: {}
-log_warning: {}
-log_info: {}
-log_debug: {}
-            ''')
 
         test_cases = [(
             EventSpec(ActionEvent, 'log_critical_action', env_var='JUJU_ACTION_NAME',
@@ -643,7 +611,7 @@ log_debug: {}
         self.assertEqual(state['observed_event_types'], [InstallEvent])
 
 
-class TestMainWithNoDispatch(TestMain, unittest.TestCase):
+class TestMainWithNoDispatch(_TestMain, unittest.TestCase):
     has_dispatch = False
     hooks_are_symlinks = True
 
@@ -695,10 +663,9 @@ class TestMainWithNoDispatch(TestMain, unittest.TestCase):
         charm_config = base64.b64encode(pickle.dumps({
             'STATE_FILE': self._state_file,
         }))
-        actions_yaml = self.JUJU_CHARM_DIR / 'actions.yaml'
-        actions_yaml.write_text('test: {}')
         self._simulate_event(EventSpec(InstallEvent, 'install', charm_config=charm_config))
-        action_hook = self.JUJU_CHARM_DIR / 'actions' / 'test'
+        # foo-bar is one of the actions defined in actions.yaml
+        action_hook = self.JUJU_CHARM_DIR / 'actions' / 'foo-bar'
         self.assertTrue(action_hook.exists())
 
 
@@ -717,7 +684,7 @@ class TestMainWithNoDispatchButScriptsAreCopies(TestMainWithNoDispatch):
         shutil.copy(charm_path, str(path))
 
 
-class TestMainWithDispatch(TestMain, unittest.TestCase):
+class TestMainWithDispatch(_TestMain, unittest.TestCase):
     has_dispatch = True
 
     def _setup_entry_point(self, directory, entry_point):
diff --git a/test/test_model.py b/test/test_model.py
index cb8189f..469e294 100755
--- a/test/test_model.py
+++ b/test/test_model.py
@@ -16,7 +16,6 @@
 from collections import OrderedDict
 import json
 import ipaddress
-import os
 import pathlib
 from textwrap import dedent
 import unittest
@@ -32,12 +31,6 @@ from test.test_helpers import fake_script, fake_script_calls
 class TestModel(unittest.TestCase):
 
     def setUp(self):
-        def restore_env(env):
-            os.environ.clear()
-            os.environ.update(env)
-        self.addCleanup(restore_env, os.environ.copy())
-
-        os.environ['JUJU_UNIT_NAME'] = 'myapp/0'
         self.harness = ops.testing.Harness(ops.charm.CharmBase, meta='''
             name: myapp
             provides:
@@ -57,9 +50,9 @@ class TestModel(unittest.TestCase):
         self.assertIs(self.model.app, self.model.unit.app)
         self.assertIsNone(self.model.name)
 
-    def test_model_name(self):
-        m = ops.model.Model('unit/0', ops.charm.CharmMeta(), self.harness._backend,
-                            model_name='default')
+    def test_model_name_from_backend(self):
+        self.harness.set_model_name('default')
+        m = ops.model.Model(ops.charm.CharmMeta(), self.harness._backend)
         self.assertEqual(m.name, 'default')
         with self.assertRaises(AttributeError):
             m.name = "changes-disallowed"
@@ -418,7 +411,7 @@ class TestModel(unittest.TestCase):
         # TODO: (jam) 2020-05-07 Harness doesn't yet support resource-get issue #262
         meta = ops.charm.CharmMeta()
         meta.resources = {'foo': None, 'bar': None}
-        model = ops.model.Model('myapp/0', meta, ops.model._ModelBackend())
+        model = ops.model.Model(meta, ops.model._ModelBackend('myapp/0'))
 
         with self.assertRaises(RuntimeError):
             model.resources.fetch('qux')
@@ -437,7 +430,7 @@ class TestModel(unittest.TestCase):
         meta = ops.charm.CharmMeta.from_yaml('''
             name: myapp
         ''')
-        model = ops.model.Model('myapp/0', meta, ops.model._ModelBackend())
+        model = ops.model.Model(meta, ops.model._ModelBackend('myapp/0'))
         fake_script(self, 'pod-spec-set', """
                     cat $2 > $(dirname $0)/spec.json
                     [[ -n $4 ]] && cat $4 > $(dirname $0)/k8s_res.json || true
@@ -472,9 +465,9 @@ class TestModel(unittest.TestCase):
         check_calls(fake_calls)
 
         # Create a new model to drop is-leader caching result.
-        self.backend = ops.model._ModelBackend()
+        self.backend = ops.model._ModelBackend('myapp/0')
         meta = ops.charm.CharmMeta()
-        model = ops.model.Model('myapp/0', meta, self.backend)
+        model = ops.model.Model(meta, self.backend)
         fake_script(self, 'is-leader', 'echo false')
         with self.assertRaises(ops.model.ModelError):
             model.pod.set_spec({'foo': 'bar'})
@@ -651,7 +644,7 @@ class TestModel(unittest.TestCase):
         # TODO: (jam) 2020-05-07 Harness doesn't yet expose storage-get issue #263
         meta = ops.charm.CharmMeta()
         meta.storages = {'disks': None, 'data': None}
-        model = ops.model.Model('myapp/0', meta, ops.model._ModelBackend())
+        model = ops.model.Model(meta, ops.model._ModelBackend('myapp/0'))
 
         fake_script(self, 'storage-list', '''
             if [ "$1" = disks ]; then
@@ -716,13 +709,6 @@ class TestModel(unittest.TestCase):
 class TestModelBindings(unittest.TestCase):
 
     def setUp(self):
-        def restore_env(env):
-            os.environ.clear()
-            os.environ.update(env)
-        self.addCleanup(restore_env, os.environ.copy())
-
-        os.environ['JUJU_UNIT_NAME'] = 'myapp/0'
-
         meta = ops.charm.CharmMeta()
         meta.relations = {
             'db0': RelationMeta(
@@ -732,8 +718,8 @@ class TestModelBindings(unittest.TestCase):
             'db2': RelationMeta(
                 RelationRole.peer, 'db2', {'interface': 'db2', 'scope': 'global'}),
         }
-        self.backend = ops.model._ModelBackend()
-        self.model = ops.model.Model('myapp/0', meta, self.backend)
+        self.backend = ops.model._ModelBackend('myapp/0')
+        self.model = ops.model.Model(meta, self.backend)
 
         fake_script(self, 'relation-ids',
                     """([ "$1" = db0 ] && echo '["db0:4"]') || echo '[]'""")
@@ -866,15 +852,12 @@ class TestModelBindings(unittest.TestCase):
 class TestModelBackend(unittest.TestCase):
 
     def setUp(self):
-        os.environ['JUJU_UNIT_NAME'] = 'myapp/0'
-        self.addCleanup(os.environ.pop, 'JUJU_UNIT_NAME')
-
         self._backend = None
 
     @property
     def backend(self):
         if self._backend is None:
-            self._backend = ops.model._ModelBackend()
+            self._backend = ops.model._ModelBackend('myapp/0')
         return self._backend
 
     def test_relation_get_set_is_app_arg(self):
@@ -897,7 +880,7 @@ class TestModelBackend(unittest.TestCase):
         meta = ops.charm.CharmMeta.from_yaml('''
             name: myapp
         ''')
-        model = ops.model.Model('myapp/0', meta, self.backend)
+        model = ops.model.Model(meta, self.backend)
         fake_script(self, 'is-leader', 'echo false')
         self.assertFalse(model.unit.is_leader())
 
@@ -915,6 +898,8 @@ class TestModelBackend(unittest.TestCase):
         self.assertTrue(model.unit.is_leader())
 
     def test_relation_tool_errors(self):
+        self.addCleanup(os.environ.pop, 'JUJU_VERSION', None)
+        os.environ['JUJU_VERSION'] = '2.8.0'
         err_msg = 'ERROR invalid value "$2" for option -r: relation not found'
 
         test_cases = [(
@@ -931,29 +916,79 @@ class TestModelBackend(unittest.TestCase):
             lambda: fake_script(self, 'relation-set', 'echo fooerror >&2 ; exit 1'),
             lambda: self.backend.relation_set(3, 'foo', 'bar', is_app=False),
             ops.model.ModelError,
-            [['relation-set', '-r', '3', 'foo=bar', '--app=False']],
+            [['relation-set', '-r', '3', 'foo=bar']],
         ), (
             lambda: fake_script(self, 'relation-set', 'echo {} >&2 ; exit 2'.format(err_msg)),
             lambda: self.backend.relation_set(3, 'foo', 'bar', is_app=False),
             ops.model.RelationNotFoundError,
-            [['relation-set', '-r', '3', 'foo=bar', '--app=False']],
+            [['relation-set', '-r', '3', 'foo=bar']],
+        ), (
+            lambda: None,
+            lambda: self.backend.relation_set(3, 'foo', 'bar', is_app=True),
+            ops.model.RelationNotFoundError,
+            [['relation-set', '-r', '3', 'foo=bar', '--app']],
         ), (
             lambda: fake_script(self, 'relation-get', 'echo fooerror >&2 ; exit 1'),
             lambda: self.backend.relation_get(3, 'remote/0', is_app=False),
             ops.model.ModelError,
-            [['relation-get', '-r', '3', '-', 'remote/0', '--app=False', '--format=json']],
+            [['relation-get', '-r', '3', '-', 'remote/0', '--format=json']],
         ), (
             lambda: fake_script(self, 'relation-get', 'echo {} >&2 ; exit 2'.format(err_msg)),
             lambda: self.backend.relation_get(3, 'remote/0', is_app=False),
             ops.model.RelationNotFoundError,
-            [['relation-get', '-r', '3', '-', 'remote/0', '--app=False', '--format=json']],
+            [['relation-get', '-r', '3', '-', 'remote/0', '--format=json']],
+        ), (
+            lambda: None,
+            lambda: self.backend.relation_get(3, 'remote/0', is_app=True),
+            ops.model.RelationNotFoundError,
+            [['relation-get', '-r', '3', '-', 'remote/0', '--app', '--format=json']],
         )]
 
-        for do_fake, run, exception, calls in test_cases:
-            do_fake()
-            with self.assertRaises(exception):
-                run()
-            self.assertEqual(fake_script_calls(self, clear=True), calls)
+        for i, (do_fake, run, exception, calls) in enumerate(test_cases):
+            with self.subTest(i):
+                do_fake()
+                with self.assertRaises(exception):
+                    run()
+                self.assertEqual(fake_script_calls(self, clear=True), calls)
+
+    def test_relation_get_juju_version_quirks(self):
+        self.addCleanup(os.environ.pop, 'JUJU_VERSION', None)
+
+        fake_script(self, 'relation-get', '''echo '{"foo": "bar"}' ''')
+
+        # on 2.7.0+, things proceed as expected
+        for v in ['2.8.0', '2.7.0']:
+            with self.subTest(v):
+                os.environ['JUJU_VERSION'] = v
+                rel_data = self.backend.relation_get(1, 'foo/0', is_app=True)
+                self.assertEqual(rel_data, {"foo": "bar"})
+                calls = [' '.join(i) for i in fake_script_calls(self, clear=True)]
+                self.assertEqual(calls, ['relation-get -r 1 - foo/0 --app --format=json'])
+
+        # before 2.7.0, it just fails (no --app support)
+        os.environ['JUJU_VERSION'] = '2.6.9'
+        with self.assertRaisesRegex(RuntimeError, 'not supported on Juju version 2.6.9'):
+            self.backend.relation_get(1, 'foo/0', is_app=True)
+        self.assertEqual(fake_script_calls(self), [])
+
+    def test_relation_set_juju_version_quirks(self):
+        self.addCleanup(os.environ.pop, 'JUJU_VERSION', None)
+
+        fake_script(self, 'relation-set', 'exit 0')
+
+        # on 2.7.0+, things proceed as expected
+        for v in ['2.8.0', '2.7.0']:
+            with self.subTest(v):
+                os.environ['JUJU_VERSION'] = v
+                self.backend.relation_set(1, 'foo', 'bar', is_app=True)
+                calls = [' '.join(i) for i in fake_script_calls(self, clear=True)]
+                self.assertEqual(calls, ['relation-set -r 1 foo=bar --app'])
+
+        # before 2.7.0, it just fails always (no --app support)
+        os.environ['JUJU_VERSION'] = '2.6.9'
+        with self.assertRaisesRegex(RuntimeError, 'not supported on Juju version 2.6.9'):
+            self.backend.relation_set(1, 'foo', 'bar', is_app=True)
+        self.assertEqual(fake_script_calls(self), [])
 
     def test_status_get(self):
         # taken from actual Juju output
@@ -1008,7 +1043,7 @@ class TestModelBackend(unittest.TestCase):
         meta = ops.charm.CharmMeta.from_yaml('''
             name: myapp
         ''')
-        model = ops.model.Model('myapp/0', meta, self.backend)
+        model = ops.model.Model(meta, self.backend)
         fake_script(self, 'status-set', 'exit 1')
         fake_script(self, 'is-leader', 'echo true')
 
diff --git a/test/test_testing.py b/test/test_testing.py
index 4b273f7..8f9e4eb 100644
--- a/test/test_testing.py
+++ b/test/test_testing.py
@@ -533,6 +533,23 @@ class TestHarness(unittest.TestCase):
         # The charm_dir also gets set
         self.assertEqual(harness.framework.charm_dir, tmp)
 
+    def test_set_model_name(self):
+        harness = Harness(CharmBase, meta='''
+            name: test-charm
+        ''')
+        harness.set_model_name('foo')
+        self.assertEqual('foo', harness.model.name)
+
+    def test_set_model_name_after_begin(self):
+        harness = Harness(CharmBase, meta='''
+            name: test-charm
+        ''')
+        harness.set_model_name('bar')
+        harness.begin()
+        with self.assertRaises(RuntimeError):
+            harness.set_model_name('foo')
+        self.assertEqual(harness.model.name, 'bar')
+
     def test_actions_from_directory(self):
         tmp = pathlib.Path(tempfile.mkdtemp())
         self.addCleanup(shutil.rmtree, str(tmp))
@@ -889,7 +906,7 @@ class TestTestingModelBackend(unittest.TestCase):
             "registrypath": "custompath",
             "username": "custom_username",
             "password": "custom_password",
-            }
+        }
         harness.add_oci_resource('image', custom)
         resource = harness._resource_dir / "image" / "contents.yaml"
         with resource.open('r') as resource_file: