Merge pull request juju#16642 from wallyworld/merge-3.4-20231129

juju#16642

Merge 3.4

juju#16598 [from wallyworld/get-owned-secrets](juju@4d0cd7d)
juju#16596 [from manadart/2.9-fix-bad-clouds-panic](juju@7027d79)
juju#16603 [from wallyworld/inhook-secret-refresh](juju@cb7ad3b)
juju#16583 [from jack-w-shaw/update_merge_bot](juju@d4b9e00)
juju#16576 [from SimonRichardson/pre-check-facades-migr…](juju@ad85101)
juju#16610 [from wallyworld/cli-help-fixes](juju@dc94b09)
juju#16465 [from hpidcock/fix-docker-auth-challenges](juju@f80096d)
juju#16616 [from wallyworld/fix-topologykey-constraint](juju@060e175)
juju#16617 [from wallyworld/remove-placeholder-port](juju@d57438a)
juju#16618 [from manadart/2.9-destroy-storage-with-test…](juju@3b81a00)
juju#16584 [from hmlanigan/fix-deploy-ci-lxd-series](juju@7c1d995)

Conflicts
```
# Conflicts:
# api/apiclient.go
# api/client/charms/downloader_s3.go
# api/export_test.go
# api/facadeversions.go
# api/facadeversions_test.go
# api/package_test.go
# apiserver/allfacades.go
# apiserver/common/secrets/access.go
# apiserver/facades/agent/secretsmanager/secrets.go
# apiserver/facades/agent/uniter/uniter.go
# apiserver/facades/agent/uniter/uniter_test.go
# apiserver/facades/controller/migrationtarget/migrationtarget.go
# apiserver/facades/controller/migrationtarget/migrationtarget_test.go
# apiserver/facades/controller/migrationtarget/register.go
# caas/kubernetes/provider/specs/admissionregistration.go
# core/charm/repository/charmhub.go
# core/charm/repository/charmhub_test.go
# go.mod
# go.sum
# internal/migration/precheck.go
# worker/uniter/runner/context/context_test.go
```

.github/workflows/merge.yml

@@ -1,7 +1,7 @@
 name: Merge
 on:
   push:
-    branches: ['2.9', '3.1', '3.2', '3.3']
+    branches: ['2.9', '3.1', '3.3', '3.4']
 
 jobs:
   check-merge:
@@ -10,9 +10,9 @@ jobs:
     env:
       MERGE_TARGETS: |
         2.9: 3.1
-        3.1: 3.2
-        3.2: 3.3
-        3.3: main
+        3.1: 3.3
+        3.3: 3.4
+        3.4: main
 
     steps:
       - name: Determine source/target branches
@@ -54,7 +54,7 @@ jobs:
           # Need to set Git username/email to do the merge (yawn)
           git config user.name 'jujubot'
           git config user.email '[email protected]'
-          
+
           set +e
           git switch "$TARGET_BRANCH"
           git merge "$SOURCE_BRANCH"

PULL_REQUEST_TEMPLATE.md

@@ -1,8 +1,8 @@
-*Why this change is needed and what it does.*
+<!-- Why this change is needed and what it does. -->
 
 ## Checklist
 
-*If an item is not applicable, use `~strikethrough~`.*
+<!-- If an item is not applicable, use `~strikethrough~`. -->
 
 - [ ] Code style: imports ordered, good names, simple structure, etc
 - [ ] Comments saying why design decisions were made
@@ -12,20 +12,17 @@
 
 ## QA steps
 
-*Commands to run to verify that the change works.*
-
-```sh
-QA steps here
-```
+<!-- Describe steps to verify that the change works. -->
 
 ## Documentation changes
 
-*How it affects user workflow (CLI or API). Delete section if not applicable.*
+<!-- How it affects user workflow (CLI or API). -->
 
 ## Links
 
-**Launchpad bug:** https://pad.lv/<bug-number>
+<!-- Link to all relevant specification, documentation, bug, issue or JIRA card. -->
+
+**Launchpad bug:** https://bugs.launchpad.net/juju/+bug/
 
-**Jira card:** JUJU-[XXXX]
+**Jira card:** JUJU-
 
-*Insert other relevant links here.*

api/agent/uniter/unit.go

@@ -1027,6 +1027,15 @@ func (b *CommitHookParamsBuilder) AddSecretUpdates(updates []SecretUpsertArg) {
 	}
 }
 
+// AddTrackLatest records the URIs for which the latest revision should be tracked.
+func (b *CommitHookParamsBuilder) AddTrackLatest(trackLatest []string) {
+	if len(trackLatest) == 0 {
+		return
+	}
+	b.arg.TrackLatest = make([]string, len(trackLatest))
+	copy(b.arg.TrackLatest, trackLatest)
+}
+
 // SecretGrantRevokeArgs holds parameters for updating a secret's access.
 type SecretGrantRevokeArgs struct {
 	URI             *secrets.URI

api/apiclient.go

@@ -35,6 +35,7 @@ import (
 	"gopkg.in/retry.v1"
 
 	"github.com/juju/juju/api/base"
+	"github.com/juju/juju/core/facades"
 	coremacaroon "github.com/juju/juju/core/macaroon"
 	"github.com/juju/juju/core/network"
 	jujuproxy "github.com/juju/juju/internal/proxy"
@@ -247,7 +248,7 @@ func Open(info *Info, opts DialOpts) (Connection, error) {
 		host = dialResult.addr
 	}
 
-	pingerFacadeVersions := FacadeVersions["Pinger"]
+	pingerFacadeVersions := facadeVersions["Pinger"]
 	if len(pingerFacadeVersions) == 0 {
 		return nil, errors.Errorf("pinger facade version is required")
 	}
@@ -1365,7 +1366,7 @@ func (c *conn) PublicDNSName() string {
 // Facade we will want to use. It needs to line up the versions that the server
 // reports to us, with the versions that our client knows how to use.
 func (c *conn) BestFacadeVersion(facade string) int {
-	return bestVersion(FacadeVersions[facade], c.facadeVersions[facade])
+	return facades.BestVersion(facadeVersions[facade], c.facadeVersions[facade])
 }
 
 // serverRoot returns the cached API server address and port used

api/controller/caasapplicationprovisioner/client.go

@@ -422,7 +422,6 @@ func (c *Client) DestroyUnits(unitNames []string) error {
 	args := params.DestroyUnitsParams{}
 	args.Units = make([]params.DestroyUnitParams, 0, len(unitNames))
 
-	fmt.Println(unitNames)
 	for _, unitName := range unitNames {
 		tag := names.NewUnitTag(unitName)
 		args.Units = append(args.Units, params.DestroyUnitParams{

api/controller/migrationmaster/client_test.go

@@ -59,7 +59,7 @@ func (s *ClientSuite) TestWatch(c *gc.C) {
 	w, err := client.Watch()
 	c.Check(err, jc.ErrorIsNil)
 	c.Check(w, gc.Equals, expectWatch)
-	stub.CheckCalls(c, []jujutesting.StubCall{{"MigrationMaster.Watch", []interface{}{"", nil}}})
+	stub.CheckCalls(c, []jujutesting.StubCall{{FuncName: "MigrationMaster.Watch", Args: []interface{}{"", nil}}})
 }
 
 func (s *ClientSuite) TestWatchCallError(c *gc.C) {
@@ -136,7 +136,7 @@ func (s *ClientSuite) TestSetPhase(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 	expectedArg := params.SetMigrationPhaseArgs{Phase: "QUIESCE"}
 	stub.CheckCalls(c, []jujutesting.StubCall{
-		{"MigrationMaster.SetPhase", []interface{}{"", expectedArg}},
+		{FuncName: "MigrationMaster.SetPhase", Args: []interface{}{"", expectedArg}},
 	})
 }
 
@@ -160,7 +160,7 @@ func (s *ClientSuite) TestSetStatusMessage(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 	expectedArg := params.SetMigrationStatusMessageArgs{Message: "foo"}
 	stub.CheckCalls(c, []jujutesting.StubCall{
-		{"MigrationMaster.SetStatusMessage", []interface{}{"", expectedArg}},
+		{FuncName: "MigrationMaster.SetStatusMessage", Args: []interface{}{"", expectedArg}},
 	})
 }
 
@@ -190,7 +190,7 @@ func (s *ClientSuite) TestModelInfo(c *gc.C) {
 	client := migrationmaster.NewClient(apiCaller, nil)
 	model, err := client.ModelInfo()
 	stub.CheckCalls(c, []jujutesting.StubCall{
-		{"MigrationMaster.ModelInfo", []interface{}{"", nil}},
+		{FuncName: "MigrationMaster.ModelInfo", Args: []interface{}{"", nil}},
 	})
 	c.Check(err, jc.ErrorIsNil)
 	c.Check(model, jc.DeepEquals, migration.ModelInfo{
@@ -218,7 +218,7 @@ func (s *ClientSuite) TestSourceControllerInfo(c *gc.C) {
 	client := migrationmaster.NewClient(apiCaller, nil)
 	info, relatedModels, err := client.SourceControllerInfo()
 	stub.CheckCalls(c, []jujutesting.StubCall{
-		{"MigrationMaster.SourceControllerInfo", []interface{}{"", nil}},
+		{FuncName: "MigrationMaster.SourceControllerInfo", Args: []interface{}{"", nil}},
 	})
 	c.Check(err, jc.ErrorIsNil)
 	c.Check(info, jc.DeepEquals, migration.SourceControllerInfo{
@@ -241,7 +241,7 @@ func (s *ClientSuite) TestPrechecks(c *gc.C) {
 	c.Check(err, gc.ErrorMatches, "blam")
 	expectedArg := params.PrechecksArgs{}
 	stub.CheckCalls(c, []jujutesting.StubCall{
-		{"MigrationMaster.Prechecks", []interface{}{"", expectedArg}},
+		{FuncName: "MigrationMaster.Prechecks", Args: []interface{}{"", expectedArg}},
 	})
 }
 
@@ -332,7 +332,7 @@ func (s *ClientSuite) TestExport(c *gc.C) {
 	out, err := client.Export()
 	c.Assert(err, jc.ErrorIsNil)
 	stub.CheckCalls(c, []jujutesting.StubCall{
-		{"MigrationMaster.Export", []interface{}{"", nil}},
+		{FuncName: "MigrationMaster.Export", Args: []interface{}{"", nil}},
 	})
 	c.Assert(out, gc.DeepEquals, migration.SerializedModel{
 		Bytes:  []byte("foo"),
@@ -441,7 +441,7 @@ func (s *ClientSuite) TestReap(c *gc.C) {
 	err := client.Reap()
 	c.Check(err, jc.ErrorIsNil)
 	stub.CheckCalls(c, []jujutesting.StubCall{
-		{"MigrationMaster.Reap", []interface{}{"", nil}},
+		{FuncName: "MigrationMaster.Reap", Args: []interface{}{"", nil}},
 	})
 }
 
@@ -474,7 +474,7 @@ func (s *ClientSuite) TestWatchMinionReports(c *gc.C) {
 	w, err := client.WatchMinionReports()
 	c.Check(err, jc.ErrorIsNil)
 	c.Check(w, gc.Equals, expectWatch)
-	stub.CheckCalls(c, []jujutesting.StubCall{{"MigrationMaster.WatchMinionReports", []interface{}{"", nil}}})
+	stub.CheckCalls(c, []jujutesting.StubCall{{FuncName: "MigrationMaster.WatchMinionReports", Args: []interface{}{"", nil}}})
 }
 
 func (s *ClientSuite) TestWatchMinionReportsError(c *gc.C) {
@@ -515,7 +515,7 @@ func (s *ClientSuite) TestMinionReports(c *gc.C) {
 	out, err := client.MinionReports()
 	c.Assert(err, jc.ErrorIsNil)
 	stub.CheckCalls(c, []jujutesting.StubCall{
-		{"MigrationMaster.MinionReports", []interface{}{"", nil}},
+		{FuncName: "MigrationMaster.MinionReports", Args: []interface{}{"", nil}},
 	})
 	c.Assert(out, gc.DeepEquals, migration.MinionReports{
 		MigrationId:             "id",

api/controller/migrationtarget/client.go

@@ -18,6 +18,7 @@ import (
 	"github.com/juju/version/v2"
 	"gopkg.in/httprequest.v1"
 
+	"github.com/juju/juju/api"
 	"github.com/juju/juju/api/base"
 	coremigration "github.com/juju/juju/core/migration"
 	"github.com/juju/juju/core/resources"
@@ -55,13 +56,26 @@ func (c *Client) BestFacadeVersion() int {
 	return c.caller.BestAPIVersion()
 }
 
+// Prechecks checks that the target controller is able to accept the
+// model being migrated.
 func (c *Client) Prechecks(model coremigration.ModelInfo) error {
+	// Pass all the known facade versions to the controller so that it
+	// can check that the target controller supports them. Passing all of them
+	// ensures that we don't have to update this code when new facades are
+	// added, or if the controller wants to change the logic service side.
+	supported := api.SupportedFacadeVersions()
+	versions := make(map[string][]int, len(supported))
+	for name, version := range supported {
+		versions[name] = version
+	}
+
 	args := params.MigrationModelInfo{
 		UUID:                   model.UUID,
 		Name:                   model.Name,
 		OwnerTag:               model.Owner.String(),
 		AgentVersion:           model.AgentVersion,
 		ControllerAgentVersion: model.ControllerAgentVersion,
+		FacadeVersions:         versions,
 	}
 	return errors.Trace(c.caller.FacadeCall(context.TODO(), "Prechecks", args, nil))
 }

api/controller/migrationtarget/client_test.go

@@ -74,9 +74,14 @@ func (s *ClientSuite) TestPrechecks(c *gc.C) {
 		AgentVersion:           vers,
 		ControllerAgentVersion: controllerVers,
 	}
-	stub.CheckCalls(c, []jujutesting.StubCall{
-		{FuncName: "MigrationTarget.Prechecks", Args: []interface{}{"", expectedArg}},
-	})
+	stub.CheckCallNames(c, "MigrationTarget.Prechecks")
+
+	arg := stub.Calls()[0].Args[1].(params.MigrationModelInfo)
+
+	mc := jc.NewMultiChecker()
+	mc.AddExpr("_.FacadeVersions", gc.Not(gc.HasLen), 0)
+
+	c.Assert(arg, mc, expectedArg)
 }
 
 func (s *ClientSuite) TestImport(c *gc.C) {

api/export_test.go

@@ -19,7 +19,7 @@ import (
 var (
 	CertDir             = &certDir
 	SlideAddressToFront = slideAddressToFront
-	BestVersion         = bestVersion
+	FacadeVersions      = &facadeVersions
 )
 
 func DialAPI(info *Info, opts DialOpts) (jsoncodec.JSONConn, string, error) {

api/facadeversions.go

@@ -3,7 +3,12 @@
 
 package api
 
-import "github.com/juju/collections/set"
+import "github.com/juju/juju/core/facades"
+
+// SupportedFacadeVersions returns the list of facades that the api supports.
+func SupportedFacadeVersions() facades.FacadeVersions {
+	return facadeVersions
+}
 
 // facadeVersions lists the best version of facades that we want to support. This
 // will be used to pick out a default version for communication, given the list
@@ -15,7 +20,7 @@ import "github.com/juju/collections/set"
 // FullStatus (client facade) and Migration (controller facade) is needed.
 // New facades should start at 1.
 // We no longer support facade versions at 0.
-var FacadeVersions = map[string][]int{
+var facadeVersions = facades.FacadeVersions{
 	"Action":                       {7},
 	"ActionPruner":                 {1},
 	"Agent":                        {3},
@@ -83,7 +88,7 @@ var FacadeVersions = map[string][]int{
 	"MigrationMaster":              {3},
 	"MigrationMinion":              {1},
 	"MigrationStatusWatcher":       {1},
-	"MigrationTarget":              {1, 2},
+	"MigrationTarget":              {1, 2, 3},
 	"ModelConfig":                  {3},
 	"ModelGeneration":              {4},
 	"ModelManager":                 {9, 10},
@@ -132,14 +137,3 @@ var FacadeVersions = map[string][]int{
 	"VolumeAttachmentsWatcher":     {2},
 	"VolumeAttachmentPlansWatcher": {1},
 }
-
-// bestVersion tries to find the newest version in the version list that we can
-// use.
-func bestVersion(desired []int, versions []int) int {
-	intersection := set.NewInts(desired...).Intersection(set.NewInts(versions...))
-	if intersection.Size() == 0 {
-		return 0
-	}
-	sorted := intersection.SortedValues()
-	return sorted[len(sorted)-1]
-}

api/facadeversions_test.go

@@ -7,6 +7,7 @@ import (
 	gc "gopkg.in/check.v1"
 
 	"github.com/juju/juju/api"
+	"github.com/juju/juju/core/facades"
 	coretesting "github.com/juju/juju/testing"
 )
 
@@ -16,8 +17,8 @@ type facadeVersionSuite struct {
 
 var _ = gc.Suite(&facadeVersionSuite{})
 
-func checkBestVersion(c *gc.C, desiredVersion []int, versions []int, expectedVersion int) {
-	resultVersion := api.BestVersion(desiredVersion, versions)
+func checkBestVersion(c *gc.C, desiredVersion, versions []int, expectedVersion int) {
+	resultVersion := facades.BestVersion(desiredVersion, versions)
 	c.Check(resultVersion, gc.Equals, expectedVersion)
 }
 
@@ -51,7 +52,7 @@ func (*facadeVersionSuite) TestBestVersionNotSorted(c *gc.C) {
 }
 
 func (s *facadeVersionSuite) TestBestFacadeVersionExactMatch(c *gc.C) {
-	s.PatchValue(&api.FacadeVersions, map[string][]int{"Client": {1}})
+	s.PatchValue(api.FacadeVersions, map[string]facades.FacadeVersion{"Client": {1}})
 	conn := api.NewTestingConnection(api.TestingConnectionParams{
 		FacadeVersions: map[string][]int{
 			"Client": {0, 1},
@@ -60,7 +61,7 @@ func (s *facadeVersionSuite) TestBestFacadeVersionExactMatch(c *gc.C) {
 }
 
 func (s *facadeVersionSuite) TestBestFacadeVersionNewerServer(c *gc.C) {
-	s.PatchValue(&api.FacadeVersions, map[string][]int{"Client": {1}})
+	s.PatchValue(api.FacadeVersions, map[string]facades.FacadeVersion{"Client": {1}})
 	conn := api.NewTestingConnection(api.TestingConnectionParams{
 		FacadeVersions: map[string][]int{
 			"Client": {0, 1, 2},
@@ -69,7 +70,7 @@ func (s *facadeVersionSuite) TestBestFacadeVersionNewerServer(c *gc.C) {
 }
 
 func (s *facadeVersionSuite) TestBestFacadeVersionNewerClient(c *gc.C) {
-	s.PatchValue(&api.FacadeVersions, map[string][]int{"Client": {1, 2}})
+	s.PatchValue(api.FacadeVersions, map[string]facades.FacadeVersion{"Client": {1, 2}})
 	conn := api.NewTestingConnection(api.TestingConnectionParams{
 		FacadeVersions: map[string][]int{
 			"Client": {0, 1},
@@ -78,7 +79,7 @@ func (s *facadeVersionSuite) TestBestFacadeVersionNewerClient(c *gc.C) {
 }
 
 func (s *facadeVersionSuite) TestBestFacadeVersionServerUnknown(c *gc.C) {
-	s.PatchValue(&api.FacadeVersions, map[string][]int{"TestingAPI": {1, 2}})
+	s.PatchValue(api.FacadeVersions, map[string]facades.FacadeVersion{"TestingAPI": {1, 2}})
 	conn := api.NewTestingConnection(api.TestingConnectionParams{
 		FacadeVersions: map[string][]int{
 			"Client": {0, 1},

api/package_test.go

@@ -33,6 +33,7 @@ func (*ImportSuite) TestImports(c *gc.C) {
 		"core/constraints",
 		"core/database",
 		"core/devices",
+		"core/facades",
 		"core/instance",
 		"core/life",
 		"core/macaroon",