> Thanks for you careful review and valuable comments.
> 
> > Overall, comments, much of the code needs some unit tests.  We simply can't
> > avoid breaking things inadvertently without them
> get_resource_disk_on_freebsd
> > and the networking code specifically.
> All right. I'll add some unit tests.

Great.

> 
> > Please bear with me... you have a lot of good things here, we just need
> > to work out some details.
> >
> > - adding the specific config/cloud.cfg-freebsd-azure
> >   I'd much prefer we adjust the already freebsd specific cloud.cfg-freebsd.
> >
> >   The changes you have there are:
> >    - add Azure to the datasource_list.  I think its right to just put
> >      Azure before Openstack and Ec2 so we have the list of 4 sources.
> Ok.
> >    - you disabled growpart and resizefs
> >      I wonder why these were not disabled by the original freebsd work.
> >      Can you explain that?  If they just aren't going to work, then lets
> >      remove them from the list and update the docstring to mention lack
> >      of support on freebsd.  If they are only broken on Azure, lets figure
> >      out why.
> On Azure, FreeBSD image used a fixed size VHD, so gpart is not necessary.
> Anyway, there is some error reported in the log if I enabled it. For the same
> reason, I disabled resizefs on Azure. Well, if we decide to use the same
> cloud-init.cfg for FreeBSD. I have to fix those error message and enable
> growpart and resizefs.

OK, Hopefully we could use Jake's suggestion above and get resizefs and growpart working even if they aren't needed on azure.  They're in general good things.

> >    - default user changes
> >      - removing of the default 'name'. This should not be necessary as
> >        linux uses the datasource provided username on azure when its there.
> >      - changing of shell to csh: my understanding (per git log) is that tcsh
> >        is the default shell on freebsd, so using that makes more sense.
> >        if Azure wishes to carry a different default shell, they can do that
> >        in local config.  Generally though, commonality is better.
> >      - removal of lock_passwd: azure can carry that locally if they want.
> For unknown reason, if I did not remove the default 'name' on Azure, it will
> stop the data source provided username. Ok, that may be a bug on Azure. In
> addition, the default username 'freebsd' is created but locked passwd, then
> why do we need it?

Maybe i wasn't clear.  I was just saying that this all works as expected on Ubuntu (linux).  We have a default config with a user 'ubuntu', but on azure, where a user is provided via the datasource, it overrides just that.  So this should be able to work correctly.


> > - why the changes set_passwd?
> >   You made 2 changes here:
> >     a.) use -H 0 and -h 0 rather than just -H or -h
> >         I'm fine to be explicit, but it seems like it shouldnt be necessary
> >         or it would never have worked.
> >     b.) you use a shell to 'echo ... |' when we were previously just
> >         providing the password as standard in to the command (via subp
> > 'data').
> >         the shell is more processes to do the same work, did it not work for
> >         you before?
> It does not work because 'pw' command only accepts password from stdin. I do
> not why the origin one works, I guess maybe it works on early FreeBSD version.
> Anyway, we should fix it.

Sorry, again, I wasn't clear.
Essentially your changes do this:
 util.subp("echo password | pw usermod USER -H 0", shell=True)
That should be pretty much the same thing as:
 util.subp(['pw', 'usermod', USER, '-H', '0'], data="password")

And the second format there does not invoke shell, but puts the bytes 'password' into the stdin of the command.

> >   comments on generate_fallback_config:
> >     - you have lots of LOG.info() that is clearly developer debug like stuff
> >       drop what you can to debug.
> >     - more generically, do you think that parsing 'ifconfig' is the right
> >       way to do this for freebsd?
> I'll remove those LOG.info(). If we have to use builtin command, for example,
> 'ifconfig', we have to parse 'ifconfig' output. Maybe there are some other
> utility commands can help, but it takes more time to investigate. I think we
> can polish this if I find better alternatives.

I'm not opposed to using 'ifconfig', just didn't know if that was "the right thing" to use on that platform.  Ie, if I was doing new linux support, i'd use the 'ip' command rather than 'ifconfig'.