Remove redundant interface indirection

Signed-off-by: Liam White <liam@tetrate.io>

pkg/binary/getenvoy/fetch.go → pkg/binary/fetch.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package getenvoy
+package binary
 
 import (
 	"errors"
@@ -31,15 +31,28 @@ import (
 	"github.com/mholt/archiver"
 	"github.com/schollz/progressbar/v2"
 	"github.com/tetratelabs/getenvoy/pkg/manifest"
+	"github.com/tetratelabs/log"
 )
 
 // Fetch downloads an Envoy binary from the passed location
 func (r *Runtime) Fetch(key *manifest.Key, binaryLocation string) error {
-	dst := r.binaryPath(key)
-	if err := os.MkdirAll(dst, 0750); err != nil {
-		return fmt.Errorf("unable to create directory %q: %v", dst, err)
+	if !r.AlreadyDownloaded(key) {
+		log.Debugf("fetching %v from %v", key, binaryLocation)
+		dst := r.binaryPath(key)
+		if err := os.MkdirAll(dst, 0750); err != nil {
+			return fmt.Errorf("unable to create directory %q: %v", dst, err)
+		}
+		return fetchEnvoy(dst, binaryLocation)
 	}
-	return fetchEnvoy(dst, binaryLocation)
+	log.Debugf("%v is already downloaded", key)
+	return nil
+}
+
+func (r *Runtime) AlreadyDownloaded(key *manifest.Key) bool {
+	_, err := os.Stat(filepath.Join(r.binaryPath(key), "envoy"))
+	// !IsNotExist != IsExist
+	// os.Stat doesn't return IsExist typed errors
+	return !os.IsNotExist(err)
 }
 
 func (r *Runtime) binaryPath(key *manifest.Key) string {

pkg/binary/getenvoy/fetch_test.go → pkg/binary/fetch_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package getenvoy
+package binary
 
 import (
 	"io/ioutil"
@@ -35,67 +35,95 @@ func TestRuntime_Fetch(t *testing.T) {
 		key              *manifest.Key
 		tarballStructure string
 		envoyLocation    string
+		alreadyLocal     bool
 		responseStatus   int
 		wantErr          bool
+		wantServerCalled bool
 	}{
 		{
 			name:             "Downloads and untars envoy to local/key",
 			key:              defaultDarwinKey,
 			tarballStructure: "golden",
 			responseStatus:   http.StatusOK,
 			envoyLocation:    "builds/standard/1.11.0/darwin/envoy",
+			wantServerCalled: true,
+		},
+		{
+			name:             "Does nothing if it already has a local copy",
+			key:              defaultDarwinKey,
+			envoyLocation:    "builds/standard/1.11.0/darwin/envoy",
+			alreadyLocal:     true,
+			wantServerCalled: false,
 		},
 		{
 			name:             "Handles directories called Envoy",
 			key:              defaultDarwinKey,
 			tarballStructure: "envoydirectory",
 			responseStatus:   http.StatusOK,
 			envoyLocation:    "builds/standard/1.11.0/darwin/envoy",
+			wantServerCalled: true,
 		},
 		{
 			name:             "errors if it can't find an envoy binary in tarball",
 			key:              defaultDarwinKey,
 			tarballStructure: "noenvoy",
 			responseStatus:   http.StatusOK,
 			wantErr:          true,
+			wantServerCalled: true,
 		},
 		{
 			name:             "errors if it gets !200 from download",
 			key:              defaultDarwinKey,
 			tarballStructure: "noenvoy",
 			responseStatus:   http.StatusTeapot,
 			wantErr:          true,
+			wantServerCalled: true,
 		},
 	}
 	for _, tt := range tests {
 		tc := tt
 		t.Run(tc.name, func(t *testing.T) {
 			tmpDir, _ := ioutil.TempDir("", "getenvoy-test-")
+			envoyLocation := filepath.Join(tmpDir, tc.envoyLocation)
 			defer os.RemoveAll(tmpDir)
-			mock := mockServer(tc.responseStatus, tc.tarballStructure, tmpDir)
+			mock, gotCalled := mockServer(tc.responseStatus, tc.tarballStructure, tmpDir)
+			if tc.alreadyLocal {
+				createLocalEnvoy(envoyLocation)
+			}
 
 			r := &Runtime{local: tmpDir}
 			err := r.Fetch(tc.key, mock.URL)
 			if tc.wantErr {
 				assert.Error(t, err)
 			} else {
 				assert.Nil(t, err)
-				f, _ := os.Open(filepath.Join(tmpDir, tc.envoyLocation))
+				f, _ := os.Open(envoyLocation)
 				bytes, _ := ioutil.ReadAll(f)
 				assert.Contains(t, string(bytes), "some complied c++")
 			}
+			assert.Equal(t, tc.wantServerCalled, *gotCalled, "mismatch of expectations for calling of remote server")
 		})
 	}
 }
 
-func mockServer(responseStatusCode int, tarballStructure string, tmpDir string) *httptest.Server {
+func createLocalEnvoy(envoyLocation string) {
+	dir, _ := filepath.Split(envoyLocation)
+	os.MkdirAll(dir, 0750)
+	f, _ := os.Create(envoyLocation)
+	f.WriteString("some complied c++")
+	f.Close()
+}
+
+func mockServer(responseStatusCode int, tarballStructure string, tmpDir string) (*httptest.Server, *bool) {
+	called := false
 	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		called = true
 		w.WriteHeader(responseStatusCode)
 		if responseStatusCode == http.StatusOK {
 			tarball := filepath.Join(tmpDir, tarballStructure+".tar.gz")
 			archiver.Archive([]string{filepath.Join("testdata", tarballStructure)}, tarball)
 			bytes, _ := ioutil.ReadFile(tarball)
 			w.Write(bytes)
 		}
-	}))
+	})), &called
 }

pkg/binary/getenvoy/procattr.go → pkg/binary/procattr.go

@@ -14,7 +14,7 @@
 
 // +build !linux
 
-package getenvoy
+package binary
 
 import (
 	"syscall"

pkg/binary/getenvoy/procattr_linux.go → pkg/binary/procattr_linux.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package getenvoy
+package binary
 
 import "syscall"
 

pkg/binary/getenvoy/run.go → pkg/binary/run.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package getenvoy
+package binary
 
 import (
 	"context"
@@ -97,6 +97,6 @@ func (r *Runtime) runEnvoy(path string, args []string, cancel context.CancelFunc
 }
 
 func (r *Runtime) initializeDebugStore() error {
-	r.debugDir = filepath.Join(r.local, "debug", strconv.FormatInt(time.Now().UnixNano(), 10))
-	return os.MkdirAll(r.debugDir, 0750)
+	r.DebugDir = filepath.Join(r.local, "debug", strconv.FormatInt(time.Now().UnixNano(), 10))
+	return os.MkdirAll(r.DebugDir, 0750)
 }

pkg/binary/getenvoy/run_test.go → pkg/binary/run_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package getenvoy
+package binary
 
 import (
 	"fmt"
@@ -89,7 +89,7 @@ func waitForProcessStart(r *Runtime) {
 func newRuntimeWithMockFunctions(t *testing.T) (*Runtime, *bool, *bool) {
 	preStartCalled := false
 	preStart := func(r *Runtime) {
-		r.registerPreStart(func(r *Runtime) error {
+		r.RegisterPreStart(func(r *Runtime) error {
 			if r.cmd != nil && r.cmd.Process != nil {
 				t.Error("preStart was called after process has started")
 			}
@@ -100,7 +100,7 @@ func newRuntimeWithMockFunctions(t *testing.T) (*Runtime, *bool, *bool) {
 
 	preTerminationCalled := false
 	preTermination := func(r *Runtime) {
-		r.registerPreTermination(func(r *Runtime) error {
+		r.RegisterPreTermination(func(r *Runtime) error {
 			if r.cmd != nil && r.cmd.Process == nil {
 				t.Error("preTermination was called before process was started")
 			}
@@ -111,6 +111,6 @@ func newRuntimeWithMockFunctions(t *testing.T) (*Runtime, *bool, *bool) {
 			return nil
 		})
 	}
-	runtime, _ := New(preStart, preTermination)
+	runtime, _ := NewRuntime(preStart, preTermination)
 	return runtime, &preStartCalled, &preTerminationCalled
 }