Merge pull request #15494 from jack-w-shaw/3.2-into-develop

#15494

Merges:
- #15399
- #15395
- #15414
- #15417
- #15418
- #15419
- #15413
- #15420
- #15422
- #15424
- #15416
- #15428
- #15423
- #15426

Conflicts resolved trivially

.github/workflows/build.yml

@@ -7,6 +7,9 @@ on:
       - '**.go'
       - 'go.mod'
       - '.github/workflows/build.yml'
+      - 'scripts/dqlite/**'
+      - 'Makefile'
+      - 'make_functions.sh'
   workflow_dispatch:
 
 permissions:

.github/workflows/client-tests.yml

@@ -7,6 +7,9 @@ on:
       - '**.go'
       - 'go.mod'
       - '.github/workflows/client-tests.yml'
+      - 'scripts/dqlite/**'
+      - 'Makefile'
+      - 'make_functions.sh'
   workflow_dispatch:
 
 permissions:

.github/workflows/gen.yml

@@ -7,6 +7,8 @@ on:
       - '**.go'
       - 'go.mod'
       - '.github/workflows/gen.yml'
+      - 'Makefile'
+      - 'make_functions.sh'
   workflow_dispatch:
 
 permissions:

.github/workflows/migrate.yml

@@ -8,6 +8,9 @@ on:
       - 'go.mod'
       - 'snap/**'
       - '.github/workflows/migrate.yml'
+      - 'scripts/dqlite/**'
+      - 'Makefile'
+      - 'make_functions.sh'
   workflow_dispatch:
 
 permissions:

.github/workflows/smoke.yml

@@ -3,14 +3,6 @@ on:
   push:
   pull_request:
     types: [opened, synchronize, reopened, ready_for_review]
-    paths:
-      - '**.go'
-      - 'go.mod'
-      - 'testcharms/**'
-      - 'tests/main.sh'
-      - 'tests/includes/**'
-      - 'tests/suites/smoke/**'
-      - '.github/workflows/smoke.yml'
   workflow_dispatch:
 
 permissions:

.github/workflows/snap.yml

@@ -8,6 +8,9 @@ on:
       - 'go.mod'
       - 'snap/**'
       - '.github/workflows/snap.yml'
+      - 'scripts/dqlite/**'
+      - 'Makefile'
+      - 'make_functions.sh'
   workflow_dispatch:
 
 permissions:

.github/workflows/upgrade.yml

@@ -11,6 +11,9 @@ on:
       - 'snap/**'
       - '.github/workflows/upgrade.yml'
       - '.github/setup-lxd/**'
+      - 'scripts/dqlite/**'
+      - 'Makefile'
+      - 'make_functions.sh'
     branches-ignore:
       - 'develop'
   workflow_dispatch:

Makefile

@@ -144,11 +144,10 @@ else
 endif
 TEST_TIMEOUT := $(TEST_TIMEOUT)
 
+TEST_ARGS ?=
 # Limit concurrency on s390x.
 ifeq ($(shell echo "${GOARCH}" | sed -E 's/.*(s390x).*/golang/'), golang)
-	TEST_ARGS ?= -p 4
-else
-	TEST_ARGS ?=
+	TEST_ARGS += -p 4
 endif
 
 # Enable coverage testing.
@@ -394,6 +393,11 @@ check: pre-check run-tests
 test: run-tests
 ## test: Verify Juju code using unit tests
 
+.PHONY: race-test
+race-test:
+## race-test: Verify Juju code using unit tests with the race detector enabled
+	+make run-tests TEST_ARGS="$(TEST_ARGS) -race"
+
 .PHONY: run-tests run-go-tests
 # Can't make the length of the TMP dir too long or it hits socket name length issues.
 run-tests: musl-install-if-missing dqlite-install-if-missing

apiserver/facades/client/modelupgrader/upgrader_test.go

@@ -4,8 +4,6 @@
 package modelupgrader_test
 
 import (
-	"net/http"
-
 	"github.com/golang/mock/gomock"
 	"github.com/juju/errors"
 	"github.com/juju/names/v4"
@@ -195,7 +193,7 @@ func (s *modelUpgradeSuite) assertUpgradeModelForControllerModelJuju3(c *gc.C, d
 	server := upgradevalidationmocks.NewMockServer(ctrl)
 	serverFactory := upgradevalidationmocks.NewMockServerFactory(ctrl)
 	s.PatchValue(&upgradevalidation.NewServerFactory,
-		func(httpClient *http.Client) lxd.ServerFactory {
+		func(_ lxd.NewHTTPClientFunc) lxd.ServerFactory {
 			return serverFactory
 		},
 	)
@@ -319,7 +317,7 @@ func (s *modelUpgradeSuite) TestUpgradeModelForControllerModelJuju3Failed(c *gc.
 	server := upgradevalidationmocks.NewMockServer(ctrl)
 	serverFactory := upgradevalidationmocks.NewMockServerFactory(ctrl)
 	s.PatchValue(&upgradevalidation.NewServerFactory,
-		func(httpClient *http.Client) lxd.ServerFactory {
+		func(_ lxd.NewHTTPClientFunc) lxd.ServerFactory {
 			return serverFactory
 		},
 	)
@@ -446,7 +444,7 @@ func (s *modelUpgradeSuite) assertUpgradeModelJuju3(c *gc.C, dryRun bool) {
 	server := upgradevalidationmocks.NewMockServer(ctrl)
 	serverFactory := upgradevalidationmocks.NewMockServerFactory(ctrl)
 	s.PatchValue(&upgradevalidation.NewServerFactory,
-		func(httpClient *http.Client) lxd.ServerFactory {
+		func(_ lxd.NewHTTPClientFunc) lxd.ServerFactory {
 			return serverFactory
 		},
 	)
@@ -525,7 +523,7 @@ func (s *modelUpgradeSuite) TestUpgradeModelJuju3Failed(c *gc.C) {
 	server := upgradevalidationmocks.NewMockServer(ctrl)
 	serverFactory := upgradevalidationmocks.NewMockServerFactory(ctrl)
 	s.PatchValue(&upgradevalidation.NewServerFactory,
-		func(httpClient *http.Client) lxd.ServerFactory {
+		func(_ lxd.NewHTTPClientFunc) lxd.ServerFactory {
 			return serverFactory
 		},
 	)

apiserver/facades/controller/caasunitprovisioner/provisioner_test.go

@@ -229,6 +229,8 @@ func (s *CAASProvisionerSuite) assertProvisioningInfo(c *gc.C, isRawK8sSpec bool
 		},
 	})
 	c.Assert(err, jc.ErrorIsNil)
+	expectedVersion := jujuversion.Current
+	expectedVersion.Build = 666
 	// Maps are harder to check...
 	// http://ci.jujucharms.com/job/make-check-juju/4853/testReport/junit/github/com_juju_juju_apiserver_facades_controller_caasunitprovisioner/TestAll/
 	expectedResult := &params.KubernetesProvisioningInfo{
@@ -237,7 +239,7 @@ func (s *CAASProvisionerSuite) assertProvisioningInfo(c *gc.C, isRawK8sSpec bool
 			ServiceType:    "loadbalancer",
 		},
 		ImageRepo: params.DockerImageInfo{
-			RegistryPath: fmt.Sprintf("jujusolutions/jujud-operator:%s", jujuversion.Current.String()+".666"),
+			RegistryPath: fmt.Sprintf("jujusolutions/jujud-operator:%s", expectedVersion.String()),
 		},
 		Devices: []params.KubernetesDeviceParams{
 			{

caas/kubernetes/provider/bootstrap_test.go

@@ -610,6 +610,8 @@ func (s *bootstrapSuite) TestBootstrap(c *gc.C) {
 		}...,
 	)
 
+	expectedVersion := jujuversion.Current
+	expectedVersion.Build = 666
 	probCmds := &core.ExecAction{
 		Command: []string{
 			"mongo",
@@ -756,7 +758,7 @@ func (s *bootstrapSuite) TestBootstrap(c *gc.C) {
 		{
 			Name:            "api-server",
 			ImagePullPolicy: core.PullIfNotPresent,
-			Image:           "test-account/jujud-operator:" + jujuversion.Current.String() + ".666",
+			Image:           "test-account/jujud-operator:" + expectedVersion.String(),
 			Env: []core.EnvVar{{
 				Name:  osenv.JujuFeatureFlagEnvKey,
 				Value: "developer-mode",
@@ -882,7 +884,7 @@ EOF
 	statefulSetSpec.Spec.Template.Spec.InitContainers = []core.Container{{
 		Name:            "charm-init",
 		ImagePullPolicy: core.PullIfNotPresent,
-		Image:           "test-account/jujud-operator:" + jujuversion.Current.String() + ".666",
+		Image:           "test-account/jujud-operator:" + expectedVersion.String(),
 		WorkingDir:      "/var/lib/juju",
 		Command:         []string{"/opt/containeragent"},
 		Args:            []string{"init", "--containeragent-pebble-dir", "/containeragent/pebble", "--charm-modified-version", "0", "--data-dir", "/var/lib/juju", "--bin-dir", "/charm/bin", "--controller"},

caas/kubernetes/provider/detectcloud.go

@@ -50,18 +50,14 @@ func localKubeConfigClouds() ([]cloud.Cloud, error) {
 
 // DetectCloud implements environs.CloudDetector.
 func (p kubernetesEnvironProvider) DetectCloud(name string) (cloud.Cloud, error) {
-	mk8sCloud, err := p.builtinCloudGetter(p.cmdRunner)
-	if err == nil && name == k8s.K8sCloudMicrok8s {
-		return mk8sCloud, nil
-	}
-
-	if !errors.IsNotFound(err) && err != nil {
-		// if the cloud is not microk8s and we get the user group error, return not found so the caller skips the cloud
-		// https://bugs.launchpad.net/juju/+bug/1937985
-		if name != k8s.K8sCloudMicrok8s {
-			return cloud.Cloud{}, errors.NotFoundf("cloud %s", name)
+	if name == k8s.K8sCloudMicrok8s {
+		// TODO: this whole thing is poorly written and we need to handle this better.
+		// Also builtinCloudGetter should really be called, microk8sCloudGetter...
+		microk8sCloud, err := p.builtinCloudGetter(p.cmdRunner)
+		if err != nil {
+			return cloud.Cloud{}, errors.Trace(err)
 		}
-		return cloud.Cloud{}, errors.Trace(err)
+		return microk8sCloud, nil
 	}
 
 	localKubeConfigClouds, err := localKubeConfigClouds()

caas/kubernetes/provider/metadata.go

@@ -172,15 +172,17 @@ func GetClusterMetadata(
 		}
 	}
 
-	if nominatedStorageClass != "" && selectedOperatorSC.Name != nominatedStorageClass {
-		return nil, &environs.NominatedStorageNotFound{
-			StorageName: nominatedStorageClass,
+	if nominatedStorageClass != "" {
+		if selectedOperatorSC == nil || selectedOperatorSC.Name != nominatedStorageClass {
+			return nil, &environs.NominatedStorageNotFound{
+				StorageName: nominatedStorageClass,
+			}
 		}
-	}
 
-	if nominatedStorageClass != "" && selectedWorkloadSC.Name != nominatedStorageClass {
-		return nil, &environs.NominatedStorageNotFound{
-			StorageName: nominatedStorageClass,
+		if selectedWorkloadSC == nil || selectedWorkloadSC.Name != nominatedStorageClass {
+			return nil, &environs.NominatedStorageNotFound{
+				StorageName: nominatedStorageClass,
+			}
 		}
 	}
 

caas/kubernetes/provider/metadata_test.go

@@ -741,3 +741,24 @@ func (s *K8sMetadataSuite) TestNominatedStorageNotFound(c *gc.C) {
 	c.Assert(errors.As(err, &notFoundError), jc.IsTrue)
 	c.Assert(notFoundError.StorageName, gc.Equals, "my-nominated-storage")
 }
+
+// TestNominatedStorageNotFoundWithNilStorageClasses is a regression test to
+// make sure that when no storage classes are defined and a nominated storage
+// class has been specified a NominatedStorageNotFoundError is returned.
+func (s *K8sMetadataSuite) TestNominatedStorageNotFoundWithNilStorageClasses(c *gc.C) {
+	clientSet := fake.NewSimpleClientset(
+		newNode(map[string]string{}),
+	)
+
+	_, err := provider.GetClusterMetadata(
+		context.TODO(),
+		"my-nominated-storage",
+		clientSet.CoreV1().Nodes(),
+		clientSet.StorageV1().StorageClasses(),
+	)
+
+	var notFoundError *environs.NominatedStorageNotFound
+	c.Assert(err, gc.NotNil)
+	c.Assert(errors.As(err, &notFoundError), jc.IsTrue)
+	c.Assert(notFoundError.StorageName, gc.Equals, "my-nominated-storage")
+}

cmd/juju/application/deployer/bundle.go

@@ -99,6 +99,11 @@ func (d *deployBundle) deploy(
 	}
 	d.printDryRunUnmarshalErrors(ctx, unmarshalErrors)
 
+	err = d.checkExplicitSeries(bundleData)
+	if err != nil {
+		return errors.Trace(err)
+	}
+
 	d.bundleDir = d.bundleDataSource.BasePath()
 
 	// Short-circuit trust checks if the operator specifies '--force'
@@ -173,6 +178,43 @@ Please repeat the deploy command with the --trust argument if you consent to tru
 	return nil
 }
 
+// checkExplicitSeries returns an error if the image-id constraint is used and
+// there is no series explicitly defined by the user.
+func (d *deployBundle) checkExplicitSeries(bundleData *charm.BundleData) error {
+	for _, applicationSpec := range bundleData.Applications {
+		// First we check if the app is deployed "to" a machine that
+		// has the image-id constraint
+		machineHasImageID := false
+		for _, to := range applicationSpec.To {
+			machineCons, err := constraints.Parse(bundleData.Machines[to].Constraints)
+			if err != nil {
+				return errors.Trace(err)
+			}
+			if machineCons.HasImageID() {
+				machineHasImageID = true
+				break
+			}
+		}
+		// Then we check if the constraints declared on the app have
+		// image-id
+		appCons, err := constraints.Parse(applicationSpec.Constraints)
+		if err != nil {
+			return errors.Trace(err)
+		}
+		appHasImageID := appCons.HasImageID()
+		// Lastly we check if the model constraints have image-id
+		modelHasImageID := d.modelConstraints.HasImageID()
+		// We check if series are defined when any of the constraints
+		// above have image-id
+		if (appHasImageID || modelHasImageID || machineHasImageID) &&
+			applicationSpec.Series == "" &&
+			bundleData.Series == "" {
+			return errors.Forbiddenf("base must be explicitly provided for %q when image-id constraint is used", applicationSpec.Charm)
+		}
+	}
+	return nil
+}
+
 func (d *deployBundle) printDryRunUnmarshalErrors(ctx *cmd.Context, unmarshalErrors []error) {
 	if !d.dryRun {
 		return