charm-ubuntu-advantage

Merge ~cmisare/charm-ubuntu-advantage:master into charm-ubuntu-advantage:master

  1. Git
  2. lp:~cmisare/charm-ubuntu-advantage
  3. master
  4. Merge into master
Proposed by Colin Misare
Status: Merged
Approved by: Colin Misare
Approved revision: 1a08cb2c61a0cbc66e7d333bf4d68e2a3ed2e579
Merged at revision: 5d93cdba18d77c1562c7e7542ac280bc47dfdb17
Proposed branch: ~cmisare/charm-ubuntu-advantage:master
Merge into: charm-ubuntu-advantage:master
Diff against target: 91 lines (+12/-3)
3 files modified
requirements.txt (+1/-1)
src/charm.py (+3/-0)
tests/test_charm.py (+8/-2)
Reviewer Review Type Date Requested Status
Haw Loeung +1 Approve
Canonical IS Reviewers Pending
Review via email: mp+402298@code.launchpad.net

Commit message

Bug fix and improved logging

- Pinning to ops==1.1.0 until until https://github.com/canonical/operator/pull/520 is published to allow the charm to run on xenial
- Adding a bit more logging
- Fixing an issue with open() in 'r+' mode where truncate() wasn't being called which led to extra content at the end of the file

Description of the change

Bug fix and improved logging

- Pinning to ops==1.1.0 until until https://github.com/canonical/operator/pull/520 is published to allow the charm to run on xenial
- Adding a bit more logging
- Fixing an issue with open() in 'r+' mode where truncate() wasn't being called which led to extra content at the end of the file

To post a comment you must log in.
Revision history for this message
Haw Loeung (hloeung) wrote :

LGTM

review: Approve (+1)
Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Revision history for this message
🤖 Canonical IS Merge Bot (canonical-is-mergebot) wrote :

Change successfully merged at revision 5d93cdba18d77c1562c7e7542ac280bc47dfdb17

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/requirements.txt b/requirements.txt
2index 2d81d3b..79f7bc1 100644
3--- a/requirements.txt
4+++ b/requirements.txt
5@@ -1 +1 @@
6-ops
7+ops==1.1.0
8diff --git a/src/charm.py b/src/charm.py
9index 16e29f6..25d7d0a 100755
10--- a/src/charm.py
11+++ b/src/charm.py
12@@ -46,6 +46,7 @@ def update_configuration(contract_url):
13 client_config["contract_url"] = contract_url
14 f.seek(0)
15 yaml.dump(client_config, f)
16+ f.truncate()
17
18
19 def detach_subscription():
20@@ -138,6 +139,7 @@ class UbuntuAdvantageCharm(CharmBase):
21 self._state.hashed_token = None
22
23 if config_changed:
24+ logger.info("Updating uaclient.conf")
25 update_configuration(contract_url)
26 self._state.contract_url = contract_url
27
28@@ -145,6 +147,7 @@ class UbuntuAdvantageCharm(CharmBase):
29 self.unit.status = BlockedStatus("No token configured")
30 return
31 elif config_changed or token_changed:
32+ logger.info("Attaching ubuntu-advantage subscription")
33 return_code = attach_subscription(token)
34 if return_code != 0:
35 message = "Error attaching, possibly an invalid token or contract_url?"
36diff --git a/tests/test_charm.py b/tests/test_charm.py
37index aeca2fa..1415f7d 100644
38--- a/tests/test_charm.py
39+++ b/tests/test_charm.py
40@@ -207,6 +207,7 @@ class TestCharm(TestCase):
41 log_level: debug
42 """)
43 self.assertEqual(_written(handle), expected)
44+ handle.truncate.assert_called_once()
45 self.assertEqual(self.mocks["call"].call_count, 1)
46 self.mocks["call"].assert_has_calls([
47 call(["ubuntu-advantage", "attach", "test-token"])
48@@ -238,6 +239,7 @@ class TestCharm(TestCase):
49 log_level: debug
50 """)
51 self.assertEqual(_written(handle), expected)
52+ handle.truncate.assert_called_once()
53 self.assertEqual(self.mocks["call"].call_count, 1)
54 self.mocks["call"].assert_has_calls([
55 call(["ubuntu-advantage", "attach", "test-token"])
56@@ -361,6 +363,7 @@ class TestCharm(TestCase):
57 log_level: debug
58 """)
59 self.assertEqual(_written(handle), expected)
60+ handle.truncate.assert_called_once()
61 self.assertEqual(self.harness.charm._state.contract_url,
62 "https://contracts.staging.canonical.com")
63
64@@ -395,6 +398,7 @@ class TestCharm(TestCase):
65 log_level: debug
66 """)
67 self.assertEqual(_written(handle), expected)
68+ handle.truncate.assert_called_once()
69 self.mocks["check_call"].assert_has_calls([
70 call(["ubuntu-advantage", "detach", "--assume-yes"])
71 ])
72@@ -431,8 +435,9 @@ class TestCharm(TestCase):
73 log_file: /var/log/ubuntu-advantage.log
74 log_level: debug
75 """)
76- self.mocks["call"].assert_not_called()
77 self.assertEqual(_written(handle), expected)
78+ handle.truncate.assert_called_once()
79+ self.mocks["call"].assert_not_called()
80 self.assertEqual(self.harness.model.unit.status, BlockedStatus("No token configured"))
81 self.mocks["open"].reset_mock()
82 self.mocks["call"].reset_mock()
83@@ -451,6 +456,7 @@ class TestCharm(TestCase):
84 log_file: /var/log/ubuntu-advantage.log
85 log_level: debug
86 """)
87- self.mocks["call"].assert_not_called()
88 self.assertEqual(_written(handle), expected)
89+ handle.truncate.assert_called_once()
90+ self.mocks["call"].assert_not_called()
91 self.assertEqual(self.harness.model.unit.status, BlockedStatus("No token configured"))

Subscribers

People subscribed via source and target branches