utils: move Network operation helpers into apiserver

These two functions were only used in the apiserver. Move them and their tests to the apiserver package.
sinanawad · Nov 11, 2015 · f289441 · f289441
1 parent a487f71
commit f289441
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 31 deletions.
diff --git a/apiserver/images.go b/apiserver/images.go
@@ -21,7 +21,6 @@ import (
 	"github.com/juju/juju/instance"
 	"github.com/juju/juju/state"
 	"github.com/juju/juju/state/imagestorage"
-	"github.com/juju/juju/utils"
 )
 
 // imagesDownloadHandler handles image download through HTTPS in the API server.
@@ -96,11 +95,10 @@ func (h *imagesDownloadHandler) loadImage(st *state.State, envuuid, kind, series
 	metadata, imageReader, err := storage.Image(kind, series, arch)
 	// Not in storage, so go fetch it.
 	if errors.IsNotFound(err) {
-		err = h.fetchAndCacheLxcImage(storage, envuuid, series, arch)
-		if err != nil {
+		if err := h.fetchAndCacheLxcImage(storage, envuuid, series, arch); err != nil {
 			return nil, nil, errors.Annotate(err, "error fetching and caching image")
 		}
-		err = utils.NetworkOperationWitDefaultRetries(func() error {
+		err = networkOperationWitDefaultRetries(func() error {
 			metadata, imageReader, err = storage.Image(string(instance.LXC), series, arch)
 			return err
 		}, "streaming os image from blobstore")()
@@ -177,7 +175,7 @@ func (h *imagesDownloadHandler) fetchAndCacheLxcImage(storage imagestorage.Stora
 	}
 
 	// Stream the image to storage.
-	err = utils.NetworkOperationWitDefaultRetries(func() error {
+	err = networkOperationWitDefaultRetries(func() error {
 		return storage.AddImage(rdr, metadata)
 	}, "add os image to blobstore")()
 	if err != nil {
@@ -186,7 +184,7 @@ func (h *imagesDownloadHandler) fetchAndCacheLxcImage(storage imagestorage.Stora
 	// Better check the downloaded image checksum.
 	downloadChecksum := fmt.Sprintf("%x", hash.Sum(nil))
 	if downloadChecksum != checksum {
-		err = utils.NetworkOperationWitDefaultRetries(func() error {
+		err := networkOperationWitDefaultRetries(func() error {
 			return storage.DeleteImage(metadata)
 		}, "delete os image from blobstore")()
 		if err != nil {

diff --git a/utils/network.go → apiserver/network.go b/utils/network.go → apiserver/network.go
@@ -1,7 +1,7 @@
 // Copyright 2015 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
-package utils
+package apiserver
 
 import (
 	"net"
@@ -17,31 +17,32 @@ var (
 	// Use the NetworkOperationWithRetries() variant to explicitly
 	// use retry values better suited to different scenarios.
 
-	// DefaultNetworkOperationRetryDelay is the default time
+	// defaultNetworkOperationRetryDelay is the default time
 	// to wait between operation retries.
-	DefaultNetworkOperationRetryDelay = 30 * time.Second
+	defaultNetworkOperationRetryDelay = 30 * time.Second
 
-	// DefaultNetworkOperationAttempts is the default number
+	// defaultNetworkOperationAttempts is the default number
 	// of attempts before giving up.
-	DefaultNetworkOperationAttempts = 10
+	defaultNetworkOperationAttempts = 10
 )
 
-// NetworkOperationWithDefaultRetries calls the supplied function and if it returns a
+// networkOperationWithDefaultRetries calls the supplied function and if it returns a
 // network error which is temporary, will retry a number of times before giving up.
 // A default attempt strategy is used.
-func NetworkOperationWitDefaultRetries(networkOp func() error, description string) func() error {
+func networkOperationWitDefaultRetries(networkOp func() error, description string) func() error {
 	attempt := utils.AttemptStrategy{
-		Delay: DefaultNetworkOperationRetryDelay,
-		Min:   DefaultNetworkOperationAttempts,
+		Delay: defaultNetworkOperationRetryDelay,
+		Min:   defaultNetworkOperationAttempts,
 	}
-	return NetworkOperationWithRetries(attempt, networkOp, description)
+	return networkOperationWithRetries(attempt, networkOp, description)
 }
 
-// NetworkOperationWithRetries calls the supplied function and if it returns a
+// networkOperationWithRetries calls the supplied function and if it returns a
 // network error which is temporary, will retry a number of times before giving up.
-func NetworkOperationWithRetries(strategy utils.AttemptStrategy, networkOp func() error, description string) func() error {
+func networkOperationWithRetries(strategy utils.AttemptStrategy, networkOp func() error, description string) func() error {
 	return func() error {
-		for a := strategy.Start(); a.Next(); {
+		for a := strategy.Start(); ; {
+			a.Next()
 			err := networkOp()
 			if !a.HasNext() || err == nil {
 				return errors.Trace(err)
@@ -51,6 +52,5 @@ func NetworkOperationWithRetries(strategy utils.AttemptStrategy, networkOp func(
 			}
 			logger.Debugf("%q error, will retry: %v", description, err)
 		}
-		panic("unreachable")
 	}
 }
diff --git a/utils/network_test.go → apiserver/network_test.go b/utils/network_test.go → apiserver/network_test.go
@@ -1,7 +1,7 @@
 // Copyright 2015 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
-package utils_test
+package apiserver
 
 import (
 	"time"
@@ -11,7 +11,6 @@ import (
 	gc "gopkg.in/check.v1"
 
 	"github.com/juju/juju/testing"
-	"github.com/juju/juju/utils"
 )
 
 type networkSuite struct {
@@ -26,52 +25,52 @@ func (s *networkSuite) TestOpSuccess(c *gc.C) {
 		isCalled = true
 		return nil
 	}
-	err := utils.NetworkOperationWitDefaultRetries(f, "do it")()
+	err := networkOperationWitDefaultRetries(f, "do it")()
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(isCalled, jc.IsTrue)
 }
 
 func (s *networkSuite) TestOpFailureNoRetry(c *gc.C) {
-	s.PatchValue(&utils.DefaultNetworkOperationRetryDelay, 1*time.Millisecond)
+	s.PatchValue(&defaultNetworkOperationRetryDelay, 1*time.Millisecond)
 	netErr := &netError{false}
 	callCount := 0
 	f := func() error {
 		callCount++
 		return netErr
 	}
-	err := utils.NetworkOperationWitDefaultRetries(f, "do it")()
+	err := networkOperationWitDefaultRetries(f, "do it")()
 	c.Assert(errors.Cause(err), gc.Equals, netErr)
 	c.Assert(callCount, gc.Equals, 1)
 }
 
 func (s *networkSuite) TestOpFailureRetries(c *gc.C) {
-	s.PatchValue(&utils.DefaultNetworkOperationRetryDelay, 1*time.Millisecond)
+	s.PatchValue(&defaultNetworkOperationRetryDelay, 1*time.Millisecond)
 	netErr := &netError{true}
 	callCount := 0
 	f := func() error {
 		callCount++
 		return netErr
 	}
-	err := utils.NetworkOperationWitDefaultRetries(f, "do it")()
+	err := networkOperationWitDefaultRetries(f, "do it")()
 	c.Assert(errors.Cause(err), gc.Equals, netErr)
 	c.Assert(callCount, gc.Equals, 10)
 }
 
 func (s *networkSuite) TestOpNestedFailureRetries(c *gc.C) {
-	s.PatchValue(&utils.DefaultNetworkOperationRetryDelay, 1*time.Millisecond)
+	s.PatchValue(&defaultNetworkOperationRetryDelay, 1*time.Millisecond)
 	netErr := &netError{true}
 	callCount := 0
 	f := func() error {
 		callCount++
 		return errors.Annotate(errors.Trace(netErr), "create a wrapped error")
 	}
-	err := utils.NetworkOperationWitDefaultRetries(f, "do it")()
+	err := networkOperationWitDefaultRetries(f, "do it")()
 	c.Assert(errors.Cause(err), gc.Equals, netErr)
 	c.Assert(callCount, gc.Equals, 10)
 }
 
 func (s *networkSuite) TestOpSucceedsAfterRetries(c *gc.C) {
-	s.PatchValue(&utils.DefaultNetworkOperationRetryDelay, 1*time.Millisecond)
+	s.PatchValue(&defaultNetworkOperationRetryDelay, 1*time.Millisecond)
 	netErr := &netError{true}
 	callCount := 0
 	f := func() error {
@@ -81,7 +80,7 @@ func (s *networkSuite) TestOpSucceedsAfterRetries(c *gc.C) {
 		}
 		return netErr
 	}
-	err := utils.NetworkOperationWitDefaultRetries(f, "do it")()
+	err := networkOperationWitDefaultRetries(f, "do it")()
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(callCount, gc.Equals, 5)
 }