Code review comment for lp:~mpontillo/maas/allow-commissioning-non-lts

Revision history for this message
Mike Pontillo (mpontillo) wrote :

@allenap, thanks for the fixes; I was going to ask you about that. I removed one of the test cases in that file because I wasn't sure that it was correct for the configuration store to allow an invalid directory to be returned. It seemed to me like we should catch that at the earliest possible time, so we don't propagate a directory that doesn't exist throughout the code and try to use it everywhere. But I can see the argument for both ways, and certainly there is less code change (and exception handling changes) required if we leave that scenario the way it is.

Also, the `maas-rack config` command has intentionally NOT been updated. Users will never see this option unless they look at the code.

I was curious about what we do in a multi-rack scenario, too. I didn't dwell on that too much, because this was strictly intended as an unsupported feature for QA/developers, and I figured if it misbehaved it's not the end of the world.

@Andres, I understand your concerns. But I think this feature is sufficiently hidden, and that the average MAAS user won't stumble upon it and try it out (at their peril). The only way they can enable this is to (1) know it exists, and (2) add it to their rackd.conf manually. If they found it by looking at the code, it should be clear from the surrounding text that it is NOT SUPPORTED and NOT RECOMMENDED. (I can call that out in clearer language, if you like.)

The reason you want to have this in a configuration file is because it makes it easier to do QA against MAAS; especially integration testing against the latest MAAS images. With this configuration option available, we can more easily test development releases in a CI environment, and thus catch problems that will occur with the *next* LTS sooner rather than later.

To be honest, I did this branch because I wanted to scratch an itch. I was trying to test out changes to the Yakkety images, and I was annoyed that I had to figure out where to hack the code each time I wanted to do that.

In conclusion, this should be something that can be easily enabled for QA or development purposes -- without hacking the Python files in the MAAS package, or using other workarounds.

« Back to merge proposal