Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Pull in new juju/utils and juju/version dependencies #6518
Conversation
anastasiamac
suggested changes
Nov 1, 2016
Overall, really awesome :)
My only concern is - to me, it's unclear why you would choose to call method that panics as opposed to not. Since .Must... can panic but is called more frequently, could you please add comment to places where you are calling .HostSeries? - explaining why you choose to call .Host rather then .Must
| @@ -256,17 +256,21 @@ func initBootstrapMachine(c agent.ConfigSetter, st *state.State, args Initialize | ||
| if args.BootstrapMachineHardwareCharacteristics != nil { | ||
| hardware = *args.BootstrapMachineHardwareCharacteristics | ||
| } | ||
| + hostSeries, err := series.HostSeries() |
| @@ -153,10 +153,14 @@ func (c *BootstrapCommand) Run(_ *cmd.Context) error { | ||
| // tools can actually be found, or else bootstrap won't complete. | ||
| stream := envtools.PreferredStream(&desiredVersion, args.ControllerModelConfig.Development(), args.ControllerModelConfig.AgentStream()) | ||
| logger.Infof("newer tools requested, looking for %v in stream %v", desiredVersion, stream) | ||
| + hostSeries, err := series.HostSeries() |
| @@ -359,10 +363,14 @@ func (c *BootstrapCommand) populateTools(st *state.State, env environs.Environ) | ||
| agentConfig := c.CurrentConfig() | ||
| dataDir := agentConfig.DataDir() | ||
| + hostSeries, err := series.HostSeries() |
| @@ -34,7 +35,11 @@ func (ci *containerInitialiser) Initialise() error { | ||
| // getPackageManager is a helper function which returns the | ||
| // package manager implementation for the current system. | ||
| func getPackageManager() (manager.PackageManager, error) { | ||
| - return manager.NewPackageManager(series.HostSeries()) | ||
| + hostSeries, err := series.HostSeries() |
| vers, err := version.ParseBinary("3.2.1-xuanhuaceratops-amd64") | ||
| - c.Assert(err, jc.Satisfies, series.IsUnknownOSForSeriesError) | ||
| + c.Assert(err, jc.ErrorIsNil) |
| @@ -138,7 +138,11 @@ func newService(name string, conf common.Conf, initSystem, series string) (Servi | ||
| // ListServices lists all installed services on the running system | ||
| func ListServices() ([]string, error) { | ||
| - initName, err := VersionInitSystem(series.HostSeries()) | ||
| + hostSeries, err := series.HostSeries() |
| @@ -358,7 +358,11 @@ func (s *SvcManager) Delete(name string) error { | ||
| func (s *SvcManager) Create(name string, conf common.Conf) error { | ||
| serviceStartName := "LocalSystem" | ||
| var passwd string | ||
| - if !series.IsWindowsNano(series.HostSeries()) { | ||
| + hostSeries, err := series.HostSeries() |
| @@ -48,7 +48,11 @@ func IsRunningLocally() (bool, error) { | ||
| return false, nil | ||
| } | ||
| - svc, err := service.NewService("lxd", common.Conf{}, series.HostSeries()) | ||
| + hostSeries, err := series.HostSeries() |
| @@ -166,7 +166,11 @@ func (w *proxyWorker) handleProxyValues(proxySettings proxyutils.Settings) { | ||
| // getPackageCommander is a helper function which returns the | ||
| // package commands implementation for the current system. | ||
| func getPackageCommander() (commands.PackageCommander, error) { | ||
| - return commands.NewPackageCommander(series.HostSeries()) | ||
| + hostSeries, err := series.HostSeries() |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
jujubot
merged commit d8b098e
into
juju:develop
Nov 1, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
wallyworld commentedNov 1, 2016
Fixes: https://bugs.launchpad.net/juju/+bug/1637079
Pull in new juju/utils and juju/version dependencies which fix issues with binary version parsing and series handling.
The change to juju/utils means that series.HostSeries() returns an error and there's a new MusHostSeries() which panics. All usages of HostSeries() in tests and a few other key places (eg var initialisation) have been converted to use the MustHostSeries() function. Other places still call HostSeries() and now handle the error. There's in progress work which obsoletes some of this but landing now fixes the immediate issue.
QA: bootstrap lxd provider and deploy a charm.