Merge pull request #15520 from ycliuhw/merge-3.0-20230421

#15520

Merge remote-tracking branch 'upstream/3.0' into 3.1:

```
# Conflicts:
# apiserver/facades/client/application/application.go
# apiserver/facades/client/application/application_unit_test.go
# cmd/juju/application/diffbundle.go
```

apiserver/facades/client/application/application.go

@@ -568,34 +568,25 @@ func deployApplication(
 	return errors.Trace(err)
 }
 
+// convertCharmOrigin converts a params CharmOrigin to a core charm
+// Origin. If the input origin is nil, a core charm Origin is deduced
+// from the provided data. It is used in both deploying and refreshing
+// charms, including from old clients which aren't charm origin aware.
+// MaybeSeries is a fallback if the origin is not provided.
 func convertCharmOrigin(origin *params.CharmOrigin, curl *charm.URL) (corecharm.Origin, error) {
-	var (
-		originType string
-		platform   corecharm.Platform
-	)
-	if origin != nil {
-		originType = origin.Type
-		base, err := series.ParseBase(origin.Base.Name, origin.Base.Channel)
-		if err != nil {
-			return corecharm.Origin{}, errors.Trace(err)
-		}
-		platform = corecharm.Platform{
-			Architecture: origin.Architecture,
-			OS:           base.OS,
-			Channel:      base.Channel.Track,
-		}
+	if origin == nil {
+		return corecharm.Origin{}, errors.NotValidf("nil charm origin")
 	}
 
-	switch {
-	case origin.Source == "local":
-		return corecharm.Origin{
-			Type:     originType,
-			Source:   corecharm.Local,
-			Revision: &curl.Revision,
-			Platform: platform,
-		}, nil
-	case origin.Source != "charm-hub":
-		return corecharm.Origin{}, errors.NotValidf("origin source not local nor charm-hub")
+	originType := origin.Type
+	base, err := series.ParseBase(origin.Base.Name, origin.Base.Channel)
+	if err != nil {
+		return corecharm.Origin{}, errors.Trace(err)
+	}
+	platform := corecharm.Platform{
+		Architecture: origin.Architecture,
+		OS:           base.OS,
+		Channel:      base.Channel.Track,
 	}
 
 	var track string

apiserver/facades/client/application/application_unit_test.go

@@ -19,6 +19,7 @@ import (
 	jc "github.com/juju/testing/checkers"
 	"github.com/juju/utils/v3"
 	"github.com/juju/version/v2"
+	"github.com/kr/pretty"
 	gc "gopkg.in/check.v1"
 	"gopkg.in/juju/environschema.v1"
 
@@ -195,7 +196,7 @@ func (s *ApplicationSuite) setup(c *gc.C) *gomock.Controller {
 		s.model,
 		s.leadershipReader,
 		func(application.Charm) *state.Charm {
-			return &state.Charm{}
+			return nil
 		},
 		func(_ application.ApplicationDeployer, _ application.Model, p application.DeployApplicationParams) (application.Application, error) {
 			s.deployParams[p.ApplicationName] = p
@@ -343,10 +344,8 @@ func (s *ApplicationSuite) TestSetCharm(c *gc.C) {
 	s.backend.EXPECT().Charm(curl).Return(ch, nil)
 
 	app := s.expectDefaultApplication(ctrl)
-	app.EXPECT().SetCharm(state.SetCharmConfig{
-		Charm:       &state.Charm{},
-		CharmOrigin: createStateCharmOriginFromURL(curl),
-	})
+	cfg := state.SetCharmConfig{CharmOrigin: createStateCharmOriginFromURL(curl)}
+	app.EXPECT().SetCharm(setCharmConfigMatcher{c: c, expected: cfg})
 	s.backend.EXPECT().Application("postgresql").Return(app, nil)
 
 	err := s.api.SetCharm(params.ApplicationSetCharm{
@@ -366,11 +365,11 @@ func (s *ApplicationSuite) TestSetCharmEverything(c *gc.C) {
 	s.backend.EXPECT().Charm(curl).Return(ch, nil)
 
 	app := s.expectDefaultApplication(ctrl)
-	app.EXPECT().SetCharm(state.SetCharmConfig{
-		Charm:          &state.Charm{},
+	cfg := state.SetCharmConfig{
 		CharmOrigin:    createStateCharmOriginFromURL(curl),
 		ConfigSettings: charm.Settings{"stringOption": "foo", "intOption": int64(666)},
-	})
+	}
+	app.EXPECT().SetCharm(setCharmConfigMatcher{c: c, expected: cfg})
 
 	schemaFields, defaults, err := application.AddTrustSchemaAndDefaults(environschema.Fields{}, schema.Defaults{})
 	c.Assert(err, jc.ErrorIsNil)
@@ -425,11 +424,12 @@ func (s *ApplicationSuite) TestSetCharmForceUnits(c *gc.C) {
 	s.backend.EXPECT().Charm(curl).Return(ch, nil)
 
 	app := s.expectDefaultApplication(ctrl)
-	app.EXPECT().SetCharm(state.SetCharmConfig{
-		Charm:       &state.Charm{},
+
+	cfg := state.SetCharmConfig{
 		CharmOrigin: createStateCharmOriginFromURL(curl),
 		ForceUnits:  true,
-	})
+	}
+	app.EXPECT().SetCharm(setCharmConfigMatcher{c: c, expected: cfg})
 
 	s.backend.EXPECT().Application("postgresql").Return(app, nil)
 	err := s.api.SetCharm(params.ApplicationSetCharm{
@@ -471,16 +471,17 @@ func (s *ApplicationSuite) TestSetCharmStorageConstraints(c *gc.C) {
 	s.backend.EXPECT().Charm(curl).Return(ch, nil)
 
 	app := s.expectDefaultApplication(ctrl)
-	app.EXPECT().SetCharm(state.SetCharmConfig{
-		Charm:       &state.Charm{},
+	cfg := state.SetCharmConfig{
 		CharmOrigin: createStateCharmOriginFromURL(curl),
 		StorageConstraints: map[string]state.StorageConstraints{
 			"a": {},
 			"b": {Pool: "radiant"},
 			"c": {Size: 123},
 			"d": {Count: 456},
 		},
-	}).Return(nil)
+	}
+	app.EXPECT().SetCharm(setCharmConfigMatcher{c: c, expected: cfg})
+
 	s.backend.EXPECT().Application("postgresql").Return(app, nil)
 
 	toUint64Ptr := func(v uint64) *uint64 {
@@ -522,6 +523,26 @@ func (s *ApplicationSuite) TestSetCAASCharmInvalid(c *gc.C) {
 	c.Assert(msg, gc.Matches, "Juju on containers does not support updating deployment info.*")
 }
 
+type setCharmConfigMatcher struct {
+	c        *gc.C
+	expected state.SetCharmConfig
+}
+
+func (m setCharmConfigMatcher) Matches(x interface{}) bool {
+	req, ok := x.(state.SetCharmConfig)
+	if !ok {
+		return false
+	}
+	m.c.Logf("req.CharmOrigin %s", pretty.Sprint(req.CharmOrigin))
+	m.c.Logf("m.expected.CharmOrigin %s", pretty.Sprint(m.expected.CharmOrigin))
+	m.c.Check(req, gc.DeepEquals, m.expected)
+	return true
+}
+
+func (setCharmConfigMatcher) String() string {
+	return "matches set charm configrequests"
+}
+
 func (s *ApplicationSuite) TestSetCharmConfigSettings(c *gc.C) {
 	ctrl := s.setup(c)
 	defer ctrl.Finish()
@@ -532,10 +553,9 @@ func (s *ApplicationSuite) TestSetCharmConfigSettings(c *gc.C) {
 
 	app := s.expectDefaultApplication(ctrl)
 	app.EXPECT().SetCharm(state.SetCharmConfig{
-		Charm:          &state.Charm{},
 		CharmOrigin:    createStateCharmOriginFromURL(curl),
 		ConfigSettings: charm.Settings{"stringOption": "value"},
-	})
+	}).Return(nil)
 	s.backend.EXPECT().Application("postgresql").Return(app, nil)
 
 	err := s.api.SetCharm(params.ApplicationSetCharm{
@@ -589,7 +609,6 @@ func (s *ApplicationSuite) TestSetCharmUpgradeFormat(c *gc.C) {
 
 	app := s.expectDefaultApplication(ctrl)
 	app.EXPECT().SetCharm(state.SetCharmConfig{
-		Charm:       &state.Charm{},
 		CharmOrigin: createStateCharmOriginFromURL(curl),
 	}).Return(nil)
 	s.backend.EXPECT().Application("postgresql").Return(app, nil)
@@ -612,7 +631,6 @@ func (s *ApplicationSuite) TestSetCharmConfigSettingsYAML(c *gc.C) {
 
 	app := s.expectDefaultApplication(ctrl)
 	app.EXPECT().SetCharm(state.SetCharmConfig{
-		Charm:          &state.Charm{},
 		CharmOrigin:    createStateCharmOriginFromURL(curl),
 		ConfigSettings: charm.Settings{"stringOption": "value"},
 	}).Return(nil)
@@ -650,7 +668,6 @@ func (s *ApplicationSuite) TestLXDProfileSetCharmWithNewerAgentVersion(c *gc.C)
 	app := s.expectApplicationWithCharm(ctrl, currentCh, "postgresql")
 	app.EXPECT().AgentTools().Return(&agentTools, nil)
 	app.EXPECT().SetCharm(state.SetCharmConfig{
-		Charm:          &state.Charm{},
 		CharmOrigin:    createStateCharmOriginFromURL(curl),
 		ConfigSettings: charm.Settings{"stringOption": "value"},
 	}).Return(nil)
@@ -704,7 +721,6 @@ func (s *ApplicationSuite) TestLXDProfileSetCharmWithEmptyProfile(c *gc.C) {
 	app := s.expectApplicationWithCharm(ctrl, currentCh, "postgresql")
 	app.EXPECT().AgentTools().Return(&agentTools, nil)
 	app.EXPECT().SetCharm(state.SetCharmConfig{
-		Charm:          &state.Charm{},
 		CharmOrigin:    createStateCharmOriginFromURL(curl),
 		ConfigSettings: charm.Settings{"stringOption": "value"},
 	}).Return(nil)

cmd/juju/application/diffbundle.go

@@ -300,9 +300,9 @@ func missingRelationEndpoint(rel string) bool {
 func (c *diffBundleCommand) bundleDataSource(ctx *cmd.Context, apiRoot base.APICallCloser, base series.Base) (charm.BundleDataSource, error) {
 	ds, err := charm.LocalBundleDataSource(c.bundle)
 
-	// NotValid/NotFound means we should try interpreting it as a charm store
-	// bundle URL.
-	if err != nil && !errors.Is(err, errors.NotValid) && !errors.Is(err, errors.NotFound) {
+	// NotFound means that the provided local file is not found, and
+	// therefore we should try interpreting it as a charm store bundle URL.
+	if err != nil && !errors.Is(err, errors.NotFound) {
 		return nil, errors.Trace(err)
 	}
 	if ds != nil {

cmd/juju/application/diffbundle_test.go

@@ -135,6 +135,12 @@ machines:
 `[1:])
 }
 
+func (s *diffSuite) TestLocalBundleInvalidYaml(c *gc.C) {
+	_, err := s.runDiffBundle(c, s.writeLocalBundle(c, invalidYaml))
+	c.Assert(err, jc.Satisfies, errors.IsNotValid)
+	c.Assert(err, gc.ErrorMatches, `.*cannot unmarshal bundle contents.*`[1:])
+}
+
 func (s *diffSuite) TestIncludeAnnotations(c *gc.C) {
 	ctx, err := s.runDiffBundle(c, "--annotations", s.writeLocalBundle(c, testCharmHubBundle))
 	c.Assert(err, jc.ErrorIsNil)
@@ -686,6 +692,13 @@ machines:
 	invalidBundle = `
 machines:
   0:
+`
+	invalidYaml = `
+applications:
+  prometheus:
+    options:
+      admin-user: lovecraft
+va
 `
 	overlay1 = `
 applications:

provider/lxd/provider.go

@@ -176,17 +176,12 @@ func (p *environProvider) Ping(ctx context.ProviderCallContext, endpoint string)
 		return errors.Trace(err)
 	}
 
-	// Make sure we have an https url
-	if lxdEndpoint != endpoint {
-		return errors.Errorf("invalid URL %q: only HTTPS is supported", endpoint)
-	}
-
 	// Connect to the remote server anonymously so we can just verify it exists
 	// as we're not sure that the certificates are loaded in time for when the
 	// ping occurs i.e. interactive add-cloud
 	_, err = lxd.ConnectRemote(lxd.NewInsecureServerSpec(lxdEndpoint))
 	if err != nil {
-		return errors.Errorf("no lxd server running at %s", lxdEndpoint)
+		return errors.Annotatef(err, "no lxd server running at %s", lxdEndpoint)
 	}
 	return nil
 }

provider/lxd/provider_test.go

@@ -5,9 +5,11 @@ package lxd_test
 
 import (
 	stdcontext "context"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"path"
+	"strings"
 
 	"github.com/golang/mock/gomock"
 	"github.com/juju/errors"
@@ -513,7 +515,9 @@ func (s *providerSuite) TestPingFailWithNoEndpoint(c *gc.C) {
 	p, err := environs.Provider("lxd")
 	c.Assert(err, jc.ErrorIsNil)
 	err = p.Ping(context.NewEmptyCloudCallContext(), server.URL)
-	c.Assert(err, gc.ErrorMatches, "no lxd server running at "+server.URL)
+	c.Assert(err, gc.ErrorMatches, fmt.Sprintf(
+		"no lxd server running at %[1]s: Failed to fetch %[1]s/1.0: 404 Not Found",
+		server.URL))
 }
 
 func (s *providerSuite) TestPingFailWithHTTP(c *gc.C) {
@@ -523,7 +527,10 @@ func (s *providerSuite) TestPingFailWithHTTP(c *gc.C) {
 	p, err := environs.Provider("lxd")
 	c.Assert(err, jc.ErrorIsNil)
 	err = p.Ping(context.NewEmptyCloudCallContext(), server.URL)
-	c.Assert(err, gc.ErrorMatches, "invalid URL \""+server.URL+"\": only HTTPS is supported")
+	httpsURL := "https://" + strings.TrimPrefix(server.URL, "http://")
+	c.Assert(err, gc.ErrorMatches, fmt.Sprintf(
+		`no lxd server running at %[1]s: Get "%[1]s/1.0": http: server gave HTTP response to HTTPS client`,
+		httpsURL))
 }
 
 type ProviderFunctionalSuite struct {

provider/maas/environprovider.go

@@ -78,19 +78,21 @@ func (p EnvironProvider) CloudSchema() *jsonschema.Schema {
 
 // Ping tests the connection to the cloud, to verify the endpoint is valid.
 func (p EnvironProvider) Ping(ctx context.ProviderCallContext, endpoint string) error {
+	var err error
 	base, version, includesVersion := gomaasapi.SplitVersionedURL(endpoint)
 	if includesVersion {
-		err := p.checkMaas(base, version)
+		err = p.checkMaas(base, version)
 		if err == nil {
 			return nil
 		}
 	} else {
-		err := p.checkMaas(endpoint, apiVersion2)
+		// No version info in the endpoint - try both in preference order.
+		err = p.checkMaas(endpoint, apiVersion2)
 		if err == nil {
 			return nil
 		}
 	}
-	return errors.Errorf("No MAAS server running at %s", endpoint)
+	return errors.Annotatef(err, "No MAAS server running at %s", endpoint)
 }
 
 func (p EnvironProvider) checkMaas(endpoint, ver string) error {

provider/maas/environprovider_test.go

@@ -171,7 +171,7 @@ func (s *MaasPingSuite) TestPingNoEndpoint(c *gc.C) {
 			return nil, errors.New("nope")
 		},
 	)
-	c.Assert(err, gc.ErrorMatches, "No MAAS server running at "+endpoint)
+	c.Assert(err, gc.ErrorMatches, "No MAAS server running at "+endpoint+": nope")
 	c.Assert(serverURLs, gc.DeepEquals, []string{
 		"https://foo.com/MAAS/api/2.0/",
 	})
@@ -219,7 +219,7 @@ func (s *MaasPingSuite) TestPingVersionURLBad(c *gc.C) {
 			return nil, errors.New("nope")
 		},
 	)
-	c.Assert(err, gc.ErrorMatches, "No MAAS server running at "+endpoint)
+	c.Assert(err, gc.ErrorMatches, "No MAAS server running at "+endpoint+": nope")
 	c.Assert(serverURLs, gc.DeepEquals, []string{
 		"https://foo.com/MAAS/api/10.1/",
 	})

provider/openstack/provider.go

@@ -249,7 +249,7 @@ func (p EnvironProvider) Ping(ctx context.ProviderCallContext, endpoint string)
 	c := p.ClientFromEndpoint(endpoint)
 	if _, err := c.IdentityAuthOptions(); err != nil {
 		handleCredentialError(err, ctx)
-		return errors.Wrap(err, errors.Errorf("No Openstack server running at %s", endpoint))
+		return errors.Annotatef(err, "No Openstack server running at %s", endpoint)
 	}
 	return nil
 }

provider/openstack/provider_test.go

@@ -521,10 +521,7 @@ func (localTests) TestPingInvalidHost(c *gc.C) {
 			c.Errorf("ping %q: expected error, but got nil.", t)
 			continue
 		}
-		expected := "No Openstack server running at " + t
-		if err.Error() != expected {
-			c.Errorf("ping %q: expected %q got %v", t, expected, err)
-		}
+		c.Check(err, gc.ErrorMatches, "(?m)No Openstack server running at "+t+".*")
 	}
 }
 func (localTests) TestPingNoEndpoint(c *gc.C) {
@@ -533,7 +530,7 @@ func (localTests) TestPingNoEndpoint(c *gc.C) {
 	p, err := environs.Provider("openstack")
 	c.Assert(err, jc.ErrorIsNil)
 	err = p.Ping(context.NewEmptyCloudCallContext(), server.URL)
-	c.Assert(err, gc.ErrorMatches, "No Openstack server running at "+server.URL)
+	c.Assert(err, gc.ErrorMatches, "(?m)No Openstack server running at "+server.URL+".*")
 }
 
 func (localTests) TestPingInvalidResponse(c *gc.C) {
@@ -544,7 +541,7 @@ func (localTests) TestPingInvalidResponse(c *gc.C) {
 	p, err := environs.Provider("openstack")
 	c.Assert(err, jc.ErrorIsNil)
 	err = p.Ping(context.NewEmptyCloudCallContext(), server.URL)
-	c.Assert(err, gc.ErrorMatches, "No Openstack server running at "+server.URL)
+	c.Assert(err, gc.ErrorMatches, "(?m)No Openstack server running at "+server.URL+".*")
 }
 
 func (localTests) TestPingOKCACertificate(c *gc.C) {

state/application.go

@@ -2671,6 +2671,17 @@ func incApplicationOffersRefOp(mb modelBackend, appName string) (txn.Op, error)
 	return incRefOp, errors.Trace(err)
 }
 
+// newApplicationOffersRefOp returns a txn.Op that creates a new reference
+// count for an application offer, starting at the count supplied. Used in
+// model migration, where offers are created in bulk.
+func newApplicationOffersRefOp(mb modelBackend, appName string, startCount int) (txn.Op, error) {
+	refcounts, closer := mb.db().GetCollection(refcountsC)
+	defer closer()
+	offerRefCountKey := applicationOffersRefCountKey(appName)
+	incRefOp, err := nsRefcounts.CreateOrIncRefOp(refcounts, offerRefCountKey, startCount)
+	return incRefOp, errors.Trace(err)
+}
+
 // countApplicationOffersRefOp returns the number of offers for an application,
 // along with a txn.Op that ensures that that does not change.
 func countApplicationOffersRefOp(mb modelBackend, appName string) (txn.Op, int, error) {