Code review comment for lp:~thumper/juju-core/container-address

Revision history for this message
John A Meinel (jameinel) wrote :

I fully support moving away from environment variables into doing this
properly via something like "agent.conf".

However, I'd rather see us have containers working on MaaS today than
wait for agent.conf refactoring, so this seems ok to me.

Can you add a comment to the "machineConfig.MachineEnvironment" line, to
make it stand out a bit more that we want to change how that works?

Everything else seems pretty straightforward. I wonder if we want a
basic smoke test for utils.GetAddressForInterface.

We could ask for the details of "eth0" and test that we either get a
meaningful error back, or we get an ipv4 address.

LGTM

https://codereview.appspot.com/13261044/

« Back to merge proposal