Change name of version sig file; ensure upgrades use locally compiled…

… binary
sdg0919 · Oct 11, 2017 · f4ab9ca · f4ab9ca
1 parent da82044
commit f4ab9ca
Show file tree

Hide file tree

Showing 12 changed files with 152 additions and 65 deletions.
diff --git a/cmd/juju/commands/bootstrap_test.go b/cmd/juju/commands/bootstrap_test.go
@@ -107,7 +107,7 @@ func (s *BootstrapSuite) SetUpTest(c *gc.C) {
 
 	// NOTE(axw) we cannot patch BundleTools here, as the "gc.C" argument
 	// is invalidated once this method returns.
-	s.PatchValue(&envtools.BundleTools, func(bool, io.Writer, *version.Number) (version.Binary, string, error) {
+	s.PatchValue(&envtools.BundleTools, func(bool, io.Writer, *version.Number) (version.Binary, bool, string, error) {
 		panic("tests must call setupAutoUploadTest or otherwise patch envtools.BundleTools")
 	})
 
@@ -1004,8 +1004,8 @@ func (s *BootstrapSuite) TestBootstrapAlreadyExists(c *gc.C) {
 
 func (s *BootstrapSuite) TestInvalidLocalSource(c *gc.C) {
 	s.PatchValue(&jujuversion.Current, version.MustParse("1.2.0"))
-	s.PatchValue(&envtools.BundleTools, func(bool, io.Writer, *version.Number) (version.Binary, string, error) {
-		return version.Binary{}, "", errors.New("no agent binaries for you")
+	s.PatchValue(&envtools.BundleTools, func(bool, io.Writer, *version.Number) (version.Binary, bool, string, error) {
+		return version.Binary{}, false, "", errors.New("no agent binaries for you")
 	})
 	resetJujuXDGDataHome(c)
 

diff --git a/cmd/juju/commands/upgradejuju.go b/cmd/juju/commands/upgradejuju.go
@@ -25,6 +25,7 @@ import (
 	"github.com/juju/juju/cmd/modelcmd"
 	"github.com/juju/juju/environs/config"
 	"github.com/juju/juju/environs/sync"
+	"github.com/juju/juju/environs/tools"
 	coretools "github.com/juju/juju/tools"
 	jujuversion "github.com/juju/juju/version"
 )
@@ -315,20 +316,16 @@ func (c *upgradeJujuCommand) Run(ctx *cmd.Context) (err error) {
 		}
 	}
 
-	context, err := c.initVersions(client, cfg, agentVersion, warnCompat)
+	context, tryImplicit, err := c.initVersions(client, cfg, agentVersion, warnCompat)
 	if err != nil {
 		return err
 	}
 	// If we're running a custom build or the user has asked for a new agent
 	// to be built, upload a local jujud binary if possible.
-	uploadLocalBinary := isControllerModel && c.Version == version.Zero && tryImplicitUpload(agentVersion)
+	uploadLocalBinary := isControllerModel && c.Version == version.Zero && tryImplicit
 	if !warnCompat && (uploadLocalBinary || c.BuildAgent) && !c.DryRun {
 		if err := context.uploadTools(c.BuildAgent); err != nil {
-			// If we've explicitly asked to build an agent binary, or the upload failed
-			// because changes were blocked, we'll return an error.
-			if err2 := block.ProcessBlockedError(err, block.BlockChange); c.BuildAgent || err2 == cmd.ErrSilent {
-				return err2
-			}
+			return block.ProcessBlockedError(err, block.BlockChange)
 		}
 		builtMsg := ""
 		if c.BuildAgent {
@@ -379,9 +376,22 @@ func (c *upgradeJujuCommand) Run(ctx *cmd.Context) (err error) {
 	return nil
 }
 
-func tryImplicitUpload(agentVersion version.Number) bool {
+func tryImplicitUpload(agentVersion version.Number) (bool, error) {
 	newerAgent := jujuversion.Current.Compare(agentVersion) > 0
-	return newerAgent || agentVersion.Build > 0 || jujuversion.Current.Build > 0
+	if newerAgent || agentVersion.Build > 0 || jujuversion.Current.Build > 0 {
+		return true, nil
+	}
+	jujudPath, err := tools.ExistingJujudLocation()
+	if err != nil {
+		return false, errors.Trace(err)
+	}
+	_, official, err := tools.JujudVersion(jujudPath)
+	// If there's an error getting jujud version, play it safe
+	// and don't implicitly do an implicit upload.
+	if err != nil {
+		return false, nil
+	}
+	return !official, nil
 }
 
 const resetPreviousUpgradeMessage = `
@@ -410,9 +420,11 @@ func (c *upgradeJujuCommand) confirmResetPreviousUpgrade(ctx *cmd.Context) (bool
 // agent and client versions, and the list of currently available tools, will
 // always be accurate; the chosen version, and the flag indicating development
 // mode, may remain blank until uploadTools or validate is called.
-func (c *upgradeJujuCommand) initVersions(client upgradeJujuAPI, cfg *config.Config, agentVersion version.Number, filterOnPrior bool) (*upgradeContext, error) {
+func (c *upgradeJujuCommand) initVersions(
+	client upgradeJujuAPI, cfg *config.Config, agentVersion version.Number, filterOnPrior bool,
+) (*upgradeContext, bool, error) {
 	if c.Version == agentVersion {
-		return nil, errUpToDate
+		return nil, false, errUpToDate
 	}
 	filterVersion := jujuversion.Current
 	if c.Version != version.Zero {
@@ -423,23 +435,27 @@ func (c *upgradeJujuCommand) initVersions(client upgradeJujuAPI, cfg *config.Con
 		// the current client version.
 		filterVersion.Major--
 	}
+	tryImplicitUpload, err := tryImplicitUpload(agentVersion)
+	if err != nil {
+		return nil, false, err
+	}
 	logger.Debugf("searching for agent binaries with major: %d", filterVersion.Major)
 	findResult, err := client.FindTools(filterVersion.Major, -1, "", "")
 	if err != nil {
-		return nil, err
+		return nil, false, err
 	}
 	err = findResult.Error
 	if findResult.Error != nil {
 		if !params.IsCodeNotFound(err) {
-			return nil, err
+			return nil, false, err
 		}
-		if !tryImplicitUpload(agentVersion) && !c.BuildAgent {
+		if !tryImplicitUpload && !c.BuildAgent {
 			// No tools found and we shouldn't upload any, so if we are not asking for a
 			// major upgrade, pretend there is no more recent version available.
 			if c.Version == version.Zero && agentVersion.Major == filterVersion.Major {
-				return nil, errUpToDate
+				return nil, false, errUpToDate
 			}
-			return nil, err
+			return nil, tryImplicitUpload, err
 		}
 	}
 	return &upgradeContext{
@@ -449,7 +465,7 @@ func (c *upgradeJujuCommand) initVersions(client upgradeJujuAPI, cfg *config.Con
 		tools:     findResult.List,
 		apiClient: client,
 		config:    cfg,
-	}, nil
+	}, tryImplicitUpload, nil
 }
 
 // upgradeContext holds the version information for making upgrade decisions.
@@ -498,7 +514,11 @@ func (context *upgradeContext) uploadTools(buildAgent bool) (err error) {
 	defer os.RemoveAll(builtTools.Dir)
 
 	uploadToolsVersion := builtTools.Version
-	uploadToolsVersion.Number = context.chosen
+	if builtTools.Official {
+		context.chosen = builtTools.Version.Number
+	} else {
+		uploadToolsVersion.Number = context.chosen
+	}
 	toolsPath := path.Join(builtTools.Dir, builtTools.StorageName)
 	logger.Infof("uploading agent binary %v (%dkB) to Juju controller", uploadToolsVersion, (builtTools.Size+512)/1024)
 	f, err := os.Open(toolsPath)

diff --git a/environs/bootstrap/bootstrap.go b/environs/bootstrap/bootstrap.go
@@ -338,7 +338,10 @@ func Bootstrap(ctx environs.BootstrapContext, environ environs.Environ, args Boo
 			version := builtTools.Version
 			version.Series = tool.Version.Series
 			version.Arch = tool.Version.Arch
-
+			// But if not an official build, use the forced version.
+			if !builtTools.Official {
+				version.Number = forceVersion
+			}
 			tool.Version = version
 			availableTools[i] = tool
 		}

diff --git a/environs/bootstrap/bootstrap_test.go b/environs/bootstrap/bootstrap_test.go
@@ -528,7 +528,8 @@ func (s *bootstrapSuite) TestBootstrapBuildAgent(c *gc.C) {
 			// If we found an official build we suppress the build number.
 			localVer.Build = 0
 			return &sync.BuiltAgent{
-				Dir: c.MkDir(),
+				Dir:      c.MkDir(),
+				Official: true,
 				Version: version.Binary{
 					Number: localVer,
 					Series: "quental",

diff --git a/environs/sync/sync.go b/environs/sync/sync.go
@@ -281,6 +281,7 @@ func cloneToolsForSeries(toolsInfo *BuiltAgent, stream string, series ...string)
 // a call to BundleTools.
 type BuiltAgent struct {
 	Version     version.Binary
+	Official    bool
 	Dir         string
 	StorageName string
 	Sha256Hash  string
@@ -319,7 +320,7 @@ func buildAgentTarball(build bool, forceVersion *version.Number, stream string)
 	clientVersion := jujuversion.Current
 	clientVersion.Build = 0
 	if builtVersion.Number.Compare(clientVersion) != 0 {
-		return nil, errors.Errorf("agent binary %v not compatibile with bootstrap client %v", toolsVersion.Number, jujuversion.Current)
+		return nil, errors.Errorf("agent binary %v not compatible with bootstrap client %v", toolsVersion.Number, jujuversion.Current)
 	}
 	fileInfo, err := f.Stat()
 	if err != nil {
@@ -330,7 +331,11 @@ func buildAgentTarball(build bool, forceVersion *version.Number, stream string)
 	if !official && forceVersion != nil {
 		reportedVersion.Number = *forceVersion
 	}
-	logger.Infof("using agent binary %v aliased to %v (%dkB)", toolsVersion, reportedVersion, (size+512)/1024)
+	if official {
+		logger.Infof("using official agent binary %v (%dkB)", toolsVersion, (size+512)/1024)
+	} else {
+		logger.Infof("using agent binary %v aliased to %v (%dkB)", toolsVersion, reportedVersion, (size+512)/1024)
+	}
 	baseToolsDir, err := ioutil.TempDir("", "juju-tools")
 	if err != nil {
 		return nil, err
@@ -353,8 +358,8 @@ func buildAgentTarball(build bool, forceVersion *version.Number, stream string)
 		return nil, err
 	}
 	return &BuiltAgent{
-		Version: reportedVersion,
-		// Version:     toolsVersion,
+		Version:     toolsVersion,
+		Official:    official,
 		Dir:         baseToolsDir,
 		StorageName: storageName,
 		Size:        size,

diff --git a/environs/tools/build.go b/environs/tools/build.go
@@ -165,20 +165,31 @@ func copyFileWithMode(from, to string, mode os.FileMode) error {
 	return nil
 }
 
-func copyExistingJujud(dir string) error {
-	// Assume that the user is running juju.
+// ExistingJujudLocation returns the directory to
+// a jujud executable in the path.
+func ExistingJujudLocation() (string, error) {
 	jujuLocation, err := findExecutable(os.Args[0])
 	if err != nil {
 		logger.Infof("%v", err)
-		return err
+		return "", err
 	}
 	jujuDir := filepath.Dir(jujuLocation)
+	return jujuDir, nil
+}
+
+func copyExistingJujud(dir string) error {
+	// Assume that the user is running juju.
+	jujuDir, err := ExistingJujudLocation()
+	if err != nil {
+		logger.Infof("couldn't find existing jujud: %v", err)
+		return errors.Trace(err)
+	}
 	jujudLocation := filepath.Join(jujuDir, names.Jujud)
 	logger.Debugf("checking: %s", jujudLocation)
 	info, err := os.Stat(jujudLocation)
 	if err != nil {
-		logger.Infof("couldn't find existing jujud")
-		return err
+		logger.Infof("couldn't find existing jujud: %v", err)
+		return errors.Trace(err)
 	}
 	logger.Infof("Found agent binary to upload (%s)", jujudLocation)
 	// TODO(thumper): break this out into a util function.
@@ -242,7 +253,7 @@ var BundleTools BundleToolsFunc = bundleTools
 // format to the given writer.  If forceVersion is not nil and the
 // file isn't an official build, a FORCE-VERSION file is included in
 // the tools bundle so it will lie about its current version number.
-func bundleTools(build bool, w io.Writer, forceVersion *version.Number) (version.Binary, bool, string, error) {
+func bundleTools(build bool, w io.Writer, forceVersion *version.Number) (_ version.Binary, official bool, sha256hash string, _ error) {
 	dir, err := ioutil.TempDir("", "juju-tools")
 	if err != nil {
 		return version.Binary{}, false, "", err
@@ -252,21 +263,10 @@ func bundleTools(build bool, w io.Writer, forceVersion *version.Number) (version
 		return version.Binary{}, false, "", err
 	}
 
-	official := true
-	tvers, err := getVersionFromFile(dir)
-	if errors.IsNotFound(err) {
-		// Extract the version number that the jujud binary was built with.
-		// This is used to check compatibility with the version of the client
-		// being used to bootstrap.
-		official = false
-		tvers, err = getVersionFromJujud(dir)
-		if err != nil {
-			return version.Binary{}, false, "", errors.Trace(err)
-		}
-	} else if err != nil {
+	tvers, official, err := JujudVersion(dir)
+	if err != nil {
 		return version.Binary{}, false, "", errors.Trace(err)
 	}
-
 	if official {
 		logger.Debugf("using official version %s", tvers)
 	} else if forceVersion != nil {
@@ -276,7 +276,7 @@ func bundleTools(build bool, w io.Writer, forceVersion *version.Number) (version
 		}
 	}
 
-	sha256hash, err := archiveAndSHA256(w, dir)
+	sha256hash, err = archiveAndSHA256(w, dir)
 	if err != nil {
 		return version.Binary{}, false, "", err
 	}
@@ -303,6 +303,41 @@ func getVersionFromJujud(dir string) (version.Binary, error) {
 	return tvers, nil
 }
 
+// JujudVersion returns the Jujud version at the specified location,
+// and whether it is an official binary.
+func JujudVersion(dir string) (version.Binary, bool, error) {
+	tvers, err := getVersionFromFile(dir)
+	official := err == nil
+	if err != nil && !errors.IsNotFound(err) && !isNoMatchingToolsChecksum(err) {
+		return version.Binary{}, false, errors.Trace(err)
+	}
+	if errors.IsNotFound(err) || isNoMatchingToolsChecksum(err) {
+		// No signature file found.
+		// Extract the version number that the jujud binary was built with.
+		// This is used to check compatibility with the version of the client
+		// being used to bootstrap.
+		tvers, err = getVersionFromJujud(dir)
+		if err != nil {
+			return version.Binary{}, false, errors.Trace(err)
+		}
+	}
+	return tvers, official, nil
+}
+
+type noMatchingToolsChecksum struct {
+	versionPath string
+	jujudPath   string
+}
+
+func (e *noMatchingToolsChecksum) Error() string {
+	return fmt.Sprintf("no SHA256 in version file %q matches binary %q", e.versionPath, e.jujudPath)
+}
+
+func isNoMatchingToolsChecksum(err error) bool {
+	_, ok := err.(*noMatchingToolsChecksum)
+	return ok
+}
+
 func getVersionFromFile(dir string) (version.Binary, error) {
 	versionPath := filepath.Join(dir, names.JujudVersions)
 	sigFile, err := os.Open(versionPath)
@@ -330,7 +365,7 @@ func getVersionFromFile(dir string) (version.Binary, error) {
 		return version.Binary{}, errors.Trace(err)
 	}
 	if len(matching) == 0 {
-		return version.Binary{}, errors.Errorf("no SHA256 in version file %q matches binary %q", versionPath, jujudPath)
+		return version.Binary{}, &noMatchingToolsChecksum{versionPath, jujudPath}
 	}
 	return selectBinary(matching)
 }