Skip to content

Commit

Permalink
Merge pull request juju#16510 from jack-w-shaw/remove_some_easy_win_c…
Browse files Browse the repository at this point in the history
…harmurls

juju#16510

Use a string param for OpenCharm instead of a parsed url

Use the charm origin source instead of charm url schema to determine the source of charms in metrics debug

## 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
- [x ] [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

```
$ juju deploy ubuntu
$ juju deploy ./testcharms/charms/metered
(wait)
$ juju set-meter-status ubuntu RED
ERROR not a local charm
$ juju set-meter-status metered RED
$ juju debug-log
...
unit-metered-1: 13:07:10 INFO unit.unit-metered-1.juju-log meter status changed
unit-metered-1: 13:07:10 INFO juju.worker.meterstatus ran "meter-status-changed" hook (via explicit, bespoke hook script)
```

```
./main.sh -v metrics
```
  • Loading branch information
jujubot authored Oct 27, 2023
2 parents 91275e8 + 9339df6 commit cdfc0d9
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 38 deletions.
15 changes: 5 additions & 10 deletions api/client/charms/downloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"io"
"net/url"

"github.com/juju/charm/v11"
"github.com/juju/errors"

"github.com/juju/juju/api/base"
Expand All @@ -21,15 +20,11 @@ import (
func NewCharmDownloader(apiCaller base.APICaller) *downloader.Downloader {
dlr := &downloader.Downloader{
OpenBlob: func(req downloader.Request) (io.ReadCloser, error) {
curl, err := charm.ParseURL(req.URL.String())
if err != nil {
return nil, errors.Annotate(err, "did not receive a valid charm URL")
}
streamer, err := NewCharmOpener(apiCaller)
if err != nil {
return nil, errors.Trace(err)
}
reader, err := streamer.OpenCharm(curl)
reader, err := streamer.OpenCharm(req.URL.String())
if err != nil {
return nil, errors.Trace(err)
}
Expand All @@ -41,15 +36,15 @@ func NewCharmDownloader(apiCaller base.APICaller) *downloader.Downloader {

// CharmOpener provides the OpenCharm method.
type CharmOpener interface {
OpenCharm(curl *charm.URL) (io.ReadCloser, error)
OpenCharm(curl string) (io.ReadCloser, error)
}

type charmOpener struct {
ctx context.Context
httpClient http.HTTPDoer
}

func (s *charmOpener) OpenCharm(curl *charm.URL) (io.ReadCloser, error) {
func (s *charmOpener) OpenCharm(curl string) (io.ReadCloser, error) {
uri, query := openCharmArgs(curl)
return http.OpenURI(s.ctx, s.httpClient, uri, query)
}
Expand All @@ -66,9 +61,9 @@ func NewCharmOpener(apiConn base.APICaller) (CharmOpener, error) {
}, nil
}

func openCharmArgs(curl *charm.URL) (string, url.Values) {
func openCharmArgs(curl string) (string, url.Values) {
query := make(url.Values)
query.Add("url", curl.String())
query.Add("url", curl)
query.Add("file", "*")
return "/charms", query
}
4 changes: 1 addition & 3 deletions api/client/charms/downloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"net/http"
"strings"

"github.com/juju/charm/v11"
jc "github.com/juju/testing/checkers"
"go.uber.org/mock/gomock"
gc "gopkg.in/check.v1"
Expand Down Expand Up @@ -54,8 +53,7 @@ func (s *charmDownloaderSuite) TestCharmOpener(c *gc.C) {

opener, err := charms.NewCharmOpener(mockCaller)
c.Assert(err, jc.ErrorIsNil)
curl := charm.MustParseURL("ch:mycharm")
reader, err := opener.OpenCharm(curl)
reader, err := opener.OpenCharm("ch:mycharm")

defer reader.Close()
c.Assert(err, jc.ErrorIsNil)
Expand Down
17 changes: 5 additions & 12 deletions apiserver/facades/client/metricsdebug/metricsdebug.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,12 @@ func (api *MetricsDebugAPI) setEntityMeterStatus(entity names.Tag, status state.
if err != nil {
return errors.Trace(err)
}
chURLStr := unit.CharmURL()
if chURLStr == nil {
return errors.New("no charm url")
}
chURL, err := charm.ParseURL(*chURLStr)
app, err := unit.Application()
if err != nil {
return errors.Trace(err)
}
if !charm.Local.Matches(chURL.Schema) {
source := app.CharmOrigin().Source
if !charm.Local.Matches(source) {
return errors.New("not a local charm")
}
err = unit.SetMeterStatus(status.Code.String(), status.Info)
Expand All @@ -199,12 +196,8 @@ func (api *MetricsDebugAPI) setEntityMeterStatus(entity names.Tag, status state.
if err != nil {
return errors.Trace(err)
}
cURL, _ := application.CharmURL()
curl, err := charm.ParseURL(*cURL)
if err != nil {
return errors.Trace(err)
}
if !charm.Local.Matches(curl.Schema) {
source := application.CharmOrigin().Source
if !charm.Local.Matches(source) {
return errors.New("not a local charm")
}
units, err := application.AllUnits()
Expand Down
14 changes: 6 additions & 8 deletions migration/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func ImportModel(importer StateImporter, getClaimer ClaimerFunc, bytes []byte) (
// CharmDownloader defines a single method that is used to download a
// charm from the source controller in a migration.
type CharmDownloader interface {
OpenCharm(*charm.URL) (io.ReadCloser, error)
OpenCharm(string) (io.ReadCloser, error)
}

// CharmUploader defines a single method that is used to upload a
Expand Down Expand Up @@ -225,13 +225,7 @@ func uploadCharms(config UploadBinariesConfig) error {

for _, charmURL := range config.Charms {
logger.Debugf("sending charm %s to target", charmURL)

curl, err := charm.ParseURL(charmURL)
if err != nil {
return errors.Annotate(err, "bad charm URL")
}

reader, err := config.CharmDownloader.OpenCharm(curl)
reader, err := config.CharmDownloader.OpenCharm(charmURL)
if err != nil {
return errors.Annotate(err, "cannot open charm")
}
Expand All @@ -243,6 +237,10 @@ func uploadCharms(config UploadBinariesConfig) error {
}
defer cleanup()

curl, err := charm.ParseURL(charmURL)
if err != nil {
return errors.Annotate(err, "bad charm URL")
}
if usedCurl, err := config.CharmUploader.UploadCharm(curl, content); err != nil {
return errors.Annotate(err, "cannot upload charm")
} else if usedCurl.String() != curl.String() {
Expand Down
7 changes: 3 additions & 4 deletions migration/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,10 @@ type fakeDownloader struct {
resources []string
}

func (d *fakeDownloader) OpenCharm(curl *charm.URL) (io.ReadCloser, error) {
urlStr := curl.String()
d.charms = append(d.charms, urlStr)
func (d *fakeDownloader) OpenCharm(curl string) (io.ReadCloser, error) {
d.charms = append(d.charms, curl)
// Return the charm URL string as the fake charm content
return io.NopCloser(bytes.NewReader([]byte(urlStr + " content"))), nil
return io.NopCloser(bytes.NewReader([]byte(curl + " content"))), nil
}

func (d *fakeDownloader) OpenURI(uri string, query url.Values) (io.ReadCloser, error) {
Expand Down
2 changes: 1 addition & 1 deletion testcharms/charms/metered/hooks/collect-metrics
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
i#!/bin/bash
#!/bin/bash
juju-log -l INFO "collect-metrics hook called."
2 changes: 2 additions & 0 deletions testcharms/charms/metered/hooks/meter-status-changed
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
#!/bin/bash
juju-log -l INFO "meter status changed"

0 comments on commit cdfc0d9

Please sign in to comment.