Merge ~ltrager/maas:lp1876217 into maas:master

Proposed by Lee Trager
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)
Reviewer Review Type Date Requested Status
Alberto Donato 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

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1876217 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 478ea10101b8e36aa7ac4f986052ca85514a9742

review: Approve
Revision history for this message
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.GetLSBRelease()
- 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.

review: Needs Fixing
Revision history for this message
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?

~ltrager/maas:lp1876217 updated
a91eff5... by Lee Trager

Merge branch 'master' into lp1876217

22ebee1... by Lee Trager

Try /etc/os-release, fallback on /etc/lsb-release

Revision history for this message
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/os-release, and /etc/lsb-release. If an os-release file has the ID of ubuntu-core its skipped as that is the base Snap. The information is then gathered from /etc/lsb-release.

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1876217 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 983f3dd298867877e6e186a1fad1ee065877b69a

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1876217 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 22ebee150f37fd210374c64fad8bdb7829864637

review: Approve
~ltrager/maas:lp1876217 updated
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.

Revision history for this message
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/os-release. LXD[1] does the same thing when getting OS information.

[1] https://github.com/lxc/lxd/blob/master/shared/osarch/release.go#L11

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1876217 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: a52321f63f9f51d592a1a8104e5e5e6b0b8ecb60

review: Approve
Revision history for this message
Alberto Donato (ack) wrote :

A couple of comments inline

~ltrager/maas:lp1876217 updated
4d3e828... by Lee Trager

Use OSInfo struct

Revision history for this message
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/os-release.

[1] https://www.freedesktop.org/software/systemd/man/os-release.html

~ltrager/maas:lp1876217 updated
b3108a3... by Lee Trager

make format

Revision history for this message
Alberto Donato (ack) wrote :

+1, thanks for the changes

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/machine-resources/Makefile b/src/machine-resources/Makefile
2index 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
21diff --git a/src/machine-resources/src/machine-resources/Gopkg.lock b/src/machine-resources/src/machine-resources/Gopkg.lock
22index 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
92diff --git a/src/machine-resources/src/machine-resources/main.go b/src/machine-resources/src/machine-resources/main.go
93index 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
233diff --git a/src/metadataserver/builtin_scripts/hooks.py b/src/metadataserver/builtin_scripts/hooks.py
234index 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
250diff --git a/src/metadataserver/builtin_scripts/tests/test_hooks.py b/src/metadataserver/builtin_scripts/tests/test_hooks.py
251index 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(

Subscribers

People subscribed via source and target branches