Skip to content

Commit

Permalink
refactor(apiserver): agenttools facade using services
Browse files Browse the repository at this point in the history 
The agenttools facade now uses a model config service to access model config,
and a model agent service to access the current agent version.

BREAKING CHANGE: agenttools now checks all preferred streams in preference
order, starting with the user-specified stream in `agent-stream`. Previously,
it always checked the "released" stream first, even if a custom `agent-stream`
was set, and it would only check the user-defined stream if nothing was found
in "released".

Add a new env variable to the Makefile (`JUJU_PUBLISH_STREAM`) that allows
`make simplestreams` to publish to a specified stream.

General modernising and code improvement on the agenttools facade, such as
moving internal functions to methods on the API struct.
  • Loading branch information
@barrettj12
barrettj12 committed Jul 3, 2024
1 parent 253290c commit fe93736
Show file tree
Hide file tree
Showing 7 changed files with 283 additions and 93 deletions.
12 changes: 8 additions & 4 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ BIN_DIR ?= ${BUILD_DIR}/${GOOS}_${GOARCH}/bin
# for built juju binaries.
JUJU_METADATA_SOURCE ?= ${BUILD_DIR}/simplestreams

# JUJU_PUBLISH_STREAM defines the simplestreams stream that we will publish
# agents to (default "released").
JUJU_PUBLISH_STREAM ?= "released"

# TEST_PACKAGE_LIST is the path to a file that is a newline delimited list of
# packages to test. This file must be sorted.
TEST_PACKAGE_LIST ?=
Expand All @@ -54,7 +58,7 @@ tool_platform_paths = $(addsuffix /${1},$(call bin_platform_paths,${2}))

# simplestream_paths takes a list of Go style platforms to calculate the
# paths to their respective simplestreams agent binary archives.
simplestream_paths = $(addprefix ${JUJU_METADATA_SOURCE}/, $(addprefix tools/released/juju-${JUJU_VERSION}-, $(addsuffix .tgz,$(subst /,-,${1}))))
simplestream_paths = $(addprefix ${JUJU_METADATA_SOURCE}/, $(addprefix tools/${JUJU_PUBLISH_STREAM}/juju-${JUJU_VERSION}-, $(addsuffix .tgz,$(subst /,-,${1}))))

# CLIENT_PACKAGE_PLATFORMS defines a white space seperated list of platforms
# to build the Juju client binaries for. Platforms are defined as GO style
Expand Down Expand Up @@ -406,14 +410,14 @@ ${BUILD_DIR}/%/bin/pebble: phony_explicit
# build for pebble
$(run_go_build)

${JUJU_METADATA_SOURCE}/tools/released/juju-${JUJU_VERSION}-%.tgz: phony_explicit juju $(BUILD_AGENT_TARGETS) $(BUILD_CGO_AGENT_TARGETS)
${JUJU_METADATA_SOURCE}/tools/${JUJU_PUBLISH_STREAM}/juju-${JUJU_VERSION}-%.tgz: phony_explicit juju $(BUILD_AGENT_TARGETS) $(BUILD_CGO_AGENT_TARGETS)
@echo "Packaging simplestream tools for juju ${JUJU_VERSION} on $*"
@mkdir -p ${JUJU_METADATA_SOURCE}/tools/released
@mkdir -p ${JUJU_METADATA_SOURCE}/tools/${JUJU_PUBLISH_STREAM}
@tar czf "$@" -C $(call bin_platform_paths,$(subst -,/,$*)) .

.PHONY: simplestreams
simplestreams: juju juju-metadata ${SIMPLESTREAMS_TARGETS}
@juju metadata generate-agent-binaries -d ${JUJU_METADATA_SOURCE} --clean --prevent-fallback ;
@juju metadata generate-agent-binaries -d ${JUJU_METADATA_SOURCE} --clean --prevent-fallback --stream ${JUJU_PUBLISH_STREAM} ;
@echo "\nRun export JUJU_METADATA_SOURCE=\"${JUJU_METADATA_SOURCE}\" if not defined in your env"

.PHONY: build
Expand Down
78 changes: 35 additions & 43 deletions apiserver/facades/controller/agenttools/agenttools.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,12 @@ import (
"github.com/juju/juju/apiserver/facade"
corelogger "github.com/juju/juju/core/logger"
"github.com/juju/juju/environs"
"github.com/juju/juju/environs/config"
"github.com/juju/juju/environs/simplestreams"
"github.com/juju/juju/environs/tools"
coretools "github.com/juju/juju/internal/tools"
"github.com/juju/juju/state"
)

var (
findTools = tools.FindTools
)

// AgentToolsAPI implements the API used by the machine model worker.
type AgentToolsAPI struct {
modelGetter ModelGetter
Expand All @@ -33,6 +28,9 @@ type AgentToolsAPI struct {
findTools toolsFinder
envVersionUpdate envVersionUpdater
logger corelogger.Logger

modelConfigService ModelConfigService
modelAgentService ModelAgentService
}

// NewAgentToolsAPI creates a new instance of the Model API.
Expand All @@ -43,14 +41,18 @@ func NewAgentToolsAPI(
envVersionUpdate func(*state.Model, version.Number) error,
authorizer facade.Authorizer,
logger corelogger.Logger,
modelConfigService ModelConfigService,
modelAgentService ModelAgentService,
) (*AgentToolsAPI, error) {
return &AgentToolsAPI{
modelGetter: modelGetter,
newEnviron: newEnviron,
authorizer: authorizer,
findTools: findTools,
envVersionUpdate: envVersionUpdate,
logger: logger,
modelGetter: modelGetter,
newEnviron: newEnviron,
authorizer: authorizer,
findTools: findTools,
envVersionUpdate: envVersionUpdate,
logger: logger,
modelConfigService: modelConfigService,
modelAgentService: modelAgentService,
}, nil
}

Expand All @@ -63,28 +65,25 @@ type newEnvironFunc func() (environs.Environ, error)
type toolsFinder func(context.Context, tools.SimplestreamsFetcher, environs.BootstrapEnviron, int, int, []string, coretools.Filter) (coretools.List, error)
type envVersionUpdater func(*state.Model, version.Number) error

func checkToolsAvailability(ctx context.Context, newEnviron newEnvironFunc, modelCfg *config.Config, finder toolsFinder) (version.Number, error) {
currentVersion, ok := modelCfg.AgentVersion()
if !ok || currentVersion == version.Zero {
return version.Zero, nil
func (api *AgentToolsAPI) checkToolsAvailability(ctx context.Context) (version.Number, error) {
currentVersion, err := api.modelAgentService.GetModelAgentVersion(ctx)
if err != nil {
return version.Zero, errors.Annotate(err, "getting agent version from service")
}

env, err := newEnviron()
env, err := api.newEnviron()
if err != nil {
return version.Zero, errors.Annotatef(err, "cannot make model")
return version.Zero, errors.Annotatef(err, "cannot make cloud provider")
}

ss := simplestreams.NewSimpleStreams(simplestreams.DefaultDataSourceFactory())

// finder receives major and minor as parameters as it uses them to filter versions and
// only return patches for the passed major.minor (from major.minor.patch).
// We'll try the released stream first, then fall back to the current configured stream
// if no released tools are found.
vers, err := finder(ctx, ss, env, currentVersion.Major, currentVersion.Minor, []string{tools.ReleasedStream}, coretools.Filter{})
preferredStream := tools.PreferredStreams(&currentVersion, modelCfg.Development(), modelCfg.AgentStream())[0]
if preferredStream != tools.ReleasedStream && errors.Cause(err) == coretools.ErrNoMatches {
vers, err = finder(ctx, ss, env, currentVersion.Major, currentVersion.Minor, []string{preferredStream}, coretools.Filter{})
modelCfg, err := api.modelConfigService.ModelConfig(ctx)
if err != nil {
return version.Zero, errors.Annotate(err, "cannot get model config")
}

preferredStreams := tools.PreferredStreams(&currentVersion, modelCfg.Development(), modelCfg.AgentStream())
vers, err := api.findTools(ctx, ss, env, currentVersion.Major, currentVersion.Minor, preferredStreams, coretools.Filter{})
if err != nil {
return version.Zero, errors.Annotatef(err, "cannot find available agent binaries")
}
Expand All @@ -94,25 +93,13 @@ func checkToolsAvailability(ctx context.Context, newEnviron newEnvironFunc, mode
return newest, nil
}

var modelConfig = func(e *state.Model) (*config.Config, error) {
return e.Config()
}

// Base implementation of envVersionUpdater
func envVersionUpdate(env *state.Model, ver version.Number) error {
return env.UpdateLatestToolsVersion(ver)
}

func updateToolsAvailability(ctx context.Context, modelGetter ModelGetter, newEnviron newEnvironFunc, finder toolsFinder, update envVersionUpdater, logger corelogger.Logger) error {
model, err := modelGetter.Model()
if err != nil {
return errors.Annotate(err, "cannot get model")
}
cfg, err := modelConfig(model)
if err != nil {
return errors.Annotate(err, "cannot get config")
}
ver, err := checkToolsAvailability(ctx, newEnviron, cfg, finder)
func (api *AgentToolsAPI) updateToolsAvailability(ctx context.Context) error {
ver, err := api.checkToolsAvailability(ctx)
if err != nil {
if errors.Is(err, errors.NotFound) {
// No newer tools, so exit silently.
Expand All @@ -121,10 +108,15 @@ func updateToolsAvailability(ctx context.Context, modelGetter ModelGetter, newEn
return errors.Annotate(err, "cannot get latest version")
}
if ver == version.Zero {
logger.Debugf("The lookup of agent binaries returned version Zero. This should only happen during bootstrap.")
api.logger.Debugf("The lookup of agent binaries returned version Zero. This should only happen during bootstrap.")
return nil
}
return update(model, ver)

model, err := api.modelGetter.Model()
if err != nil {
return errors.Annotate(err, "cannot get model")
}
return api.envVersionUpdate(model, ver)
}

// UpdateToolsAvailable invokes a lookup and further update in environ
Expand All @@ -133,5 +125,5 @@ func (api *AgentToolsAPI) UpdateToolsAvailable(ctx context.Context) error {
if !api.authorizer.AuthController() {
return apiservererrors.ErrPerm
}
return updateToolsAvailability(ctx, api.modelGetter, api.newEnviron, api.findTools, api.envVersionUpdate, api.logger)
return api.updateToolsAvailability(ctx)
}
105 changes: 60 additions & 45 deletions apiserver/facades/controller/agenttools/agenttools_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/juju/errors"
jc "github.com/juju/testing/checkers"
"github.com/juju/version/v2"
"go.uber.org/mock/gomock"
gc "gopkg.in/check.v1"

"github.com/juju/juju/environs"
Expand All @@ -24,27 +25,31 @@ var _ = gc.Suite(&AgentToolsSuite{})

type AgentToolsSuite struct {
coretesting.BaseSuite
modelConfigService *MockModelConfigService
modelAgentService *MockModelAgentService
}

type dummyEnviron struct {
environs.Environ
}

type configGetter struct {
cfg *config.Config
}

func (s *configGetter) ModelConfig() (*config.Config, error) {
return s.cfg, nil
func (s *AgentToolsSuite) setupMocks(c *gc.C) *gomock.Controller {
ctrl := gomock.NewController(c)
s.modelConfigService = NewMockModelConfigService(ctrl)
s.modelAgentService = NewMockModelAgentService(ctrl)
return ctrl
}

func (s *AgentToolsSuite) TestCheckTools(c *gc.C) {
sConfig := coretesting.FakeConfig()
sConfig = sConfig.Merge(coretesting.Attrs{
"agent-version": "2.5.0",
})
cfg, err := config.New(config.NoDefaults, sConfig)
defer s.setupMocks(c).Finish()

expVer, err := version.Parse("2.5.0")
c.Assert(err, jc.ErrorIsNil)
s.modelAgentService.EXPECT().GetModelAgentVersion(gomock.Any()).Return(expVer, nil)
modelConfig, err := config.New(config.NoDefaults, coretesting.FakeConfig())
c.Assert(err, jc.ErrorIsNil)
s.modelConfigService.EXPECT().ModelConfig(gomock.Any()).Return(modelConfig, nil)

var (
calledWithMajor, calledWithMinor int
)
Expand All @@ -59,20 +64,29 @@ func (s *AgentToolsSuite) TestCheckTools(c *gc.C) {
return coretools.List{&t}, nil
}

ver, err := checkToolsAvailability(context.Background(), getDummyEnviron, cfg, fakeToolFinder)
api, err := NewAgentToolsAPI(nil, getDummyEnviron, fakeToolFinder, nil, nil, loggertesting.WrapCheckLog(c), s.modelConfigService, s.modelAgentService)
c.Assert(err, jc.ErrorIsNil)

obtainedVer, err := api.checkToolsAvailability(context.Background())
c.Assert(err, jc.ErrorIsNil)
c.Assert(ver, gc.Not(gc.Equals), version.Zero)
c.Assert(ver, gc.Equals, version.Number{Major: 2, Minor: 5, Patch: 0})
c.Assert(obtainedVer, gc.Equals, expVer)
}

func (s *AgentToolsSuite) TestCheckToolsNonReleasedStream(c *gc.C) {
defer s.setupMocks(c).Finish()

expVer, err := version.Parse("2.5-alpha1")
c.Assert(err, jc.ErrorIsNil)
s.modelAgentService.EXPECT().GetModelAgentVersion(gomock.Any()).Return(expVer, nil)

sConfig := coretesting.FakeConfig()
sConfig = sConfig.Merge(coretesting.Attrs{
"agent-version": "2.5-alpha1",
"agent-stream": "proposed",
"agent-stream": "proposed",
})
cfg, err := config.New(config.NoDefaults, sConfig)
c.Assert(err, jc.ErrorIsNil)
s.modelConfigService.EXPECT().ModelConfig(gomock.Any()).Return(cfg, nil)

var (
calledWithMajor, calledWithMinor int
calledWithStreams [][]string
Expand All @@ -90,30 +104,31 @@ func (s *AgentToolsSuite) TestCheckToolsNonReleasedStream(c *gc.C) {
c.Assert(calledWithMinor, gc.Equals, 5)
return coretools.List{&t}, nil
}
ver, err := checkToolsAvailability(context.Background(), getDummyEnviron, cfg, fakeToolFinder)

api, err := NewAgentToolsAPI(nil, getDummyEnviron, fakeToolFinder, nil, nil, loggertesting.WrapCheckLog(c), s.modelConfigService, s.modelAgentService)
c.Assert(err, jc.ErrorIsNil)
c.Assert(calledWithStreams, gc.DeepEquals, [][]string{{"released"}, {"proposed"}})
c.Assert(ver, gc.Not(gc.Equals), version.Zero)
c.Assert(ver, gc.Equals, version.Number{Major: 2, Minor: 5, Patch: 0})
}

type mockState struct {
configGetter
obtainedVer, err := api.checkToolsAvailability(context.Background())
c.Assert(err, jc.ErrorIsNil)
c.Assert(calledWithStreams, gc.DeepEquals, [][]string{{"proposed", "released"}})
c.Assert(obtainedVer, gc.Equals, version.Number{Major: 2, Minor: 5, Patch: 0})
}

type mockState struct{}

func (e *mockState) Model() (*state.Model, error) {
return &state.Model{}, nil
}

func (s *AgentToolsSuite) TestUpdateToolsAvailability(c *gc.C) {
fakeModelConfig := func(_ *state.Model) (*config.Config, error) {
sConfig := coretesting.FakeConfig()
sConfig = sConfig.Merge(coretesting.Attrs{
"agent-version": "2.5.0",
})
return config.New(config.NoDefaults, sConfig)
}
s.PatchValue(&modelConfig, fakeModelConfig)
defer s.setupMocks(c).Finish()

expVer, err := version.Parse("2.5.0")
c.Assert(err, jc.ErrorIsNil)
s.modelAgentService.EXPECT().GetModelAgentVersion(gomock.Any()).Return(expVer, nil)
modelConfig, err := config.New(config.NoDefaults, coretesting.FakeConfig())
c.Assert(err, jc.ErrorIsNil)
s.modelConfigService.EXPECT().ModelConfig(gomock.Any()).Return(modelConfig, nil)

fakeToolFinder := func(_ context.Context, _ tools.SimplestreamsFetcher, _ environs.BootstrapEnviron, _ int, _ int, _ []string, _ coretools.Filter) (coretools.List, error) {
ver := version.Binary{Number: version.Number{Major: 2, Minor: 5, Patch: 2}}
Expand All @@ -129,24 +144,23 @@ func (s *AgentToolsSuite) TestUpdateToolsAvailability(c *gc.C) {
return nil
}

cfg, err := config.New(config.NoDefaults, coretesting.FakeConfig())
c.Assert(err, jc.ErrorIsNil)
err = updateToolsAvailability(context.Background(), &mockState{configGetter{cfg}}, getDummyEnviron, fakeToolFinder, fakeUpdate, loggertesting.WrapCheckLog(c))
api, err := NewAgentToolsAPI(&mockState{}, getDummyEnviron, fakeToolFinder, fakeUpdate, nil, loggertesting.WrapCheckLog(c), s.modelConfigService, s.modelAgentService)
c.Assert(err, jc.ErrorIsNil)

c.Assert(ver, gc.Not(gc.Equals), version.Zero)
err = api.updateToolsAvailability(context.Background())
c.Assert(err, jc.ErrorIsNil)
c.Assert(ver, gc.Equals, version.Number{Major: 2, Minor: 5, Patch: 2})
}

func (s *AgentToolsSuite) TestUpdateToolsAvailabilityNoMatches(c *gc.C) {
fakeModelConfig := func(_ *state.Model) (*config.Config, error) {
sConfig := coretesting.FakeConfig()
sConfig = sConfig.Merge(coretesting.Attrs{
"agent-version": "2.5.0",
})
return config.New(config.NoDefaults, sConfig)
}
s.PatchValue(&modelConfig, fakeModelConfig)
defer s.setupMocks(c).Finish()

expVer, err := version.Parse("2.5.0")
c.Assert(err, jc.ErrorIsNil)
s.modelAgentService.EXPECT().GetModelAgentVersion(gomock.Any()).Return(expVer, nil)
modelConfig, err := config.New(config.NoDefaults, coretesting.FakeConfig())
c.Assert(err, jc.ErrorIsNil)
s.modelConfigService.EXPECT().ModelConfig(gomock.Any()).Return(modelConfig, nil)

// No new tools available.
fakeToolFinder := func(_ context.Context, _ tools.SimplestreamsFetcher, _ environs.BootstrapEnviron, _ int, _ int, _ []string, _ coretools.Filter) (coretools.List, error) {
Expand All @@ -159,9 +173,10 @@ func (s *AgentToolsSuite) TestUpdateToolsAvailabilityNoMatches(c *gc.C) {
return nil
}

cfg, err := config.New(config.NoDefaults, coretesting.FakeConfig())
api, err := NewAgentToolsAPI(&mockState{}, getDummyEnviron, fakeToolFinder, fakeUpdate, nil, loggertesting.WrapCheckLog(c), s.modelConfigService, s.modelAgentService)
c.Assert(err, jc.ErrorIsNil)
err = updateToolsAvailability(context.Background(), &mockState{configGetter{cfg}}, getDummyEnviron, fakeToolFinder, fakeUpdate, loggertesting.WrapCheckLog(c))

err = api.updateToolsAvailability(context.Background())
c.Assert(err, jc.ErrorIsNil)
}

Expand Down
2 changes: 2 additions & 0 deletions apiserver/facades/controller/agenttools/package_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"github.com/juju/juju/testing"
)

//go:generate go run go.uber.org/mock/mockgen -typed -package agenttools -destination service_mock_test.go github.com/juju/juju/apiserver/facades/controller/agenttools ModelConfigService,ModelAgentService

func TestAll(t *stdtesting.T) {
testing.MgoTestPackage(t)
}
12 changes: 11 additions & 1 deletion apiserver/facades/controller/agenttools/register.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/juju/juju/apiserver/facade"
"github.com/juju/juju/environs"
"github.com/juju/juju/environs/tools"
"github.com/juju/juju/state/stateenvirons"
)

Expand All @@ -32,5 +33,14 @@ func newFacade(ctx facade.ModelContext) (*AgentToolsAPI, error) {
newEnviron := stateenvirons.GetNewEnvironFunc(environs.New)
return newEnviron(model, ctx.ServiceFactory().Cloud(), ctx.ServiceFactory().Credential())
}
return NewAgentToolsAPI(st, newEnviron, findTools, envVersionUpdate, ctx.Auth(), ctx.Logger().Child("model"))
return NewAgentToolsAPI(
st,
newEnviron,
tools.FindTools,
envVersionUpdate,
ctx.Auth(),
ctx.Logger().Child("model"),
ctx.ServiceFactory().Config(),
ctx.ServiceFactory().Agent(),
)
}
Loading

0 comments on commit fe93736

Please sign in to comment.