Repository: canonical/operator
Pull request #100 (2020-01-10 09:51:14)

Add a Juju version comparison module
Introduce support for Juju version comparison in order to allow for
version-specific checks and warnings in the framework.


Patch:
diff --git a/.gitignore b/.gitignore
index 36b14f6..cf0f371 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,2 +1,3 @@
 __pycache__
 /sandbox
+.idea
diff --git a/README.md b/README.md
index 375a962..f289ebc 100644
--- a/README.md
+++ b/README.md
@@ -5,26 +5,43 @@ for early testing.
 
 ## Getting Started
 
-Start by creating a charm directory with at least the following files:
+Your charm directory should have the following overall structure:
 
-* `src/charm.py` (must be executable and use Python 3.6+)
-* `hooks/install` (or `hooks/start` for K8s charms) sym-linked to `../src/charm.py`
-* `metadata.yaml`
+```
+.
++-- metadata.yaml
++-- mod/
++-- lib/
+|   +-- ops -> ../mod/operator/ops
++-- src/
+|   +-- charm.py
++-- hooks/
+    +-- install -> ../src/charm.py
+```
 
-Then install the framework into the `lib/` directory using:
+The `mod/` directory will contain the operator framework dependency as a git
+submodule:
 
 ```
-mkdir lib/
-pip install -t lib/ https://github.com/canonical/operator
+git submodule add https://github.com/canonical/operator mod/operator
 ```
 
-Your `src/charm.py` is the entry point for your charm logic. At a minimum, it
-needs to define a subclass of `CharmBase` and pass that into the framework's
-`main` function:
+Other dependencies included as git submodules can be added there as well.
+
+The `lib/` directory will then contain symlinks to subdirectories of your
+submodule dependencies to enable them to be imported into the charm:
+
+```
+ln -s ../mod/operator/ops lib/ops
+```
+
+Your `src/charm.py` is the entry point for your charm logic. It should be set
+to executable and use Python 3.6 or greater. At a minimum, it needs to define a
+subclass of `CharmBase` and pass that into the framework's `main` function:
 
 ```python
 import sys
-sys.path.insert.append('lib')
+sys.path.append('lib')
 
 from ops.charm import CharmBase
 from ops.main import main
@@ -53,11 +70,20 @@ class MyCharm(CharmBase):
 Every standard event in Juju may be observed that way, and you can also easily
 define your own events in your custom types.
 
+The `hooks/` directory will then contain symlinks to your `src/charm.py` entry
+point so that Juju can call it. You only need to set up the `hooks/install` link
+(`hooks/start` for K8s charms, until [lp#1854635](https://bugs.launchpad.net/juju/+bug/1854635)
+is resolved), and the framework will fill out all others at runtime.
+
 Once your charm is ready, deploy it as normal with:
 
 ```
 juju deploy .
 ```
 
-You can sync subsequent changes from the framework by running the `pip`
-command again with `--upgrade`.
+You can sync subsequent changes from the framework and other submodule
+dependencies by running:
+
+```
+git submodule update
+```
diff --git a/ops/framework.py b/ops/framework.py
index 9c27bf1..3181aa1 100755
--- a/ops/framework.py
+++ b/ops/framework.py
@@ -194,6 +194,9 @@ class Object:
         kind = self.handle_kind
         if isinstance(parent, Framework):
             self.framework = parent
+            # Avoid Framework instances having a circular reference to themselves.
+            if self.framework is self:
+                self.framework = weakref.proxy(self.framework)
             self.handle = Handle(None, kind, key)
         else:
             self.framework = parent.framework
@@ -212,6 +215,18 @@ class Object:
 
         # TODO Detect conflicting handles here.
 
+    @property
+    def model(self):
+        return self.framework.model
+
+    @property
+    def meta(self):
+        return self.framework.meta
+
+    @property
+    def charm_dir(self):
+        return self.framework.charm_dir
+
 
 class EventsBase(Object):
     """Convenience type to allow defining .on attributes at class level."""
@@ -376,6 +391,11 @@ class Framework(Object):
 
     on = FrameworkEvents()
 
+    # Override properties from Object so that we can set them in __init__.
+    model = None
+    meta = None
+    charm_dir = None
+
     def __init__(self, data_path, charm_dir, meta, model):
 
         super().__init__(self, None)
diff --git a/ops/jujuversion.py b/ops/jujuversion.py
new file mode 100755
index 0000000..9e062c0
--- /dev/null
+++ b/ops/jujuversion.py
@@ -0,0 +1,63 @@
+import re
+from functools import total_ordering
+
+
+@total_ordering
+class JujuVersion:
+
+    PATTERN = r'^(?P<major>\d{1,9})\.(?P<minor>\d{1,9})((?:\.|-(?P<tag>[a-z]+))(?P<patch>\d{1,9}))?(\.(?P<build>\d{1,9}))?$'
+
+    def __init__(self, version):
+        m = re.match(self.PATTERN, version)
+        if not m:
+            raise RuntimeError(f"{version} is not a valid Juju version string.")
+
+        d = m.groupdict()
+        self.major = int(m.group('major'))
+        self.minor = int(m.group('minor'))
+        self.tag = d['tag'] or ''
+        self.patch = int(d['patch'] or 0)
+        self.build = int(d['build'] or 0)
+
+    def __repr__(self):
+        if self.tag:
+            s = f'{self.major}.{self.minor}-{self.tag}{self.patch}'
+        else:
+            s = f'{self.major}.{self.minor}.{self.patch}'
+        if self.build > 0:
+            s += f'.{self.build}'
+        return s
+
+    def __eq__(self, other):
+        if self is other:
+            return True
+        if isinstance(other, str):
+            other = type(self)(other)
+        elif not isinstance(other, JujuVersion):
+            raise RuntimeError(f"Cannot compare Juju version {self} with {other}")
+        return self.major == other.major and self.minor == other.minor\
+            and self.tag == other.tag and self.build == other.build and self.patch == other.patch
+
+    def __lt__(self, other):
+        if self is other:
+            return False
+        if isinstance(other, str):
+            other = type(self)(other)
+        elif not isinstance(other, JujuVersion):
+            raise RuntimeError(f"Cannot compare Juju version {self} with {other}")
+
+        if self.major != other.major:
+            return self.major < other.major
+        elif self.minor != other.minor:
+            return self.minor < other.minor
+        elif self.tag != other.tag:
+            if not self.tag:
+                return False
+            elif not other.tag:
+                return True
+            return self.tag < other.tag
+        elif self.patch != other.patch:
+            return self.patch < other.patch
+        elif self.build != other.build:
+            return self.build < other.build
+        return False


Test patch:
diff --git a/test/test_framework.py b/test/test_framework.py
index 4fa38aa..7551d9d 100755
--- a/test/test_framework.py
+++ b/test/test_framework.py
@@ -700,6 +700,16 @@ class TestFramework(unittest.TestCase):
         # (The event key goes up by 2 due to the pre-commit and commit events.)
         self.assertEqual(obs2.seen, [('4', 'second'), ('1', 'first')])
 
+    def test_helper_properties(self):
+        framework = self.create_framework()
+        framework.model = 'test-model'
+        framework.meta = 'test-meta'
+
+        my_obj = Object(framework, 'my_obj')
+        self.assertEqual(my_obj.model, framework.model)
+        self.assertEqual(my_obj.meta, framework.meta)
+        self.assertEqual(my_obj.charm_dir, framework.charm_dir)
+
 
 class TestStoredState(unittest.TestCase):
 
diff --git a/test/test_jujuversion.py b/test/test_jujuversion.py
new file mode 100644
index 0000000..25bbd5a
--- /dev/null
+++ b/test/test_jujuversion.py
@@ -0,0 +1,120 @@
+#!/usr/bin/env python3
+
+import unittest
+from ops.jujuversion import JujuVersion
+
+
+class TestJujuVersion(unittest.TestCase):
+
+    def test_parsing(self):
+        test_cases = [(
+            "0.0.0", 0, 0, '', 0, 0,
+        ), (
+            "0.0.2", 0, 0, '', 2, 0,
+        ), (
+            "0.1.0", 0, 1, '', 0, 0,
+        ), (
+            "0.2.3", 0, 2, '', 3, 0,
+        ), (
+            "10.234.3456", 10, 234, '', 3456, 0,
+        ), (
+            "10.234.3456.1", 10, 234, '', 3456, 1,
+        ), (
+            "1.21-alpha12", 1, 21, 'alpha', 12, 0,
+        ), (
+            "1.21-alpha1.34", 1, 21, 'alpha', 1, 34,
+        ), (
+            "2.7", 2, 7, '', 0, 0,
+        )]
+
+        for vs, major, minor, tag, patch, build in test_cases:
+            v = JujuVersion(vs)
+            self.assertEqual(v.major, major)
+            self.assertEqual(v.minor, minor)
+            self.assertEqual(v.tag, tag)
+            self.assertEqual(v.patch, patch)
+            self.assertEqual(v.build, build)
+
+    def test_parsing_errors(self):
+        invalid_versions = [
+            "xyz",
+            "foo.bar",
+            "foo.bar.baz",
+            "dead.beef.ca.fe",
+            "1234567890.2.1",     # The major version is too long.
+            "0.2..1",             # Two periods next to each other.
+            "1.21.alpha1",        # Tag comes after period.
+            "1.21-alpha",         # No patch number but a tag is present.
+            "1.21-alpha1beta",    # Non-numeric string after the patch number.
+            "1.21-alpha-dev",     # Tag duplication.
+            "1.21-alpha_dev3",    # Underscore in a tag.
+            "1.21-alpha123dev3",  # Non-numeric string after the patch number.
+        ]
+        for v in invalid_versions:
+            with self.assertRaises(RuntimeError):
+                JujuVersion(v)
+
+    def test_equality(self):
+        test_cases = [
+            ("1.0.0", "1.0.0", True),
+            ("01.0.0", "1.0.0", True),
+            ("10.0.0", "9.0.0", False),
+            ("1.0.0", "1.0.1", False),
+            ("1.0.1", "1.0.0", False),
+            ("1.0.0", "1.1.0", False),
+            ("1.1.0", "1.0.0", False),
+            ("1.0.0", "2.0.0", False),
+            ("1.2-alpha1", "1.2.0", False),
+            ("1.2-alpha2", "1.2-alpha1", False),
+            ("1.2-alpha2.1", "1.2-alpha2", False),
+            ("1.2-alpha2.2", "1.2-alpha2.1", False),
+            ("1.2-beta1", "1.2-alpha1", False),
+            ("1.2-beta1", "1.2-alpha2.1", False),
+            ("1.2-beta1", "1.2.0", False),
+            ("1.2.1", "1.2.0", False),
+            ("2.0.0", "1.0.0", False),
+            ("2.0.0.0", "2.0.0", True),
+            ("2.0.0.0", "2.0.0.0", True),
+            ("2.0.0.1", "2.0.0.0", False),
+            ("2.0.1.10", "2.0.0.0", False),
+        ]
+
+        for a, b, expected in test_cases:
+            self.assertEqual(JujuVersion(a) == JujuVersion(b), expected)
+            self.assertEqual(JujuVersion(a) == b, expected)
+
+    def test_comparison(self):
+        test_cases = [
+            ("1.0.0", "1.0.0", False, True),
+            ("01.0.0", "1.0.0", False, True),
+            ("10.0.0", "9.0.0", False, False),
+            ("1.0.0", "1.0.1", True, True),
+            ("1.0.1", "1.0.0", False, False),
+            ("1.0.0", "1.1.0", True, True),
+            ("1.1.0", "1.0.0", False, False),
+            ("1.0.0", "2.0.0", True, True),
+            ("1.2-alpha1", "1.2.0", True, True),
+            ("1.2-alpha2", "1.2-alpha1", False, False),
+            ("1.2-alpha2.1", "1.2-alpha2", False, False),
+            ("1.2-alpha2.2", "1.2-alpha2.1", False, False),
+            ("1.2-beta1", "1.2-alpha1", False, False),
+            ("1.2-beta1", "1.2-alpha2.1", False, False),
+            ("1.2-beta1", "1.2.0", True, True),
+            ("1.2.1", "1.2.0", False, False),
+            ("2.0.0", "1.0.0", False, False),
+            ("2.0.0.0", "2.0.0", False, True),
+            ("2.0.0.0", "2.0.0.0", False, True),
+            ("2.0.0.1", "2.0.0.0", False, False),
+            ("2.0.1.10", "2.0.0.0", False, False),
+        ]
+
+        for a, b, expected_strict, expected_weak in test_cases:
+            self.assertEqual(JujuVersion(a) < JujuVersion(b), expected_strict)
+            self.assertEqual(JujuVersion(a) <= JujuVersion(b), expected_weak)
+            self.assertEqual(JujuVersion(b) > JujuVersion(a), expected_strict)
+            self.assertEqual(JujuVersion(b) >= JujuVersion(a), expected_weak)
+            # Implicit conversion.
+            self.assertEqual(JujuVersion(a) < b, expected_strict)
+            self.assertEqual(JujuVersion(a) <= b, expected_weak)
+            self.assertEqual(b > JujuVersion(a), expected_strict)
+            self.assertEqual(b >= JujuVersion(a), expected_weak)