From 8d4713b3e48576519bc13341584d5a43247c9461 Mon Sep 17 00:00:00 2001 From: Tim Pepper Date: Tue, 23 Aug 2016 15:05:56 -0700 Subject: [PATCH 1/5] osprepare: don't pass PackageRequirements by reference For this call stack there's no need to pass by reference. Trust in the memory manager and garbage collector ye olde C programmers. Signed-off-by: Tim Pepper --- osprepare/main.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/osprepare/main.go b/osprepare/main.go index 295f940a7..1e970a7aa 100644 --- a/osprepare/main.go +++ b/osprepare/main.go @@ -63,7 +63,7 @@ var BootstrapRequirements = PackageRequirements{ // CollectPackages returns a list of non-installed packages from // the PackageRequirements received -func collectPackages(dist distro, reqs *PackageRequirements) []string { +func collectPackages(dist distro, reqs PackageRequirements) []string { // For now just support keys like "ubuntu" vs "ubuntu:16.04" var pkgsMissing []string if reqs == nil { @@ -71,7 +71,7 @@ func collectPackages(dist distro, reqs *PackageRequirements) []string { } id := dist.getID() - if pkgs, success := (*reqs)[id]; success { + if pkgs, success := reqs[id]; success { for _, pkg := range pkgs { // Have the path existing, skip. if pathExists(pkg.BinaryName) { @@ -85,9 +85,9 @@ func collectPackages(dist distro, reqs *PackageRequirements) []string { return nil } -// PrepareOsDeps installs all the dependencies defined in -// PackageRequirements in order to run the ciao component -func PrepareOsDeps(reqs *PackageRequirements) { +// PrepareOsDeps installs all the dependencies defined in a component +// specific PackageRequirements in order to enable running the component +func PrepareOsDeps(reqs PackageRequirements) { distro := getDistro() if distro == nil { @@ -113,5 +113,5 @@ func PrepareOsDeps(reqs *PackageRequirements) { // Bootstrap installs all the core dependencies required to bootstrap the core // configuration of all Ciao components func Bootstrap() { - PrepareOsDeps(&BootstrapRequirements) + PrepareOsDeps(BootstrapRequirements) } From 0ccd02f1b48c2d4b636f8403df80945017ad1df2 Mon Sep 17 00:00:00 2001 From: Tim Pepper Date: Tue, 23 Aug 2016 15:07:05 -0700 Subject: [PATCH 2/5] scheduler: explicitly declare no OS deps We want to enable a pattern of each component having a deps.go listing the requirements of the component. It's clean and consistent. The component owner knows best what it needs and can maintain that list. Signed-off-by: Tim Pepper --- ciao-scheduler/deps.go | 31 +++++++++++++++++++++++++++++++ ciao-scheduler/scheduler.go | 2 +- 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 ciao-scheduler/deps.go diff --git a/ciao-scheduler/deps.go b/ciao-scheduler/deps.go new file mode 100644 index 000000000..1542c7648 --- /dev/null +++ b/ciao-scheduler/deps.go @@ -0,0 +1,31 @@ +// +// Copyright (c) 2016 Intel Corporation +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package main + +import "github.com/01org/ciao/osprepare" + +var schedDeps = osprepare.PackageRequirements{ + "clearlinux": { + {"", ""}, + }, + "fedora": { + {"", ""}, + }, + "ubuntu": { + {"", ""}, + }, +} diff --git a/ciao-scheduler/scheduler.go b/ciao-scheduler/scheduler.go index 13538adfc..85c49ed6f 100644 --- a/ciao-scheduler/scheduler.go +++ b/ciao-scheduler/scheduler.go @@ -1021,7 +1021,7 @@ func configSchedulerServer() (sched *ssntpSchedulerServer) { func main() { flag.Parse() - osprepare.PrepareOsDeps(nil) + osprepare.PrepareOsDeps(schedDeps) sched := configSchedulerServer() if sched == nil { From 245b16f6d9968c6e595d0f5639869c7ec677380f Mon Sep 17 00:00:00 2001 From: Tim Pepper Date: Tue, 23 Aug 2016 15:12:58 -0700 Subject: [PATCH 3/5] osprepare: fix go vet warnings A few testing.Fatal's should be testing.Fatalf. Signed-off-by: Tim Pepper --- osprepare/os_release_test.go | 2 +- osprepare/versions_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/osprepare/os_release_test.go b/osprepare/os_release_test.go index 992c071eb..327d67311 100644 --- a/osprepare/os_release_test.go +++ b/osprepare/os_release_test.go @@ -45,6 +45,6 @@ func TestGetOsRelease(t *testing.T) { func TestParseReleaseFileNonExistent(t *testing.T) { if res := ParseReleaseFile(NON_EXISTENT_FILE); res != nil { - t.Fatal("Expected nil, got %v\n", res) + t.Fatalf("Expected nil, got %v\n", res) } } diff --git a/osprepare/versions_test.go b/osprepare/versions_test.go index ae84f56c0..46bad0450 100644 --- a/osprepare/versions_test.go +++ b/osprepare/versions_test.go @@ -43,7 +43,7 @@ func TestGetQemu(t *testing.T) { // this tests is expected to pass func TestVersionLessThanEqualVersion(t *testing.T) { if res := VersionLessThan(MinQemuVersion, MinQemuVersion); res != false { - t.Fatal("expected false, got %v\n", res) + t.Fatalf("expected false, got %v\n", res) } } @@ -52,7 +52,7 @@ func TestVersionLessThanEqualVersion(t *testing.T) { // this tests is expected to pass func TestVersionLessThanGreaterVersion(t *testing.T) { if res := VersionLessThan(MinQemuVersion, "0.0.1"); res != false { - t.Fatal("expected false, got %v\n", res) + t.Fatalf("expected false, got %v\n", res) } } @@ -61,6 +61,6 @@ func TestVersionLessThanGreaterVersion(t *testing.T) { // this tests is expected to pass func TestVersionLessThanLowerVersion(t *testing.T) { if res := VersionLessThan("0.0.1", MinQemuVersion); res != true { - t.Fatal("expected true, got %v\n", res) + t.Fatalf("expected true, got %v\n", res) } } From 77df18da251184c494c20ce67cd7a9b508c1235c Mon Sep 17 00:00:00 2001 From: Tim Pepper Date: Tue, 23 Aug 2016 15:34:04 -0700 Subject: [PATCH 4/5] osprepare: make gofmt / vet / lint clean A few minor cleanups for go idioms and unexporting local-only stuff. Signed-off-by: Tim Pepper --- osprepare/distro.go | 2 +- osprepare/main.go | 5 +-- osprepare/os_release.go | 30 ++++++++-------- osprepare/os_release_test.go | 14 ++++---- osprepare/versions.go | 70 +++++++++++++++++++----------------- osprepare/versions_test.go | 10 +++--- 6 files changed, 68 insertions(+), 63 deletions(-) diff --git a/osprepare/distro.go b/osprepare/distro.go index f4f7f4d11..60b8bee2b 100644 --- a/osprepare/distro.go +++ b/osprepare/distro.go @@ -184,7 +184,7 @@ type distro interface { // getDistro will return a distro based on what // is read from GetOsRelease func getDistro() distro { - osRelease := GetOsRelease() + osRelease := getOSRelease() if osRelease == nil { return nil diff --git a/osprepare/main.go b/osprepare/main.go index 1e970a7aa..9047dba58 100644 --- a/osprepare/main.go +++ b/osprepare/main.go @@ -50,7 +50,8 @@ type PackageRequirement struct { // ) type PackageRequirements map[string][]*PackageRequirement -// Required for absolutely core functionality across all Ciao components +// BootstrapRequirements lists required dependencies for absolutely core +// functionality across all Ciao components var BootstrapRequirements = PackageRequirements{ "ubuntu": { {"/usr/bin/cephfs", "ceph-fs-common"}, @@ -92,7 +93,7 @@ func PrepareOsDeps(reqs PackageRequirements) { if distro == nil { fmt.Fprintf(os.Stderr, "Running on an unsupported distro\n") - if rel := GetOsRelease(); rel != nil { + if rel := getOSRelease(); rel != nil { fmt.Fprintf(os.Stderr, "Unsupported distro: %s %s\n", rel.Name, rel.Version) } else { fmt.Fprintln(os.Stderr, "No os-release found on this host") diff --git a/osprepare/os_release.go b/osprepare/os_release.go index b8d7d9482..04e4015ef 100644 --- a/osprepare/os_release.go +++ b/osprepare/os_release.go @@ -22,7 +22,7 @@ import ( "strings" ) -type OsRelease struct { +type osRelease struct { Name string ID string PrettyName string @@ -33,10 +33,10 @@ type OsRelease struct { // Parse the given path and attempt to return a valid // OsRelease for it -func ParseReleaseFile(path string) *OsRelease { +func parseReleaseFile(path string) *osRelease { fi, err := os.Open(path) - var os_rel OsRelease - os_rel.mapping = make(map[string]string) + var r osRelease + r.mapping = make(map[string]string) if err != nil { return nil @@ -57,25 +57,25 @@ func ParseReleaseFile(path string) *OsRelease { value = strings.Replace(value, "'", "", -1) if key == "name" { - os_rel.Name = value + r.Name = value } else if key == "id" { - os_rel.ID = value + r.ID = value } else if key == "pretty_name" { - os_rel.PrettyName = value + r.PrettyName = value } else if key == "version" { - os_rel.Version = value + r.Version = value } else if key == "version_id" { - os_rel.VersionID = value + r.VersionID = value } // Store it for use by Distro - os_rel.mapping[key] = value + r.mapping[key] = value } - return &os_rel + return &r } // Try all known paths to get the right OsRelease instance -func GetOsRelease() *OsRelease { +func getOSRelease() *osRelease { paths := []string{ "/etc/os-release", "/usr/lib/os-release", @@ -83,14 +83,14 @@ func GetOsRelease() *OsRelease { } for _, item := range paths { - if os_rel := ParseReleaseFile(item); os_rel != nil { - return os_rel + if r := parseReleaseFile(item); r != nil { + return r } } return nil } -func (o *OsRelease) GetValue(key string) string { +func (o *osRelease) GetValue(key string) string { if val, succ := o.mapping[strings.ToLower(key)]; succ { return val } diff --git a/osprepare/os_release_test.go b/osprepare/os_release_test.go index 327d67311..0acc89a21 100644 --- a/osprepare/os_release_test.go +++ b/osprepare/os_release_test.go @@ -22,7 +22,7 @@ import ( ) const ( - NON_EXISTENT_FILE = "/nonexistentpath/this/file/doesnt/exists" + nonExistentFile = "/nonexistentpath/this/file/doesnt/exists" ) func TestGetOsRelease(t *testing.T) { @@ -30,21 +30,21 @@ func TestGetOsRelease(t *testing.T) { if d == nil { t.Skip("Unknown distro, cannot test") } - os_rel := GetOsRelease() - if os_rel == nil { + r := getOSRelease() + if r == nil { t.Fatal("Could not get os-release file for known distro") } - if d.getID() == "clearlinux" && !strings.Contains(os_rel.ID, "clear") { + if d.getID() == "clearlinux" && !strings.Contains(r.ID, "clear") { t.Fatal("Invalid os-release for clearlinux") - } else if d.getID() == "ubuntu" && !strings.Contains(os_rel.ID, "ubuntu") { + } else if d.getID() == "ubuntu" && !strings.Contains(r.ID, "ubuntu") { t.Fatal("Invalid os-release for Ubuntu") - } else if d.getID() == "fedora" && !strings.Contains(os_rel.ID, "fedora") { + } else if d.getID() == "fedora" && !strings.Contains(r.ID, "fedora") { t.Fatal("Invalid os-release for Fedora") } } func TestParseReleaseFileNonExistent(t *testing.T) { - if res := ParseReleaseFile(NON_EXISTENT_FILE); res != nil { + if res := parseReleaseFile(nonExistentFile); res != nil { t.Fatalf("Expected nil, got %v\n", res) } } diff --git a/osprepare/versions.go b/osprepare/versions.go index 314a63c79..af12df2a9 100644 --- a/osprepare/versions.go +++ b/osprepare/versions.go @@ -24,7 +24,7 @@ import ( "strings" ) -func get_command_output(command string) string { +func getCommandOutput(command string) string { splits := strings.Split(command, " ") c := exec.Command(splits[0], splits[1:]...) c.Env = os.Environ() @@ -33,73 +33,77 @@ func get_command_output(command string) string { c.Env = append(c.Env, "LANG=C") c.Stderr = os.Stderr - if out, err := c.Output(); err != nil { + out, err := c.Output() + if err != nil { fmt.Fprintf(os.Stderr, "Failed to run %s: %s\n", splits[0], err) return "" - } else { - return string(out) } + + return string(out) } -func GetDockerVersion() string { - ret := get_command_output("docker --version") +func getDockerVersion() string { + ret := getCommandOutput("docker --version") var version string + if n, _ := fmt.Sscanf(ret, "Docker version %s, build", &version); n != 1 { return "" - } else { - if strings.HasSuffix(version, ",") { - return string(version[0 : len(version)-1]) - } - return version } + + if strings.HasSuffix(version, ",") { + return string(version[0 : len(version)-1]) + } + return version } -func GetQemuVersion() string { - ret := get_command_output("qemu-system-x86_64 --version") +func getQemuVersion() string { + ret := getCommandOutput("qemu-system-x86_64 --version") var version string + if n, _ := fmt.Sscanf(ret, "QEMU emulator version %s, Copyright (c)", &version); n != 1 { return "" - } else { - if strings.HasSuffix(version, ",") { - return string(version[0 : len(version)-1]) - } - return version } + + if strings.HasSuffix(version, ",") { + return string(version[0 : len(version)-1]) + } + return version } // Determine if the given current version is less than the test version // Note: Can only compare equal version schemas (i.e. same level of dots) -func VersionLessThan(current_version string, test_version string) bool { - cur_splits := strings.Split(current_version, ".") - test_splits := strings.Split(test_version, ".") +func versionLessThan(currentVer string, testVer string) bool { + curSplits := strings.Split(currentVer, ".") + testSplits := strings.Split(testVer, ".") + + max := len(curSplits) - max_range := len(cur_splits) - if l2 := len(test_splits); l2 < max_range { - max_range = l2 + if l2 := len(testSplits); l2 < max { + max = l2 } - cur_isplits := make([]int, max_range) - cur_tsplits := make([]int, max_range) + iSplits := make([]int, max) + tSplits := make([]int, max) - for i := 0; i < max_range; i++ { - cur_isplits[i], _ = strconv.Atoi(cur_splits[i]) - cur_tsplits[i], _ = strconv.Atoi(test_splits[i]) + for i := 0; i < max; i++ { + iSplits[i], _ = strconv.Atoi(curSplits[i]) + tSplits[i], _ = strconv.Atoi(testSplits[i]) } - for i := 0; i < max_range; i++ { + for i := 0; i < max; i++ { if i == 0 { - if cur_isplits[i] < cur_tsplits[i] { + if iSplits[i] < tSplits[i] { return true } } else { match := true for j := 0; j < i; j++ { - if cur_isplits[j] != cur_tsplits[j] { + if iSplits[j] != tSplits[j] { match = false break } } - if match && cur_isplits[i] < cur_tsplits[i] { + if match && iSplits[i] < tSplits[i] { return true } } diff --git a/osprepare/versions_test.go b/osprepare/versions_test.go index 46bad0450..500f19cb6 100644 --- a/osprepare/versions_test.go +++ b/osprepare/versions_test.go @@ -24,7 +24,7 @@ func TestGetDocker(t *testing.T) { if pathExists("/usr/bin/docker") == false { t.Skip("Docker not installed, cannot validate version get") } - if vers := GetDockerVersion(); vers == "" { + if vers := getDockerVersion(); vers == "" { t.Fatal("Cannot determine docker version") } } @@ -33,7 +33,7 @@ func TestGetQemu(t *testing.T) { if pathExists("/usr/bin/qemu-system-x86_64") == false { t.Skip("Qemu not installed, cannot validate version get") } - if vers := GetQemuVersion(); vers == "" { + if vers := getQemuVersion(); vers == "" { t.Fatal("Cannot determine qemu version") } } @@ -42,7 +42,7 @@ func TestGetQemu(t *testing.T) { // false when given same version to tests. e.g: VersionLessThan("1.11.0", "1.11.0") // this tests is expected to pass func TestVersionLessThanEqualVersion(t *testing.T) { - if res := VersionLessThan(MinQemuVersion, MinQemuVersion); res != false { + if res := versionLessThan(MinQemuVersion, MinQemuVersion); res != false { t.Fatalf("expected false, got %v\n", res) } } @@ -51,7 +51,7 @@ func TestVersionLessThanEqualVersion(t *testing.T) { // false when given greater version. e.g: VersionLessThan("1.11.0", "0.0.1") // this tests is expected to pass func TestVersionLessThanGreaterVersion(t *testing.T) { - if res := VersionLessThan(MinQemuVersion, "0.0.1"); res != false { + if res := versionLessThan(MinQemuVersion, "0.0.1"); res != false { t.Fatalf("expected false, got %v\n", res) } } @@ -60,7 +60,7 @@ func TestVersionLessThanGreaterVersion(t *testing.T) { // true when given lower version. e.g: VersionLessThan("0.0.1", "99.9.9") // this tests is expected to pass func TestVersionLessThanLowerVersion(t *testing.T) { - if res := VersionLessThan("0.0.1", MinQemuVersion); res != true { + if res := versionLessThan("0.0.1", MinQemuVersion); res != true { t.Fatalf("expected true, got %v\n", res) } } From 0bd171b6c5831f4b48e16b19a587912eb56a644b Mon Sep 17 00:00:00 2001 From: Tim Pepper Date: Tue, 23 Aug 2016 15:44:12 -0700 Subject: [PATCH 5/5] osprepare: fix misspell warning Man o man this tool is picky. But it's easily tricked^H^H^Hfixed. Signed-off-by: Tim Pepper --- osprepare/os_release_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/osprepare/os_release_test.go b/osprepare/os_release_test.go index 0acc89a21..f23a634c2 100644 --- a/osprepare/os_release_test.go +++ b/osprepare/os_release_test.go @@ -22,7 +22,7 @@ import ( ) const ( - nonExistentFile = "/nonexistentpath/this/file/doesnt/exists" + nonExistentFile = "/nonexistentpath/this/file/doesnot/exists" ) func TestGetOsRelease(t *testing.T) {