Code review comment for lp:~axwalk/juju-core/lp1271144-maas-bridge-utils

Revision history for this message
Roger Peppe (rogpeppe) wrote :

LGTM with a couple of thoughts below.

https://codereview.appspot.com/77890045/diff/1/provider/maas/environ.go
File provider/maas/environ.go (right):

https://codereview.appspot.com/77890045/diff/1/provider/maas/environ.go#newcode316
provider/maas/environ.go:316:
utils.CommandString(utils.AptGetCommand("update")...),
It seems a pity that we can't just call SetAptUpdate on the
cloudinit config, but that would require a change to ComposeUserData
(might it make sense to pass in a possibly-nil cloudinit.Config to that
function?)

Aside: CommandString should also escape backquotes, although it won't be
a problem here

https://codereview.appspot.com/77890045/diff/1/provider/maas/environ_test.go
File provider/maas/environ_test.go (right):

https://codereview.appspot.com/77890045/diff/1/provider/maas/environ_test.go#newcode208
provider/maas/environ_test.go:208: const aptGetPrefix = "env
DEBIAN_FRONTEND=noninteractive apt-get
--option=Dpkg::Options::=--force-confold
--option=Dpkg::options::=--force-unsafe-io --assume-yes --quiet"
or aptGetPrefix := utils.AptGetCommand()
?

https://codereview.appspot.com/77890045/

« Back to merge proposal