Merge ~cjwatson/launchpad:charm-assets-fix-build into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 03a8c8937bb36c0128f93a3a966182abf9e81521
Merged at revision: 31e8974697a1499593c01de0837f179f64e02334
Proposed branch: ~cjwatson/launchpad:charm-assets-fix-build
Merge into: launchpad:master
Diff against target: 117 lines (+36/-7)
5 files modified
charm/launchpad-assets/config.yaml (+4/-0)
charm/launchpad-assets/layer.yaml (+0/-2)
charm/launchpad-assets/reactive/launchpad-assets.py (+11/-0)
charm/launchpad-assets/templates/launchpad-assets-lazr.conf (+17/-1)
charm/launchpad-assets/templates/vhost.conf.j2 (+4/-4)
Reviewer Review Type Date Requested Status
Simone Pelosi Approve
Jürgen Gmach Approve
Review via email: mp+445284@code.launchpad.net

Commit message

charm: Fix assets build arrangements

Description of the change

`launchpad-assets` needs to build the files it serves (particularly API documentation) with the correct `LPCONFIG` so that they embed the correct host names. The previous arrangements of simply running `make build` when creating the virtualenv couldn't possibly achieve this, because the necessary configuration file didn't exist yet. Run `make build` after creating the configuration file instead.

Doing this uncovered various problems with the configuration file we write out: it didn't have enough entries to support running `utilities/create-lp-wadl-and-apidoc.py`. Also, since the `/srv/launchpad/code` symlink is now changed before all the assets are built, the Apache configuration now has to refer to the most-recently-configured payload directly using its build label rather than using `/srv/launchpad/code` as a shortcut.

To post a comment you must log in.
Revision history for this message
Jürgen Gmach (jugmac00) wrote :

> it didn't have enough entries to support running `utilities/create-lp-wadl-and-apidoc.py`.

I do not understand what that means.

That certainly should not block a merge. Please go ahead.

review: Approve
Revision history for this message
Simone Pelosi (pelpsi) wrote :

LGTM!

review: Approve
Revision history for this message
Colin Watson (cjwatson) wrote :

> > it didn't have enough entries to support running `utilities/create-lp-wadl-
> and-apidoc.py`.
>
> I do not understand what that means.

It's the program that `make build` runs to build the API documentation. It calls `execute_zcml_for_scripts` early in its setup, which requires various bits of configuration even though they aren't strictly needed for the API documentation itself.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/charm/launchpad-assets/config.yaml b/charm/launchpad-assets/config.yaml
2index dd00954..c440578 100644
3--- a/charm/launchpad-assets/config.yaml
4+++ b/charm/launchpad-assets/config.yaml
5@@ -3,6 +3,10 @@ options:
6 type: string
7 description: Domain name for this instance.
8 default: "launchpad.test"
9+ domain_xmlrpc_private:
10+ type: string
11+ description: Domain name for this instance's private XML-RPC service.
12+ default: "xmlrpc-private.launchpad.test"
13 port:
14 type: int
15 description: Port for the assets website.
16diff --git a/charm/launchpad-assets/layer.yaml b/charm/launchpad-assets/layer.yaml
17index 53a6ccf..f7e6593 100644
18--- a/charm/launchpad-assets/layer.yaml
19+++ b/charm/launchpad-assets/layer.yaml
20@@ -8,8 +8,6 @@ options:
21 packages:
22 - nodejs
23 - python3-convoy
24- launchpad-payload:
25- build_target: build
26 ols:
27 service_name: launchpad-assets
28 tarball_payload_name: launchpad
29diff --git a/charm/launchpad-assets/reactive/launchpad-assets.py b/charm/launchpad-assets/reactive/launchpad-assets.py
30index 331cb58..ee2b74e 100644
31--- a/charm/launchpad-assets/reactive/launchpad-assets.py
32+++ b/charm/launchpad-assets/reactive/launchpad-assets.py
33@@ -95,6 +95,17 @@ def configure():
34 group=base.user(),
35 perms=0o444,
36 )
37+ # Build assets now that we have the correct configuration in place.
38+ subprocess.run(
39+ [
40+ "make",
41+ "build",
42+ f"PYTHON={base.python_bin()}",
43+ "LPCONFIG=launchpad-assets",
44+ ],
45+ cwd=base.code_dir(),
46+ check=True,
47+ )
48 configure_convoy(config)
49 set_flag("service.configured")
50
51diff --git a/charm/launchpad-assets/templates/launchpad-assets-lazr.conf b/charm/launchpad-assets/templates/launchpad-assets-lazr.conf
52index 24637f3..169fc2d 100644
53--- a/charm/launchpad-assets/templates/launchpad-assets-lazr.conf
54+++ b/charm/launchpad-assets/templates/launchpad-assets-lazr.conf
55@@ -7,8 +7,24 @@
56 # true, false, and none are treated as True, False, and None.
57
58 [meta]
59-extends: ../lib/lp/services/config/schema-lazr.conf
60+extends: ../../lib/lp/services/config/schema-lazr.conf
61+
62+[error_reports]
63+# ErrorReportingUtility.configure crashes without this.
64+error_dir: {{ base_dir }}/oopses
65+
66+[vhost.mainsite]
67+hostname: {{ domain }}
68
69 [vhost.api]
70 hostname: api.{{ domain }}
71
72+[vhost.feeds]
73+hostname: feeds.{{ domain }}
74+
75+[vhost.xmlrpc]
76+hostname: xmlrpc.{{ domain }}
77+
78+[vhost.xmlrpc_private]
79+hostname: {{ domain_xmlrpc_private }}
80+
81diff --git a/charm/launchpad-assets/templates/vhost.conf.j2 b/charm/launchpad-assets/templates/vhost.conf.j2
82index 320e7cc..7d427d7 100644
83--- a/charm/launchpad-assets/templates/vhost.conf.j2
84+++ b/charm/launchpad-assets/templates/vhost.conf.j2
85@@ -13,7 +13,7 @@
86 Header set Cache-Control "public,max-age=5184000"
87 Require all granted
88 </Location>
89- Alias "/+apidoc/" "{{ code_dir }}/lib/canonical/launchpad/apidoc/"
90+ Alias "/+apidoc/" "{{ payloads_dir }}/{{ build_label }}/lib/canonical/launchpad/apidoc/"
91
92 <Location "/+combo/">
93 Header set Cache-Control "public,max-age=5184000"
94@@ -30,20 +30,20 @@
95 Header set Cache-Control "public,max-age=5184000"
96 Require all granted
97 </Location>
98- Alias "/+icing/" "{{ code_dir }}/lib/canonical/launchpad/icing/"
99+ Alias "/+icing/" "{{ payloads_dir }}/{{ build_label }}/lib/canonical/launchpad/icing/"
100
101 <Location "/@@/">
102 Options MultiViews
103 Header set Cache-Control "public,max-age=5184000"
104 Require all granted
105 </Location>
106- Alias "/@@/" "{{ code_dir }}/lib/canonical/launchpad/images/"
107+ Alias "/@@/" "{{ payloads_dir }}/{{ build_label }}/lib/canonical/launchpad/images/"
108
109 <LocationMatch "^/favicon\.(?:ico|gif|png)$">
110 Header set Cache-Control "public,max-age=5184000"
111 Require all granted
112 </LocationMatch>
113- AliasMatch "^/favicon\.(?:ico|gif|png)$" "{{ code_dir }}/lib/canonical/launchpad/images/launchpad.png"
114+ AliasMatch "^/favicon\.(?:ico|gif|png)$" "{{ payloads_dir }}/{{ build_label }}/lib/canonical/launchpad/images/launchpad.png"
115
116 <Location "/_status/check">
117 Require all granted

Subscribers

People subscribed via source and target branches

to status/vote changes: