Merge ~ltrager/maas:lp1876217 into maas:master
- Git
- lp:~ltrager/maas
- lp1876217
- Merge into master
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Lee Trager | ||||
Approved revision: | b3108a300dc6e7c0d390a4c0d2d035be6b2d5207 | ||||
Merge reported by: | MAAS Lander | ||||
Merged at revision: | not available | ||||
Proposed branch: | ~ltrager/maas:lp1876217 | ||||
Merge into: | maas:master | ||||
Diff against target: |
300 lines (+118/-35) 5 files modified
src/machine-resources/Makefile (+2/-2) src/machine-resources/src/machine-resources/Gopkg.lock (+12/-15) src/machine-resources/src/machine-resources/main.go (+79/-11) src/metadataserver/builtin_scripts/hooks.py (+5/-1) src/metadataserver/builtin_scripts/tests/test_hooks.py (+20/-6) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alberto Donato (community) | Approve | ||
MAAS Lander | Approve | ||
Review via email: mp+383467@code.launchpad.net |
Commit message
LP: #1876217 - Gather OS information from /etc/os-release or /etc/lsb-release
Description of the change
Alberto Donato (ack) wrote : | # |
I think GetLSBRelease should do the following:
- try to open /etc/lsb-release
- if it fails, get the info from osarch.
- otherwise, parse /etc/lsb-release
- return a struct with just "name" and "version" fields (so the calling code doesn't need to know where does the info come from.
Lee Trager (ltrager) wrote : | # |
In the bug Ian suggested we use a system-files interface for /etc/os-release. Do you see any reason not to do that and dump the modification to main.go?
- a91eff5... by Lee Trager
-
Merge branch 'master' into lp1876217
- 22ebee1... by Lee Trager
-
Try /etc/os-release, fallback on /etc/lsb-release
Lee Trager (ltrager) wrote : | # |
Adam updated me on the meeting with snapd that happened last night. My understanding is snapd will expose /etc/os-release in the future by default. As such I've updated this MP to try /etc/os-release, /usr/lib/
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b lp1876217 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: SUCCESS
COMMIT: 983f3dd29886787
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b lp1876217 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: SUCCESS
COMMIT: 22ebee150f37fd2
- 1036759... by Lee Trager
-
Merge branch 'master' into lp1876217
- a52321f... by Lee Trager
-
Don't send OS information if it isn't detectable. If no OS information is
sent don't update the controller's OS.
Lee Trager (ltrager) wrote : | # |
I've updated this to make this highly resilient. Machine resources will only fail to detect OS information when running the Snap on a host OS other then Ubuntu. This will be fixed once snapd passes through /etc/os-release. In the case that detecting OS information fails nothing is sent, MAAS will keep the current OS information.
To answer your question on IRC I'm scanning /etc/os-release and /usr/lib/os-release incase the host OS only provides /usr/lib/
[1] https:/
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b lp1876217 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: SUCCESS
COMMIT: a52321f63f9f51d
Alberto Donato (ack) wrote : | # |
A couple of comments inline
- 4d3e828... by Lee Trager
-
Use OSInfo struct
Lee Trager (ltrager) wrote : | # |
Updated to use an OSInfo struct as suggested.
/usr/lib/os-release is actually defined by the freedesktop.org spec[1]. It is suggested both are searched. Fedora and CentOS both provide /usr/lib/
[1] https:/
- b3108a3... by Lee Trager
-
make format
Alberto Donato (ack) wrote : | # |
+1, thanks for the changes
Preview Diff
1 | diff --git a/src/machine-resources/Makefile b/src/machine-resources/Makefile |
2 | index a083aec..d28199d 100644 |
3 | --- a/src/machine-resources/Makefile |
4 | +++ b/src/machine-resources/Makefile |
5 | @@ -27,7 +27,7 @@ DEP := cd $(PACKAGE_DIR); dep |
6 | build: $(BINARIES) |
7 | .PHONY: build |
8 | |
9 | -$(BINARIES): $(PACKAGE_VENDOR_DIR) |
10 | +$(BINARIES): $(PACKAGE_VENDOR_DIR) $(PACKAGE_DIR)/main.go |
11 | GOCACHE=$(GO_CACHE_DIR) GOARCH=$(GOARCH_$(notdir $@)) go build -ldflags '-s -w' -o $@ machine-resources |
12 | |
13 | $(PACKAGE_VENDOR_DIR) vendor: |
14 | @@ -41,5 +41,5 @@ clean: |
15 | .PHONY: clean |
16 | |
17 | update-deps: |
18 | - $(DEP) ensure -update github.com/lxc/lxd |
19 | + $(DEP) ensure -update |
20 | .PHONY: update-deps |
21 | diff --git a/src/machine-resources/src/machine-resources/Gopkg.lock b/src/machine-resources/src/machine-resources/Gopkg.lock |
22 | index 6c2f652..ce7f9ec 100644 |
23 | --- a/src/machine-resources/src/machine-resources/Gopkg.lock |
24 | +++ b/src/machine-resources/src/machine-resources/Gopkg.lock |
25 | @@ -4,30 +4,26 @@ |
26 | [[projects]] |
27 | name = "github.com/flosch/pongo2" |
28 | packages = ["."] |
29 | - revision = "bbf5a6c351f4d4e883daa40046a404d7553e0a00" |
30 | + revision = "5e81b817a0c48c1c57cdf1a9056cf76bdee02ca9" |
31 | + version = "v3.0" |
32 | |
33 | [[projects]] |
34 | name = "github.com/gorilla/websocket" |
35 | packages = ["."] |
36 | - revision = "" |
37 | + revision = "b65e62901fc1c0d968042419e74789f6af455eb9" |
38 | version = "v1.4.2" |
39 | |
40 | [[projects]] |
41 | name = "github.com/jaypipes/pcidb" |
42 | packages = ["."] |
43 | - revision = "49e68a8beb0c155efb2db43bea4b354ec4843ff1" |
44 | - version = "0.5.0" |
45 | - |
46 | -[[projects]] |
47 | - name = "github.com/juju/errors" |
48 | - packages = ["."] |
49 | - revision = "3fe23663418fc1d724868c84f21b7519bbac7441" |
50 | + revision = "98ef3ee36c69f20650fe7855047ad042338c6983" |
51 | + version = "v0.5.0" |
52 | |
53 | [[projects]] |
54 | branch = "master" |
55 | name = "github.com/lxc/lxd" |
56 | packages = ["lxd/resources","shared","shared/api","shared/cancel","shared/ioprogress","shared/logger","shared/osarch","shared/units","shared/usbid","shared/version"] |
57 | - revision = "7516f0a2968b025efd1ea5066e78b16d6646b1c8" |
58 | + revision = "a101c996de697dda597538ffb95eb4949711f3ec" |
59 | |
60 | [[projects]] |
61 | name = "github.com/mitchellh/go-homedir" |
62 | @@ -38,16 +34,17 @@ |
63 | [[projects]] |
64 | name = "github.com/pkg/errors" |
65 | packages = ["."] |
66 | - revision = "ba968bfe8b2f7e042a574c888954fccecfa385b4" |
67 | - version = "v0.8.1" |
68 | + revision = "614d223910a179a466c1767a985424175c39b465" |
69 | + version = "v0.9.1" |
70 | |
71 | [[projects]] |
72 | branch = "master" |
73 | name = "golang.org/x/sys" |
74 | - packages = ["unix","windows"] |
75 | - revision = "b09406accb4736d857a32bf9444cd7edae2ffa79" |
76 | + packages = ["internal/unsafeheader","unix","windows"] |
77 | + revision = "fe76b779f299728f3bd63f77ea2c815504229c3b" |
78 | |
79 | [[projects]] |
80 | + branch = "v2" |
81 | name = "gopkg.in/robfig/cron.v2" |
82 | packages = ["."] |
83 | revision = "be2e0b0deed5a68ffee390b4583a13aff8321535" |
84 | @@ -55,6 +52,6 @@ |
85 | [solve-meta] |
86 | analyzer-name = "dep" |
87 | analyzer-version = 1 |
88 | - inputs-digest = "c48e39bda2f6db455f595dad9fbb6171c11d952e37c8b1b1a0f4e85ca1330e8e" |
89 | + inputs-digest = "0aa702d9956536c2ac11ab31b412046932bc18980bb395c40153f94d457e48c6" |
90 | solver-name = "gps-cdcl" |
91 | solver-version = 1 |
92 | diff --git a/src/machine-resources/src/machine-resources/main.go b/src/machine-resources/src/machine-resources/main.go |
93 | index be9f16d..7ba5884 100644 |
94 | --- a/src/machine-resources/src/machine-resources/main.go |
95 | +++ b/src/machine-resources/src/machine-resources/main.go |
96 | @@ -4,26 +4,31 @@ |
97 | package main |
98 | |
99 | import ( |
100 | + "bufio" |
101 | "encoding/json" |
102 | "fmt" |
103 | "os" |
104 | + "strings" |
105 | |
106 | "github.com/lxc/lxd/lxd/resources" |
107 | "github.com/lxc/lxd/shared" |
108 | - "github.com/lxc/lxd/shared/osarch" |
109 | "github.com/lxc/lxd/shared/version" |
110 | ) |
111 | |
112 | +type OSInfo struct { |
113 | + OSName string `json:"os_name" yaml:"os_name"` |
114 | + OSVersion string `json:"os_version" yaml:"os_version"` |
115 | +} |
116 | + |
117 | // Subset of github.com/lxc/lxd/shared/api/server.ServerEnvironment |
118 | type ServerEnvironment struct { |
119 | Kernel string `json:"kernel" yaml:"kernel"` |
120 | KernelArchitecture string `json:"kernel_architecture" yaml:"kernel_architecture"` |
121 | KernelVersion string `json:"kernel_version" yaml:"kernel_version"` |
122 | - OSName string `json:"os_name" yaml:"os_name"` |
123 | - OSVersion string `json:"os_version" yaml:"os_version"` |
124 | - Server string `json:"server" yaml:"server"` |
125 | - ServerName string `json:"server_name" yaml:"server_name"` |
126 | - ServerVersion string `json:"server_version" yaml:"server_version"` |
127 | + OSInfo |
128 | + Server string `json:"server" yaml:"server"` |
129 | + ServerName string `json:"server_name" yaml:"server_name"` |
130 | + ServerVersion string `json:"server_version" yaml:"server_version"` |
131 | } |
132 | |
133 | // Subset of github.com/lxc/lxd/shared/api/server.HostInfo |
134 | @@ -45,6 +50,66 @@ func check_error(err error) { |
135 | } |
136 | } |
137 | |
138 | +func parseKeyValueFile(path string) (map[string]string, error) { |
139 | + parsed_file := make(map[string]string) |
140 | + file, err := os.Open(path) |
141 | + if err != nil { |
142 | + return parsed_file, err |
143 | + } |
144 | + defer file.Close() |
145 | + |
146 | + scanner := bufio.NewScanner(file) |
147 | + for scanner.Scan() { |
148 | + line := strings.TrimSpace(scanner.Text()) |
149 | + if strings.HasPrefix(line, "#") { |
150 | + continue |
151 | + } |
152 | + |
153 | + tokens := strings.SplitN(line, "=", 2) |
154 | + if len(tokens) != 2 { |
155 | + continue |
156 | + } |
157 | + parsed_file[strings.Trim(tokens[0], `'"`)] = strings.Trim(tokens[1], `'"`) |
158 | + } |
159 | + |
160 | + return parsed_file, nil |
161 | +} |
162 | + |
163 | +func getOSNameVersion() OSInfo { |
164 | + // Search both pathes as suggested by |
165 | + // https://www.freedesktop.org/software/systemd/man/os-release.html |
166 | + for _, path := range []string{"/etc/os-release", "/usr/lib/os-release"} { |
167 | + parsed_file, err := parseKeyValueFile(path) |
168 | + // LP:1876217 - As of 2.44 snapd only gives confined Snaps |
169 | + // access to /etc/lsb-release from the host OS. /etc/os-release |
170 | + // currently contains the Ubuntu Core version the Snap is |
171 | + // running. Try /etc/os-release first for controllers installed |
172 | + // with the Debian packages. At some point in the future snapd |
173 | + // may provide the host OS version of /etc/os-release. |
174 | + if err == nil && parsed_file["ID"] != "ubuntu-core" { |
175 | + return OSInfo{ |
176 | + OSName: strings.ToLower(parsed_file["ID"]), |
177 | + OSVersion: strings.ToLower(parsed_file["VERSION_ID"]), |
178 | + } |
179 | + } |
180 | + } |
181 | + // If /etc/os-release isn't found or only contains the Ubuntu Core |
182 | + // version try to get OS information from /etc/lsb-release as this |
183 | + // file is passed from the running host OS. Only Ubuntu and Manjaro |
184 | + // provide /etc/lsb-release. |
185 | + parsed_file, err := parseKeyValueFile("/etc/lsb-release") |
186 | + if err == nil { |
187 | + return OSInfo{ |
188 | + OSName: strings.ToLower(parsed_file["DISTRIB_ID"]), |
189 | + OSVersion: strings.ToLower(parsed_file["DISTRIB_RELEASE"]), |
190 | + } |
191 | + } else { |
192 | + // If the OS information isn't detectable don't send anything. |
193 | + // MAAS will keep the current OS information. |
194 | + return OSInfo{} |
195 | + } |
196 | +} |
197 | + |
198 | func GetHostInfo() HostInfo { |
199 | hostname, err := os.Hostname() |
200 | check_error(err) |
201 | @@ -52,14 +117,18 @@ func GetHostInfo() HostInfo { |
202 | uname, err := shared.Uname() |
203 | check_error(err) |
204 | |
205 | - osInfo, err := osarch.GetLSBRelease() |
206 | - check_error(err) |
207 | - |
208 | return HostInfo{ |
209 | // These are the API extensions machine-resources reproduces. |
210 | APIExtensions: []string{ |
211 | "resources", |
212 | + "resources_cpu_socket", |
213 | + "resources_gpu", |
214 | + "resources_numa", |
215 | "resources_v2", |
216 | + "resources_disk_sata", |
217 | + "resources_usb_pci", |
218 | + "resources_cpu_threads_numa", |
219 | + "resources_cpu_core_die", |
220 | "api_os", |
221 | "resources_system", |
222 | }, |
223 | @@ -71,8 +140,7 @@ func GetHostInfo() HostInfo { |
224 | Kernel: uname.Sysname, |
225 | KernelArchitecture: uname.Machine, |
226 | KernelVersion: uname.Release, |
227 | - OSName: osInfo["NAME"], |
228 | - OSVersion: osInfo["VERSION_ID"], |
229 | + OSInfo: getOSNameVersion(), |
230 | Server: "maas-machine-resources", |
231 | ServerName: hostname, |
232 | // Use the imported LXD version as the data comes from |
233 | diff --git a/src/metadataserver/builtin_scripts/hooks.py b/src/metadataserver/builtin_scripts/hooks.py |
234 | index d247e5a..056af1a 100644 |
235 | --- a/src/metadataserver/builtin_scripts/hooks.py |
236 | +++ b/src/metadataserver/builtin_scripts/hooks.py |
237 | @@ -662,7 +662,11 @@ def _process_lxd_environment(node, data): |
238 | # on the running machine and LXD Pods are getting this data from LXD |
239 | # on the running machine. In those cases the information gathered below |
240 | # is correct. |
241 | - if node.is_controller or node.is_pod: |
242 | + if ( |
243 | + (node.is_controller or node.is_pod) |
244 | + and data.get("os_name") |
245 | + and data.get("os_version") |
246 | + ): |
247 | # This is how the hostname gets set on controllers and stays in sync on Pods. |
248 | node.hostname = data["server_name"] |
249 | |
250 | diff --git a/src/metadataserver/builtin_scripts/tests/test_hooks.py b/src/metadataserver/builtin_scripts/tests/test_hooks.py |
251 | index de3424a..5616ee6 100644 |
252 | --- a/src/metadataserver/builtin_scripts/tests/test_hooks.py |
253 | +++ b/src/metadataserver/builtin_scripts/tests/test_hooks.py |
254 | @@ -490,19 +490,19 @@ def make_lxd_host_info( |
255 | "api_os", |
256 | "resources_system", |
257 | ] |
258 | - if not api_version: |
259 | + if api_version is None: |
260 | api_version = "1.0" |
261 | - if not kernel_architecture: |
262 | + if kernel_architecture is None: |
263 | kernel_architecture = random.choice( |
264 | ["i686", "x86_64", "aarch64", "ppc64le", "s390x", "mips", "mips64"] |
265 | ) |
266 | - if not kernel_version: |
267 | + if kernel_version is None: |
268 | kernel_version = factory.make_name("kernel_version") |
269 | - if not os_name: |
270 | + if os_name is None: |
271 | os_name = "Ubuntu" |
272 | - if not os_version: |
273 | + if os_version is None: |
274 | os_version = random.choice(["16.04", "18.04", "20.04"]) |
275 | - if not server_name: |
276 | + if server_name is None: |
277 | server_name = factory.make_hostname() |
278 | return { |
279 | "api_extensions": api_extensions, |
280 | @@ -1270,6 +1270,20 @@ class TestProcessLXDResults(MAASServerTestCase): |
281 | self.assertEquals(ubuntu_release["series"], rack.distro_series) |
282 | self.assertEquals(server_name, rack.hostname) |
283 | |
284 | + def test__doesnt_set_os_for_controller_if_blank(self): |
285 | + osystem = factory.make_name("osystem") |
286 | + distro_series = factory.make_name("distro_series") |
287 | + rack = factory.make_RackController( |
288 | + osystem=osystem, distro_series=distro_series |
289 | + ) |
290 | + create_IPADDR_OUTPUT_NAME_script(rack, IP_ADDR_OUTPUT) |
291 | + process_lxd_results( |
292 | + rack, make_lxd_output_json(os_name="", os_version=""), 0 |
293 | + ) |
294 | + rack = reload_object(rack) |
295 | + self.assertEquals(osystem, rack.osystem) |
296 | + self.assertEquals(distro_series, rack.distro_series) |
297 | + |
298 | def test__sets_os_for_pod(self): |
299 | pod = factory.make_Pod() |
300 | node = factory.make_Node( |
UNIT TESTS
-b lp1876217 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: SUCCESS aa7ac4f986052ca 85514a9742
COMMIT: 478ea10101b8e36