Skip to content

Commit

Permalink
Simplestreams: Thread through the context value
Browse files Browse the repository at this point in the history 
Once we start passing in the right context value we start to see tests
passing.

The idea is that we only ever create a NewDataSource using the factory
and we should remove as many as possible ones that just create
themselves on the fly.
  • Loading branch information
@SimonRichardson
SimonRichardson committed Jun 3, 2021
1 parent 47b7df6 commit 23c1fa0
Show file tree
Hide file tree
Showing 19 changed files with 108 additions and 63 deletions.
3 changes: 2 additions & 1 deletion apiserver/facades/agent/provisioner/provisioninginfo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,8 @@ func (api *ProvisionerAPI) imageMetadataFromState(constraint *imagemetadata.Imag

// imageMetadataFromDataSources finds image metadata that match specified criteria in existing data sources.
func (api *ProvisionerAPI) imageMetadataFromDataSources(env environs.Environ, constraint *imagemetadata.ImageConstraint) ([]params.CloudImageMetadata, error) {
sources, err := environs.ImageMetadataSources(env)
factory := simplestreams.DefaultDataSourceFactory()
sources, err := environs.ImageMetadataSources(env, factory)
if err != nil {
return nil, errors.Trace(err)
}
Expand Down
5 changes: 3 additions & 2 deletions cmd/plugins/juju-metadata/validateagentsmetadata.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ func (c *validateAgentsMetadataCommand) Init(args []string) error {
}

func (c *validateAgentsMetadataCommand) Run(context *cmd.Context) error {
ss := simplestreams.NewSimpleStreams(simplestreams.DefaultDataSourceFactory())

var params *simplestreams.MetadataLookupParams
if c.providerType == "" {
context.Infof("no provider type specified, using bootstrapped cloud")
Expand All @@ -168,7 +170,7 @@ func (c *validateAgentsMetadataCommand) Run(context *cmd.Context) error {
if err != nil {
return err
}
params.Sources, err = tools.GetMetadataSources(environ)
params.Sources, err = tools.GetMetadataSources(environ, ss)
if err != nil {
return err
}
Expand Down Expand Up @@ -207,7 +209,6 @@ func (c *validateAgentsMetadataCommand) Run(context *cmd.Context) error {
params.Endpoint = c.endpoint
}

ss := simplestreams.NewSimpleStreams(simplestreams.DefaultDataSourceFactory())
if c.metadataDir != "" {
if _, err := c.Filesystem().Stat(c.metadataDir); err != nil {
return err
Expand Down
10 changes: 6 additions & 4 deletions cmd/plugins/juju-metadata/validateimagemetadata.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ func (c *validateImageMetadataCommand) Run(context *cmd.Context) error {
}

func (c *validateImageMetadataCommand) createLookupParams(context *cmd.Context) (*simplestreams.MetadataLookupParams, error) {
ss := simplestreams.NewSimpleStreams(simplestreams.DefaultDataSourceFactory())

controllerName, err := c.ControllerName()
if err != nil {
return nil, errors.Trace(err)
Expand All @@ -198,7 +200,7 @@ func (c *validateImageMetadataCommand) createLookupParams(context *cmd.Context)
return nil, err
}
oes := &overrideEnvStream{environ, c.stream}
params.Sources, err = environs.ImageMetadataSources(oes)
params.Sources, err = environs.ImageMetadataSources(oes, ss)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -231,16 +233,16 @@ func (c *validateImageMetadataCommand) createLookupParams(context *cmd.Context)
if _, err := c.Filesystem().Stat(dir); err != nil {
return nil, err
}
params.Sources = imagesDataSources(dir)
params.Sources = imagesDataSources(ss, dir)
}
return params, nil
}

var imagesDataSources = func(urls ...string) []simplestreams.DataSource {
var imagesDataSources = func(ss *simplestreams.Simplestreams, urls ...string) []simplestreams.DataSource {
dataSources := make([]simplestreams.DataSource, len(urls))
publicKey, _ := simplestreams.UserPublicSigningKey()
for i, url := range urls {
dataSources[i] = simplestreams.NewDataSource(
dataSources[i] = ss.NewDataSource(
simplestreams.Config{
Description: "local metadata directory",
BaseURL: "file://" + url,
Expand Down
4 changes: 3 additions & 1 deletion cmd/plugins/juju-metadata/validateimagemetadata_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/juju/juju/environs/filestorage"
"github.com/juju/juju/environs/imagemetadata"
"github.com/juju/juju/environs/simplestreams"
sstestings "github.com/juju/juju/environs/simplestreams/testing"
"github.com/juju/juju/jujuclient"
"github.com/juju/juju/jujuclient/jujuclienttesting"
coretesting "github.com/juju/juju/testing"
Expand Down Expand Up @@ -256,7 +257,8 @@ func (s *ValidateImageMetadataSuite) TestOpenstackLocalMetadataNoMatch(c *gc.C)
}

func (s *ValidateImageMetadataSuite) TestImagesDataSourceHasKey(c *gc.C) {
ds := imagesDataSources("test.me")
ss := simplestreams.NewSimpleStreams(sstestings.TestDataSourceFactory())
ds := imagesDataSources(ss, "test.me")
// This data source does not require to contain signed data.
// However, it may still contain it.
// Since we will always try to read signed data first,
Expand Down
7 changes: 7 additions & 0 deletions core/context/key.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Copyright 2021 Canonical Ltd.
// Licensed under the AGPLv3, see LICENCE file for details.

package context

// ContextKey defines a type for selecting values within a given context.
type ContextKey string
41 changes: 21 additions & 20 deletions environs/bootstrap/bootstrap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,12 @@ import (

"github.com/juju/juju/api"
"github.com/juju/juju/cloud"
jujucloud "github.com/juju/juju/cloud"
"github.com/juju/juju/cloudconfig/instancecfg"
"github.com/juju/juju/cloudconfig/podcfg"
"github.com/juju/juju/controller"
"github.com/juju/juju/core/constraints"
corecontext "github.com/juju/juju/core/context"
"github.com/juju/juju/core/series"
coreseries "github.com/juju/juju/core/series"
"github.com/juju/juju/environs"
"github.com/juju/juju/environs/config"
"github.com/juju/juju/environs/context"
Expand All @@ -47,7 +46,7 @@ import (
const (
// SimplestreamsFetcherContextKey defines a way to change the simplestreams
// fetcher within a context.
SimplestreamsFetcherContextKey = "simplestreams-fetcher"
SimplestreamsFetcherContextKey corecontext.ContextKey = "simplestreams-fetcher"
)

const noToolsMessage = `Juju cannot bootstrap because no agent binaries are available for your model.
Expand Down Expand Up @@ -323,13 +322,22 @@ func bootstrapIAAS(
disableNetworkManagement, _ := cfg.DisableNetworkManagement()
logger.Debugf("network management by juju enabled: %v", !disableNetworkManagement)

var ss *simplestreams.Simplestreams
if value := ctx.Context().Value(SimplestreamsFetcherContextKey); value == nil {
ss = simplestreams.NewSimpleStreams(simplestreams.DefaultDataSourceFactory())
} else if s, ok := value.(*simplestreams.Simplestreams); ok {
ss = s
} else {
return errors.Errorf("expected a valid simple streams type")
}

// Set default tools metadata source, add image metadata source,
// then verify constraints. Providers may rely on image metadata
// for constraint validation.
var customImageMetadata []*imagemetadata.ImageMetadata
if args.MetadataDir != "" {
var err error
customImageMetadata, err = setPrivateMetadataSources(args.MetadataDir)
customImageMetadata, err = setPrivateMetadataSources(ss, args.MetadataDir)
if err != nil {
return errors.Trace(err)
}
Expand Down Expand Up @@ -359,7 +367,7 @@ func bootstrapIAAS(
}
}

requestedBootstrapSeries, err := coreseries.ValidateSeries(
requestedBootstrapSeries, err := series.ValidateSeries(
args.SupportedBootstrapSeries,
args.BootstrapSeries,
config.PreferredSeries(cfg),
Expand Down Expand Up @@ -389,6 +397,7 @@ func bootstrapIAAS(

ctx.Verbosef("Loading image metadata")
imageMetadata, err := bootstrapImageMetadata(environ,
ss,
bootstrapSeries,
bootstrapArchForImageSearch,
args.BootstrapImage,
Expand Down Expand Up @@ -461,16 +470,7 @@ func bootstrapIAAS(
}
ctx.Infof("Looking for %vpackaged Juju agent version %s for %s", latestPatchTxt, versionTxt, bootstrapArch)

var fetcher tools.SimplestreamsFetcher
if value := ctx.Context().Value(SimplestreamsFetcherContextKey); value == nil {
fetcher = simplestreams.NewSimpleStreams(simplestreams.DefaultDataSourceFactory())
} else if f, ok := value.(tools.SimplestreamsFetcher); ok {
fetcher = f
} else {
return errors.Errorf("expected a valid simple streams fetcher")
}

availableTools, err = findPackagedTools(environ, fetcher, args.AgentVersion, &bootstrapArch, bootstrapSeries)
availableTools, err = findPackagedTools(environ, ss, args.AgentVersion, &bootstrapArch, bootstrapSeries)
if err != nil && !errors.IsNotFound(err) {
return err
}
Expand Down Expand Up @@ -587,7 +587,7 @@ func bootstrapIAAS(
return errors.Trace(err)
}

osType := coreseries.DefaultOSTypeNameFromSeries(result.Series)
osType := series.DefaultOSTypeNameFromSeries(result.Series)
matchingTools, err := bootstrapParams.AvailableTools.Match(coretools.Filter{
Arch: result.Arch,
OSType: osType,
Expand Down Expand Up @@ -659,7 +659,7 @@ func Bootstrap(
ExtraAgentValuesForTesting: args.ExtraAgentValuesForTesting,
}
doBootstrap := bootstrapIAAS
if jujucloud.CloudIsCAAS(args.Cloud) {
if cloud.CloudIsCAAS(args.Cloud) {
doBootstrap = bootstrapCAAS
}

Expand Down Expand Up @@ -854,6 +854,7 @@ func userPublicSigningKey() (string, error) {
// state database will have the synthesised image metadata added to it.
func bootstrapImageMetadata(
environ environs.BootstrapEnviron,
dataSourceFactory simplestreams.DataSourceFactory,
bootstrapSeries *string,
bootstrapArch string,
bootstrapImageId string,
Expand Down Expand Up @@ -904,7 +905,7 @@ func bootstrapImageMetadata(
// For providers that support making use of simplestreams
// image metadata, search public image metadata. We need
// to pass this onto Bootstrap for selecting images.
sources, err := environs.ImageMetadataSources(environ)
sources, err := environs.ImageMetadataSources(environ, dataSourceFactory)
if err != nil {
return nil, errors.Trace(err)
}
Expand Down Expand Up @@ -1005,7 +1006,7 @@ func isCompatibleVersion(v1, v2 version.Number) bool {
// and adds an image metadata source after verifying the contents. If the
// directory ends in tools, only the default tools metadata source will be
// set. Same for images.
func setPrivateMetadataSources(metadataDir string) ([]*imagemetadata.ImageMetadata, error) {
func setPrivateMetadataSources(dataSourceFactory simplestreams.DataSourceFactory, metadataDir string) ([]*imagemetadata.ImageMetadata, error) {
if _, err := os.Stat(metadataDir); err != nil {
if !os.IsNotExist(err) {
return nil, errors.Annotate(err, "cannot access simplestreams metadata directory")
Expand Down Expand Up @@ -1068,7 +1069,7 @@ func setPrivateMetadataSources(metadataDir string) ([]*imagemetadata.ImageMetada
if err := dataSourceConfig.Validate(); err != nil {
return nil, errors.Annotate(err, "simplestreams config validation failed")
}
dataSource := simplestreams.NewDataSource(dataSourceConfig)
dataSource := dataSourceFactory.NewDataSource(dataSourceConfig)

// Read the image metadata, as we'll want to upload it to the environment.
imageConstraint, err := imagemetadata.NewImageConstraint(simplestreams.LookupParams{})
Expand Down
34 changes: 24 additions & 10 deletions environs/bootstrap/bootstrap_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -577,8 +577,9 @@ func (s *bootstrapSuite) TestBootstrapImageMetadataFromAllSources(c *gc.C) {
}
s.setDummyStorage(c, env.bootstrapEnviron)

ctx, ss := bootstrapContext(c)
bootstrapCons := constraints.MustParse("arch=amd64")
err = bootstrap.Bootstrap(envtesting.BootstrapTODOContext(c), env,
err = bootstrap.Bootstrap(ctx, env,
s.callContext, bootstrap.BootstrapParams{
ControllerConfig: coretesting.FakeControllerConfig(),
AdminSecret: "admin-secret",
Expand All @@ -589,7 +590,7 @@ func (s *bootstrapSuite) TestBootstrapImageMetadataFromAllSources(c *gc.C) {
})
c.Assert(err, jc.ErrorIsNil)

datasources, err := environs.ImageMetadataSources(env)
datasources, err := environs.ImageMetadataSources(env, ss)
c.Assert(err, jc.ErrorIsNil)
for _, source := range datasources {
// make sure we looked in each and all...
Expand Down Expand Up @@ -1139,9 +1140,10 @@ func (s *bootstrapSuite) TestBootstrapMetadata(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
envtesting.UploadFakeTools(c, stor, "released", "released")

ctx, ss := bootstrapContext(c)
env := newEnviron("foo", useDefaultKeys, nil)
s.setDummyStorage(c, env)
err = bootstrap.Bootstrap(envtesting.BootstrapTODOContext(c), env,
err = bootstrap.Bootstrap(ctx, env,
s.callContext, bootstrap.BootstrapParams{
ControllerConfig: coretesting.FakeControllerConfig(),
AdminSecret: "admin-secret",
Expand All @@ -1153,7 +1155,7 @@ func (s *bootstrapSuite) TestBootstrapMetadata(c *gc.C) {
c.Assert(env.bootstrapCount, gc.Equals, 1)
c.Assert(envtools.DefaultBaseURL, gc.Equals, metadataDir)

datasources, err := environs.ImageMetadataSources(env)
datasources, err := environs.ImageMetadataSources(env, ss)
c.Assert(err, jc.ErrorIsNil)
c.Assert(datasources, gc.HasLen, 2)
c.Assert(datasources[0].Description(), gc.Equals, "bootstrap metadata")
Expand Down Expand Up @@ -1198,7 +1200,9 @@ func (s *bootstrapSuite) TestBootstrapMetadataImagesNoTools(c *gc.C) {
startingDefaultBaseURL := envtools.DefaultBaseURL
for i, suffix := range []string{"", "images"} {
environs.UnregisterImageDataSourceFunc("bootstrap metadata")
err := bootstrap.Bootstrap(envtesting.BootstrapTODOContext(c), env,

ctx, ss := bootstrapContext(c)
err := bootstrap.Bootstrap(ctx, env,
s.callContext, bootstrap.BootstrapParams{
ControllerConfig: coretesting.FakeControllerConfig(),
AdminSecret: "admin-secret",
Expand All @@ -1210,7 +1214,7 @@ func (s *bootstrapSuite) TestBootstrapMetadataImagesNoTools(c *gc.C) {
c.Assert(env.bootstrapCount, gc.Equals, i+1)
c.Assert(envtools.DefaultBaseURL, gc.Equals, startingDefaultBaseURL)

datasources, err := environs.ImageMetadataSources(env)
datasources, err := environs.ImageMetadataSources(env, ss)
c.Assert(err, jc.ErrorIsNil)
c.Assert(datasources, gc.HasLen, 2)
c.Assert(datasources[0].Description(), gc.Equals, "bootstrap metadata")
Expand All @@ -1232,7 +1236,8 @@ func (s *bootstrapSuite) TestBootstrapMetadataToolsNoImages(c *gc.C) {
env := newEnviron("foo", useDefaultKeys, nil)
s.setDummyStorage(c, env)
for i, suffix := range []string{"", "tools"} {
err = bootstrap.Bootstrap(envtesting.BootstrapTODOContext(c), env,
ctx, ss := bootstrapContext(c)
err = bootstrap.Bootstrap(ctx, env,
s.callContext, bootstrap.BootstrapParams{
ControllerConfig: coretesting.FakeControllerConfig(),
AdminSecret: "admin-secret",
Expand All @@ -1243,7 +1248,8 @@ func (s *bootstrapSuite) TestBootstrapMetadataToolsNoImages(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
c.Assert(env.bootstrapCount, gc.Equals, i+1)
c.Assert(envtools.DefaultBaseURL, gc.Equals, metadataDir)
datasources, err := environs.ImageMetadataSources(env)

datasources, err := environs.ImageMetadataSources(env, ss)
c.Assert(err, jc.ErrorIsNil)
c.Assert(datasources, gc.HasLen, 1)
c.Assert(datasources[0].Description(), gc.Not(gc.Equals), "bootstrap metadata")
Expand Down Expand Up @@ -1357,9 +1363,11 @@ func (s *bootstrapSuite) TestBootstrapMetadataImagesMissing(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
envtesting.UploadFakeTools(c, stor, "released", "released")

ctx, ss := bootstrapContext(c)

env := newEnviron("foo", useDefaultKeys, nil)
s.setDummyStorage(c, env)
err = bootstrap.Bootstrap(envtesting.BootstrapTODOContext(c), env,
err = bootstrap.Bootstrap(ctx, env,
s.callContext, bootstrap.BootstrapParams{
ControllerConfig: coretesting.FakeControllerConfig(),
AdminSecret: "admin-secret",
Expand All @@ -1370,7 +1378,7 @@ func (s *bootstrapSuite) TestBootstrapMetadataImagesMissing(c *gc.C) {
c.Assert(err, jc.ErrorIsNil)
c.Assert(env.bootstrapCount, gc.Equals, 1)

datasources, err := environs.ImageMetadataSources(env)
datasources, err := environs.ImageMetadataSources(env, ss)
c.Assert(err, jc.ErrorIsNil)
c.Assert(datasources, gc.HasLen, 1)
c.Assert(datasources[0].Description(), gc.Equals, "default ubuntu cloud images")
Expand Down Expand Up @@ -1722,3 +1730,9 @@ func (e bootstrapEnvironWithHardwareDetection) DetectSeries() (string, error) {
func (e bootstrapEnvironWithHardwareDetection) DetectHardware() (*instance.HardwareCharacteristics, error) {
return e.detectedHW, nil
}

func bootstrapContext(c *gc.C) (environs.BootstrapContext, *simplestreams.Simplestreams) {
ss := simplestreams.NewSimpleStreams(sstesting.TestDataSourceFactory())
ctx := context.WithValue(context.TODO(), bootstrap.SimplestreamsFetcherContextKey, ss)
return envtesting.BootstrapContext(ctx, c), ss
}
10 changes: 4 additions & 6 deletions environs/imagemetadata.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func UnregisterImageDataSourceFunc(id string) {

// ImageMetadataSources returns the sources to use when looking for
// simplestreams image id metadata for the given stream.
func ImageMetadataSources(env BootstrapEnviron) ([]simplestreams.DataSource, error) {
func ImageMetadataSources(env BootstrapEnviron, dataSourceFactory simplestreams.DataSourceFactory) ([]simplestreams.DataSource, error) {
config := env.Config()

// Add configured and environment-specific datasources.
Expand All @@ -104,7 +104,7 @@ func ImageMetadataSources(env BootstrapEnviron) ([]simplestreams.DataSource, err
if err := cfg.Validate(); err != nil {
return nil, errors.Trace(err)
}
dataSource := simplestreams.NewDataSource(cfg)
dataSource := dataSourceFactory.NewDataSource(cfg)
sources = append(sources, dataSource)
}

Expand All @@ -115,13 +115,11 @@ func ImageMetadataSources(env BootstrapEnviron) ([]simplestreams.DataSource, err
sources = append(sources, envDataSources...)

// Add the official image metadata datasources.
officialDataSources, err := imagemetadata.OfficialDataSources(config.ImageStream())
officialDataSources, err := imagemetadata.OfficialDataSources(dataSourceFactory, config.ImageStream())
if err != nil {
return nil, err
}
for _, source := range officialDataSources {
sources = append(sources, source)
}
sources = append(sources, officialDataSources...)
for _, ds := range sources {
logger.Debugf("obtained image datasource %q", ds.Description())
}
Expand Down
Loading

0 comments on commit 23c1fa0

Please sign in to comment.