Merge pull request #16457 from hpidcock/merge-2.9-3.1-20231018

#16457

Forward ports:
- #16426
- #16451
- #16456

Conflicts:
- apiserver/facades/client/modelupgrader/upgrader_test.go
- core/base/supportedseries_linux_test.go
- core/base/supportedseries_test.go
- environs/bootstrap/bootstrap_test.go
- go.mod
- go.sum
- provider/openstack/local_test.go
- upgrades/upgradevalidation/migrate_test.go
- upgrades/upgradevalidation/upgrade_test.go
- upgrades/upgradevalidation/validation_test.go

CONTRIBUTING.md

@@ -50,12 +50,12 @@ that Go versions are not incremented during a release cycle. This means that
 branch may lag by one version or so.  Check the `go.mod` file at the root of
 the project for the targeted version of Go, as this is authoritative.
 
-For example, the following indicates that Go 1.20 is targeted:
+For example, the following indicates that Go 1.21 is targeted:
 
 ```
 module github.com/juju/juju
 
-go 1.20
+go 1.21
 ```
 
 ### Official distribution
@@ -66,7 +66,7 @@ Go can be [installed](https://golang.org/doc/install#install) from the official
 
 [Snap](https://snapcraft.io/go) may also be used to install Go on Linux.
 
-    snap install go --channel=1.19/stable --classic
+    snap install go --channel=1.21/stable --classic
 
 ## Build Juju and its dependencies
 

apiserver/facades/agent/provisioner/imagemetadata_test.go

@@ -4,6 +4,9 @@
 package provisioner_test
 
 import (
+	"net/http"
+	"net/http/httptest"
+
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
 
@@ -35,9 +38,16 @@ var _ = gc.Suite(&ImageMetadataSuite{})
 func (s *ImageMetadataSuite) SetUpSuite(c *gc.C) {
 	s.provisionerSuite.SetUpSuite(c)
 
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(404)
+	}))
+	s.AddCleanup(func(c *gc.C) {
+		server.Close()
+	})
+
 	// Make sure that there is nothing in data sources.
 	// Each individual tests will decide if it needs metadata there.
-	imagetesting.PatchOfficialDataSources(&s.CleanupSuite, "test:/daily")
+	imagetesting.PatchOfficialDataSources(&s.CleanupSuite, server.URL)
 	s.PatchValue(&imagemetadata.SimplestreamsImagesPublicKey, sstesting.SignedMetadataPublicKey)
 	s.PatchValue(&keys.JujuPublicKey, sstesting.SignedMetadataPublicKey)
 	useTestImageData(c, nil)
@@ -151,6 +161,7 @@ func (s *ImageMetadataSuite) assertImageMetadataResults(
 ) {
 	c.Assert(obtained.Results, gc.HasLen, len(expected))
 	for i, one := range obtained.Results {
+		c.Assert(one.Error, gc.IsNil)
 		// We are only concerned with images here
 		c.Assert(one.Result.ImageMetadata, gc.DeepEquals, expected[i])
 	}

apiserver/facades/agent/provisioner/provisioninginfo.go

@@ -721,11 +721,15 @@ func (api *ProvisionerAPI) imageMetadataFromDataSources(env environs.Environ, co
 	for _, source := range sources {
 		logger.Debugf("looking in data source %v", source.Description())
 		found, info, err := imagemetadata.Fetch(fetcher, []simplestreams.DataSource{source}, constraint)
-		if err != nil {
+		if errors.Is(err, errors.NotFound) || errors.Is(err, errors.Unauthorized) {
 			// Do not stop looking in other data sources if there is an issue here.
 			logger.Warningf("encountered %v while getting published images metadata from %v", err, source.Description())
 			continue
+		} else if err != nil {
+			// When we get an actual protocol/unexpected error, we need to stop.
+			return nil, errors.Annotatef(err, "failed getting published images metadata from %s", source.Description())
 		}
+
 		for _, m := range found {
 			metadataState = append(metadataState, toModel(m, info.Source, source.Priority()))
 		}

cmd/juju/commands/main_test.go

@@ -235,6 +235,11 @@ func (s *MainSuite) TestNoWarn2xFirstRun(c *gc.C) {
 	path := c.MkDir()
 	s.PatchEnvironment("JUJU_HOME", path)
 
+	s.PatchValue(&cloud.FetchAndMaybeUpdatePublicClouds,
+		func(access cloud.PublicCloudsAccessDetails, updateClient bool) (map[string]jujucloud.Cloud, string, error) {
+			return nil, "", nil
+		})
+
 	var code int
 	stdout, stderr := jujutesting.CaptureOutput(c, func() {
 		code = jujuMain{

core/base/supportedseries_test.go

@@ -267,16 +267,20 @@ func (s *SupportedSeriesSuite) TestUbuntuVersions(c *gc.C) {
 			WorkloadType: ControllerWorkloadType,
 			Version:      "23.04",
 		},
+		Mantic: {
+			WorkloadType: ControllerWorkloadType,
+			Version:      "23.10",
+		},
 	}
 
 	result := ubuntuVersions(nil, nil, ubuntuSeries)
-	c.Check(result, gc.DeepEquals, map[string]string{"artful": "17.10", "bionic": "18.04", "cosmic": "18.10", "disco": "19.04", "eoan": "19.10", "focal": "20.04", "groovy": "20.10", "hirsute": "21.04", "impish": "21.10", "jammy": "22.04", "kinetic": "22.10", "lunar": "23.04", "precise": "12.04", "quantal": "12.10", "raring": "13.04", "saucy": "13.10", "trusty": "14.04", "utopic": "14.10", "vivid": "15.04", "wily": "15.10", "xenial": "16.04", "yakkety": "16.10", "zesty": "17.04"})
+	c.Check(result, gc.DeepEquals, map[string]string{"artful": "17.10", "bionic": "18.04", "cosmic": "18.10", "disco": "19.04", "eoan": "19.10", "focal": "20.04", "groovy": "20.10", "hirsute": "21.04", "impish": "21.10", "jammy": "22.04", "kinetic": "22.10", "lunar": "23.04", "mantic": "23.10", "precise": "12.04", "quantal": "12.10", "raring": "13.04", "saucy": "13.10", "trusty": "14.04", "utopic": "14.10", "vivid": "15.04", "wily": "15.10", "xenial": "16.04", "yakkety": "16.10", "zesty": "17.04"})
 
 	result = ubuntuVersions(boolPtr(true), boolPtr(true), ubuntuSeries)
 	c.Check(result, gc.DeepEquals, map[string]string{"focal": "20.04", "jammy": "22.04"})
 
 	result = ubuntuVersions(boolPtr(false), boolPtr(false), ubuntuSeries)
-	c.Check(result, gc.DeepEquals, map[string]string{"artful": "17.10", "cosmic": "18.10", "disco": "19.04", "eoan": "19.10", "groovy": "20.10", "hirsute": "21.04", "impish": "21.10", "kinetic": "22.10", "lunar": "23.04", "precise": "12.04", "quantal": "12.10", "raring": "13.04", "saucy": "13.10", "utopic": "14.10", "vivid": "15.04", "wily": "15.10", "yakkety": "16.10", "zesty": "17.04"})
+	c.Check(result, gc.DeepEquals, map[string]string{"artful": "17.10", "cosmic": "18.10", "disco": "19.04", "eoan": "19.10", "groovy": "20.10", "hirsute": "21.04", "impish": "21.10", "kinetic": "22.10", "lunar": "23.04", "mantic": "23.10", "precise": "12.04", "quantal": "12.10", "raring": "13.04", "saucy": "13.10", "utopic": "14.10", "vivid": "15.04", "wily": "15.10", "yakkety": "16.10", "zesty": "17.04"})
 
 	result = ubuntuVersions(boolPtr(true), boolPtr(false), ubuntuSeries)
 	c.Check(result, gc.DeepEquals, map[string]string{})
@@ -288,13 +292,13 @@ func (s *SupportedSeriesSuite) TestUbuntuVersions(c *gc.C) {
 	c.Check(result, gc.DeepEquals, map[string]string{"focal": "20.04", "jammy": "22.04"})
 
 	result = ubuntuVersions(boolPtr(false), nil, ubuntuSeries)
-	c.Check(result, gc.DeepEquals, map[string]string{"artful": "17.10", "bionic": "18.04", "cosmic": "18.10", "disco": "19.04", "eoan": "19.10", "groovy": "20.10", "hirsute": "21.04", "impish": "21.10", "kinetic": "22.10", "lunar": "23.04", "precise": "12.04", "quantal": "12.10", "raring": "13.04", "saucy": "13.10", "trusty": "14.04", "utopic": "14.10", "vivid": "15.04", "wily": "15.10", "xenial": "16.04", "yakkety": "16.10", "zesty": "17.04"})
+	c.Check(result, gc.DeepEquals, map[string]string{"artful": "17.10", "bionic": "18.04", "cosmic": "18.10", "disco": "19.04", "eoan": "19.10", "groovy": "20.10", "hirsute": "21.04", "impish": "21.10", "kinetic": "22.10", "lunar": "23.04", "mantic": "23.10", "precise": "12.04", "quantal": "12.10", "raring": "13.04", "saucy": "13.10", "trusty": "14.04", "utopic": "14.10", "vivid": "15.04", "wily": "15.10", "xenial": "16.04", "yakkety": "16.10", "zesty": "17.04"})
 
 	result = ubuntuVersions(nil, boolPtr(true), ubuntuSeries)
 	c.Check(result, gc.DeepEquals, map[string]string{"bionic": "18.04", "focal": "20.04", "jammy": "22.04", "trusty": "14.04", "xenial": "16.04"})
 
 	result = ubuntuVersions(nil, boolPtr(false), ubuntuSeries)
-	c.Check(result, gc.DeepEquals, map[string]string{"artful": "17.10", "cosmic": "18.10", "disco": "19.04", "eoan": "19.10", "groovy": "20.10", "hirsute": "21.04", "impish": "21.10", "kinetic": "22.10", "lunar": "23.04", "precise": "12.04", "quantal": "12.10", "raring": "13.04", "saucy": "13.10", "utopic": "14.10", "vivid": "15.04", "wily": "15.10", "yakkety": "16.10", "zesty": "17.04"})
+	c.Check(result, gc.DeepEquals, map[string]string{"artful": "17.10", "cosmic": "18.10", "disco": "19.04", "eoan": "19.10", "groovy": "20.10", "hirsute": "21.04", "impish": "21.10", "kinetic": "22.10", "lunar": "23.04", "mantic": "23.10", "precise": "12.04", "quantal": "12.10", "raring": "13.04", "saucy": "13.10", "utopic": "14.10", "vivid": "15.04", "wily": "15.10", "yakkety": "16.10", "zesty": "17.04"})
 }
 
 func makeTempFile(c *gc.C, content string) (*os.File, func()) {

environs/bootstrap/bootstrap.go

@@ -987,10 +987,13 @@ func bootstrapImageMetadata(
 	var publicImageMetadata []*imagemetadata.ImageMetadata
 	for _, source := range sources {
 		sourceMetadata, _, err := imagemetadata.Fetch(fetcher, []simplestreams.DataSource{source}, imageConstraint)
-		if err != nil {
+		if errors.Is(err, errors.NotFound) || errors.Is(err, errors.Unauthorized) {
 			logger.Debugf("ignoring image metadata in %s: %v", source.Description(), err)
 			// Just keep looking...
 			continue
+		} else if err != nil {
+			// When we get an actual protocol/unexpected error, we need to stop.
+			return nil, errors.Annotatef(err, "failed looking for image metadata in %s", source.Description())
 		}
 		logger.Debugf("found %d image metadata in %s", len(sourceMetadata), source.Description())
 		publicImageMetadata = append(publicImageMetadata, sourceMetadata...)
@@ -1141,7 +1144,7 @@ func setPrivateMetadataSources(fetcher imagemetadata.SimplestreamsFetcher, metad
 	}
 	existingMetadata, _, err := imagemetadata.Fetch(fetcher, []simplestreams.DataSource{dataSource}, imageConstraint)
 	if err != nil && !errors.IsNotFound(err) {
-		return nil, errors.Annotate(err, "cannot read image metadata")
+		return nil, errors.Annotatef(err, "cannot read image metadata in %s", dataSource.Description())
 	}
 
 	// Add an image metadata datasource for constraint validation, etc.

environs/bootstrap/bootstrap_test.go

@@ -6,6 +6,8 @@ package bootstrap_test
 import (
 	"context"
 	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"strings"
@@ -572,6 +574,12 @@ func (s *bootstrapSuite) setupProviderWithNoSupportedArches(c *gc.C) bootstrapEn
 // despite image metadata in other data sources compatible with the same configuration as well.
 // Related to bug#1560625.
 func (s *bootstrapSuite) TestBootstrapImageMetadataFromAllSources(c *gc.C) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(404)
+	}))
+	defer server.Close()
+
+	s.PatchValue(&imagemetadata.DefaultUbuntuBaseURL, server.URL)
 	s.PatchValue(&osseries.HostSeries, func() (string, error) { return "raring", nil })
 	s.PatchValue(&arch.HostArch, func() string { return arch.AMD64 })
 

environs/simplestreams/datasource.go

@@ -10,9 +10,12 @@ import (
 	"net/http"
 	"net/url"
 	"strings"
+	"time"
 
+	"github.com/juju/clock"
 	"github.com/juju/errors"
 	jujuhttp "github.com/juju/http/v2"
+	"github.com/juju/retry"
 	"github.com/juju/utils/v3"
 
 	corelogger "github.com/juju/juju/core/logger"
@@ -36,10 +39,6 @@ type DataSource interface {
 	// PublicSigningKey returns the public key used to validate signed metadata.
 	PublicSigningKey() string
 
-	// SetAllowRetry sets the flag which determines if the datasource will retry fetching the metadata
-	// if it is not immediately available.
-	SetAllowRetry(allow bool)
-
 	// Priority is an importance factor for Data Source. Higher number means higher priority.
 	// This is will allow to sort data sources in order of importance.
 	Priority() int
@@ -78,6 +77,7 @@ type urlDataSource struct {
 	priority         int
 	requireSigned    bool
 	httpClient       *jujuhttp.Client
+	clock            clock.Clock
 }
 
 // Config has values to be used in constructing a datasource.
@@ -108,6 +108,9 @@ type Config struct {
 	// of cloud infrastructure components
 	// The contents are Base64 encoded x.509 certs.
 	CACertificates []string
+
+	// Clock is used for retry. Will use clock.WallClock if nil.
+	Clock clock.Clock
 }
 
 // Validate checks that the baseURL is valid and the description is set.
@@ -139,13 +142,18 @@ func NewDataSource(cfg Config) DataSource {
 // NewDataSourceWithClient returns a new DataSource as defines by the given
 // Config, but with the addition of a http.Client.
 func NewDataSourceWithClient(cfg Config, client *jujuhttp.Client) DataSource {
+	clk := cfg.Clock
+	if clk == nil {
+		clk = clock.WallClock
+	}
 	return &urlDataSource{
 		description:      cfg.Description,
 		baseURL:          cfg.BaseURL,
 		publicSigningKey: cfg.PublicSigningKey,
 		priority:         cfg.Priority,
 		requireSigned:    cfg.RequireSigned,
 		httpClient:       client,
+		clock:            clk,
 	}
 }
 
@@ -171,31 +179,50 @@ func urlJoin(baseURL, relpath string) string {
 
 // Fetch is defined in simplestreams.DataSource.
 func (h *urlDataSource) Fetch(path string) (io.ReadCloser, string, error) {
+	var readCloser io.ReadCloser
 	dataURL := urlJoin(h.baseURL, path)
 	// dataURL can be http:// or file://
 	// MakeFileURL will only modify the URL if it's a file URL
 	dataURL = utils.MakeFileURL(dataURL)
-	resp, err := h.httpClient.Get(context.TODO(), dataURL)
+
+	err := retry.Call(retry.CallArgs{
+		Func: func() error {
+			var err error
+			readCloser, err = h.fetch(dataURL)
+			return err
+		},
+		IsFatalError: func(err error) bool {
+			return errors.Is(err, errors.NotFound) || errors.Is(err, errors.Unauthorized)
+		},
+		Attempts:    3,
+		Delay:       time.Second,
+		MaxDelay:    time.Second * 5,
+		BackoffFunc: retry.DoubleDelay,
+		Clock:       h.clock,
+	})
+	return readCloser, dataURL, err
+}
+
+func (h *urlDataSource) fetch(path string) (io.ReadCloser, error) {
+	resp, err := h.httpClient.Get(context.TODO(), path)
 	if err != nil {
-		if !errors.IsNotFound(err) {
-			// Callers of this mask the actual error.  Therefore warn here.
-			// This is called multiple times when a machine is created, we
-			// only need one success for images and one for tools.
-			logger.Warningf("Got error requesting %q: %v", dataURL, err)
-		}
-		return nil, dataURL, errors.NewNotFound(err, fmt.Sprintf("%q", dataURL))
+		// Callers of this mask the actual error.  Therefore warn here.
+		// This is called multiple times when a machine is created, we
+		// only need one success for images and one for tools.
+		logger.Warningf("Got error requesting %q: %v", path, err)
+		return nil, fmt.Errorf("cannot access URL %q: %w", path, err)
 	}
 	if resp.StatusCode != http.StatusOK {
 		_ = resp.Body.Close()
 		switch resp.StatusCode {
 		case http.StatusNotFound:
-			return nil, dataURL, errors.NotFoundf("%q", dataURL)
+			return nil, errors.NotFoundf("%q", path)
 		case http.StatusUnauthorized:
-			return nil, dataURL, errors.Unauthorizedf("unauthorised access to URL %q", dataURL)
+			return nil, errors.Unauthorizedf("unauthorised access to URL %q", path)
 		}
-		return nil, dataURL, fmt.Errorf("cannot access URL %q, %q", dataURL, resp.Status)
+		return nil, fmt.Errorf("cannot access URL %q, %q", path, resp.Status)
 	}
-	return resp.Body, dataURL, nil
+	return resp.Body, nil
 }
 
 // URL is defined in simplestreams.DataSource.
@@ -208,11 +235,6 @@ func (u *urlDataSource) PublicSigningKey() string {
 	return u.publicSigningKey
 }
 
-// SetAllowRetry is defined in simplestreams.DataSource.
-func (h *urlDataSource) SetAllowRetry(allow bool) {
-	// This is a NOOP for url datasources.
-}
-
 // Priority is defined in simplestreams.DataSource.
 func (h *urlDataSource) Priority() int {
 	return h.priority