Landscape Charm

Merge lp:~simpoir/landscape-charm/1682105_revisit_config into lp:~landscape/landscape-charm/trunk

  1. 1682105_revisit_config
  2. Merge into trunk
Proposed by Simon Poirier
Status: Merged
Approved by: Simon Poirier
Approved revision: 406
Merged at revision: 408
Proposed branch: lp:~simpoir/landscape-charm/1682105_revisit_config
Merge into: lp:~landscape/landscape-charm/trunk
Diff against target: 374 lines (+225/-21)
5 files modified
config.yaml (+23/-0)
lib/callbacks/scripts.py (+43/-12)
lib/callbacks/tests/test_scripts.py (+144/-7)
lib/services.py (+7/-2)
lib/tests/stubs.py (+8/-0)
To merge this branch: bzr merge lp:~simpoir/landscape-charm/1682105_revisit_config
Reviewer Review Type Date Requested Status
🤖 Landscape Builder test results Approve
Kevin Nasto Approve
Review via email: mp+412618@code.launchpad.net

Commit message

Make proxy, system-email and root-url configurations updatable through the charm config.

Description of the change

Adds a few configuration to the charm.
Those were previously only updatable from the UI (proxy, system-email, root-url) or on bootstrap.

Requires changes which are fix-commited to landscape/trunk, for it to do anything.

Testing:

I have repackaged 19.10 with the trunk patch to actually be able to test this branch.
Trying to change those conf with the current 19.10 ppa will do nothing (the branch checks if the flag exists)

download https://pastebin.canonical.com/p/hk84D8kqtg/ as bundle.yml
juju deploy ./bundle.yml
juju config landscape-server system-email=noreply@localhost

log to the ui, check the settings tab.

To post a comment you must log in.
Revision history for this message
🤖 Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
🤖 Landscape Builder (landscape-builder) wrote :

Command: make ci-test
Result: Fail
Revno: 404
Branch: lp:~simpoir/landscape-charm/1682105_revisit_config
Jenkins: https://jenkins.canonical.com/landscape/job/latch-test-xenial/1476/

review: Needs Fixing (test results)
lp:~simpoir/landscape-charm/1682105_revisit_config updated
405. By Simon Poirier

lint

Revision history for this message
🤖 Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
🤖 Landscape Builder (landscape-builder) wrote :

Command: make ci-test
Result: Success
Revno: 405
Branch: lp:~simpoir/landscape-charm/1682105_revisit_config
Jenkins: https://jenkins.canonical.com/landscape/job/latch-test-xenial/1477/

review: Approve (test results)
Revision history for this message
Kevin Nasto (silverdrake11) wrote :

LGTM except for the typo in the yaml

review: Approve
lp:~simpoir/landscape-charm/1682105_revisit_config updated
406. By Simon Poirier

typo

Revision history for this message
🤖 Landscape Builder (landscape-builder) :
review: Abstain (executing tests)
Revision history for this message
🤖 Landscape Builder (landscape-builder) wrote :

Command: make ci-test
Result: Success
Revno: 406
Branch: lp:~simpoir/landscape-charm/1682105_revisit_config
Jenkins: https://jenkins.canonical.com/landscape/job/latch-test-xenial/1486/

review: Approve (test results)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'config.yaml'
--- config.yaml 2021-11-09 09:20:31 +0000
+++ config.yaml 2021-12-08 22:56:37 +0000
@@ -99,6 +99,11 @@
99 IP of the first related HAproxy unit will be used instead.99 IP of the first related HAproxy unit will be used instead.
100 type: string100 type: string
101 default: ""101 default: ""
102 system-email:
103 type: string
104 description: |
105 The address emails from Landscape will appear to come from.
106 default: ""
102 ssl-cert:107 ssl-cert:
103 type: string108 type: string
104 description: |109 description: |
@@ -117,6 +122,24 @@
117 description: |122 description: |
118 The SMTP server to use to deliver outgoing mail.123 The SMTP server to use to deliver outgoing mail.
119 default: ""124 default: ""
125 http-proxy:
126 default:
127 type: string
128 description: |
129 The http proxy URL Landscape will use. If left blank, the
130 model-config value will be used.
131 https-proxy:
132 default:
133 type: string
134 description: |
135 The https proxy Landscape will use. If left blank, the model-config
136 value will be used.
137 no-proxy:
138 default:
139 type: string
140 description: |
141 Comma separated list of hosts for which no proxy should be used.
142 If left blank, the model-config value will be used.
120 site-name:143 site-name:
121 type: string144 type: string
122 default: ''145 default: ''
123146
=== modified file 'lib/callbacks/scripts.py'
--- lib/callbacks/scripts.py 2019-07-10 19:02:28 +0000
+++ lib/callbacks/scripts.py 2021-12-08 22:56:37 +0000
@@ -1,4 +1,3 @@
1import os
2import subprocess1import subprocess
32
4from charmhelpers.core import hookenv3from charmhelpers.core import hookenv
@@ -14,12 +13,15 @@
14# Database relation keys for which, in case of change, a restart is not needed.13# Database relation keys for which, in case of change, a restart is not needed.
15NO_RESTART_DB_RELATION_KEYS = {"allowed-units"}14NO_RESTART_DB_RELATION_KEYS = {"allowed-units"}
1615
16CONFIG_ONLY_FLAG = "--configure-lds-only"
17
1718
18class ScriptCallback(ManagerCallback):19class ScriptCallback(ManagerCallback):
19 """Callback class for invoking Landscape scripts."""20 """Callback class for invoking Landscape scripts."""
2021
21 def __init__(self, subprocess=subprocess):22 def __init__(self, subprocess=subprocess, hookenv=hookenv):
22 self._subprocess = subprocess23 self._subprocess = subprocess
24 self._hookenv = hookenv
2325
24 def _run(self, name, options=()):26 def _run(self, name, options=()):
25 """Run the script with the given name and options."""27 """Run the script with the given name and options."""
@@ -34,12 +36,21 @@
34 This will invoke the schema script with the --bootstrap flag, if it hasn't36 This will invoke the schema script with the --bootstrap flag, if it hasn't
35 been called yet.37 been called yet.
36 """38 """
39
37 def __call__(self, manager, service_name, event_name):40 def __call__(self, manager, service_name, event_name):
41 self._schema_doc = self._subprocess.check_output([SCHEMA_SCRIPT, "-h"])
38 if not manager.was_ready(service_name):42 if not manager.was_ready(service_name):
39 options = ["--bootstrap"]43 options = ["--bootstrap"]
40 options.extend(self._get_proxy_options())44 options.extend(self._get_proxy_options())
41 self._run(SCHEMA_SCRIPT, options)45 self._run(SCHEMA_SCRIPT, options)
4246
47 if CONFIG_ONLY_FLAG in self._schema_doc:
48 options = [CONFIG_ONLY_FLAG]
49 options.extend(self._get_proxy_options())
50 options.extend(self._get_settings_options())
51 if len(options) > 1:
52 self._run(SCHEMA_SCRIPT, options)
53
43 def _get_proxy_options(self):54 def _get_proxy_options(self):
44 """Return the HTTP proxy options to set.55 """Return the HTTP proxy options to set.
4556
@@ -48,15 +59,39 @@
48 at the environment variables that Juju sets for us.59 at the environment variables that Juju sets for us.
49 """60 """
50 options = []61 options = []
62 config = self._hookenv.config()
5163
52 help_output = self._subprocess.check_output([SCHEMA_SCRIPT, "-h"])64 if "--with-http-proxy" in self._schema_doc:
53 if "--with-http-proxy" in help_output:
54 # Forward any proxy configuration set in the environment65 # Forward any proxy configuration set in the environment
66 model_proxy = self._hookenv.env_proxy_settings() or {}
55 for proxy_variable in ("http_proxy", "https_proxy", "no_proxy"):67 for proxy_variable in ("http_proxy", "https_proxy", "no_proxy"):
56 if proxy_variable in os.environ:68 model_proxy_value = model_proxy.get(proxy_variable, "")
57 options.append("--with-%s=%s" % (69
58 proxy_variable.replace("_", "-"),70 # XXX There is no charm hook for model-config changes,
59 os.environ[proxy_variable]))71 # so updating proxies has to be done using charm config.
72 # See juju issue (LP: #1835050).
73 proxy_variable = proxy_variable.replace("_", "-")
74 proxy_value = config.get(proxy_variable)
75 if proxy_value is None:
76 proxy_value = model_proxy_value
77 options.append("--with-{}".format(proxy_variable))
78 options.append(proxy_value)
79
80 return options
81
82 def _get_settings_options(self):
83 """Return options for updating charm-managed settings."""
84 config = self._hookenv.config()
85 options = []
86 root_url = config.get("root-url")
87 if root_url:
88 options.append("--with-root-url")
89 options.append(root_url)
90
91 system_email = config.get("system-email")
92 if system_email:
93 options.append("--with-system-email")
94 options.append(system_email)
6095
61 return options96 return options
6297
@@ -64,10 +99,6 @@
64class LSCtl(ScriptCallback):99class LSCtl(ScriptCallback):
65 """Call the lsctl script to start or stop services."""100 """Call the lsctl script to start or stop services."""
66101
67 def __init__(self, subprocess=subprocess, hookenv=hookenv):
68 super(LSCtl, self).__init__(subprocess=subprocess)
69 self._hookenv = hookenv
70
71 def __call__(self, manager, service_name, event_name):102 def __call__(self, manager, service_name, event_name):
72 current_status, current_status_message = self._hookenv.status_get()103 current_status, current_status_message = self._hookenv.status_get()
73 action_status_message = ""104 action_status_message = ""
74105
=== modified file 'lib/callbacks/tests/test_scripts.py'
--- lib/callbacks/tests/test_scripts.py 2019-07-15 20:01:06 +0000
+++ lib/callbacks/tests/test_scripts.py 2021-12-08 22:56:37 +0000
@@ -3,7 +3,7 @@
3from charmhelpers.core.services.base import ServiceManager3from charmhelpers.core.services.base import ServiceManager
44
5from lib.paths import LSCTL, SCHEMA_SCRIPT5from lib.paths import LSCTL, SCHEMA_SCRIPT
6from lib.callbacks.scripts import SchemaBootstrap, LSCtl6from lib.callbacks.scripts import SchemaBootstrap, LSCtl, CONFIG_ONLY_FLAG
7from lib.utils import update_persisted_data7from lib.utils import update_persisted_data
8from lib.tests.helpers import HookenvTest8from lib.tests.helpers import HookenvTest
9from lib.tests.stubs import SubprocessStub9from lib.tests.stubs import SubprocessStub
@@ -19,7 +19,8 @@
19 self.subprocess = SubprocessStub()19 self.subprocess = SubprocessStub()
20 self.subprocess.add_fake_executable(SCHEMA_SCRIPT)20 self.subprocess.add_fake_executable(SCHEMA_SCRIPT)
21 self.manager = ServiceManager(services=[{"service": "landscape"}])21 self.manager = ServiceManager(services=[{"service": "landscape"}])
22 self.callback = SchemaBootstrap(subprocess=self.subprocess)22 self.callback = SchemaBootstrap(
23 subprocess=self.subprocess, hookenv=self.hookenv)
2324
24 def test_options(self):25 def test_options(self):
25 """26 """
@@ -56,18 +57,154 @@
56 self.callback(self.manager, "landscape", None)57 self.callback(self.manager, "landscape", None)
57 self.assertEqual(58 self.assertEqual(
58 ["/usr/bin/landscape-schema", "--bootstrap",59 ["/usr/bin/landscape-schema", "--bootstrap",
59 "--with-http-proxy=http://foo:3128",60 "--with-http-proxy", "http://foo:3128",
60 "--with-https-proxy=http://bar:3128",61 "--with-https-proxy", "http://bar:3128",
61 "--with-no-proxy=localhost"],62 "--with-no-proxy", "localhost"],
62 self.subprocess.calls[1][0])63 self.subprocess.calls[1][0])
6364
64 def test_was_ready(self):65 def test_was_ready(self):
65 """66 """
66 If the services was ready, the schema script is not invoked again.67 If the services was ready, the schema bootstrap is not invoked again.
67 """68 """
68 self.manager.save_ready("landscape")69 self.manager.save_ready("landscape")
69 self.callback(self.manager, "landscape", None)70 self.callback(self.manager, "landscape", None)
70 self.assertEqual([], self.subprocess.calls)71 self.assertEqual([
72 (['/usr/bin/landscape-schema', '-h'], {})], self.subprocess.calls)
73
74 def test_update_config(self):
75 """Updating config are updated in Landscape."""
76 self.subprocess.add_fake_executable(
77 SCHEMA_SCRIPT, args=["-h"], stdout="Usage: " + CONFIG_ONLY_FLAG)
78
79 self.hookenv.hook = "config-changed"
80 config = self.hookenv.config()
81 config["root-url"] = "https://old.sad/"
82 config.save()
83 self.manager.save_ready("landscape")
84
85 config["root-url"] = "https://happy.new/"
86 self.callback(self.manager, "landscape", None)
87 self.assertEqual([
88 (["/usr/bin/landscape-schema", "-h"], {}),
89 (["/usr/bin/landscape-schema", CONFIG_ONLY_FLAG, "--with-root-url",
90 "https://happy.new/"], {}),
91 ], self.subprocess.calls)
92
93 def test_update_config_unsupported_flag(self):
94 """Configuration is NOOP if the configure-lds flag is not supported."""
95 self.subprocess.add_fake_executable(
96 SCHEMA_SCRIPT, args=["-h"], stdout="Usage: --spam")
97
98 self.hookenv.hook = "config-changed"
99 config = self.hookenv.config()
100 config["root-url"] = "https://old.sad/"
101 config.save()
102 self.manager.save_ready("landscape")
103 config["root-url"] = "https://happy.new/"
104
105 self.callback(self.manager, "landscape", None)
106 self.assertEqual([
107 (["/usr/bin/landscape-schema", "-h"], {}),
108 ], self.subprocess.calls)
109
110 def test_bootstrap_and_configure(self):
111 """Configuration is done after bootstrap, if config values exist."""
112 self.subprocess.add_fake_executable(
113 SCHEMA_SCRIPT, args=["-h"], stdout="Usage: " + CONFIG_ONLY_FLAG)
114
115 self.hookenv.hook = "config-changed"
116 config = self.hookenv.config()
117 config["system-email"] = "noreply@spam"
118 config.save()
119 config["system-email"] = "noreply@scape"
120
121 self.callback(self.manager, "landscape", None)
122 self.assertEqual([
123 (["/usr/bin/landscape-schema", "-h"], {}),
124 (["/usr/bin/landscape-schema", "--bootstrap"], {}),
125 (["/usr/bin/landscape-schema", CONFIG_ONLY_FLAG,
126 "--with-system-email", "noreply@scape"], {}),
127 ], self.subprocess.calls)
128
129 def test_inherit_model_proxy(self):
130 """Proxy config is inherited from the model if unset on charm."""
131 self.subprocess.add_fake_executable(
132 SCHEMA_SCRIPT, args=["-h"],
133 stdout="Usage: --with-http-proxy " + CONFIG_ONLY_FLAG)
134 self.hookenv.hook = "config-changed"
135 config = self.hookenv.config()
136 config["http-proxy"] = "spam"
137 config["https-proxy"] = "spam"
138 config["no-proxy"] = "spam"
139 config.save()
140 del config["http-proxy"]
141 del config["https-proxy"]
142 del config["no-proxy"]
143 self.useFixture(EnvironmentVariable("http_proxy", "http://foo:3128"))
144 self.useFixture(EnvironmentVariable("https_proxy", "http://bar:3128"))
145 self.useFixture(EnvironmentVariable("no_proxy", "localhost"))
146 self.manager.save_ready("landscape")
147
148 self.callback(self.manager, "landscape", None)
149 self.assertEqual([
150 (["/usr/bin/landscape-schema", "-h"], {}),
151 (["/usr/bin/landscape-schema", CONFIG_ONLY_FLAG,
152 "--with-http-proxy", "http://foo:3128",
153 "--with-https-proxy", "http://bar:3128",
154 "--with-no-proxy", "localhost"], {}),
155 ], self.subprocess.calls)
156
157 def test_change_model_proxy(self):
158 """Proxy config override model proxy if configured on charm."""
159 self.subprocess.add_fake_executable(
160 SCHEMA_SCRIPT, args=["-h"],
161 stdout="Usage: --with-http-proxy " + CONFIG_ONLY_FLAG)
162 self.hookenv.hook = "config-changed"
163 config = self.hookenv.config()
164 self.useFixture(EnvironmentVariable("http_proxy", "spam"))
165 self.useFixture(EnvironmentVariable("https_proxy", "spam"))
166 self.useFixture(EnvironmentVariable("no_proxy", "spam"))
167 self.manager.save_ready("landscape")
168 config["http-proxy"] = "http://foo:3128"
169 config["https-proxy"] = "http://bar:3128"
170 config["no-proxy"] = "localhost"
171
172 self.callback(self.manager, "landscape", None)
173 self.assertEqual([
174 (["/usr/bin/landscape-schema", "-h"], {}),
175 (["/usr/bin/landscape-schema", CONFIG_ONLY_FLAG,
176 "--with-http-proxy", "http://foo:3128",
177 "--with-https-proxy", "http://bar:3128",
178 "--with-no-proxy", "localhost"], {}),
179 ], self.subprocess.calls)
180
181 def test_unset_model_proxy(self):
182 """Proxy config can explicitly unset model proxy."""
183 self.subprocess.add_fake_executable(
184 SCHEMA_SCRIPT, args=["-h"],
185 stdout="Usage: --with-http-proxy " + CONFIG_ONLY_FLAG)
186 self.hookenv.hook = "config-changed"
187 config = self.hookenv.config()
188 self.useFixture(EnvironmentVariable("http_proxy", "spam"))
189 self.useFixture(EnvironmentVariable("https_proxy", "spam"))
190 self.useFixture(EnvironmentVariable("no_proxy", "spam"))
191 self.manager.save_ready("landscape")
192 config["http-proxy"] = ""
193 config["https-proxy"] = ""
194 config["no-proxy"] = ""
195
196 self.callback(self.manager, "landscape", None)
197 self.assertEqual([
198 (["/usr/bin/landscape-schema", "-h"], {}),
199 (["/usr/bin/landscape-schema", CONFIG_ONLY_FLAG,
200 "--with-http-proxy", "",
201 "--with-https-proxy", "",
202 "--with-no-proxy", ""], {}),
203 ], self.subprocess.calls)
204
205 def test_reconfigure_noop(self):
206 """Nothing happens if there is no proxy and no config change."""
207 # TODO
71208
72209
73class LSCtlTest(HookenvTest):210class LSCtlTest(HookenvTest):
74211
=== modified file 'lib/services.py'
--- lib/services.py 2021-11-09 09:20:31 +0000
+++ lib/services.py 2021-12-08 22:56:37 +0000
@@ -86,7 +86,8 @@
86 subprocess=self._subprocess),86 subprocess=self._subprocess),
87 EnsureConfigDir(paths=self._paths),87 EnsureConfigDir(paths=self._paths),
88 WriteCustomSSLCertificate(paths=self._paths),88 WriteCustomSSLCertificate(paths=self._paths),
89 SchemaBootstrap(subprocess=self._subprocess),89 SchemaBootstrap(
90 subprocess=self._subprocess, hookenv=self._hookenv),
90 WriteLicenseFile(host=self._host, paths=self._paths),91 WriteLicenseFile(host=self._host, paths=self._paths),
91 ConfigureSMTP(92 ConfigureSMTP(
92 hookenv=self._hookenv, subprocess=self._subprocess),93 hookenv=self._hookenv, subprocess=self._subprocess),
@@ -95,4 +96,8 @@
95 ],96 ],
96 "start": LSCtl(subprocess=self._subprocess, hookenv=self._hookenv),97 "start": LSCtl(subprocess=self._subprocess, hookenv=self._hookenv),
97 }])98 }])
98 manager.manage()99 try:
100 manager.manage()
101 except Exception as e:
102 self._hookenv.log(str(e), level=hookenv.ERROR)
103 raise
99104
=== modified file 'lib/tests/stubs.py'
--- lib/tests/stubs.py 2021-11-10 05:34:36 +0000
+++ lib/tests/stubs.py 2021-12-08 22:56:37 +0000
@@ -1,3 +1,4 @@
1import os
1import subprocess2import subprocess
23
3from charmhelpers.core.hookenv import Config4from charmhelpers.core.hookenv import Config
@@ -33,6 +34,13 @@
33 def config(self):34 def config(self):
34 return self._config35 return self._config
3536
37 def env_proxy_settings(self):
38 return {
39 k: os.environ[k]
40 for k in ("http_proxy", "https_proxy", "no_proxy")
41 if k in os.environ
42 } or None
43
36 def relations_of_type(self, reltype):44 def relations_of_type(self, reltype):
37 return self.relations.get(reltype, None)45 return self.relations.get(reltype, None)
3846

Subscribers

People subscribed via source and target branches