Code review comment for lp:~jtv/maas-test/bootresources-config

[0]

Looks like this doesn't work. I extracted the generated bootresources.yaml generated by this change (http://d-jenkins.ubuntu-ci:8080/view/MAAS/job/maas-test-manual/60/console) and ran the import script manually on canonistack and got http://paste.ubuntu.com/7225943/.

[1]

fwiw, I disagree with the approach taken here. I understand it's been approved by Julian so I won't block this but I really think this is wrong.

Having maas-test generate bootresources.yaml instead of just taking its content from a command line argument will force us to change maas-test every single time we will want to get it to tweak bootresources.yaml: to use a different label, to tweak the keyring or the simplestream path. This is the same mistake we've done with the power parameters settings. maas-test should try to do as little as possible and just pass things around, act as a dumb intermediate whenever possible.

[2]

47 + self.kvm_fixture.upload_file(config.name, 'bootresources.yaml')
48 + self.kvm_fixture.run_command(
49 + ['sudo', 'mv', 'bootresources.yaml', '/etc/maas/'],
50 + check_call=True)

Shouldn't we put the file in /tmp/bootresources.yaml? This looks like it will work just fine but maybe it's best to have the entire path under our control, just to be on the safe side.

[3]

> This includes i386/generic images as well as those for the requested architecture, all for the requested Ubuntu
> release series.

This was https://bugs.launchpad.net/ubuntu/+source/maas/+bug/1181334 and it's fixed now. Since maas-test can only be used to run MAAS 1.5 and up, we might as well be rid of this and avoid downloading the i386 image when testing an amd64 node. I know it has nothing to do with you branch per se, I'm just mentioning it here so that we don't forget.