Merge lp:~free.ekanayaka/charms/trusty/landscape-client/drop-unit-name-from-juju-info into lp:charms/trusty/landscape-client
- Trusty Tahr (14.04)
- drop-unit-name-from-juju-info
- Merge into trunk
Proposed by
Free Ekanayaka
Status: | Superseded |
---|---|
Proposed branch: | lp:~free.ekanayaka/charms/trusty/landscape-client/drop-unit-name-from-juju-info |
Merge into: | lp:charms/trusty/landscape-client |
Diff against target: |
251 lines (+40/-54) 4 files modified
hooks/common.py (+17/-8) hooks/hooks.py (+5/-21) hooks/install.py (+6/-2) hooks/test_hooks.py (+12/-23) |
To merge this branch: | bzr merge lp:~free.ekanayaka/charms/trusty/landscape-client/drop-unit-name-from-juju-info |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Landscape | Pending | ||
Landscape | Pending | ||
Review via email: mp+235756@code.launchpad.net |
Commit message
Description of the change
This branch drops the unit-name field from the content of the juju info file, and changes the code to generate a single json file as opposed to one per principal unit, since we now only need the machine->computer association.
To post a comment you must log in.
- 47. By Free Ekanayaka
-
Drop private-address from juju-info
Unmerged revisions
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'hooks/common.py' | |||
2 | --- hooks/common.py 2014-08-28 09:54:11 +0000 | |||
3 | +++ hooks/common.py 2014-09-24 07:49:01 +0000 | |||
4 | @@ -30,6 +30,19 @@ | |||
5 | 30 | """Get relation configuration from Juju.""" | 30 | """Get relation configuration from Juju.""" |
6 | 31 | return self._run_juju_tool("relation-get") | 31 | return self._run_juju_tool("relation-get") |
7 | 32 | 32 | ||
8 | 33 | def get_machine_id(self): | ||
9 | 34 | """Return the Juju machine ID of this unit.""" | ||
10 | 35 | # XXX once #1359714 is fixed this method can be dropped and we can use | ||
11 | 36 | # the JUJU_MACHINE_ID environment variable. | ||
12 | 37 | |||
13 | 38 | # Sniff the value of the machine ID by looking at the name of the | ||
14 | 39 | # directory where hooks live. It will be something like | ||
15 | 40 | # 'machine-0-lxc-1'. | ||
16 | 41 | pattern = "../../machine-*" | ||
17 | 42 | match = glob(pattern)[0] | ||
18 | 43 | dirname = os.path.basename(match) | ||
19 | 44 | return dirname.lstrip("machine-").replace("-", "/") | ||
20 | 45 | |||
21 | 33 | def get_service_config(self): | 46 | def get_service_config(self): |
22 | 34 | """Get service configuration from Juju.""" | 47 | """Get service configuration from Juju.""" |
23 | 35 | return self._run_juju_tool("config-get") | 48 | return self._run_juju_tool("config-get") |
24 | @@ -88,7 +101,7 @@ | |||
25 | 88 | is_configured_enough = all([ | 101 | is_configured_enough = all([ |
26 | 89 | self.config.get("account_name"), | 102 | self.config.get("account_name"), |
27 | 90 | self.config.get("computer_title"), | 103 | self.config.get("computer_title"), |
29 | 91 | get_juju_info_filenames(self.juju_directory)]) | 104 | os.path.exists(self.juju_filename)]) |
30 | 92 | if not try_to_register: | 105 | if not try_to_register: |
31 | 93 | # We don't need to further check the configuration if we are | 106 | # We don't need to further check the configuration if we are |
32 | 94 | # deleting it. | 107 | # deleting it. |
33 | @@ -102,11 +115,11 @@ | |||
34 | 102 | return 0 | 115 | return 0 |
35 | 103 | 116 | ||
36 | 104 | @property | 117 | @property |
38 | 105 | def juju_directory(self): | 118 | def juju_filename(self): |
39 | 106 | try: | 119 | try: |
41 | 107 | return self.config.juju_directory | 120 | return self.config.juju_filename |
42 | 108 | except AttributeError: # older landscape-client versions | 121 | except AttributeError: # older landscape-client versions |
44 | 109 | return os.path.join(self.config.data_path, "juju-info.d") | 122 | return os.path.join(self.config.data_path, "juju-info.json") |
45 | 110 | 123 | ||
46 | 111 | def clear_registration(self): | 124 | def clear_registration(self): |
47 | 112 | """ | 125 | """ |
48 | @@ -187,7 +200,3 @@ | |||
49 | 187 | """Dump content in JSON format to the specified file.""" | 200 | """Dump content in JSON format to the specified file.""" |
50 | 188 | with open(filename, "w") as dumpfile: | 201 | with open(filename, "w") as dumpfile: |
51 | 189 | json.dump(content, dumpfile) | 202 | json.dump(content, dumpfile) |
52 | 190 | |||
53 | 191 | |||
54 | 192 | def get_juju_info_filenames(juju_directory): | ||
55 | 193 | return glob("%s/*.json" % juju_directory) | ||
56 | 194 | 203 | ||
57 | === modified file 'hooks/hooks.py' | |||
58 | --- hooks/hooks.py 2014-08-28 09:54:11 +0000 | |||
59 | +++ hooks/hooks.py 2014-09-24 07:49:01 +0000 | |||
60 | @@ -11,8 +11,7 @@ | |||
61 | 11 | from subprocess import CalledProcessError | 11 | from subprocess import CalledProcessError |
62 | 12 | 12 | ||
63 | 13 | from common import ( | 13 | from common import ( |
66 | 14 | write_json_file, JujuBroker, LandscapeBroker, chown, | 14 | write_json_file, JujuBroker, LandscapeBroker, chown) |
65 | 15 | get_juju_info_filenames) | ||
67 | 16 | from ceph import ( | 15 | from ceph import ( |
68 | 17 | get_ceph_client_path, write_ceph_client_keyring, write_ceph_client_config) | 16 | get_ceph_client_path, write_ceph_client_keyring, write_ceph_client_config) |
69 | 18 | 17 | ||
70 | @@ -71,29 +70,15 @@ | |||
71 | 71 | "" % landscape_broker.local_unit) | 70 | "" % landscape_broker.local_unit) |
72 | 72 | landscape_broker.config.reload() | 71 | landscape_broker.config.reload() |
73 | 73 | relation_conf = juju_broker.get_relation_config() | 72 | relation_conf = juju_broker.get_relation_config() |
74 | 74 | remote_unit_name = juju_broker.environment.get("JUJU_REMOTE_UNIT") | ||
75 | 75 | # We use the remote unit for the unit name, since we want to associate this | 73 | # We use the remote unit for the unit name, since we want to associate this |
76 | 76 | # client with the unit it's managing, not its own unit. | 74 | # client with the unit it's managing, not its own unit. |
77 | 77 | juju_info = { | 75 | juju_info = { |
78 | 78 | "environment-uuid": juju_broker.environment.get("JUJU_ENV_UUID"), | 76 | "environment-uuid": juju_broker.environment.get("JUJU_ENV_UUID"), |
80 | 79 | "unit-name": remote_unit_name, | 77 | "machine-id": juju_broker.get_machine_id(), |
81 | 80 | "api-addresses": juju_broker.environment.get("JUJU_API_ADDRESSES"), | 78 | "api-addresses": juju_broker.environment.get("JUJU_API_ADDRESSES"), |
82 | 81 | "private-address": relation_conf.get("private-address")} | 79 | "private-address": relation_conf.get("private-address")} |
83 | 82 | 80 | ||
98 | 83 | # Let's not use any "/" in a filename. | 81 | write_json_file(landscape_broker.juju_filename, juju_info) |
85 | 84 | unit_name = remote_unit_name.replace("/", "-") | ||
86 | 85 | juju_file = "%s.json" % unit_name | ||
87 | 86 | |||
88 | 87 | juju_info_dir = landscape_broker.juju_directory | ||
89 | 88 | |||
90 | 89 | try: | ||
91 | 90 | os.mkdir(juju_info_dir) | ||
92 | 91 | except OSError: | ||
93 | 92 | pass # The file already exists. | ||
94 | 93 | |||
95 | 94 | juju_info_file = os.path.join(juju_info_dir, juju_file) | ||
96 | 95 | |||
97 | 96 | write_json_file(juju_info_file, juju_info) | ||
99 | 97 | exit_code = landscape_broker.update_client_config( | 82 | exit_code = landscape_broker.update_client_config( |
100 | 98 | {"computer-title": socket.gethostname()}) | 83 | {"computer-title": socket.gethostname()}) |
101 | 99 | 84 | ||
102 | @@ -111,9 +96,8 @@ | |||
103 | 111 | landscape_broker.clear_registration() | 96 | landscape_broker.clear_registration() |
104 | 112 | 97 | ||
105 | 113 | # Delete all of the juju-info files. | 98 | # Delete all of the juju-info files. |
109 | 114 | files_to_delete = get_juju_info_filenames(landscape_broker.juju_directory) | 99 | if os.path.exists(landscape_broker.juju_filename): |
110 | 115 | for to_delete in files_to_delete: | 100 | os.remove(landscape_broker.juju_filename) |
108 | 116 | os.remove(to_delete) | ||
111 | 117 | landscape_broker.update_client_config( | 101 | landscape_broker.update_client_config( |
112 | 118 | {"computer-title": ""}, try_to_register=False) | 102 | {"computer-title": ""}, try_to_register=False) |
113 | 119 | 103 | ||
114 | 120 | 104 | ||
115 | === modified file 'hooks/install.py' | |||
116 | --- hooks/install.py 2014-08-28 10:09:11 +0000 | |||
117 | +++ hooks/install.py 2014-09-24 07:49:01 +0000 | |||
118 | @@ -8,6 +8,9 @@ | |||
119 | 8 | import os | 8 | import os |
120 | 9 | import subprocess | 9 | import subprocess |
121 | 10 | import sys | 10 | import sys |
122 | 11 | |||
123 | 12 | from glob import glob | ||
124 | 13 | |||
125 | 11 | from charmhelpers.core.hookenv import ( | 14 | from charmhelpers.core.hookenv import ( |
126 | 12 | Hooks, UnregisteredHookError, log) | 15 | Hooks, UnregisteredHookError, log) |
127 | 13 | from charmhelpers.fetch import ( | 16 | from charmhelpers.fetch import ( |
128 | @@ -43,6 +46,7 @@ | |||
129 | 43 | data_path]) | 46 | data_path]) |
130 | 44 | 47 | ||
131 | 45 | 48 | ||
132 | 49 | |||
133 | 46 | def build_from_launchpad(url): | 50 | def build_from_launchpad(url): |
134 | 47 | """The charm will install the code from the passed lp branch. | 51 | """The charm will install the code from the passed lp branch. |
135 | 48 | """ | 52 | """ |
136 | @@ -55,8 +59,8 @@ | |||
137 | 55 | subprocess.check_call(["make", "package"], env=env) | 59 | subprocess.check_call(["make", "package"], env=env) |
138 | 56 | #TODO: The following call should be retried (potential race condition to | 60 | #TODO: The following call should be retried (potential race condition to |
139 | 57 | # acquire the dpkg lock). | 61 | # acquire the dpkg lock). |
142 | 58 | subprocess.call(["dpkg", "-i", "../landscape-client_*.deb", | 62 | subprocess.call(["dpkg", "-i", glob("../landscape-client_*.deb")[0], |
143 | 59 | "../landscape-common_*.deb"]) | 63 | glob("../landscape-common_*.deb")[0]]) |
144 | 60 | # The _run_apt_command will ensure the command is retried in case we cannot | 64 | # The _run_apt_command will ensure the command is retried in case we cannot |
145 | 61 | # acquire the lock for some reason. | 65 | # acquire the lock for some reason. |
146 | 62 | _run_apt_command(["apt-get", "-f", "install"], fatal=True) | 66 | _run_apt_command(["apt-get", "-f", "install"], fatal=True) |
147 | 63 | 67 | ||
148 | === modified file 'hooks/test_hooks.py' | |||
149 | --- hooks/test_hooks.py 2014-08-28 09:54:11 +0000 | |||
150 | +++ hooks/test_hooks.py 2014-09-24 07:49:01 +0000 | |||
151 | @@ -30,6 +30,9 @@ | |||
152 | 30 | def log(self, message): | 30 | def log(self, message): |
153 | 31 | self.logs.append(message) | 31 | self.logs.append(message) |
154 | 32 | 32 | ||
155 | 33 | def get_machine_id(self): | ||
156 | 34 | return "1" | ||
157 | 35 | |||
158 | 33 | def _run_juju_tool(self, command): | 36 | def _run_juju_tool(self, command): |
159 | 34 | """Override _run_juju_tool not not execute any commands.""" | 37 | """Override _run_juju_tool not not execute any commands.""" |
160 | 35 | return self.commands[command] | 38 | return self.commands[command] |
161 | @@ -64,12 +67,8 @@ | |||
162 | 64 | self.landscape_broker.config.write() | 67 | self.landscape_broker.config.write() |
163 | 65 | self.juju_broker = FakeJujuBroker() | 68 | self.juju_broker = FakeJujuBroker() |
164 | 66 | 69 | ||
171 | 67 | def touch_example_juju_info_file(self): | 70 | def touch_juju_info_file(self): |
172 | 68 | if not os.path.exists(self.landscape_broker.juju_directory): | 71 | write_json_file(self.landscape_broker.juju_filename, content={}) |
167 | 69 | os.mkdir(self.landscape_broker.juju_directory) | ||
168 | 70 | filename = os.path.join(self.landscape_broker.juju_directory, | ||
169 | 71 | "whatever.json") | ||
170 | 72 | write_json_file(filename=filename, content={}) | ||
173 | 73 | 72 | ||
174 | 74 | def mkdtemp(self): | 73 | def mkdtemp(self): |
175 | 75 | directory = mkdtemp(prefix=self.__class__.__name__) | 74 | directory = mkdtemp(prefix=self.__class__.__name__) |
176 | @@ -99,6 +98,7 @@ | |||
177 | 99 | single JSON file. | 98 | single JSON file. |
178 | 100 | """ | 99 | """ |
179 | 101 | juju_info = {"juju-env-uuid": "0afcd28d-6263-43fb-8131-afa696a984f8", | 100 | juju_info = {"juju-env-uuid": "0afcd28d-6263-43fb-8131-afa696a984f8", |
180 | 101 | "juju-machine-id": "1", | ||
181 | 102 | "juju-unit-name": "postgresql/0", | 102 | "juju-unit-name": "postgresql/0", |
182 | 103 | "juju-api-addresses": "10.0.3.1:17070", | 103 | "juju-api-addresses": "10.0.3.1:17070", |
183 | 104 | "juju-private-address": "10.0.3.205"} | 104 | "juju-private-address": "10.0.3.205"} |
184 | @@ -171,20 +171,17 @@ | |||
185 | 171 | """ | 171 | """ |
186 | 172 | self.juju_broker.environment.update({ | 172 | self.juju_broker.environment.update({ |
187 | 173 | "JUJU_ENV_UUID": "uuid1", | 173 | "JUJU_ENV_UUID": "uuid1", |
188 | 174 | "JUJU_REMOTE_UNIT": "unit/0", | ||
189 | 175 | "JUJU_API_ADDRESSES": "10.0.0.1:1234"}) | 174 | "JUJU_API_ADDRESSES": "10.0.0.1:1234"}) |
190 | 176 | self.juju_broker.commands["relation-get"] = { | 175 | self.juju_broker.commands["relation-get"] = { |
191 | 177 | "private-address": "10.0.0.99"} | 176 | "private-address": "10.0.0.99"} |
192 | 178 | hooks.container_relation_joined(juju_broker=self.juju_broker, | 177 | hooks.container_relation_joined(juju_broker=self.juju_broker, |
193 | 179 | landscape_broker=self.landscape_broker) | 178 | landscape_broker=self.landscape_broker) |
194 | 180 | 179 | ||
198 | 181 | juju_info_path = os.path.join( | 180 | juju_info_path = self.landscape_broker.juju_filename |
196 | 182 | self.landscape_broker.juju_directory, "unit-0.json") | ||
197 | 183 | |||
199 | 184 | juju_info = json.loads(read_file(juju_info_path)) | 181 | juju_info = json.loads(read_file(juju_info_path)) |
200 | 185 | self.assertEqual( | 182 | self.assertEqual( |
201 | 186 | {"environment-uuid": "uuid1", | 183 | {"environment-uuid": "uuid1", |
203 | 187 | "unit-name": "unit/0", | 184 | "machine-id": "1", |
204 | 188 | "api-addresses": "10.0.0.1:1234", | 185 | "api-addresses": "10.0.0.1:1234", |
205 | 189 | "private-address": "10.0.0.99"}, | 186 | "private-address": "10.0.0.99"}, |
206 | 190 | juju_info) | 187 | juju_info) |
207 | @@ -198,9 +195,7 @@ | |||
208 | 198 | registration_attempts = [] | 195 | registration_attempts = [] |
209 | 199 | 196 | ||
210 | 200 | def record_registration_attempt(): | 197 | def record_registration_attempt(): |
214 | 201 | juju_directory = self.landscape_broker.juju_directory | 198 | if os.path.exists(self.landscape_broker.juju_filename): |
212 | 202 | juju_file_list = glob("%s/*.json" % juju_directory) | ||
213 | 203 | for _ in juju_file_list: | ||
215 | 204 | registration_attempts.append(True) | 199 | registration_attempts.append(True) |
216 | 205 | 200 | ||
217 | 206 | self.landscape_broker.try_to_register = record_registration_attempt | 201 | self.landscape_broker.try_to_register = record_registration_attempt |
218 | @@ -249,14 +244,8 @@ | |||
219 | 249 | since it contains information about the remove unit (which is | 244 | since it contains information about the remove unit (which is |
220 | 250 | now gone). | 245 | now gone). |
221 | 251 | """ | 246 | """ |
228 | 252 | juju_info_path = os.path.join( | 247 | juju_info_path = self.landscape_broker.juju_filename |
223 | 253 | self.landscape_broker.juju_directory, "unit-0.json") | ||
224 | 254 | |||
225 | 255 | if not os.path.exists(self.landscape_broker.juju_directory): | ||
226 | 256 | os.mkdir(self.landscape_broker.juju_directory) | ||
227 | 257 | |||
229 | 258 | juju_info = {"environment-uuid": "uuid1", | 248 | juju_info = {"environment-uuid": "uuid1", |
230 | 259 | "unit-name": "unit/0", | ||
231 | 260 | "api-addresses": "10.0.0.1:1234", | 249 | "api-addresses": "10.0.0.1:1234", |
232 | 261 | "private-address": "10.0.0.99"} | 250 | "private-address": "10.0.0.99"} |
233 | 262 | write_json_file(juju_info_path, juju_info) | 251 | write_json_file(juju_info_path, juju_info) |
234 | @@ -406,7 +395,7 @@ | |||
235 | 406 | super(ConfigChangedTest, self).setUp() | 395 | super(ConfigChangedTest, self).setUp() |
236 | 407 | self.landscape_broker.config.account_name = "account1" | 396 | self.landscape_broker.config.account_name = "account1" |
237 | 408 | self.landscape_broker.config.computer_title = "title" | 397 | self.landscape_broker.config.computer_title = "title" |
239 | 409 | self.touch_example_juju_info_file() | 398 | self.touch_juju_info_file() |
240 | 410 | self.addCleanup(setattr, hooks, "CERT_FILE", hooks.CERT_FILE) | 399 | self.addCleanup(setattr, hooks, "CERT_FILE", hooks.CERT_FILE) |
241 | 411 | tmp_dir = self.mkdtemp() | 400 | tmp_dir = self.mkdtemp() |
242 | 412 | hooks.CERT_FILE = "%s/cert" % tmp_dir | 401 | hooks.CERT_FILE = "%s/cert" % tmp_dir |
243 | @@ -435,7 +424,7 @@ | |||
244 | 435 | super(RegistrationRelationJoinedTest, self).setUp() | 424 | super(RegistrationRelationJoinedTest, self).setUp() |
245 | 436 | self.landscape_broker.config.account_name = "account1" | 425 | self.landscape_broker.config.account_name = "account1" |
246 | 437 | self.landscape_broker.config.computer_title = "title" | 426 | self.landscape_broker.config.computer_title = "title" |
248 | 438 | self.touch_example_juju_info_file() | 427 | self.touch_juju_info_file() |
249 | 439 | self.addCleanup(setattr, hooks, "CERT_FILE", hooks.CERT_FILE) | 428 | self.addCleanup(setattr, hooks, "CERT_FILE", hooks.CERT_FILE) |
250 | 440 | tmp_dir = self.mkdtemp() | 429 | tmp_dir = self.mkdtemp() |
251 | 441 | hooks.CERT_FILE = "%s/cert" % tmp_dir | 430 | hooks.CERT_FILE = "%s/cert" % tmp_dir |