cmd/juju/addmachine.go might fail with unsupported container

[Dimiter Naydenov]

http://bazaar.launchpad.net/~go-bot/juju-core/trunk/view/head:/cmd/juju/addmachine.go#L63

In addmachine.go there's this code:

59 if c.ContainerType, err = instance.ParseSupportedContainerType(containerSpec); err != nil {
60 if !names.IsMachineOrNewContainer(containerSpec) {
61 return fmt.Errorf("malformed container argument %q", containerSpec)
62 }
63 sep := strings.Index(containerSpec, ":")
65 c.MachineId = containerSpec[sep+1:]
66 c.ContainerType, err = instance.ParseSupportedContainerType(containerSpec[:sep])
67 }

There are a few issues with it:
1) If containerSpec is not one of the supported container types ("none", "lxc", "kvm") it assumes it's in the form "type:machine", which might not be the case (on 59 err will be != nil)
2) IsMachineOrNewContainer returning true does not mean there's a ":" in containerSpec (it might be 0/lxc/1 for example), leading to sep being -1 on line 63, and as a consequence containerSpec[:sep] on 66 will panic at run time.

Roger has proposed to add a names.SplitContainerSpec(containerSpec string) (containerType string, machineId string, err error)
and use that here to handle the case better.
There should be better tests for the invalid cases specified above: 1) and 2).

[Dimiter Naydenov (dimitern)]

The correct behavior is to accept either "type:machine" or "type", where "type" is one of the supported container types. If just a machine id is passed, it should display a more meaningful error message.