Merge ~ltrager/maas:lp1876217 into maas:master

[Lee Trager]

LP: #1876217 - Gather OS information from /etc/os-release or /etc/lsb-release

[MAAS Lander (maas-lander)]

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

STATUS: SUCCESS
COMMIT: 478ea10101b8e36aa7ac4f986052ca85514a9742

review: Approve

[Alberto Donato (ack)]

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

[Lee Trager (ltrager)]

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 on 2020-05-15

Merge branch 'master' into lp1876217

22ebee1... by Lee Trager on 2020-05-15

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

[Lee Trager (ltrager)]

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.

[MAAS Lander (maas-lander)]

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

STATUS: SUCCESS
COMMIT: 983f3dd298867877e6e186a1fad1ee065877b69a

review: Approve

[MAAS Lander (maas-lander)]

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

STATUS: SUCCESS
COMMIT: 22ebee150f37fd210374c64fad8bdb7829864637

review: Approve

1036759... by Lee Trager on 2020-05-20

Merge branch 'master' into lp1876217

a52321f... by Lee Trager on 2020-05-20

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)]

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

[MAAS Lander (maas-lander)]

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

STATUS: SUCCESS
COMMIT: a52321f63f9f51d592a1a8104e5e5e6b0b8ecb60

review: Approve

[Alberto Donato (ack)]

A couple of comments inline

4d3e828... by Lee Trager on 2020-05-21

Use OSInfo struct

[Lee Trager (ltrager)]

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

b3108a3... by Lee Trager on 2020-05-21

make format

[Alberto Donato (ack)]

+1, thanks for the changes

review: Approve

diff --git a/src/machine-resources/Makefile b/src/machine-resources/Makefile
index a083aec..d28199d 100644
--- a/src/machine-resources/Makefile
+++ b/src/machine-resources/Makefile
@@ -27,7 +27,7 @@ DEP := cd $(PACKAGE_DIR); dep
 build: $(BINARIES)
 .PHONY: build
 
-$(BINARIES): $(PACKAGE_VENDOR_DIR)
+$(BINARIES): $(PACKAGE_VENDOR_DIR) $(PACKAGE_DIR)/main.go
 	GOCACHE=$(GO_CACHE_DIR) GOARCH=$(GOARCH_$(notdir $@)) go build -ldflags '-s -w' -o $@ machine-resources
 
 $(PACKAGE_VENDOR_DIR) vendor:
@@ -41,5 +41,5 @@ clean:
 .PHONY: clean
 
 update-deps:
-	$(DEP) ensure -update github.com/lxc/lxd
+	$(DEP) ensure -update
 .PHONY: update-deps
diff --git a/src/machine-resources/src/machine-resources/Gopkg.lock b/src/machine-resources/src/machine-resources/Gopkg.lock
index 6c2f652..ce7f9ec 100644
--- a/src/machine-resources/src/machine-resources/Gopkg.lock
+++ b/src/machine-resources/src/machine-resources/Gopkg.lock
@@ -4,30 +4,26 @@
 [[projects]]
   name = "github.com/flosch/pongo2"
   packages = ["."]
-  revision = "bbf5a6c351f4d4e883daa40046a404d7553e0a00"
+  revision = "5e81b817a0c48c1c57cdf1a9056cf76bdee02ca9"
+  version = "v3.0"
 
 [[projects]]
   name = "github.com/gorilla/websocket"
   packages = ["."]
-  revision = ""
+  revision = "b65e62901fc1c0d968042419e74789f6af455eb9"
   version = "v1.4.2"
 
 [[projects]]
   name = "github.com/jaypipes/pcidb"
   packages = ["."]
-  revision = "49e68a8beb0c155efb2db43bea4b354ec4843ff1"
-  version = "0.5.0"
-
-[[projects]]
-  name = "github.com/juju/errors"
-  packages = ["."]
-  revision = "3fe23663418fc1d724868c84f21b7519bbac7441"
+  revision = "98ef3ee36c69f20650fe7855047ad042338c6983"
+  version = "v0.5.0"
 
 [[projects]]
   branch = "master"
   name = "github.com/lxc/lxd"
   packages = ["lxd/resources","shared","shared/api","shared/cancel","shared/ioprogress","shared/logger","shared/osarch","shared/units","shared/usbid","shared/version"]
-  revision = "7516f0a2968b025efd1ea5066e78b16d6646b1c8"
+  revision = "a101c996de697dda597538ffb95eb4949711f3ec"
 
 [[projects]]
   name = "github.com/mitchellh/go-homedir"
@@ -38,16 +34,17 @@
 [[projects]]
   name = "github.com/pkg/errors"
   packages = ["."]
-  revision = "ba968bfe8b2f7e042a574c888954fccecfa385b4"
-  version = "v0.8.1"
+  revision = "614d223910a179a466c1767a985424175c39b465"
+  version = "v0.9.1"
 
 [[projects]]
   branch = "master"
   name = "golang.org/x/sys"
-  packages = ["unix","windows"]
-  revision = "b09406accb4736d857a32bf9444cd7edae2ffa79"
+  packages = ["internal/unsafeheader","unix","windows"]
+  revision = "fe76b779f299728f3bd63f77ea2c815504229c3b"
 
 [[projects]]
+  branch = "v2"
   name = "gopkg.in/robfig/cron.v2"
   packages = ["."]
   revision = "be2e0b0deed5a68ffee390b4583a13aff8321535"
@@ -55,6 +52,6 @@
 [solve-meta]
   analyzer-name = "dep"
   analyzer-version = 1
-  inputs-digest = "c48e39bda2f6db455f595dad9fbb6171c11d952e37c8b1b1a0f4e85ca1330e8e"
+  inputs-digest = "0aa702d9956536c2ac11ab31b412046932bc18980bb395c40153f94d457e48c6"
   solver-name = "gps-cdcl"
   solver-version = 1
diff --git a/src/machine-resources/src/machine-resources/main.go b/src/machine-resources/src/machine-resources/main.go
index be9f16d..7ba5884 100644
--- a/src/machine-resources/src/machine-resources/main.go
+++ b/src/machine-resources/src/machine-resources/main.go
@@ -4,26 +4,31 @@
 package main
 
 import (
+	"bufio"
 	"encoding/json"
 	"fmt"
 	"os"
+	"strings"
 
 	"github.com/lxc/lxd/lxd/resources"
 	"github.com/lxc/lxd/shared"
-	"github.com/lxc/lxd/shared/osarch"
 	"github.com/lxc/lxd/shared/version"
 )
 
+type OSInfo struct {
+	OSName    string `json:"os_name" yaml:"os_name"`
+	OSVersion string `json:"os_version" yaml:"os_version"`
+}
+
 // Subset of github.com/lxc/lxd/shared/api/server.ServerEnvironment
 type ServerEnvironment struct {
 	Kernel             string `json:"kernel" yaml:"kernel"`
 	KernelArchitecture string `json:"kernel_architecture" yaml:"kernel_architecture"`
 	KernelVersion      string `json:"kernel_version" yaml:"kernel_version"`
-	OSName             string `json:"os_name" yaml:"os_name"`
-	OSVersion          string `json:"os_version" yaml:"os_version"`
-	Server             string `json:"server" yaml:"server"`
-	ServerName         string `json:"server_name" yaml:"server_name"`
-	ServerVersion      string `json:"server_version" yaml:"server_version"`
+	OSInfo
+	Server        string `json:"server" yaml:"server"`
+	ServerName    string `json:"server_name" yaml:"server_name"`
+	ServerVersion string `json:"server_version" yaml:"server_version"`
 }
 
 // Subset of github.com/lxc/lxd/shared/api/server.HostInfo
@@ -45,6 +50,66 @@ func check_error(err error) {
 	}
 }
 
+func parseKeyValueFile(path string) (map[string]string, error) {
+	parsed_file := make(map[string]string)
+	file, err := os.Open(path)
+	if err != nil {
+		return parsed_file, err
+	}
+	defer file.Close()
+
+	scanner := bufio.NewScanner(file)
+	for scanner.Scan() {
+		line := strings.TrimSpace(scanner.Text())
+		if strings.HasPrefix(line, "#") {
+			continue
+		}
+
+		tokens := strings.SplitN(line, "=", 2)
+		if len(tokens) != 2 {
+			continue
+		}
+		parsed_file[strings.Trim(tokens[0], `'"`)] = strings.Trim(tokens[1], `'"`)
+	}
+
+	return parsed_file, nil
+}
+
+func getOSNameVersion() OSInfo {
+	// Search both pathes as suggested by
+	// https://www.freedesktop.org/software/systemd/man/os-release.html
+	for _, path := range []string{"/etc/os-release", "/usr/lib/os-release"} {
+		parsed_file, err := parseKeyValueFile(path)
+		// LP:1876217 - As of 2.44 snapd only gives confined Snaps
+		// access to /etc/lsb-release from the host OS. /etc/os-release
+		// currently contains the Ubuntu Core version the Snap is
+		// running. Try /etc/os-release first for controllers installed
+		// with the Debian packages. At some point in the future snapd
+		// may provide the host OS version of /etc/os-release.
+		if err == nil && parsed_file["ID"] != "ubuntu-core" {
+			return OSInfo{
+				OSName:    strings.ToLower(parsed_file["ID"]),
+				OSVersion: strings.ToLower(parsed_file["VERSION_ID"]),
+			}
+		}
+	}
+	// If /etc/os-release isn't found or only contains the Ubuntu Core
+	// version try to get OS information from /etc/lsb-release as this
+	// file is passed from the running host OS. Only Ubuntu and Manjaro
+	// provide /etc/lsb-release.
+	parsed_file, err := parseKeyValueFile("/etc/lsb-release")
+	if err == nil {
+		return OSInfo{
+			OSName:    strings.ToLower(parsed_file["DISTRIB_ID"]),
+			OSVersion: strings.ToLower(parsed_file["DISTRIB_RELEASE"]),
+		}
+	} else {
+		// If the OS information isn't detectable don't send anything.
+		// MAAS will keep the current OS information.
+		return OSInfo{}
+	}
+}
+
 func GetHostInfo() HostInfo {
 	hostname, err := os.Hostname()
 	check_error(err)
@@ -52,14 +117,18 @@ func GetHostInfo() HostInfo {
 	uname, err := shared.Uname()
 	check_error(err)
 
-	osInfo, err := osarch.GetLSBRelease()
-	check_error(err)
-
 	return HostInfo{
 		// These are the API extensions machine-resources reproduces.
 		APIExtensions: []string{
 			"resources",
+			"resources_cpu_socket",
+			"resources_gpu",
+			"resources_numa",
 			"resources_v2",
+			"resources_disk_sata",
+			"resources_usb_pci",
+			"resources_cpu_threads_numa",
+			"resources_cpu_core_die",
 			"api_os",
 			"resources_system",
 		},
@@ -71,8 +140,7 @@ func GetHostInfo() HostInfo {
 			Kernel:             uname.Sysname,
 			KernelArchitecture: uname.Machine,
 			KernelVersion:      uname.Release,
-			OSName:             osInfo["NAME"],
-			OSVersion:          osInfo["VERSION_ID"],
+			OSInfo:             getOSNameVersion(),
 			Server:             "maas-machine-resources",
 			ServerName:         hostname,
 			// Use the imported LXD version as the data comes from
diff --git a/src/metadataserver/builtin_scripts/hooks.py b/src/metadataserver/builtin_scripts/hooks.py
index d247e5a..056af1a 100644
--- a/src/metadataserver/builtin_scripts/hooks.py
+++ b/src/metadataserver/builtin_scripts/hooks.py
@@ -662,7 +662,11 @@ def _process_lxd_environment(node, data):
     # on the running machine and LXD Pods are getting this data from LXD
     # on the running machine. In those cases the information gathered below
     # is correct.
-    if node.is_controller or node.is_pod:
+    if (
+        (node.is_controller or node.is_pod)
+        and data.get("os_name")
+        and data.get("os_version")
+    ):
         # This is how the hostname gets set on controllers and stays in sync on Pods.
         node.hostname = data["server_name"]
 
diff --git a/src/metadataserver/builtin_scripts/tests/test_hooks.py b/src/metadataserver/builtin_scripts/tests/test_hooks.py
index de3424a..5616ee6 100644
--- a/src/metadataserver/builtin_scripts/tests/test_hooks.py
+++ b/src/metadataserver/builtin_scripts/tests/test_hooks.py
@@ -490,19 +490,19 @@ def make_lxd_host_info(
             "api_os",
             "resources_system",
         ]
-    if not api_version:
+    if api_version is None:
         api_version = "1.0"
-    if not kernel_architecture:
+    if kernel_architecture is None:
         kernel_architecture = random.choice(
             ["i686", "x86_64", "aarch64", "ppc64le", "s390x", "mips", "mips64"]
         )
-    if not kernel_version:
+    if kernel_version is None:
         kernel_version = factory.make_name("kernel_version")
-    if not os_name:
+    if os_name is None:
         os_name = "Ubuntu"
-    if not os_version:
+    if os_version is None:
         os_version = random.choice(["16.04", "18.04", "20.04"])
-    if not server_name:
+    if server_name is None:
         server_name = factory.make_hostname()
     return {
         "api_extensions": api_extensions,
@@ -1270,6 +1270,20 @@ class TestProcessLXDResults(MAASServerTestCase):
         self.assertEquals(ubuntu_release["series"], rack.distro_series)
         self.assertEquals(server_name, rack.hostname)
 
+    def test__doesnt_set_os_for_controller_if_blank(self):
+        osystem = factory.make_name("osystem")
+        distro_series = factory.make_name("distro_series")
+        rack = factory.make_RackController(
+            osystem=osystem, distro_series=distro_series
+        )
+        create_IPADDR_OUTPUT_NAME_script(rack, IP_ADDR_OUTPUT)
+        process_lxd_results(
+            rack, make_lxd_output_json(os_name="", os_version=""), 0
+        )
+        rack = reload_object(rack)
+        self.assertEquals(osystem, rack.osystem)
+        self.assertEquals(distro_series, rack.distro_series)
+
     def test__sets_os_for_pod(self):
         pod = factory.make_Pod()
         node = factory.make_Node(