Add test for removing legacy caas charm symlink fix;

apiserver/facades/agent/uniter/uniter.go

@@ -3478,7 +3478,7 @@ func (u *UniterAPI) CommitHookChanges(args params.CommitHookChangesArgs) (params
 	return params.ErrorResults{Results: res}, nil
 }
 
-func (u *UniterAPI) commitHookChangesForOneUnit(unitTag names.UnitTag, changes params.CommitHookChangesArg, canAccessUnit, canAccessApp common.AuthFunc) error {
+func (u *UniterAPI) commitHookChangesForOneUnit(unitTag names.UnitTag, changes params.CommitHookChangesArg, canAccessUnit, canAccessApp common.AuthFunc) (err error) {
 	unit, err := u.getUnit(unitTag)
 	if err != nil {
 		return errors.Trace(err)

caas/kubernetes/provider/k8s.go

@@ -2171,7 +2171,7 @@ func (k *kubernetesClient) Units(appName string, mode caas.DeploymentMode) ([]ca
 				fsInfo, err = k.volumeInfoForEmptyDir(vol, volMount, now)
 			} else {
 				// Ignore volumes which are not Juju managed filesystems.
-				logger.Debugf("Ignoring blank EmptyDir, PersistentVolumeClaim or ClaimName")
+				logger.Debugf("ignoring blank EmptyDir, PersistentVolumeClaim or ClaimName")
 				continue
 			}
 			if err != nil {

caas/kubernetes/provider/metadata.go

@@ -127,7 +127,7 @@ func (k *kubernetesClient) GetClusterMetadata(storageClass string) (*caas.Cluste
 			return nil, errors.Trace(err)
 		}
 		if err == nil {
-			logger.Debugf("Use %q for nominated storage class", sc.Name)
+			logger.Debugf("use %q for nominated storage class", sc.Name)
 			result.NominatedStorageClass = toCaaSStorageProvisioner(*sc)
 		}
 	}
@@ -147,7 +147,7 @@ func (k *kubernetesClient) GetClusterMetadata(storageClass string) (*caas.Cluste
 		}
 
 		if k8sannotations.New(sc.GetAnnotations()).Has(operatorStorageClassAnnotationKey, "true") {
-			logger.Debugf("Use %q with annotations %v for operator storage class", sc.Name, sc.GetAnnotations())
+			logger.Debugf("use %q with annotations %v for operator storage class", sc.Name, sc.GetAnnotations())
 			result.OperatorStorageClass = maybeStorage
 		} else if hasPreferredOperatorStorage {
 			err := storageClassMatches(preferredOperatorStorage, maybeStorage)
@@ -159,7 +159,7 @@ func (k *kubernetesClient) GetClusterMetadata(storageClass string) (*caas.Cluste
 				// Prefer operator storage from the default storage class.
 				result.OperatorStorageClass = maybeStorage
 				logger.Debugf(
-					"Use the default Storage class %q for operator storage class because it also matches Juju preferred config %v",
+					"use the default Storage class %q for operator storage class because it also matches Juju preferred config %v",
 					maybeStorage.Name, preferredOperatorStorage,
 				)
 			} else {
@@ -174,12 +174,12 @@ func (k *kubernetesClient) GetClusterMetadata(storageClass string) (*caas.Cluste
 		}
 
 		if k8sannotations.New(sc.GetAnnotations()).Has(workloadStorageClassAnnotationKey, "true") {
-			logger.Debugf("Use %q with annotations %v for nominated storage class", sc.Name, sc.GetAnnotations())
+			logger.Debugf("use %q with annotations %v for nominated storage class", sc.Name, sc.GetAnnotations())
 			result.NominatedStorageClass = maybeStorage
 		} else if isDefaultStorageClass(sc) {
 			// no nominated storage class specified, so use the default one;
 			result.NominatedStorageClass = maybeStorage
-			logger.Debugf("Use the default Storage class %q for nominated storage class", maybeStorage.Name)
+			logger.Debugf("use the default Storage class %q for nominated storage class", maybeStorage.Name)
 		} else {
 			possibleWorkloadStorage = append(possibleWorkloadStorage, maybeStorage)
 		}
@@ -196,18 +196,18 @@ func (k *kubernetesClient) GetClusterMetadata(storageClass string) (*caas.Cluste
 
 	if result.OperatorStorageClass == nil && len(possibleOperatorStorage) > 0 {
 		result.OperatorStorageClass = possibleOperatorStorage[0]
-		logger.Debugf("Use %q for operator storage class", possibleOperatorStorage[0].Name)
+		logger.Debugf("use %q for operator storage class", possibleOperatorStorage[0].Name)
 	}
 	// Even if no storage class was marked as default for the cluster, if there's only
 	// one of them, use it for workload storage.
 	if result.NominatedStorageClass == nil && len(possibleWorkloadStorage) == 1 {
 		result.NominatedStorageClass = possibleWorkloadStorage[0]
-		logger.Debugf("Use %q for nominated storage class", possibleWorkloadStorage[0].Name)
+		logger.Debugf("use %q for nominated storage class", possibleWorkloadStorage[0].Name)
 	}
 	if result.OperatorStorageClass == nil && result.NominatedStorageClass != nil {
 		// use workload storage class if no operator storage class preference found.
 		result.OperatorStorageClass = result.NominatedStorageClass
-		logger.Debugf("Use nominated storage class %q for operator storage class", result.NominatedStorageClass.Name)
+		logger.Debugf("use nominated storage class %q for operator storage class", result.NominatedStorageClass.Name)
 	}
 	return &result, nil
 }

state/podspec_ops.go

@@ -7,15 +7,12 @@ import (
 	"github.com/juju/charm/v7"
 	"github.com/juju/errors"
 	"github.com/juju/names/v4"
-	"github.com/kr/pretty"
 	"gopkg.in/mgo.v2/bson"
 	"gopkg.in/mgo.v2/txn"
 
 	"github.com/juju/juju/core/leadership"
 )
 
-var logPodSpec = logger.Child("podspec")
-
 type setPodSpecOperation struct {
 	m       *CAASModel
 	appTag  names.ApplicationTag
@@ -132,7 +129,6 @@ func (op *setPodSpecOperation) buildTxn(_ int) ([]txn.Op, error) {
 		}
 		sop.Assert = asserts
 		sop.Update = updates
-		logPodSpec.Criticalf("newSetPodSpecOperation err == nil buildTxn -> %s", pretty.Sprint(sop))
 	} else if errors.IsNotFound(err) {
 		sop.Assert = txn.DocMissing
 		newDoc := containerSpecDoc{}
@@ -143,7 +139,6 @@ func (op *setPodSpecOperation) buildTxn(_ int) ([]txn.Op, error) {
 			newDoc.RawSpec = *op.rawSpec
 		}
 		sop.Insert = newDoc
-		logPodSpec.Criticalf("newSetPodSpecOperation errors.IsNotFound(err) buildTxn -> %s", pretty.Sprint(sop))
 	} else {
 		return nil, errors.Annotate(err, "setting pod spec")
 	}

version/version.go

@@ -21,7 +21,7 @@ import (
 // The presence and format of this constant is very important.
 // The debian/rules build recipe uses this value for the version
 // number of the release package.
-const version = "2.8-rc16"
+const version = "2.8-rc1"
 
 const (
 	// TreeStateDirty when the build was made with a dirty checkout.

worker/caasoperator/caasoperator.go

@@ -301,6 +301,7 @@ func (op *caasOperator) makeAgentSymlinks(unitTag names.UnitTag) error {
 		return errors.Trace(err)
 	}
 	if isUnitCharmDirSymlink {
+		op.config.Logger.Warningf("removing legacy charm symlink for %q", unitTag.String())
 		if err := os.Remove(unitCharmDir); err != nil {
 			return errors.Trace(err)
 		}

worker/caasoperator/caasoperator_test.go

@@ -20,6 +20,7 @@ import (
 	jc "github.com/juju/testing/checkers"
 	"github.com/juju/utils"
 	"github.com/juju/utils/arch"
+	"github.com/juju/utils/symlink"
 	"github.com/juju/version"
 	"github.com/juju/worker/v2"
 	"github.com/juju/worker/v2/workertest"
@@ -434,6 +435,36 @@ func (s *WorkerSuite) TestRemovedApplication(c *gc.C) {
 }
 
 func (s *WorkerSuite) TestMakeAgentSymlinks(c *gc.C) {
+	w, err := caasoperator.NewWorker(s.config)
+	c.Assert(err, jc.ErrorIsNil)
+	defer workertest.CleanKill(c, w)
+
+	unitTag := names.NewUnitTag("gitlab/0")
+	op := w.(*caasoperator.CaasOperator)
+	unitDir := filepath.Join(op.GetDataDir(), "agents", unitTag.String())
+	err = os.MkdirAll(unitDir, 0755)
+	c.Assert(err, jc.ErrorIsNil)
+
+	unitCharmLegacySymlink := filepath.Join(unitDir, "charm")
+	fakeAppDir := c.MkDir()
+	err = symlink.New(fakeAppDir, unitCharmLegacySymlink)
+	c.Assert(err, jc.ErrorIsNil)
+	assertSymlinkExist(c, unitCharmLegacySymlink)
+
+	err = op.MakeAgentSymlinks(unitTag)
+	c.Assert(err, jc.ErrorIsNil)
+	assertSymlinkNotExist(c, unitCharmLegacySymlink)
+}
+
+func assertSymlinkExist(c *gc.C, path string) {
+	symlinkExists, err := symlink.IsSymlink(path)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(symlinkExists, jc.IsTrue)
+}
+
+func assertSymlinkNotExist(c *gc.C, path string) {
+	_, err := symlink.IsSymlink(path)
+	c.Assert(errors.Cause(err), jc.Satisfies, os.IsNotExist)
 }
 
 func (s *WorkerSuite) TestContainerStart(c *gc.C) {

worker/caasoperator/export_test.go

@@ -7,8 +7,23 @@ import (
 	"github.com/juju/names/v4"
 )
 
-var GetNewRunnerExecutor = getNewRunnerExecutor
+var (
+	GetNewRunnerExecutor = getNewRunnerExecutor
+	JujudSymlinks        = jujudSymlinks
+)
+
+type (
+	CaasOperator = caasOperator
+)
 
 func (op *caasOperator) MakeAgentSymlinks(unitTag names.UnitTag) error {
 	return op.makeAgentSymlinks(unitTag)
 }
+
+func (op *caasOperator) GetDataDir() string {
+	return op.config.DataDir
+}
+
+func (op *caasOperator) GetToolsDir() string {
+	return op.paths.GetToolsDir()
+}

worker/uniter/runner/context/context.go

@@ -18,7 +18,6 @@ import (
 	"github.com/juju/loggo"
 	"github.com/juju/names/v4"
 	"github.com/juju/proxy"
-	"github.com/kr/pretty"
 
 	"github.com/juju/juju/api/base"
 	"github.com/juju/juju/api/uniter"
@@ -1133,7 +1132,6 @@ func (ctx *HookContext) doFlush(process string) error {
 	commitReq, numChanges := b.Build()
 	if numChanges > 0 {
 		if err := ctx.unit.CommitHookChanges(commitReq); err != nil {
-			logger.Criticalf("CommitHookChanges commitReq -> %s", pretty.Sprint(commitReq))
 			err = errors.Annotatef(err, "cannot apply changes")
 			logger.Errorf("%v", err)
 			return errors.Trace(err)
@@ -1149,7 +1147,6 @@ func (ctx *HookContext) doFlush(process string) error {
 // we'll still trigger a change to a counter on the podspec so that we can
 // ensure any other charm changes (eg storage) are acted on.
 func (ctx *HookContext) addCommitHookChangesForCAAS(builder *uniter.CommitHookParamsBuilder, process string) error {
-	logger.Criticalf("addCommitHookChangesForCAAS ctx.podSpecYaml -> %v, ctx.k8sRawSpecYaml -> %v, hooks.UpgradeCharm -> %q", ctx.podSpecYaml, ctx.k8sRawSpecYaml, string(hooks.UpgradeCharm))
 	if ctx.podSpecYaml == nil && ctx.k8sRawSpecYaml == nil {
 		// No ops for any situation unless any k8s spec needs to be set.
 		// The "upgrade-charm" hook always runs with non nil k8s spec for the leader but with nil k8s spec for non leaders.