Skip to content

Commit

Permalink
Merge pull request juju#18726 from gfouillet/v4/dqlite/resources/wire…
Browse files Browse the repository at this point in the history
…up/6401-revision-worker

juju#18726

> [!WARNING]
> Include commits from juju#18654 
>
> prdesc [x] Wait for merge juju#18654 and rebase

This Pull request introduce the support of resources updates:

* It add the required logic to insert `potential` resources on application creation
* It wire up the revision worker to fetch updates and update revision on potential resources in state.

## Checklist

- [X] Code style: imports ordered, good names, simple structure, etc
- [X] Comments saying why design decisions were made
- [X] Go unit tests, with comments saying what you're testing
- [ ] [Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing
- [ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages

## QA steps

### setup

We need to make the revision worker pass more often that once a day (unless you are really patient). Apply the following diff:

```diff
diff --git a/cmd/jujud-controller/agent/machine.go b/cmd/jujud-controller/agent/machine.go
index f552f6d596..cf6c70cb30 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,
 NewEnvironFunc: newEnvirons,
 NewContainerBrokerFunc: newCAASBroker,
 NewMigrationMaster: migrationmaster.NewWorker,

```
Build.

Create and populate controller

```sh
juju bootstrap lxd lxd
juju add-model m
juju deploy juju-qa-test qa1 --resource foo-file=1
juju deploy juju-qa-test qa2 --resource foo-file=test.txt
juju deploy juju-qa-test qa3 --resource foo-file=2
```

### Assert after a while (at least 30s)

```sh
juju resources qa1
juju resources qa2
juju resources qa3
```

Expected for qa1

```sh
Resource Supplied by Revision
foo-file store 1

[Updates Available]
Resource Revision
foo-file 2
```

Expected for qa2

```sh
Resource Supplied by Revision
foo-file upload 2025-01-29T16:27
```

Expected for qa3

```sh
Resource Supplied by Revision
foo-file store 2
```




## Documentation changes

None

## Links

**Jira card:** [JUJU\-6401](https://warthogs.atlassian.net/browse/JUJU-6401)
  • Loading branch information
jujubot authored Feb 3, 2025
2 parents b65dd7e + 7ff1d5c commit 8268dac
Show file tree
Hide file tree
Showing 18 changed files with 751 additions and 79 deletions.
26 changes: 18 additions & 8 deletions domain/application/service/charm.go
Original file line number Diff line number Diff line change
Expand Up @@ -932,14 +932,10 @@ func (s *Service) setCharm(ctx context.Context, args charm.SetCharmArgs) (setCha
ch.Architecture = architecture

charmID, locator, err := s.st.SetCharm(ctx, ch, args.DownloadInfo, args.RequiresSequencing)
if err != nil {
return setCharmResult{}, warnings, errors.Trace(err)
}

return setCharmResult{
ID: charmID,
Locator: locator,
}, warnings, nil
}, warnings, errors.Trace(err)
}

// ReserveCharmRevision creates a new charm revision placeholder in state. This
Expand All @@ -951,6 +947,9 @@ func (s *Service) setCharm(ctx context.Context, args charm.SetCharmArgs) (setCha
//
// If there are any non-blocking issues with the charm metadata, actions, config
// or manifest, a set of warnings will be returned.
//
// If the placeholder already exists, it is a noop but still return the
// corresponding charm ID.
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,
Expand All @@ -961,10 +960,21 @@ func (s *Service) ReserveCharmRevision(ctx context.Context, args charm.ReserveCh
Architecture: args.Architecture,
DownloadInfo: args.DownloadInfo,
})
if err != nil {
return "", nil, errors.Trace(err)
if errors.Is(err, applicationerrors.CharmAlreadyExists) {
var charmSource charm.CharmSource
switch args.Source {
case corecharm.CharmHub:
charmSource = charm.CharmHubSource
case corecharm.Local:
charmSource = charm.LocalSource
default:
return "", nil, applicationerrors.CharmSourceNotValid
}

// retrieve the charm ID
result.ID, err = s.st.GetCharmID(ctx, args.ReferenceName, args.Revision, charmSource)
}
return result.ID, warnings, nil
return result.ID, warnings, errors.Trace(err)
}

// GetLatestPendingCharmhubCharm returns the latest charm that is pending from
Expand Down
68 changes: 68 additions & 0 deletions domain/application/service/charm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1798,6 +1798,74 @@ func (s *charmServiceSuite) TestReserveCharmRevision(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
}

func (s *charmServiceSuite) TestReserveCharmRevisionAlreadyExists(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{}

charmBase := internalcharm.NewCharmBase(metadata, manifest, config, actions, lxdProfile)

// We don't check the expected, we only care about the result
s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return("id", charm.CharmLocator{},
applicationerrors.CharmAlreadyExists)
s.state.EXPECT().GetCharmID(gomock.Any(), charmBase.Meta().Name, 42, charm.LocalSource).Return("id", nil)

id, _, err := s.service.ReserveCharmRevision(context.Background(), charm.ReserveCharmRevisionArgs{
// We only define required fields to pass code before SetCharm call
Charm: charmBase,
Source: corecharm.Local,
ReferenceName: "foo",
Revision: 42,
})
c.Assert(id, gc.Equals, corecharm.ID("id"))
c.Assert(err, jc.ErrorIsNil)
}

func (s *charmServiceSuite) TestReserveCharmRevisionAlreadyExistsGetCharmIdError(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{}

charmBase := internalcharm.NewCharmBase(metadata, manifest, config, actions, lxdProfile)
expectedError := errors.New("boom")

// We don't check the expected, we only care about the result
s.state.EXPECT().SetCharm(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return("id", charm.CharmLocator{},
applicationerrors.CharmAlreadyExists)
s.state.EXPECT().GetCharmID(gomock.Any(), charmBase.Meta().Name, 42, charm.LocalSource).Return("", expectedError)

_, _, err := s.service.ReserveCharmRevision(context.Background(), charm.ReserveCharmRevisionArgs{
// We only define required fields to pass code before SetCharm call
Charm: charmBase,
Source: corecharm.Local,
ReferenceName: "foo",
Revision: 42,
})
c.Assert(err, jc.ErrorIs, expectedError)
}

func (s *charmServiceSuite) TestGetLatestPendingCharmhubCharm(c *gc.C) {
defer s.setupMocks(c).Finish()

Expand Down
7 changes: 6 additions & 1 deletion domain/application/state/application.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,12 @@ func (st *State) CreateApplication(
if err := tx.Query(ctx, createScaleStmt, scaleInfo).Run(); err != nil {
return errors.Errorf("inserting scale row for application %q: %w", name, err)
}
if err := st.insertResources(ctx, tx, appDetails, args.Resources); err != nil {
if err := st.insertResources(ctx, tx, insertResourcesArgs{
appID: appDetails.UUID,
charmUUID: appDetails.CharmID,
charmSource: args.Charm.Source,
appResources: args.Resources,
}); err != nil {
return errors.Errorf("inserting resources for application %q: %w", name, err)
}
if err := st.insertApplicationConfig(ctx, tx, appDetails.UUID, args.Config); err != nil {
Expand Down
50 changes: 43 additions & 7 deletions domain/application/state/application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,9 @@ FROM application
WHERE name=?`, "666").Scan(&appUUID, &charmUUID)
})
var (
foundCharmResources []charm.Resource
foundAppResources []application.AddApplicationResourceArg
foundCharmResources []charm.Resource
foundAppAvailableResources []application.AddApplicationResourceArg
foundAppPotentialResources []application.AddApplicationResourceArg
)
assertTxn("Fetch charm resources", func(ctx context.Context, tx *sql.Tx) error {
rows, err := tx.QueryContext(ctx, `
Expand All @@ -426,11 +427,36 @@ WHERE charm_uuid=?`, charmUUID)
}
return nil
})
assertTxn("Fetch application resources", func(ctx context.Context, tx *sql.Tx) error {
assertTxn("Fetch application available resources", func(ctx context.Context, tx *sql.Tx) error {
rows, err := tx.QueryContext(ctx, `
SELECT vr.name, revision, origin_type
FROM v_resource vr
WHERE application_uuid = ?`, appUUID)
WHERE application_uuid = ?
AND state = 'available'`, appUUID)
if err != nil {
return errors.Capture(err)
}
defer rows.Close()
for rows.Next() {
var res application.AddApplicationResourceArg
var originName string
if err := rows.Scan(&res.Name, &res.Revision, &originName); err != nil {
return errors.Capture(err)
}
if res.Origin, err = charmresource.ParseOrigin(originName); err != nil {
return errors.Capture(err)
}
foundAppAvailableResources = append(foundAppAvailableResources, res)
}
return nil
})

assertTxn("Fetch application potential resources", func(ctx context.Context, tx *sql.Tx) error {
rows, err := tx.QueryContext(ctx, `
SELECT vr.name, revision, origin_type
FROM v_resource vr
WHERE application_uuid = ?
AND state = 'potential'`, appUUID)
if err != nil {
return errors.Capture(err)
}
Expand All @@ -444,14 +470,24 @@ WHERE application_uuid = ?`, appUUID)
if res.Origin, err = charmresource.ParseOrigin(originName); err != nil {
return errors.Capture(err)
}
foundAppResources = append(foundAppResources, res)
foundAppPotentialResources = append(foundAppPotentialResources, res)
}
return nil
})
c.Check(foundCharmResources, jc.SameContents, slices.Collect(maps.Values(charmResources)),
gc.Commentf("(Assert) mismatch between charm resources and inserted resources"))
c.Check(foundAppResources, jc.SameContents, addResourcesArgs,
gc.Commentf("(Assert) mismatch between app resources and inserted resources"))
c.Check(foundAppAvailableResources, jc.SameContents, addResourcesArgs,
gc.Commentf("(Assert) mismatch between app available app resources and inserted resources"))
expectedPotentialResources := make([]application.AddApplicationResourceArg, 0, len(addResourcesArgs))
for _, res := range addResourcesArgs {
expectedPotentialResources = append(expectedPotentialResources, application.AddApplicationResourceArg{
Name: res.Name,
Revision: nil, // nil revision
Origin: charmresource.OriginStore, // origin should always be store
})
}
c.Check(foundAppPotentialResources, jc.SameContents, expectedPotentialResources,
gc.Commentf("(Assert) mismatch between potential app resources and inserted resources"))
}

// TestCreateApplicationWithExistingCharmWithResources ensures that two
Expand Down
70 changes: 57 additions & 13 deletions domain/application/state/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ import (

"github.com/canonical/sqlair"

coreapplication "github.com/juju/juju/core/application"
coreresource "github.com/juju/juju/core/resource"
"github.com/juju/juju/domain/application"
"github.com/juju/juju/domain/application/charm"
charmresource "github.com/juju/juju/internal/charm/resource"
"github.com/juju/juju/internal/errors"
)

Expand All @@ -19,34 +22,75 @@ import (
// creation.
func (st *State) buildResourcesToAdd(
charmUUID string,
charmSource charm.CharmSource,
appResources []application.AddApplicationResourceArg,
) ([]resourceToAdd, error) {
resources := make([]resourceToAdd, 0, len(appResources))
var resources []resourceToAdd
now := st.clock.Now()
for _, r := range appResources {
// Available resources are resources actually available for use to the
// related application.
uuid, err := coreresource.NewUUID()
if err != nil {
return nil, errors.Capture(err)
}
resources = append(resources, resourceToAdd{
UUID: uuid.String(),
CharmUUID: charmUUID,
Name: r.Name,
Revision: r.Revision,
Origin: r.Origin.String(),
State: coreresource.StateAvailable.String(),
CreatedAt: now,
})
resources = append(resources,
resourceToAdd{
UUID: uuid.String(),
CharmUUID: charmUUID,
Name: r.Name,
Revision: r.Revision,
Origin: r.Origin.String(),
State: coreresource.StateAvailable.String(),
CreatedAt: now,
})
if charmSource != charm.CharmHubSource {
continue
}
// Potential resources are possible updates from charm hub for resource
// linked to the application.
//
// In the case of charm from charm hub, juju will regularly fetch the
// repository to find out if there is newer revision for each resource.
// Those resources are defined in the state as "potential" resources,
// and we need to create them "empty" (with no revision) at the creation
// of the application.
//
// - They are updated by the CharmRevisionWorker, which check charmhub and
// updates the charmUUID, revision and polled_at field in the state.
// - They are used by resources facade to be compared with actual
// resources and provides information on potential updates.
uuid, err = coreresource.NewUUID()
if err != nil {
return nil, errors.Capture(err)
}
resources = append(resources,
resourceToAdd{
UUID: uuid.String(),
CharmUUID: charmUUID,
Name: r.Name,
Revision: nil, // No revision yet
Origin: charmresource.OriginStore.String(),
State: coreresource.StatePotential.String(),
CreatedAt: now,
})
}
return resources, nil
}

type insertResourcesArgs struct {
appID coreapplication.ID
charmUUID string
charmSource charm.CharmSource
appResources []application.AddApplicationResourceArg
}

// insertResources constructs a transaction to insert resources into the
// database. It returns a function which, when executed, inserts resources and
// links them to applications.
func (st *State) insertResources(ctx context.Context, tx *sqlair.TX, appDetails applicationDetails, appResources []application.AddApplicationResourceArg) error {
func (st *State) insertResources(ctx context.Context, tx *sqlair.TX, args insertResourcesArgs) error {

resources, err := st.buildResourcesToAdd(appDetails.CharmID, appResources)
resources, err := st.buildResourcesToAdd(args.charmUUID, args.charmSource, args.appResources)
if err != nil {
return errors.Capture(err)
}
Expand Down Expand Up @@ -79,7 +123,7 @@ VALUES ($linkResourceApplication.*)`, linkResourceApplication{})
}

// Insert resources
appUUID := appDetails.UUID.String()
appUUID := args.appID.String()
for _, res := range resources {
// Insert the resource.
if err = tx.Query(ctx, insertStmt, res).Run(); err != nil {
Expand Down
7 changes: 7 additions & 0 deletions domain/application/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ WHERE uuid = $charmID.uuid;
return nil
}

// checkCharmReferenceExists checks if a charm with the given reference name and
// revision exists in the database.
//
// - If the charm doesn't exists, it returns an empty id and no error
// - If the charm exists, it returns the id and the error
// [applicationerrors.CharmAlreadyExists]
// - Any other error are returned if the check fails
func (s *State) checkCharmReferenceExists(ctx context.Context, tx *sqlair.TX, referenceName string, revision int) (corecharm.ID, error) {
selectQuery := `
SELECT &charmID.*
Expand Down
4 changes: 4 additions & 0 deletions domain/resource/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ const (
// being operated on does not exist.
ApplicationNotFound = errors.ConstError("application not found")

// CharmIDNotValid describes an error when the charm ID is
// not valid.
CharmIDNotValid = errors.ConstError("charm ID not valid")

// ArgumentNotValid describes an error that occurs when an argument to
// the service is invalid.
ArgumentNotValid = errors.ConstError("argument not valid")
Expand Down
19 changes: 11 additions & 8 deletions domain/resource/service/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -514,29 +514,32 @@ func (s *Service) SetApplicationResource(
return nil
}

// SetRepositoryResources sets the "polled" resource for the application to
// the provided values. These are resource collected from the repository for
// the application.
// SetRepositoryResources updates the last available revision of resources
// from charm repository for a specific application.
//
// The following error types can be expected to be returned:
// - [coreerrors.NotValid] is returned if the Application ID is not valid.
// - [resourceerrors.ArgumentNotValid] is returned if LastPolled is zero.
// - [resourceerrors.ArgumentNotValid] is returned if the length of Info is zero.
// - [resourceerrors.ApplicationIDNotValid] is returned if the Application ID is not valid.
// - [resourceerrors.CharmIDNotValid] is returned if the charm ID is not valid.
// - [resourceerrors.ArgumentNotValid] is returned if LastPolled is zero,
// if the length of Info is zero or if any info are invalid.
// - [resourceerrors.ApplicationNotFound] if the specified application does
// not exist.
func (s *Service) SetRepositoryResources(
ctx context.Context,
args resource.SetRepositoryResourcesArgs,
) error {
if err := args.ApplicationID.Validate(); err != nil {
return errors.Errorf("application id: %w", err)
return errors.Errorf("%w: %w", resourceerrors.ApplicationIDNotValid, err)
}
if err := args.CharmID.Validate(); err != nil {
return errors.Errorf("%w: %w", resourceerrors.CharmIDNotValid, err)
}
if len(args.Info) == 0 {
return errors.Errorf("empty Info: %w", resourceerrors.ArgumentNotValid)
}
for _, info := range args.Info {
if err := info.Validate(); err != nil {
return errors.Errorf("resource: %w", err)
return errors.Errorf("%w: resource: %w", resourceerrors.ArgumentNotValid, err)
}
}
if args.LastPolled.IsZero() {
Expand Down
Loading

0 comments on commit 8268dac

Please sign in to comment.