Skip to content

Commit

Permalink
InstanceKey for Origins.
Browse files Browse the repository at this point in the history 
Related to charmhub KPIs and LP: 1944582. Provide a stable, unique InstanceKey
when using the refresh action for charmhub. The easiest one is to combine the
model UUID and the application name.

Setup for most calls in GetCharmURLOrigin, to be used when creating a
RefreshConfig down in refreshOne.  All client initiated refresh actions end up
there.  However the data required isn't easily available.

The charmrevisionupdater refresh action is put together in a facade and
has easy access to the model UUID and application name.

For compatibilty with older clients, if no InstanceKey is provided to
RefreshOne, create a uuid.

Do not add to state's origin.
  • Loading branch information
@hmlanigan
hmlanigan committed Oct 5, 2021
1 parent 1f7d67f commit 3212461
Show file tree
Hide file tree
Showing 15 changed files with 139 additions and 77 deletions.
10 changes: 10 additions & 0 deletions api/common/charm/charmorigin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ type Origin struct {
OS string
// Series describes the series of the OS intended to be used by the charm.
Series string

// InstanceKey is a unique string associated with the application. To
// assist with keeping KPI data in charmhub, it must be the same for every
// charmhub Refresh action related to an application. Create with the
// charmhub.CreateInstanceKey method. LP: 1944582
InstanceKey string
}

// WithSeries allows to update the series of an origin.
Expand Down Expand Up @@ -83,6 +89,7 @@ func (o Origin) ParamsCharmOrigin() params.CharmOrigin {
Architecture: o.Architecture,
OS: o.OS,
Series: o.Series,
InstanceKey: o.InstanceKey,
}
}

Expand Down Expand Up @@ -111,6 +118,7 @@ func (o Origin) CoreCharmOrigin() corecharm.Origin {
OS: o.OS,
Series: o.Series,
},
InstanceKey: o.InstanceKey,
}
}

Expand All @@ -128,6 +136,7 @@ func APICharmOrigin(origin params.CharmOrigin) Origin {
Architecture: origin.Architecture,
OS: origin.OS,
Series: origin.Series,
InstanceKey: origin.InstanceKey,
}
}

Expand All @@ -153,5 +162,6 @@ func CoreCharmOrigin(origin corecharm.Origin) Origin {
Architecture: origin.Platform.Architecture,
OS: origin.Platform.OS,
Series: origin.Platform.Series,
InstanceKey: origin.InstanceKey,
}
}
1 change: 1 addition & 0 deletions apiserver/facades/client/application/application.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1489,6 +1489,7 @@ func (api *APIBase) GetCharmURLOrigin(args params.ApplicationGet) (params.CharmU
return result, nil
}
result.Origin = makeParamsCharmOrigin(chOrigin)
result.Origin.InstanceKey = charmhub.CreateInstanceKey(api.model.UUID(), args.ApplicationName)
return result, nil
}

Expand Down
5 changes: 4 additions & 1 deletion apiserver/facades/client/application/application_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/juju/juju/apiserver/facades/client/application"
"github.com/juju/juju/apiserver/params"
apiservertesting "github.com/juju/juju/apiserver/testing"
"github.com/juju/juju/charmhub"
"github.com/juju/juju/core/arch"
"github.com/juju/juju/core/constraints"
"github.com/juju/juju/core/instance"
Expand Down Expand Up @@ -985,12 +986,13 @@ func (s *applicationSuite) TestApplicationGetCharmURLOrigin(c *gc.C) {
Series: "focal",
},
}
s.AddTestingApplicationWithOrigin(c, "wordpress", ch, &expectedOrigin)
app := s.AddTestingApplicationWithOrigin(c, "wordpress", ch, &expectedOrigin)
result, err := s.applicationAPI.GetCharmURLOrigin(params.ApplicationGet{ApplicationName: "wordpress"})
c.Assert(err, jc.ErrorIsNil)
c.Assert(result.Error, gc.IsNil)
c.Assert(result.URL, gc.Equals, "local:quantal/wordpress-3")
latest := "latest"

c.Assert(result.Origin, jc.DeepEquals, params.CharmOrigin{
Source: "local",
Risk: "stable",
Expand All @@ -999,6 +1001,7 @@ func (s *applicationSuite) TestApplicationGetCharmURLOrigin(c *gc.C) {
Architecture: "amd64",
OS: "ubuntu",
Series: "focal",
InstanceKey: charmhub.CreateInstanceKey(s.Model.UUID(), app.Name()),
})
}

Expand Down
1 change: 1 addition & 0 deletions apiserver/facades/client/application/backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ type Model interface {
Owner() names.UserTag
Tag() names.Tag
Type() state.ModelType
UUID() string
ModelConfig() (*config.Config, error)
AgentVersion() (version.Number, error)
OpenedPortRangesForMachine(string) (state.MachinePortRanges, error)
Expand Down
17 changes: 9 additions & 8 deletions apiserver/facades/client/charms/charmhubrepo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,14 @@ func (c *chRepo) ResolveWithPreferredChannel(curl *charm.URL, origin corecharm.O
// Only charms being upgraded will have an ID and Hash. Those values should
// only ever be updated in FindDownloadURL.
resOrigin := corecharm.Origin{
Source: origin.Source,
ID: origin.ID,
Hash: origin.Hash,
Type: string(res.Entity.Type),
Channel: &channel,
Revision: &revision,
Platform: input.Platform,
Source: origin.Source,
ID: origin.ID,
Hash: origin.Hash,
Type: string(res.Entity.Type),
Channel: &channel,
Revision: &revision,
Platform: input.Platform,
InstanceKey: origin.InstanceKey,
}

outputOrigin, err := sanitizeCharmOrigin(resOrigin, origin)
Expand Down Expand Up @@ -464,7 +465,7 @@ func refreshConfig(curl *charm.URL, origin corecharm.Origin) (charmhub.RefreshCo
case MethodID:
// This must be a charm upgrade if we have an ID. Use the refresh
// action for metric keeping on the CharmHub side.
cfg, err = charmhub.RefreshOne(origin.ID, rev, channel, base)
cfg, err = charmhub.RefreshOne(origin.InstanceKey, origin.ID, rev, channel, base)
default:
return nil, errors.NotValidf("origin %v", origin)
}
Expand Down
12 changes: 8 additions & 4 deletions apiserver/facades/client/charms/charmhubrepo_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (s *charmHubRepositoriesSuite) TestResolveForDeploy(c *gc.C) {

func (s *charmHubRepositoriesSuite) TestResolveForUpgrade(c *gc.C) {
defer s.setupMocks(c).Finish()
cfg, err := charmhub.RefreshOne("charmCHARMcharmCHARMcharmCHARM01", 16, "latest/stable", charmhub.RefreshBase{
cfg, err := charmhub.RefreshOne("instance-key", "charmCHARMcharmCHARMcharmCHARM01", 16, "latest/stable", charmhub.RefreshBase{
Architecture: arch.DefaultArchitecture,
})
c.Assert(err, jc.ErrorIsNil)
Expand All @@ -60,6 +60,9 @@ func (s *charmHubRepositoriesSuite) testResolve(c *gc.C, id string) {
Series: "focal",
},
}
if id != "" {
origin.InstanceKey = "instance-key"
}

resolver := &chRepo{client: s.client}
obtainedCurl, obtainedOrigin, obtainedSeries, err := resolver.ResolveWithPreferredChannel(curl, origin)
Expand Down Expand Up @@ -521,9 +524,10 @@ func (refreshConfigSuite) TestRefreshByID(c *gc.C) {
curl := charm.MustParseURL("ch:wordpress")
platform := corecharm.MustParsePlatform("amd64/ubuntu/focal")
origin := corecharm.Origin{
ID: id,
Platform: platform,
Revision: &revision,
ID: id,
Platform: platform,
Revision: &revision,
InstanceKey: "instance-key",
}

cfg, err := refreshConfig(curl, origin)
Expand Down
2 changes: 2 additions & 0 deletions apiserver/facades/client/charms/conversions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func convertOrigin(origin corecharm.Origin) params.CharmOrigin {
Architecture: origin.Platform.Architecture,
OS: origin.Platform.OS,
Series: origin.Platform.Series,
InstanceKey: origin.InstanceKey,
}
}

Expand All @@ -53,5 +54,6 @@ func convertParamsOrigin(origin params.CharmOrigin) corecharm.Origin {
OS: origin.OS,
Series: origin.Series,
},
InstanceKey: origin.InstanceKey,
}
}
1 change: 1 addition & 0 deletions apiserver/facades/client/resources/facade.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,5 +312,6 @@ func convertParamsOrigin(origin params.CharmOrigin) corecharm.Origin {
OS: origin.OS,
Series: origin.Series,
},
InstanceKey: origin.InstanceKey,
}
}
10 changes: 8 additions & 2 deletions apiserver/facades/controller/charmrevisionupdater/charmhub.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ type charmhubID struct {
series string
arch string
metrics map[metrics.MetricKey]string
// Required for charmhub only. instanceKey is a unique string associated
// with the application. To assist with keeping KPI data in charmhub, it
// must be the same for every charmhub Refresh action related to an
// application. Create with the charmhub.CreateInstanceKey method.
// LP: 1944582
instanceKey string
}

// charmhubResult is the type charmhubLatestCharmInfo returns: information
Expand All @@ -47,15 +53,15 @@ type CharmhubRefreshClient interface {

// charmhubLatestCharmInfo fetches the latest information about the given
// charms from charmhub's "charm_refresh" API.
func charmhubLatestCharmInfo(client CharmhubRefreshClient, metrics map[metrics.MetricKey]map[metrics.MetricKey]string, ids []charmhubID) ([]charmhubResult, error) {
func charmhubLatestCharmInfo(client CharmhubRefreshClient, metrics map[metrics.MetricKey]map[metrics.MetricKey]string, ids []charmhubID, modelUUID string) ([]charmhubResult, error) {
cfgs := make([]charmhub.RefreshConfig, len(ids))
for i, id := range ids {
base := charmhub.RefreshBase{
Architecture: id.arch,
Name: id.os,
Channel: id.series,
}
cfg, err := charmhub.RefreshOne(id.id, id.revision, id.channel, base)
cfg, err := charmhub.RefreshOne(id.instanceKey, id.id, id.revision, id.channel, base)
if err != nil {
return nil, errors.Trace(err)
}
Expand Down
15 changes: 8 additions & 7 deletions apiserver/facades/controller/charmrevisionupdater/updater.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,13 @@ func (api *CharmRevisionUpdaterAPI) retrieveLatestCharmInfo() ([]latestCharmInfo
return nil, errors.Trace(err)
}
cid := charmhubID{
id: origin.ID,
revision: *origin.Revision,
channel: channel.String(),
os: strings.ToLower(origin.Platform.OS), // charmhub API requires lowercase OS name
series: origin.Platform.Series,
arch: origin.Platform.Architecture,
id: origin.ID,
revision: *origin.Revision,
channel: channel.String(),
os: strings.ToLower(origin.Platform.OS), // charmhub API requires lowercase OS key
series: origin.Platform.Series,
arch: origin.Platform.Architecture,
instanceKey: charmhub.CreateInstanceKey(cfg.UUID(), application.ApplicationTag().Name),
}
if telemetry {
cid.metrics = map[charmmetrics.MetricKey]string{
Expand Down Expand Up @@ -393,7 +394,7 @@ func (api *CharmRevisionUpdaterAPI) fetchCharmhubInfos(cfg *config.Config, ids [
if err != nil {
return nil, errors.Trace(err)
}
results, err := charmhubLatestCharmInfo(client, requestMetrics, ids)
results, err := charmhubLatestCharmInfo(client, requestMetrics, ids, cfg.UUID())
if err != nil {
return nil, errors.Trace(err)
}
Expand Down
9 changes: 9 additions & 0 deletions apiserver/facades/schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3166,6 +3166,9 @@
"id": {
"type": "string"
},
"instance-key": {
"type": "string"
},
"os": {
"type": "string"
},
Expand Down Expand Up @@ -15551,6 +15554,9 @@
"id": {
"type": "string"
},
"instance-key": {
"type": "string"
},
"os": {
"type": "string"
},
Expand Down Expand Up @@ -38998,6 +39004,9 @@
"id": {
"type": "string"
},
"instance-key": {
"type": "string"
},
"os": {
"type": "string"
},
Expand Down
6 changes: 6 additions & 0 deletions apiserver/params/applications.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ type CharmOrigin struct {
Architecture string `json:"architecture,omitempty"`
OS string `json:"os,omitempty"`
Series string `json:"series,omitempty"`

// InstanceKey is a unique string associated with the application. To
// assist with keeping KPI data in charmhub, it must be the same for every
// charmhub Refresh action related to an application. Create with the
// charmhub.CreateInstanceKey method. LP: 1944582
InstanceKey string `json:"instance-key,omitempty"`
}

// ApplicationDeploy holds the parameters for making the application Deploy
Expand Down
27 changes: 21 additions & 6 deletions charmhub/refresh.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,23 +174,38 @@ func (c refreshOne) String() string {
}

// RefreshOne creates a request config for requesting only one charm.
func RefreshOne(id string, revision int, channel string, base RefreshBase) (RefreshConfig, error) {
if err := validateBase(base); err != nil {
return nil, errors.Trace(err)
func RefreshOne(key, id string, revision int, channel string, base RefreshBase) (RefreshConfig, error) {
if key == "" {
// This is for compatibility reasons. With older clients, the
// key created in GetCharmURLOrigin will be lost to and from
// the client. Since a key is required, ensure we have one.
uuid, err := utils.NewUUID()
if err != nil {
return nil, errors.Trace(err)
}
key = uuid.String()
}
uuid, err := utils.NewUUID()
if err != nil {
if err := validateBase(base); err != nil {
return nil, errors.Trace(err)
}
return refreshOne{
instanceKey: uuid.String(),
instanceKey: key,
ID: id,
Revision: revision,
Channel: channel,
Base: base,
}, nil
}

// CreateInstanceKey creates an InstanceKey which can be unique and stable
// from Refresh action to Refresh action. Required for KPI collection
// on the charmhub side, see LP:1944582. Rather than saving in
// state, use the model uuid + the app name, which are unique. Modeled
// after the applicationDoc DocID and globalKey in state.
func CreateInstanceKey(uuid, appName string) string {
return uuid + ":a#" + appName
}

// Build a refresh request that can be past to the API.
func (c refreshOne) Build() (transport.RefreshRequest, Headers, error) {
base, err := constructRefreshBase(c.Base)
Expand Down
Loading

0 comments on commit 3212461

Please sign in to comment.