internal/googlecloud: refactor OS-dependent code (#5239)

2022-03-21 22:37:55 +01:00
parent 1ffd63de37
commit 7ea4af98f1
5 changed files with 125 additions and 118 deletions
--- a/internal/googlecloud/googlecloud.go
+++ b/internal/googlecloud/googlecloud.go
@@ -20,13 +20,6 @@
 package googlecloud

 import (
-	"errors"
-	"fmt"
-	"io"
-	"io/ioutil"
-	"os"
-	"os/exec"
-	"regexp"
 	"runtime"
 	"strings"
 	"sync"
@@ -35,43 +28,9 @@ import (
 	internalgrpclog "google.golang.org/grpc/internal/grpclog"
 )

-const (
-	linuxProductNameFile     = "/sys/class/dmi/id/product_name"
-	windowsCheckCommand      = "powershell.exe"
-	windowsCheckCommandArgs  = "Get-WmiObject -Class Win32_BIOS"
-	powershellOutputFilter   = "Manufacturer"
-	windowsManufacturerRegex = ":(.*)"
-
-	logPrefix = "[googlecloud]"
-)
+const logPrefix = "[googlecloud]"

 var (
-	// The following two variables will be reassigned in tests.
-	runningOS          = runtime.GOOS
-	manufacturerReader = func() (io.Reader, error) {
-		switch runningOS {
-		case "linux":
-			return os.Open(linuxProductNameFile)
-		case "windows":
-			cmd := exec.Command(windowsCheckCommand, windowsCheckCommandArgs)
-			out, err := cmd.Output()
-			if err != nil {
-				return nil, err
-			}
-			for _, line := range strings.Split(strings.TrimSuffix(string(out), "\n"), "\n") {
-				if strings.HasPrefix(line, powershellOutputFilter) {
-					re := regexp.MustCompile(windowsManufacturerRegex)
-					name := re.FindString(line)
-					name = strings.TrimLeft(name, ":")
-					return strings.NewReader(name), nil
-				}
-			}
-			return nil, errors.New("cannot determine the machine's manufacturer")
-		default:
-			return nil, fmt.Errorf("%s is not supported", runningOS)
-		}
-	}
-
 	vmOnGCEOnce sync.Once
 	vmOnGCE     bool

@@ -84,21 +43,21 @@ var (
 // package. We keep this to avoid depending on the cloud library module.
 func OnGCE() bool {
 	vmOnGCEOnce.Do(func() {
-		vmOnGCE = isRunningOnGCE()
+		mf, err := manufacturer()
+		if err != nil {
+			logger.Infof("failed to read manufacturer, setting onGCE=false: %v")
+			return
+		}
+		vmOnGCE = isRunningOnGCE(mf, runtime.GOOS)
 	})
 	return vmOnGCE
 }

-// isRunningOnGCE checks whether the local system, without doing a network request is
+// isRunningOnGCE checks whether the local system, without doing a network request, is
 // running on GCP.
-func isRunningOnGCE() bool {
-	manufacturer, err := readManufacturer()
-	if err != nil {
-		logger.Infof("failed to read manufacturer %v, returning OnGCE=false", err)
-		return false
-	}
+func isRunningOnGCE(manufacturer []byte, goos string) bool {
 	name := string(manufacturer)
-	switch runningOS {
+	switch goos {
 	case "linux":
 		name = strings.TrimSpace(name)
 		return name == "Google" || name == "Google Compute Engine"
@@ -111,18 +70,3 @@ func isRunningOnGCE() bool {
 		return false
 	}
 }
-
-func readManufacturer() ([]byte, error) {
-	reader, err := manufacturerReader()
-	if err != nil {
-		return nil, err
-	}
-	if reader == nil {
-		return nil, errors.New("got nil reader")
-	}
-	manufacturer, err := ioutil.ReadAll(reader)
-	if err != nil {
-		return nil, fmt.Errorf("failed reading %v: %v", linuxProductNameFile, err)
-	}
-	return manufacturer, nil
-}
--- a/internal/googlecloud/googlecloud_test.go
+++ b/internal/googlecloud/googlecloud_test.go
@@ -19,68 +19,28 @@
 package googlecloud

 import (
-	"io"
-	"os"
-	"strings"
 	"testing"
 )

-func setupManufacturerReader(testOS string, reader func() (io.Reader, error)) func() {
-	tmpOS := runningOS
-	tmpReader := manufacturerReader
-
-	// Set test OS and reader function.
-	runningOS = testOS
-	manufacturerReader = reader
-	return func() {
-		runningOS = tmpOS
-		manufacturerReader = tmpReader
-	}
-}
-
-func setup(testOS string, testReader io.Reader) func() {
-	reader := func() (io.Reader, error) {
-		return testReader, nil
-	}
-	return setupManufacturerReader(testOS, reader)
-}
-
-func setupError(testOS string, err error) func() {
-	reader := func() (io.Reader, error) {
-		return nil, err
-	}
-	return setupManufacturerReader(testOS, reader)
-}
-
 func TestIsRunningOnGCE(t *testing.T) {
 	for _, tc := range []struct {
-		description string
-		testOS      string
-		testReader  io.Reader
-		out         bool
+		description      string
+		testOS           string
+		testManufacturer string
+		out              bool
 	}{
 		// Linux tests.
-		{"linux: not a GCP platform", "linux", strings.NewReader("not GCP"), false},
-		{"Linux: GCP platform (Google)", "linux", strings.NewReader("Google"), true},
-		{"Linux: GCP platform (Google Compute Engine)", "linux", strings.NewReader("Google Compute Engine"), true},
-		{"Linux: GCP platform (Google Compute Engine) with extra spaces", "linux", strings.NewReader("  Google Compute Engine        "), true},
+		{"linux: not a GCP platform", "linux", "not GCP", false},
+		{"Linux: GCP platform (Google)", "linux", "Google", true},
+		{"Linux: GCP platform (Google Compute Engine)", "linux", "Google Compute Engine", true},
+		{"Linux: GCP platform (Google Compute Engine) with extra spaces", "linux", "  Google Compute Engine        ", true},
 		// Windows tests.
-		{"windows: not a GCP platform", "windows", strings.NewReader("not GCP"), false},
-		{"windows: GCP platform (Google)", "windows", strings.NewReader("Google"), true},
-		{"windows: GCP platform (Google) with extra spaces", "windows", strings.NewReader("  Google     "), true},
+		{"windows: not a GCP platform", "windows", "not GCP", false},
+		{"windows: GCP platform (Google)", "windows", "Google", true},
+		{"windows: GCP platform (Google) with extra spaces", "windows", "  Google     ", true},
 	} {
-		reverseFunc := setup(tc.testOS, tc.testReader)
-		if got, want := isRunningOnGCE(), tc.out; got != want {
+		if got, want := isRunningOnGCE([]byte(tc.testManufacturer), tc.testOS), tc.out; got != want {
 			t.Errorf("%v: isRunningOnGCE()=%v, want %v", tc.description, got, want)
 		}
-		reverseFunc()
 	}
 }
-
-func TestIsRunningOnGCENoProductNameFile(t *testing.T) {
-	reverseFunc := setupError("linux", os.ErrNotExist)
-	if isRunningOnGCE() {
-		t.Errorf("ErrNotExist: isRunningOnGCE()=true, want false")
-	}
-	reverseFunc()
-}
--- a/internal/googlecloud/manufacturer.go
+++ b/internal/googlecloud/manufacturer.go
@@ -0,0 +1,26 @@
+//go:build !(linux || windows)
+// +build !linux,!windows
+
+/*
+ *
+ * Copyright 2022 gRPC authors.
+ *
+ * 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 googlecloud
+
+func manufacturer() ([]byte, error) {
+	return nil, nil
+}
--- a/internal/googlecloud/manufacturer_linux.go
+++ b/internal/googlecloud/manufacturer_linux.go
@@ -0,0 +1,27 @@
+/*
+ *
+ * Copyright 2022 gRPC authors.
+ *
+ * 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 googlecloud
+
+import "io/ioutil"
+
+const linuxProductNameFile = "/sys/class/dmi/id/product_name"
+
+func manufacturer() ([]byte, error) {
+	return ioutil.ReadFile(linuxProductNameFile)
+}
--- a/internal/googlecloud/manufacturer_windows.go
+++ b/internal/googlecloud/manufacturer_windows.go
@@ -0,0 +1,50 @@
+/*
+ *
+ * Copyright 2022 gRPC authors.
+ *
+ * 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 googlecloud
+
+import (
+	"errors"
+	"os/exec"
+	"regexp"
+	"strings"
+)
+
+const (
+	windowsCheckCommand      = "powershell.exe"
+	windowsCheckCommandArgs  = "Get-WmiObject -Class Win32_BIOS"
+	powershellOutputFilter   = "Manufacturer"
+	windowsManufacturerRegex = ":(.*)"
+)
+
+func manufacturer() ([]byte, error) {
+	cmd := exec.Command(windowsCheckCommand, windowsCheckCommandArgs)
+	out, err := cmd.Output()
+	if err != nil {
+		return nil, err
+	}
+	for _, line := range strings.Split(strings.TrimSuffix(string(out), "\n"), "\n") {
+		if strings.HasPrefix(line, powershellOutputFilter) {
+			re := regexp.MustCompile(windowsManufacturerRegex)
+			name := re.FindString(line)
+			name = strings.TrimLeft(name, ":")
+			return []byte(name), nil
+		}
+	}
+	return nil, errors.New("cannot determine the machine's manufacturer")
+}