Fix agent lookup on upgraded 2.8 controllers

sdg0919 · Mar 25, 2021 · 1ab5629 · 1ab5629
1 parent 10b55c3
commit 1ab5629
Show file tree

Hide file tree

Showing 8 changed files with 191 additions and 40 deletions.
diff --git a/acceptancetests/assess_upgrade.py b/acceptancetests/assess_upgrade.py
@@ -72,7 +72,7 @@ def assess_upgrade_from_stable_to_develop(args, stable_bsm, devel_client):
             stable_client, base_dir, 'released')
         setup_agent_metadata(
             stream_server, args.devel_juju_agent,
-            devel_client, base_dir, 'released')
+            devel_client, base_dir, 'develop')
         with stream_server.server() as url:
             stable_client.env.update_config({
                 'agent-metadata-url': url,
@@ -177,12 +177,6 @@ def setup_agent_metadata(
         version_parts.version,
         version_parts.arch,
         agent_details)
-    # Trusty needed for wikimedia charm.
-    stream_server.add_product(
-        stream,
-        version_parts.version,
-        version_parts.arch,
-        agent_details)
 
 
 def get_version_parts(version_string):

diff --git a/acceptancetests/jujupy/stream_server.py b/acceptancetests/jujupy/stream_server.py
@@ -208,7 +208,7 @@ def agent_tgz_from_juju_binary(
     return tgz_path
 
 
-def _generate_product_json(content_id, version, arch, series, agent_tgz_path):
+def _generate_product_json(content_id, version, arch, agent_tgz_path):
     """Return dict containing product metadata from provided args."""
     tgz_name = os.path.basename(agent_tgz_path)
     file_details = _get_tgz_file_details(agent_tgz_path)
@@ -233,21 +233,6 @@ def _generate_product_json(content_id, version, arch, series, agent_tgz_path):
     )
 
 
-def _get_series_details(series):
-    # Ubuntu agents use series and a code (i.e. trusty:14.04), others don't.
-    _series_lookup = dict(
-        trusty=14.04,
-        xenial=16.04,
-        artful=17.10,
-        bionic=18.04,
-    )
-    try:
-        series_code = _series_lookup[series]
-    except KeyError:
-        return series, series
-    return series, series_code
-
-
 def _get_tgz_file_details(agent_tgz_path):
     file_details = dict(size=os.path.getsize(agent_tgz_path))
     with open(agent_tgz_path, 'rb') as f:

diff --git a/apiserver/common/tools.go b/apiserver/common/tools.go
@@ -6,6 +6,7 @@ package common
 import (
 	"fmt"
 	"sort"
+	"strings"
 
 	"github.com/juju/errors"
 	"github.com/juju/names/v4"
@@ -14,6 +15,7 @@ import (
 	apiservererrors "github.com/juju/juju/apiserver/errors"
 	"github.com/juju/juju/apiserver/params"
 	"github.com/juju/juju/core/network"
+	coreos "github.com/juju/juju/core/os"
 	coreseries "github.com/juju/juju/core/series"
 	"github.com/juju/juju/environs"
 	envtools "github.com/juju/juju/environs/tools"
@@ -277,13 +279,51 @@ func (f *ToolsFinder) findTools(args params.FindToolsParams) (coretools.List, er
 // and is found in tools storage, then simplestreams will not be searched.
 func (f *ToolsFinder) findMatchingTools(args params.FindToolsParams) (coretools.List, error) {
 	exactMatch := args.Number != version.Zero && (args.OSType != "" || args.Series != "") && args.Arch != ""
+
+	// TODO(juju4) - remove this logic
+	// Older versions of Juju publish agent binary metadata based on series.
+	// So we need to strip out OSType and match on everything else, then filter below.
+	compatibleMatch := false
+	wantedOSType := args.OSType
+	if args.Number.Major == 2 && args.Number.Minor <= 8 && args.OSType != "" {
+		args.OSType = ""
+		compatibleMatch = true
+	}
+
 	storageList, err := f.matchingStorageTools(args)
-	if err == nil && exactMatch {
-		return storageList, nil
-	} else if err != nil && err != coretools.ErrNoMatches {
+	if err != nil && err != coretools.ErrNoMatches {
 		return nil, err
 	}
 
+	// For a given list of tools, return those which match the required
+	// os type based on an exact os type match or series match.
+	compatibleTools := func(tools coretools.List) coretools.List {
+		added := make(map[version.Binary]bool)
+		var matched coretools.List
+		for _, t := range tools {
+			converted := *t
+			osTypeName, _ := coreseries.GetOSFromSeries(t.Version.Release)
+			if osTypeName != coreos.Unknown {
+				converted.Version.Release = strings.ToLower(osTypeName.String())
+			}
+			if added[converted.Version] {
+				continue
+			}
+			if converted.Version.Release == wantedOSType || wantedOSType == "" {
+				matched = append(matched, &converted)
+				added[converted.Version] = true
+			}
+		}
+		return matched
+	}
+
+	if compatibleMatch {
+		storageList = compatibleTools(storageList)
+	}
+	if len(storageList) > 0 && exactMatch {
+		return storageList, nil
+	}
+
 	// Look for tools in simplestreams too, but don't replace
 	// any versions found in storage.
 	env, err := f.newEnviron()
@@ -303,6 +343,9 @@ func (f *ToolsFinder) findMatchingTools(args params.FindToolsParams) (coretools.
 	if len(storageList) == 0 && err != nil {
 		return nil, err
 	}
+	if compatibleMatch {
+		simplestreamsList = compatibleTools(simplestreamsList)
+	}
 
 	list := storageList
 	found := make(map[version.Binary]bool)

diff --git a/apiserver/common/tools_test.go b/apiserver/common/tools_test.go
@@ -261,6 +261,66 @@ func (s *toolsSuite) TestFindTools(c *gc.C) {
 	}
 }
 
+// TODO(juju4) - remove
+func (s *toolsSuite) TestFindToolsOldAgent(c *gc.C) {
+	for i, test := range []struct {
+		agentStreamRequested string
+		agentStreamsUsed     []string
+	}{{
+		agentStreamsUsed: []string{"released"},
+	}, {
+		agentStreamRequested: "pretend",
+		agentStreamsUsed:     []string{"pretend"},
+	}} {
+		c.Logf("test %d", i)
+		envtoolsList := coretools.List{
+			&coretools.Tools{
+				Version: version.MustParseBinary("2.8.9-focal-amd64"),
+				Size:    2048,
+				SHA256:  "badf00d",
+			},
+		}
+		storageMetadata := []binarystorage.Metadata{{
+			Version: "2.8.9-win2012-amd64",
+			Size:    1024,
+			SHA256:  "feedface",
+		}}
+
+		s.PatchValue(common.EnvtoolsFindTools, func(e environs.BootstrapEnviron, major, minor int, streams []string, filter coretools.Filter) (coretools.List, error) {
+			c.Assert(major, gc.Equals, 2)
+			c.Assert(minor, gc.Equals, 8)
+			c.Assert(streams, gc.DeepEquals, test.agentStreamsUsed)
+			c.Assert(filter.OSType, gc.Equals, "")
+			c.Assert(filter.Arch, gc.Equals, "amd64")
+			return envtoolsList, nil
+		})
+		newEnviron := func() (environs.BootstrapEnviron, error) {
+			return s.Environ, nil
+		}
+		toolsFinder := common.NewToolsFinder(
+			stateenvirons.EnvironConfigGetter{Model: s.Model}, &mockToolsStorage{metadata: storageMetadata}, sprintfURLGetter("tools:%s"), newEnviron,
+		)
+		result, err := toolsFinder.FindTools(params.FindToolsParams{
+			Number: version.MustParse("2.8.9"),
+			MajorVersion: 2,
+			MinorVersion: 8,
+			OSType:       "ubuntu",
+			Arch:         "amd64",
+			AgentStream:  test.agentStreamRequested,
+		})
+		c.Assert(err, jc.ErrorIsNil)
+		c.Assert(result.Error, gc.IsNil)
+		c.Check(result.List, jc.DeepEquals, coretools.List{
+			&coretools.Tools{
+				Version: version.MustParseBinary("2.8.9-ubuntu-amd64"),
+				Size:    2048,
+				SHA256:  "badf00d",
+				URL:     "tools:2.8.9-ubuntu-amd64",
+			},
+		})
+	}
+}
+
 func (s *toolsSuite) TestFindToolsNotFound(c *gc.C) {
 	s.PatchValue(common.EnvtoolsFindTools, func(e environs.BootstrapEnviron, major, minor int, stream []string, filter coretools.Filter) (list coretools.List, err error) {
 		return nil, errors.NotFoundf("tools")

diff --git a/apiserver/tools.go b/apiserver/tools.go
@@ -13,6 +13,7 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
+	"strings"
 
 	"github.com/juju/errors"
 	jujuhttp "github.com/juju/http"
@@ -21,6 +22,7 @@ import (
 	"github.com/juju/juju/apiserver/common"
 	"github.com/juju/juju/apiserver/httpcontext"
 	"github.com/juju/juju/apiserver/params"
+	coreos "github.com/juju/juju/core/os"
 	coreseries "github.com/juju/juju/core/series"
 	"github.com/juju/juju/environs"
 	envtools "github.com/juju/juju/environs/tools"
@@ -133,32 +135,81 @@ func (h *toolsUploadHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 // getToolsForRequest retrieves the compressed agent binaries tarball from state
 // based on the input HTTP request.
 // It is returned with the size of the file as recorded in the stored metadata.
-func (h *toolsDownloadHandler) getToolsForRequest(r *http.Request, st *state.State) (io.ReadCloser, int64, error) {
-	version, err := version.ParseBinary(r.URL.Query().Get(":version"))
+func (h *toolsDownloadHandler) getToolsForRequest(r *http.Request, st *state.State) (_ io.ReadCloser, _ int64, err error) {
+	vers, err := version.ParseBinary(r.URL.Query().Get(":version"))
 	if err != nil {
 		return nil, 0, errors.Annotate(err, "error parsing version")
 	}
-	logger.Debugf("request for agent binaries: %s", version)
+	logger.Debugf("request for agent binaries: %s", vers)
 
 	storage, err := st.ToolsStorage()
 	if err != nil {
 		return nil, 0, errors.Annotate(err, "error getting storage for agent binaries")
 	}
+	defer func() {
+		if err != nil {
+			_ = storage.Close()
+		}
+	}()
+
+	// TODO(juju4) = remove this compatibility logic
+	// Looked for stored tools which are recorded for a series
+	// but which have the same os type as the wanted version.
+	storageVers := vers
+	var osTypeName string
+	if vers.Number.Major == 2 && vers.Number.Minor <= 8 {
+		all, err := storage.AllMetadata()
+		if err != nil {
+			return nil, 0, errors.Trace(err)
+		}
+		var osMatchVersion *version.Binary
+		for _, m := range all {
+			// Exact match so just use that with os type name substitution.
+			if m.Version == vers.String() {
+				break
+			}
+			if osMatchVersion != nil {
+				continue
+			}
+			metaVers, err := version.ParseBinary(m.Version)
+			if err != nil {
+				return nil, 0, errors.Annotate(err, "error parsing metadata version")
+			}
+			osType, _ := coreseries.GetOSFromSeries(metaVers.Release)
+			if osType == coreos.Unknown {
+				continue
+			}
+			toCompare := metaVers
+			toCompare.Release = strings.ToLower(osType.String())
+			if toCompare.String() == vers.String() {
+				logger.Debugf("using os based version %s for requested %s", toCompare, vers)
+				osMatchVersion = &metaVers
+				osTypeName = toCompare.Release
+			}
+		}
+		// Set the version to store to be the match we found
+		// for any compatible series.
+		if osMatchVersion != nil {
+			storageVers = *osMatchVersion
+		}
+	}
 
-	md, reader, err := storage.Open(version.String())
+	md, reader, err := storage.Open(storageVers.String())
 	if errors.IsNotFound(err) {
 		// Tools could not be found in tools storage,
 		// so look for them in simplestreams,
 		// fetch them and cache in tools storage.
-		logger.Infof("%v agent binaries not found locally, fetching", version)
-		md, reader, err = h.fetchAndCacheTools(version, storage, st)
+		logger.Infof("%v agent binaries not found locally, fetching", vers)
+		if osTypeName != "" {
+			storageVers.Release = osTypeName
+		}
+		md, reader, err = h.fetchAndCacheTools(vers, storageVers, storage, st)
 		if err != nil {
 			err = errors.Annotate(err, "error fetching agent binaries")
 		}
 	}
 	if err != nil {
-		storage.Close()
-		return nil, 0, err
+		return nil, 0, errors.Trace(err)
 	}
 
 	return &toolsReadCloser{f: reader, st: storage}, md.Size, nil
@@ -169,6 +220,7 @@ func (h *toolsDownloadHandler) getToolsForRequest(r *http.Request, st *state.Sta
 // to the caller.
 func (h *toolsDownloadHandler) fetchAndCacheTools(
 	v version.Binary,
+	storageVers version.Binary,
 	stor binarystorage.Storage,
 	st *state.State,
 ) (binarystorage.Metadata, io.ReadCloser, error) {
@@ -217,6 +269,7 @@ func (h *toolsDownloadHandler) fetchAndCacheTools(
 	}
 	md.SHA256 = exactTools.SHA256
 
+	md.Version = storageVers.String()
 	if err := stor.Add(bytes.NewReader(data), md); err != nil {
 		return md, nil, errors.Annotate(err, "error caching agent binaries")
 	}
@@ -257,7 +310,7 @@ func (h *toolsUploadHandler) processPost(r *http.Request, st *state.State) (*too
 	}
 
 	logger.Debugf("request to upload agent binaries: %s", toolsVersion)
-	// TODO(juju3) - drop this compatibility with series params
+	// TODO(juju4) - drop this compatibility with series params
 	// If the binary is for a workload series, convert the release to an OS type name.
 	allSeries, err := coreseries.AllWorkloadSeries("", "")
 	if err != nil {

diff --git a/apiserver/tools_test.go b/apiserver/tools_test.go
@@ -385,6 +385,24 @@ func (s *toolsSuite) TestDownloadModelUUIDPath(c *gc.C) {
 	s.testDownload(c, tools, s.State.ModelUUID())
 }
 
+// TODO(juju4) - remove
+func (s *toolsSuite) TestDownloadOldAgent(c *gc.C) {
+	tools := s.storeFakeTools(c, s.State, "abc", binarystorage.Metadata{
+		Version: "2.8.9-focal-amd64",
+		Size:    3,
+		SHA256:  "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad",
+	})
+	resp := s.downloadRequest(c, version.MustParseBinary("2.8.9-ubuntu-amd64"), s.State.ModelUUID())
+	defer resp.Body.Close()
+	data, err := ioutil.ReadAll(resp.Body)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(data, gc.HasLen, int(tools.Size))
+
+	hash := sha256.New()
+	hash.Write(data)
+	c.Assert(fmt.Sprintf("%x", hash.Sum(nil)), gc.Equals, tools.SHA256)
+}
+
 func (s *toolsSuite) TestDownloadOtherModelUUIDPath(c *gc.C) {
 	newSt := s.Factory.MakeModel(c, nil)
 	defer newSt.Close()

diff --git a/cmd/juju/commands/upgrademodel.go b/cmd/juju/commands/upgrademodel.go
@@ -898,6 +898,7 @@ func (context *upgradeContext) uploadTools(
 	// Newer 2.9+ controllers can deal with this but not older controllers.
 	// Look at the model and get all series for all machines
 	// and use those to create additional tools.
+	// TODO(juju4) - remove this logic
 	additionalSeries := set.NewStrings()
 	if controllerAgentVersion.Major == 2 && controllerAgentVersion.Minor <= 8 {
 		fullStatus, err := client.Status(nil)

diff --git a/state/migration_import.go b/state/migration_import.go
@@ -721,11 +721,8 @@ func (i *importer) makeTools(t description.AgentTools) (*tools.Tools, error) {
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	for _, ser := range allSeries.SortedValues() {
-		if result.Version.Release == ser {
-			result.Version.Release = coreseries.DefaultOSTypeNameFromSeries(ser)
-			break
-		}
+	if allSeries.Contains(result.Version.Release) {
+		result.Version.Release = coreseries.DefaultOSTypeNameFromSeries(result.Version.Release)
 	}
 	return result, nil
 }