Find agent binaries using fallback across streams #8092

Merged
merged 3 commits into from Nov 16, 2017

Conversation

Projects
None yet
3 participants
Member

babbageclunk commented Nov 16, 2017

Description of change

If someone bootstraps with a beta or with an explicitly set devel stream, we weren't then considering released agent binaries as possible candidates for upgrades (because they are on the released stream).
Change the metadata lookup so that we check the released stream if a controller is on the devel stream.

This is another go at #8080 without trying to change the simplestreams code - handling mirrors and resolve info was too involved. This version combines the results just above tools/simplestreams/Fetch, which means it handles mirrors right for each stream and doesn't need to combine resolve info because it's thrown away at that point anyway.

QA steps

Bootstrap a controller with 2.2-beta3 (using a released 2.2 binary). Then run juju upgrade-juju -m controller. The controller should be upgraded to 2.2.6.

Documentation changes

It's possible this needs documentation? I couldn't find any documentation for how we find agent binaries in streams, but if there is we should add a mention of the fallback to it.

@@ -118,7 +118,7 @@ func (c *toolsMetadataCommand) Run(context *cmd.Context) error {
if err != nil {
return errors.Trace(err)
}
- toolsList, err = envtools.FindToolsForCloud(toolsDataSources(source), simplestreams.CloudSpec{}, c.stream, -1, -1, coretools.Filter{})
+ toolsList, err = envtools.FindToolsForCloud(toolsDataSources(source), simplestreams.CloudSpec{}, []string{c.stream}, -1, -1, coretools.Filter{})
@wallyworld

wallyworld Nov 16, 2017

Owner

this should have the same behaviour as juju - so it needs to use the streams fallbacks based on what the user specifies

@babbageclunk

babbageclunk Nov 16, 2017

Member

Hang on, won't that mean that if they pass --stream=devel to the command the resulting metadata it writes out will have everything from devel, proposed and released as if it was in devel? That seems wrong. If someone wants to generate metadata for all streams they can run the tool for each stream. Or should we do the looping over streams here and merge each stream's tools into the metadata separately?

environs/tools/tools.go
// If the use has already nominated a specific stream, we'll use that.
if stream != "" && stream != ReleasedStream {
- return stream
+ return copyStrings(streamFallbacks[stream])
@wallyworld

wallyworld Nov 16, 2017

Owner

if the user specifies a stream not in streamFallbacks, we should just use that stream verbatum

environs.tools.PreferredStream -> PreferredStreams
It now returns a slice of fallback streams in preference order. For the
moment callers have been updated to just use the first one, they'll be
updated to do the fallback next.

thanks!

Make environs.tools.FindTools accept multiple streams
It will query the metadata for each stream in turn, combining the
binaries found.

Update callers that already use PreferredStreams to pass them.

@babbageclunk babbageclunk changed the title from WIP: stream fallback in a less invasive way to Find agent binaries using fallback across streams Nov 16, 2017

Member

babbageclunk commented Nov 16, 2017

In the interests of getting things into the release (hopefully) I'm going to merge this despite the question about juju-metadata. Can you change it if I turn out to be wrong? Thanks!

Member

babbageclunk commented Nov 16, 2017

$$merge$$

Member

babbageclunk commented Nov 16, 2017

!!build!!

Member

babbageclunk commented Nov 16, 2017

Hmm, it looks like the mergebot's down.

Contributor

jujubot commented Nov 16, 2017

Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju

Contributor

jujubot commented Nov 16, 2017

Build failed: Tests failed
build url: http://ci.jujucharms.com/job/github-merge-juju/537

Member

babbageclunk commented Nov 16, 2017

$$merge$$

Contributor

jujubot commented Nov 16, 2017

Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju

@jujubot jujubot merged commit 91ad765 into juju:develop Nov 16, 2017

0 of 2 checks passed

github-check-merge-juju-pipeline Built PR, ran unit tests, and tested LXD deploy. Use !!.*!! to request another build. IE, !!build!!, !!retry!!
Details
continuous-integration/jenkins/pr-merge This commit is being built
Details

@babbageclunk babbageclunk deleted the babbageclunk:stream-fallback branch Nov 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment