Upgrades: Ensure we have an empty manifest from state

In order to have better determination of when a charm meta data format is either v1 or v2, it helps if we don't have charms that have empty manifests. Empty manifests with empty bases means the logic for determine if it's a format v2 now has to check for the presence of values rather than checking for nil. Lots of code to just say `version: 2` in a metadata.
SimonRichardson · Jul 20, 2021 · fe61f9e · fe61f9e
1 parent c53038c
commit fe61f9e
Show file tree

Hide file tree

Showing 7 changed files with 167 additions and 0 deletions.
diff --git a/state/upgrades.go b/state/upgrades.go
@@ -3609,3 +3609,35 @@ func AddSpawnedTaskCountToOperations(pool *StatePool) error {
 	}
 	return nil
 }
+
+func TransformEmptyManifestsToNil(pool *StatePool) error {
+	return errors.Trace(runForAllModelStates(pool, func(st *State) error {
+		col, closer := st.db().GetCollection(charmsC)
+		defer closer()
+
+		var docs []charmDoc
+		if err := col.Find(nil).All(&docs); err != nil {
+			return errors.Trace(err)
+		}
+
+		var ops []txn.Op
+		for _, doc := range docs {
+			if doc.Manifest == nil || len(doc.Manifest.Bases) == 0 {
+				ops = append(ops, txn.Op{
+					C:      charmsC,
+					Id:     doc.DocID,
+					Assert: txn.DocExists,
+					Update: bson.D{{
+						"$unset", bson.D{{
+							"manifest", nil,
+						}},
+					}},
+				})
+			}
+		}
+		if len(ops) > 0 {
+			return errors.Trace(st.db().RunTransaction(ops))
+		}
+		return nil
+	}))
+}
diff --git a/state/upgrades_test.go b/state/upgrades_test.go
@@ -5325,6 +5325,91 @@ func (s *upgradesSuite) TestAddSpawnedTaskCountToOperations(c *gc.C) {
 	)
 }
 
+func (s *upgradesSuite) TestTransformEmptyManifestsToNil(c *gc.C) {
+	model1 := s.makeModel(c, "model-1", coretesting.Attrs{})
+	model2 := s.makeModel(c, "model-2", coretesting.Attrs{})
+	defer func() {
+		_ = model1.Close()
+		_ = model2.Close()
+	}()
+
+	uuid1 := model1.ModelUUID()
+	uuid2 := model2.ModelUUID()
+
+	coll, closer := s.state.db().GetRawCollection(charmsC)
+	defer closer()
+
+	err := coll.Insert(bson.M{
+		"_id":        ensureModelUUID(uuid1, "charm1"),
+		"model-uuid": uuid1,
+		"url":        charm.MustParseURL("cs:test").String(),
+		"manifest":   nil,
+	})
+	c.Assert(err, jc.ErrorIsNil)
+	err = coll.Insert(bson.M{
+		"_id":        ensureModelUUID(uuid2, "charm2"),
+		"model-uuid": uuid2,
+		"url":        charm.MustParseURL("local:test").String(),
+		"manifest":   &charm.Manifest{},
+	})
+	c.Assert(err, jc.ErrorIsNil)
+	err = coll.Insert(bson.M{
+		"_id":        ensureModelUUID(uuid2, "charm3"),
+		"model-uuid": uuid2,
+		"url":        charm.MustParseURL("ch:test").String(),
+		"manifest": &charm.Manifest{
+			Bases: []charm.Base{},
+		},
+	})
+	c.Assert(err, jc.ErrorIsNil)
+	err = coll.Insert(bson.M{
+		"_id":        ensureModelUUID(uuid1, "charm4"),
+		"model-uuid": uuid1,
+		"url":        charm.MustParseURL("ch:test2").String(),
+		"manifest": &charm.Manifest{
+			Bases: []charm.Base{
+				{Name: "ubuntu"},
+			},
+		},
+	})
+	c.Assert(err, jc.ErrorIsNil)
+
+	expected := bsonMById{
+		{
+			"_id":        ensureModelUUID(uuid1, "charm1"),
+			"model-uuid": uuid1,
+			"url":        "cs:test",
+		},
+		{
+			"_id":        ensureModelUUID(uuid2, "charm2"),
+			"model-uuid": uuid2,
+			"url":        "local:test",
+		},
+		{
+			"_id":        ensureModelUUID(uuid2, "charm3"),
+			"model-uuid": uuid2,
+			"url":        "ch:test",
+		},
+		{
+			"_id":        ensureModelUUID(uuid1, "charm4"),
+			"model-uuid": uuid1,
+			"url":        "ch:test2",
+			"manifest": bson.M{
+				"bases": []interface{}{
+					bson.M{
+						"name": "ubuntu",
+					},
+				},
+			},
+		},
+	}
+
+	sort.Sort(expected)
+	s.assertUpgradedData(c, TransformEmptyManifestsToNil,
+		upgradedData(coll, expected),
+	)
+}
+
 type docById []bson.M
 
 func (d docById) Len() int           { return len(d) }

diff --git a/upgrades/backend.go b/upgrades/backend.go
@@ -96,6 +96,7 @@ type StateBackend interface {
 	UpdateDHCPAddressConfigs() error
 	KubernetesInClusterCredentialSpec() (environscloudspec.CloudSpec, *config.Config, string, error)
 	AddSpawnedTaskCountToOperations() error
+	TransformEmptyManifestsToNil() error
 }
 
 // Model is an interface providing access to the details of a model within the
@@ -409,3 +410,7 @@ func (s stateBackend) KubernetesInClusterCredentialSpec() (
 func (s stateBackend) AddSpawnedTaskCountToOperations() error {
 	return state.AddSpawnedTaskCountToOperations(s.pool)
 }
+
+func (s stateBackend) TransformEmptyManifestsToNil() error {
+	return state.TransformEmptyManifestsToNil(s.pool)
+}
diff --git a/upgrades/operations.go b/upgrades/operations.go
@@ -49,6 +49,7 @@ var stateUpgradeOperations = func() []Operation {
 		upgradeToVersion{version.MustParse("2.9.5"), stateStepsFor295()},
 		upgradeToVersion{version.MustParse("2.9.6"), stateStepsFor296()},
 		upgradeToVersion{version.MustParse("2.9.9"), stateStepsFor299()},
+		upgradeToVersion{version.MustParse("2.9.10"), stateStepsFor2910()},
 	}
 	return steps
 }

diff --git a/upgrades/steps_2910.go b/upgrades/steps_2910.go
@@ -0,0 +1,17 @@
+// Copyright 2021 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package upgrades
+
+// stateStepsFor2910 returns upgrade steps for juju 2.9.10
+func stateStepsFor2910() []Step {
+	return []Step{
+		&upgradeStep{
+			description: `transform empty manifests to nil`,
+			targets:     []Target{DatabaseMaster},
+			run: func(context Context) error {
+				return context.State().TransformEmptyManifestsToNil()
+			},
+		},
+	}
+}
diff --git a/upgrades/steps_2910_test.go b/upgrades/steps_2910_test.go
@@ -0,0 +1,26 @@
+// Copyright 2021 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package upgrades_test
+
+import (
+	jc "github.com/juju/testing/checkers"
+	"github.com/juju/version/v2"
+	gc "gopkg.in/check.v1"
+
+	"github.com/juju/juju/testing"
+	"github.com/juju/juju/upgrades"
+)
+
+var v2910 = version.MustParse("2.9.10")
+
+type steps2910Suite struct {
+	testing.BaseSuite
+}
+
+var _ = gc.Suite(&steps2910Suite{})
+
+func (s *steps2910Suite) TestTransformEmptyManifestsToNil(c *gc.C) {
+	step := findStateStep(c, v2910, `transform empty manifests to nil`)
+	c.Assert(step.Targets(), jc.DeepEquals, []upgrades.Target{upgrades.DatabaseMaster})
+}
diff --git a/upgrades/upgrade_test.go b/upgrades/upgrade_test.go
@@ -639,6 +639,7 @@ func (s *upgradeSuite) TestStateUpgradeOperationsVersions(c *gc.C) {
 		"2.9.5",
 		"2.9.6",
 		"2.9.9",
+		"2.9.10",
 	})
 }