From d502ee1b715da62bf0605abe8e60301098c2437a Mon Sep 17 00:00:00 2001 From: Abe Winter Date: Mon, 21 Oct 2024 10:26:33 -0400 Subject: [PATCH 1/6] revertme point at branch --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index f740924dcbc..f16185e369e 100644 --- a/go.mod +++ b/go.mod @@ -85,7 +85,7 @@ require ( go.uber.org/atomic v1.11.0 go.uber.org/multierr v1.11.0 go.uber.org/zap v1.27.0 - go.viam.com/api v0.1.348 + go.viam.com/api v0.1.349-0.20241020221202-d70a1c788662 go.viam.com/test v1.1.1-0.20220913152726-5da9916c08a2 go.viam.com/utils v0.1.106 goji.io v2.0.2+incompatible diff --git a/go.sum b/go.sum index cfecd7db71c..ea55b69dfab 100644 --- a/go.sum +++ b/go.sum @@ -1645,8 +1645,8 @@ go.uber.org/zap v1.18.1/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI= go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY= go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= -go.viam.com/api v0.1.348 h1:pLT6UiYaAvt8ckx2KA38MvyBU1s/KoiGn4/vzC/T2uU= -go.viam.com/api v0.1.348/go.mod h1:5lpVRxMsKFCaahqsnJfPGwJ9baoQ6PIKQu3lxvy6Wtw= +go.viam.com/api v0.1.349-0.20241020221202-d70a1c788662 h1:XWqxF4TOwCRbhwpicIrBY6DA3Xd6TbPzBDW2EtuG9Wk= +go.viam.com/api v0.1.349-0.20241020221202-d70a1c788662/go.mod h1:5lpVRxMsKFCaahqsnJfPGwJ9baoQ6PIKQu3lxvy6Wtw= go.viam.com/test v1.1.1-0.20220913152726-5da9916c08a2 h1:oBiK580EnEIzgFLU4lHOXmGAE3MxnVbeR7s1wp/F3Ps= go.viam.com/test v1.1.1-0.20220913152726-5da9916c08a2/go.mod h1:XM0tej6riszsiNLT16uoyq1YjuYPWlRBweTPRDanIts= go.viam.com/utils v0.1.106 h1:LcmVUBBV7ipzl4to4RJ4knj5bVdATU8reO77HNMimdA= From c2e37ccc2bbbe4ce6398c715b0d7e6756b532d2e Mon Sep 17 00:00:00 2001 From: Abe Winter Date: Mon, 21 Oct 2024 10:29:44 -0400 Subject: [PATCH 2/6] pathexists function --- utils/file.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/utils/file.go b/utils/file.go index 55b6de77755..4b159f5e1d7 100644 --- a/utils/file.go +++ b/utils/file.go @@ -47,3 +47,10 @@ func SafeJoinDir(parent, subdir string) (string, error) { } return res, nil } + +// PathExists returns true if the file exists, false if it doesn't or in +// any error case. No guarantees about doing the right thing with links. +func PathExists(path string) bool { + _, err := os.Stat(path) + return err == nil +} From 0eae2a79fb3c1f8380c2c30510de12bb0d84b5f7 Mon Sep 17 00:00:00 2001 From: Abe Winter Date: Mon, 21 Oct 2024 10:29:57 -0400 Subject: [PATCH 3/6] pass new agentinfo --- config/reader.go | 48 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/config/reader.go b/config/reader.go index a68f461abb4..59f9cc5a7a5 100644 --- a/config/reader.go +++ b/config/reader.go @@ -1,6 +1,7 @@ package config import ( + "bufio" "bytes" "context" "encoding/json" @@ -11,6 +12,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "time" "github.com/a8m/envsubst" @@ -42,6 +44,39 @@ const ( LocalPackagesSuffix = "-local" ) +func parseOsRelease(body *bufio.Reader) map[string]string { + ret := make(map[string]string) + for { + line, err := body.ReadString('\n') + if err != nil { + return ret + } + before, after, _ := strings.Cut(line, "=") + ret[before] = after + } +} + +// This reads the granular platform constraints (os version, distro, etc). +// This further constrains the basic runtime.GOOS/GOARCH stuff in getAgentInfo +// so module authors can publish builds with ABI or SDK dependencies. The +// list of tags returned by this function is expected to grow. +func readExtendedPlatformTags() []string { + // TODO(APP-6696): CI in multiple environments (alpine + mac), darwin support. + tags := make([]string, 0, 4) + if runtime.GOOS == "linux" && rutils.PathExists("/etc/os-release") { + if body, err := os.Open("/etc/os-release"); err != nil { + logging.Global().Errorw("can't open /etc/os-release, modules may not load correctly", "err", err) + } else { + defer body.Close() //nolint:errcheck + osRelease := parseOsRelease(bufio.NewReader(body)) + tags = append(tags, "distro:"+osRelease["ID"]) + tags = append(tags, "os_version:"+osRelease["VERSION_ID"]) + tags = append(tags, "codename:"+osRelease["VERSION_CODENAME"]) + } + } + return tags +} + func getAgentInfo() (*apppb.AgentInfo, error) { hostname, err := os.Hostname() if err != nil { @@ -70,12 +105,13 @@ func getAgentInfo() (*apppb.AgentInfo, error) { platform := fmt.Sprintf("%s/%s", runtime.GOOS, arch) return &apppb.AgentInfo{ - Host: hostname, - Ips: ips, - Os: runtime.GOOS, - Version: Version, - GitRevision: GitRevision, - Platform: &platform, + Host: hostname, + Ips: ips, + Os: runtime.GOOS, + Version: Version, + GitRevision: GitRevision, + Platform: &platform, + PlatformTags: readExtendedPlatformTags(), }, nil } From 305eac4ab92802ddcce4cd11c31a7c7b796f0e3f Mon Sep 17 00:00:00 2001 From: Abe Winter Date: Mon, 21 Oct 2024 14:21:55 -0400 Subject: [PATCH 4/6] trimming, test --- config/reader.go | 5 +++-- config/reader_test.go | 9 +++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/config/reader.go b/config/reader.go index 59f9cc5a7a5..ec984c2340a 100644 --- a/config/reader.go +++ b/config/reader.go @@ -51,8 +51,9 @@ func parseOsRelease(body *bufio.Reader) map[string]string { if err != nil { return ret } - before, after, _ := strings.Cut(line, "=") - ret[before] = after + key, value, _ := strings.Cut(line, "=") + // note: we trim `value` rather than `line` because os_version value is quoted sometimes. + ret[key] = strings.Trim(value, "\n\"") } } diff --git a/config/reader_test.go b/config/reader_test.go index 0767f5d9a0e..12988250aee 100644 --- a/config/reader_test.go +++ b/config/reader_test.go @@ -5,6 +5,7 @@ import ( "fmt" "io/fs" "os" + "runtime" "strings" "testing" "time" @@ -394,3 +395,11 @@ func TestReadTLSFromCache(t *testing.T) { test.That(t, err, test.ShouldBeNil) }) } + +func TestReadExtendedPlatformTags(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("skipping platform tags test on non-linux") + } + tags := readExtendedPlatformTags() + test.That(t, len(tags), test.ShouldBeGreaterThanOrEqualTo, 3) +} From 3d008802d7e7607a14ddb4ee301b5fa9111b97d8 Mon Sep 17 00:00:00 2001 From: Abe Winter Date: Mon, 21 Oct 2024 18:43:34 -0400 Subject: [PATCH 5/6] don't set empty tags, simplify file load --- config/reader.go | 22 ++++++++++++++++------ config/reader_test.go | 9 ++++++++- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/config/reader.go b/config/reader.go index ec984c2340a..7920ccbf378 100644 --- a/config/reader.go +++ b/config/reader.go @@ -57,22 +57,32 @@ func parseOsRelease(body *bufio.Reader) map[string]string { } } +// append key:value pair to orig if value is non-empty +func appendPairIfNonempty(orig []string, key, value string) []string { + if value != "" { + return append(orig, key+":"+value) + } + return orig +} + // This reads the granular platform constraints (os version, distro, etc). // This further constrains the basic runtime.GOOS/GOARCH stuff in getAgentInfo // so module authors can publish builds with ABI or SDK dependencies. The // list of tags returned by this function is expected to grow. func readExtendedPlatformTags() []string { // TODO(APP-6696): CI in multiple environments (alpine + mac), darwin support. - tags := make([]string, 0, 4) - if runtime.GOOS == "linux" && rutils.PathExists("/etc/os-release") { + tags := make([]string, 0, 3) + if runtime.GOOS == "linux" { if body, err := os.Open("/etc/os-release"); err != nil { - logging.Global().Errorw("can't open /etc/os-release, modules may not load correctly", "err", err) + if !os.IsNotExist(err) { + logging.Global().Errorw("can't open /etc/os-release, modules may not load correctly", "err", err) + } } else { defer body.Close() //nolint:errcheck osRelease := parseOsRelease(bufio.NewReader(body)) - tags = append(tags, "distro:"+osRelease["ID"]) - tags = append(tags, "os_version:"+osRelease["VERSION_ID"]) - tags = append(tags, "codename:"+osRelease["VERSION_CODENAME"]) + tags = appendPairIfNonempty(tags, "distro", osRelease["ID"]) + tags = appendPairIfNonempty(tags, "os_version", osRelease["VERSION_ID"]) + tags = appendPairIfNonempty(tags, "codename", osRelease["VERSION_CODENAME"]) } } return tags diff --git a/config/reader_test.go b/config/reader_test.go index 12988250aee..26ffd2442c7 100644 --- a/config/reader_test.go +++ b/config/reader_test.go @@ -401,5 +401,12 @@ func TestReadExtendedPlatformTags(t *testing.T) { t.Skip("skipping platform tags test on non-linux") } tags := readExtendedPlatformTags() - test.That(t, len(tags), test.ShouldBeGreaterThanOrEqualTo, 3) + test.That(t, len(tags), test.ShouldBeGreaterThanOrEqualTo, 2) +} + +func TestAppendPairIfNonempty(t *testing.T) { + arr := make([]string, 0, 1) + arr = appendPairIfNonempty(arr, "x", "y") + arr = appendPairIfNonempty(arr, "a", "") + test.That(t, arr, test.ShouldResemble, []string{"x:y"}) } From a532b417176b6a8c6cfb072defb9276a77f19d6c Mon Sep 17 00:00:00 2001 From: Abe Winter Date: Mon, 21 Oct 2024 18:46:20 -0400 Subject: [PATCH 6/6] rm unused PathExists, lint --- config/reader.go | 2 +- utils/file.go | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/config/reader.go b/config/reader.go index 7920ccbf378..6f99b0dbcee 100644 --- a/config/reader.go +++ b/config/reader.go @@ -57,7 +57,7 @@ func parseOsRelease(body *bufio.Reader) map[string]string { } } -// append key:value pair to orig if value is non-empty +// append key:value pair to orig if value is non-empty. func appendPairIfNonempty(orig []string, key, value string) []string { if value != "" { return append(orig, key+":"+value) diff --git a/utils/file.go b/utils/file.go index 4b159f5e1d7..55b6de77755 100644 --- a/utils/file.go +++ b/utils/file.go @@ -47,10 +47,3 @@ func SafeJoinDir(parent, subdir string) (string, error) { } return res, nil } - -// PathExists returns true if the file exists, false if it doesn't or in -// any error case. No guarantees about doing the right thing with links. -func PathExists(path string) bool { - _, err := os.Stat(path) - return err == nil -}