Merge pull request juju#18605 from SimonRichardson/store-new-revisions

juju#18605 Once the charm revision worker has run, we need to store the new revisions. These revisions need to contain all the essential metadata so that when the charm is swapped to the new one, it can be downloaded with all the correct data. Effectively, the charm is deployed in all circumstances. Reworking the charmrevisioner to remove the charm URL and just using the charm locator removes a needless encoding of the charm URL. The architecture is tricky because it should just be the same as the existing application, but there is no way to verify that using the response. We just have to trust the API. ----- This pull request includes several changes to the `charmrevisioner` worker to improve the handling of charm revisions and their metadata. The most important changes include the addition of a new method to reserve charm revisions, updates to the application state to include a new identifier, and enhancements to the worker's revision update process. ### Enhancements to charm revision handling: * [`domain/application/service/charm.go`](diffhunk://#diff-f634e196c8c372b1bc36ad4d6db384d26713e39a92b90c981816480a2d1e91d6R889-R899): Added a new method `ReserveCharmRevision` to reserve a charm revision for a given charm ID, returning warnings if there are non-blocking issues. * [`internal/worker/charmrevisioner/worker.go`](diffhunk://#diff-8bd3a975a51af8c1205ea4861c5103f7097a13c3b4c4b1849da3d20739524374R67-R71): Updated the `ApplicationService` interface and the `revisionUpdateWorker` to include the `ReserveCharmRevision` method and handle new charm revision storage. Added methods to store new revisions and handle telemetry configuration. [[1]](diffhunk://#diff-8bd3a975a51af8c1205ea4861c5103f7097a13c3b4c4b1849da3d20739524374R67-R71) [[2]](diffhunk://#diff-8bd3a975a51af8c1205ea4861c5103f7097a13c3b4c4b1849da3d20739524374R236-R259) [[3]](diffhunk://#diff-8bd3a975a51af8c1205ea4861c5103f7097a13c3b4c4b1849da3d20739524374L277-L297) [[4]](diffhunk://#diff-8bd3a975a51af8c1205ea4861c5103f7097a13c3b4c4b1849da3d20739524374L313-R350) [[5]](diffhunk://#diff-8bd3a975a51af8c1205ea4861c5103f7097a13c3b4c4b1849da3d20739524374R393-R416) [[6]](diffhunk://#diff-8bd3a975a51af8c1205ea4861c5103f7097a13c3b4c4b1849da3d20739524374R452-R524) [[7]](diffhunk://#diff-8bd3a975a51af8c1205ea4861c5103f7097a13c3b4c4b1849da3d20739524374R587-R603) ### Updates to application state: * [`domain/application/state/application.go`](diffhunk://#diff-994df01f80f2f62fa018f73b1f15b583e059dd830bda4c524f4824df8764bb08R2178): Added a new field `CharmhubIdentifier` to the `Origin` struct to store the charmhub identifier. * [`domain/application/state/types.go`](diffhunk://#diff-1129494fcd03573fb4e70eca0f2e6bd6d86cb7ccdace444e6cd1a5effb6795e1R664): Added the `CharmhubIdentifier` field to the `revisionUpdaterApplication` struct. * [`domain/schema/model/sql/0025-revision-updater.sql`](diffhunk://#diff-aab4704c2a1516a3dc1522d668e07ba0e245252ccd404567832a62682ea8bb68L13-R19): Updated the SQL schema to include the `charmhub_identifier` field in the `v_revision_updater_application_unit` view. ### Test and mock updates: * [`internal/worker/charmrevisioner/package_mocks_test.go`](diffhunk://#diff-1775e1c26124f986647875f18015714002ff3d4b44d6067a63396d802d7b6211R290-R329): Added mocks for the `ReserveCharmRevision` method. * [`internal/worker/charmrevisioner/worker_test.go`](diffhunk://#diff-a339279173bc60eecdeecd5d28d248bb3d4f575e332f1cbf60af2793059b1b4bR18-R19): Updated tests to include the new `ReserveCharmRevision` method and added a utility function `ptr`. [[1]](diffhunk://#diff-a339279173bc60eecdeecd5d28d248bb3d4f575e332f1cbf60af2793059b1b4bR18-R19) [[2]](diffhunk://#diff-a339279173bc60eecdeecd5d28d248bb3d4f575e332f1cbf60af2793059b1b4bL29-R31) [[3]](diffhunk://#diff-a339279173bc60eecdeecd5d28d248bb3d4f575e332f1cbf60af2793059b1b4bR52-R53) [[4]](diffhunk://#diff-23db611b4cb037ae2785eee165720f96713ed880b4312cebbfc3c0b98fb498d9R22-R25) These changes collectively enhance the charm revision handling capabilities and improve the robustness of the `charmrevisioner` worker. ## QA steps Apply the following diff: ``` diff --git a/cmd/jujud-controller/agent/machine.go b/cmd/jujud-controller/agent/machine.go index 3f9ebeca0a..baa329e557 100644 --- a/cmd/jujud-controller/agent/machine.go +++ b/cmd/jujud-controller/agent/machine.go @@ -980,7 +980,7 @@ func (a *MachineAgent) startModelWorkers(cfg modelworkermanager.NewModelConfig) Clock: clock.WallClock, LoggingContext: loggingContext, RunFlagDuration: time.Minute, - CharmRevisionUpdateInterval: 24 prdesc time.Hour, + CharmRevisionUpdateInterval: 30 prdesc time.Second, StatusHistoryPrunerInterval: 5 prdesc time.Minute, ActionPrunerInterval: 24 prdesc time.Hour, NewEnvironFunc: newEnvirons, ``` ``` $ juju bootstrap lxd test $ juju add-model default $ juju deploy ubuntu --revision=24 --channel=stable $ juju ssh -m controller 0 $ sudo /var/lib/juju/tools/machine-0/jujud db-repl --machine-id 0 repl (controller)> .switch model-controller repl (model-controller)> SELECT prdesc FROM charm uuid archive_path object_store_uuid available version lxd_profile source_id revision architecture_id reference_name 328b315e-df08-4d7e-8ab8-a5ec252f92e7 dbvXVgKNSL6JQ1LTyMQ/Rw 9050925d-2dc1-4d14-81cc-7c15abb078e7 true [] 1 117 0 juju-controller repl (model-controller)> .switch model-default repl (model-default)> SELECT prdesc FROM charm uuid archive_path object_store_uuid available version lxd_profile source_id revision architecture_id reference_name 5e135085-c4c2-49ce-8632-1e1d5a6c4052 1nmQUJneSDSLSz8ah8cCew cd3e18c0-2d08-441b-893d-e16b63367384 true [] 1 24 0 ubuntu repl (model-default)> SELECT prdesc FROM charm uuid archive_path object_store_uuid available version lxd_profile source_id revision architecture_id reference_name 5e135085-c4c2-49ce-8632-1e1d5a6c4052 1nmQUJneSDSLSz8ah8cCew cd3e18c0-2d08-441b-893d-e16b63367384 true [] 1 24 0 ubuntu repl (model-default)> SELECT prdesc FROM charm uuid archive_path object_store_uuid available version lxd_profile source_id revision architecture_id reference_name 5e135085-c4c2-49ce-8632-1e1d5a6c4052 1nmQUJneSDSLSz8ah8cCew cd3e18c0-2d08-441b-893d-e16b63367384 true [] 1 24 0 ubuntu repl (model-default)> SELECT prdesc FROM charm uuid archive_path object_store_uuid available version lxd_profile source_id revision architecture_id reference_name 5e135085-c4c2-49ce-8632-1e1d5a6c4052 1nmQUJneSDSLSz8ah8cCew cd3e18c0-2d08-441b-893d-e16b63367384 true [] 1 24 0 ubuntu 81a123be-9268-472b-8795-b077323d9dd7 <nil> false [] 1 25 0 ubuntu repl (model-default)> SELECT prdesc FROM charm_hash charm_uuid hash_kind_id hash 5e135085-c4c2-49ce-8632-1e1d5a6c4052 0 9b2877f7ebf04b348cf40ace32fdfc6d1cc0157dea7fd6740c4ba74e0e201e5b 81a123be-9268-472b-8795-b077323d9dd7 0 75e3bf60b79f9902cae89cf242c6e197d9744b2d25bd5835f7101b77913f0e10 repl (model-default)> SELECT prdesc FROM charm_download_info charm_uuid provenance_id charmhub_identifier download_url download_size 5e135085-c4c2-49ce-8632-1e1d5a6c4052 0 DksXQKAQTZfsUmBAGanZAhpoS4dpmXel https://api.charmhub.io/api/v1/charms/download/DksXQKAQTZfsUmBAGanZAhpoS4dpmXel_24.charm 2139977 81a123be-9268-472b-8795-b077323d9dd7 0 DksXQKAQTZfsUmBAGanZAhpoS4dpmXel https://api.charmhub.io/api/v1/charms/download/DksXQKAQTZfsUmBAGanZAhpoS4dpmXel_25.charm 5227288 repl (model-default)> SELECT prdesc FROM charm_hash charm_uuid hash_kind_id hash 5e135085-c4c2-49ce-8632-1e1d5a6c4052 0 9b2877f7ebf04b348cf40ace32fdfc6d1cc0157dea7fd6740c4ba74e0e201e5b 81a123be-9268-472b-8795-b077323d9dd7 0 75e3bf60b79f9902cae89cf242c6e197d9744b2d25bd5835f7101b77913f0e10 repl (model-default)> SELECT prdesc FROM charm_metadata charm_uuid name description summary subordinate min_juju_version run_as_id assumes 5e135085-c4c2-49ce-8632-1e1d5a6c4052 ubuntu This simply deploys the Ubuntu Cloud/Server image A pristine Ubuntu Server false 0.0.0 0 [] 81a123be-9268-472b-8795-b077323d9dd7 ubuntu This simply deploys the Ubuntu Cloud/Server image A pristine Ubuntu Server false 0.0.0 0 [] repl (model-default)> SELECT prdesc FROM charm_hash charm_uuid hash_kind_id hash 5e135085-c4c2-49ce-8632-1e1d5a6c4052 0 9b2877f7ebf04b348cf40ace32fdfc6d1cc0157dea7fd6740c4ba74e0e201e5b 81a123be-9268-472b-8795-b077323d9dd7 0 75e3bf60b79f9902cae89cf242c6e197d9744b2d25bd5835f7101b77913f0e10 repl (model-default)> SELECT prdesc FROM charm uuid archive_path object_store_uuid available version lxd_profile source_id revision architecture_id reference_name 5e135085-c4c2-49ce-8632-1e1d5a6c4052 1nmQUJneSDSLSz8ah8cCew cd3e18c0-2d08-441b-893d-e16b63367384 true [] 1 24 0 ubuntu 81a123be-9268-472b-8795-b077323d9dd7 <nil> false [] 1 25 0 ubuntu ``` ## Links **Jira card:** [ JUJU-7279](https://warthogs.atlassian.net/browse/JUJU-7279)
manadart · Jan 10, 2025 · 942e5e7 · 942e5e7
2 parents ae6b415 + 5afb9c1
commit 942e5e7
Show file tree

Hide file tree

Showing 12 changed files with 584 additions and 104 deletions.
diff --git a/domain/application/charm/types.go b/domain/application/charm/types.go
@@ -97,6 +97,34 @@ type SetCharmArgs struct {
 	RequiresSequencing bool
 }
 
+// ReserveCharmRevisionArgs holds the arguments for the ReserveCharmRevision method.
+type ReserveCharmRevisionArgs struct {
+	// Charm is the charm to set.
+	Charm internalcharm.Charm
+	// Source is the source of the charm.
+	Source charm.Source
+	// ReferenceName is the given name of the charm that is stored in the
+	// persistent storage. The proxy name should either be the application name
+	// or the charm metadata name.
+	//
+	// The name of a charm can differ from the charm name stored in the metadata
+	// in the cases where the application name is selected by the user. In order
+	// to select that charm again via the name, we need to use the proxy name to
+	// locate it. You can't go via the application and select it via the
+	// application name, as no application might be referencing it at that
+	// specific revision. The only way to then locate the charm directly via the
+	// name is use the proxy name.
+	ReferenceName string
+	// Revision is the revision of the charm.
+	Revision int
+	// Hash is the sha256 of the charm.
+	Hash string
+	// Architecture is the architecture of the charm.
+	Architecture arch.Arch
+	// DownloadInfo holds the information needed to download a charmhub charm.
+	DownloadInfo *DownloadInfo
+}
+
 // ResolveUploadCharm holds the arguments for the ResolveUploadCharm method.
 type ResolveUploadCharm struct {
 	// Name is the name of the charm.

diff --git a/domain/application/service/charm.go b/domain/application/service/charm.go
@@ -886,6 +886,31 @@ func (s *Service) setCharm(ctx context.Context, args charm.SetCharmArgs) (setCha
 	}, warnings, nil
 }
 
+// ReserveCharmRevision creates a new charm revision placeholder in state. This
+// includes all the metadata, actions, config and manifest for the charm. The
+// charm revision placeholder will include all the associated hash, and download
+// information for the charm. Once the new charm revision place holder is linked
+// to an application, the async charm downloader will download the charm archive
+// and set the charm as available.
+//
+// If there are any non-blocking issues with the charm metadata, actions, config
+// or manifest, a set of warnings will be returned.
+func (s *Service) ReserveCharmRevision(ctx context.Context, args charm.ReserveCharmRevisionArgs) (corecharm.ID, []string, error) {
+	result, warnings, err := s.setCharm(ctx, charm.SetCharmArgs{
+		Charm:         args.Charm,
+		Source:        args.Source,
+		ReferenceName: args.ReferenceName,
+		Hash:          args.Hash,
+		Revision:      args.Revision,
+		Architecture:  args.Architecture,
+		DownloadInfo:  args.DownloadInfo,
+	})
+	if err != nil {
+		return "", nil, errors.Trace(err)
+	}
+	return result.ID, warnings, nil
+}
+
 // WatchCharms returns a watcher that observes changes to charms.
 func (s *WatchableService) WatchCharms() (watcher.StringsWatcher, error) {
 	return s.watcherFactory.NewUUIDsWatcher(

diff --git a/domain/application/service/charm_test.go b/domain/application/service/charm_test.go
@@ -1659,6 +1659,54 @@ func (s *charmServiceSuite) TestResolveUploadCharmLocalCharmImportingFailedResol
 	c.Assert(err, jc.ErrorIs, errors.NotValid)
 }
 
+func (s *charmServiceSuite) TestReserveCharmRevision(c *gc.C) {
+	defer s.setupMocks(c).Finish()
+
+	metadata := &internalcharm.Meta{
+		Name: "foo",
+	}
+	manifest := &internalcharm.Manifest{
+		Bases: []internalcharm.Base{{
+			Name:          "ubuntu",
+			Channel:       internalcharm.Channel{Risk: internalcharm.Beta},
+			Architectures: []string{"arm64"},
+		}},
+	}
+	config := &internalcharm.Config{}
+	actions := &internalcharm.Actions{}
+	lxdProfile := &internalcharm.LXDProfile{}
+
+	downloadInfo := &charm.DownloadInfo{
+		Provenance:         charm.ProvenanceDownload,
+		CharmhubIdentifier: "foo",
+		DownloadURL:        "http://example.com/foo",
+		DownloadSize:       42,
+	}
+
+	charmBase := internalcharm.NewCharmBase(metadata, manifest, config, actions, lxdProfile)
+	ch, _, err := encodeCharm(charmBase)
+	c.Assert(err, jc.ErrorIsNil)
+
+	ch.Source = charm.CharmHubSource
+	ch.ReferenceName = "foo"
+	ch.Revision = 1
+	ch.Hash = "hash"
+	ch.Architecture = architecture.AMD64
+
+	s.state.EXPECT().SetCharm(gomock.Any(), ch, downloadInfo, false).Return(corecharm.ID("id"), charm.CharmLocator{}, nil)
+
+	_, _, err = s.service.ReserveCharmRevision(context.Background(), charm.ReserveCharmRevisionArgs{
+		Charm:         charmBase,
+		Source:        corecharm.CharmHub,
+		ReferenceName: "foo",
+		Revision:      1,
+		Hash:          "hash",
+		Architecture:  arch.AMD64,
+		DownloadInfo:  downloadInfo,
+	})
+	c.Assert(err, jc.ErrorIsNil)
+}
+
 type watchableServiceSuite struct {
 	baseSuite
 

diff --git a/domain/application/state/application.go b/domain/application/state/application.go
@@ -2175,6 +2175,7 @@ FROM v_revision_updater_application_unit
 				Architecture: charmArch,
 			},
 			Origin: application.Origin{
+				ID:       r.CharmhubIdentifier,
 				Revision: r.Revision,
 				Channel: application.Channel{
 					Track:  r.ChannelTrack,

diff --git a/domain/application/state/application_test.go b/domain/application/state/application_test.go
@@ -2430,6 +2430,7 @@ func (s *applicationStateSuite) TestGetApplicationsForRevisionUpdater(c *gc.C) {
 				Architecture: architecture.ARM64,
 			},
 			Revision: 42,
+			ID:       "ident",
 		},
 		NumUnits: 0,
 	}, {
@@ -2452,6 +2453,7 @@ func (s *applicationStateSuite) TestGetApplicationsForRevisionUpdater(c *gc.C) {
 				Architecture: architecture.ARM64,
 			},
 			Revision: 42,
+			ID:       "ident",
 		},
 		NumUnits: 1,
 	}})

diff --git a/domain/application/state/types.go b/domain/application/state/types.go
@@ -661,6 +661,7 @@ type revisionUpdaterApplication struct {
 	PlatformOSID           sql.NullInt64 `db:"platform_os_id"`
 	PlatformChannel        string        `db:"platform_channel"`
 	PlatformArchitectureID sql.NullInt64 `db:"platform_architecture_id"`
+	CharmhubIdentifier     string        `db:"charmhub_identifier"`
 }
 
 type revisionUpdaterApplicationNumUnits struct {

diff --git a/domain/schema/model/sql/0025-revision-updater.sql b/domain/schema/model/sql/0025-revision-updater.sql
@@ -10,11 +10,13 @@ SELECT
     ac.branch AS channel_branch,
     ap.os_id AS platform_os_id,
     ap.channel AS platform_channel,
-    ap.architecture_id AS platform_architecture_id
+    ap.architecture_id AS platform_architecture_id,
+    cdi.charmhub_identifier
 FROM application AS a
 LEFT JOIN charm AS c ON a.charm_uuid = c.uuid
 LEFT JOIN application_channel AS ac ON a.uuid = ac.application_uuid
 LEFT JOIN application_platform AS ap ON a.uuid = ap.application_uuid
+LEFT JOIN charm_download_info AS cdi ON c.uuid = cdi.charm_uuid
 WHERE a.life_id = 0 AND c.source_id = 1;
 
 CREATE VIEW v_revision_updater_application_unit AS

diff --git a/internal/worker/charmrevisioner/package_mocks_test.go b/internal/worker/charmrevisioner/package_mocks_test.go
diff --git a/internal/worker/charmrevisioner/package_test.go b/internal/worker/charmrevisioner/package_test.go
@@ -19,3 +19,7 @@ func TestPackage(t *testing.T) {
 
 	gc.TestingT(t)
 }
+
+func ptr[T any](v T) *T {
+	return &v
+}