Merge ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

Proposed by Hongjiang Zhang
Status: Merged
Merged at revision: 0a71d5a870b416f2c86c8bc196004bb3fc0768a0
Proposed branch: ~redriver/cloud-init:frbsd-azure-branch
Merge into: cloud-init:master
Diff against target: 1374 lines (+782/-87)
22 files modified
cloudinit/config/cc_resizefs.py (+95/-0)
cloudinit/distros/__init__.py (+3/-0)
cloudinit/distros/freebsd.py (+261/-16)
cloudinit/settings.py (+1/-1)
cloudinit/sources/DataSourceAzure.py (+169/-11)
cloudinit/sources/helpers/azure.py (+10/-1)
cloudinit/stages.py (+1/-1)
cloudinit/util.py (+51/-1)
config/cloud.cfg-freebsd (+1/-1)
setup.py (+6/-4)
sysvinit/freebsd/cloudconfig (+0/-10)
sysvinit/freebsd/cloudfinal (+0/-10)
sysvinit/freebsd/cloudinit (+0/-10)
sysvinit/freebsd/cloudinitlocal (+1/-11)
tests/unittests/test_datasource/test_azure.py (+65/-0)
tests/unittests/test_datasource/test_azure_helper.py (+2/-2)
tests/unittests/test_datasource/test_cloudstack.py (+5/-0)
tests/unittests/test_distros/test_netconfig.py (+46/-1)
tests/unittests/test_handler/test_handler_resizefs.py (+59/-0)
tests/unittests/test_net.py (+3/-3)
tests/unittests/test_util.py (+2/-1)
tools/build-on-freebsd (+1/-3)
Reviewer Review Type Date Requested Status
Scott Moser Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+314895@code.launchpad.net

Commit message

This patch targets to make FreeBSD 10.3 or 11 work on Azure. The modifications abide by the rule of
(1) making as less modification as possible
(2) delegating the platform or cloud specific operations to the specific classes instead of adding platform checking.
The main modifications constitute the following items:
1. Commands for network configuration are different from the ones on Ubuntu, for example, get ipv4/ipv6/mac, so, some of the functions called by apply_network_config are moved distro/FreeBSD.py instead of being handled in net/__init__.py.
2. Password setting through "pw" can only work through pipe, like "echo PASSWD|pw ... -H 0".
3. Default group for root user is "wheel" on FreeBSD. So, 'root:wheel' should be added syslog_fix_perms field.
4. There are some changes related to FreeBSD on Azure (DataSourceAzure):
(a) The leases file for dhclient is different from the one on Linux, so, the lease file path should be generated according to platform. The other similar platform related values: default primary network interface (hn0)and default file system (ufs)
(b) Mounting issues, including mounting CD, and get mount information
(c) Azure protocol depends on a field of dhclient lease file, but its name is different on FreeBSD. (azure.py, "option-245" vs. "unknown-245")
(d) Method to find resource (ephemeral) disk is a little complex because FreeBSD does not provide /dev/disk/cloud/azure_resource. This method is also used by WALinuxAgent 2.2
(e) Add a cloud.cfg for FreeBSD on Azure. Azure sets user information through ovf file, so the default configuration should not contain any user information.
Some of the features have not been fully implemented, for example, get seed from "/sys/firmware/acpi/tables/OEM0" on Linux, but this information on FreeBSD cannot be directly fetched. The config modules are also needed to be checked whether they work for FreeBSD on Azure.

To post a comment you must log in.
Revision history for this message
Scott Moser (smoser) wrote :

Hongjjang,
Thanks for submitting this, I will try to take a look next week.
Sorry for the slow response.

Revision history for this message
Scott Moser (smoser) wrote :
Download full text (4.5 KiB)

Note, I really dont have a lot of experience on FreeBSD. I relied heavily
on others who added freebsd support to tell me what made sense, and I'll do
the same for you.

Thank you for your efforts here... There are definitely some things we need
to re-work to better support different "distros" throughout the code base.
We'll see these same sorts of things as aixtools is working on getting better
aix support in.

One such example is your moving generate_fallback_config to the distro object.

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.

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.
   - 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.
   - 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.

- resizefs: it seems like you made some changes for this
  in an attempt to make it work, but then disabled it in your
  config. If it isn't going to work, lets not make ancillary changes there.
   these include
     tests/unittests/test_handler/test_handler_growpart.py
     cloudinit/config/cc_resizefs.py
     get_mount_info (i think)

  I'm not opposed to doing this stuff at some point, but just want to make
  sure all changes are intended.

- you moved 'get_mount_info' from the distro to the datasource?
  that doesn't seem right. Possibly related only to resizefs.

- 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?

  - is there a reason you are not using set_passwd from add_user ?

- util.py:
  freebsd really calls its x86_64 arch 'amd64' ? wow. maybe at this point we
  should just s...

Read more...

review: Needs Fixing
Revision history for this message
Jake Smith (c0by) wrote :

Hi,

I have been doing some testing on FreeBSD.

The current growpart seems to work fine with FreeBSD 11.0 with GPT/UFS - the partition gets resized. The issue is with filesystem resize in resizefs. Its a simple fix, before you can modify the MBR you must first set kern.geom.debugflags=16, then when calling growfs you must pass the '-y' flag. You should then set debugflags back to it's original value.

Basically this, I did a quick hack to test and seems ok:

current_debugflags = sysctl -n kern.geom.debugflags
sysctl kern.geom.debugflags=16
growfs -y /dev/vtbd0p2
sysctl kern.geom.debugflags=current_debugflags

Revision history for this message
Hongjiang Zhang (redriver) wrote :
Download full text (6.8 KiB)

Thanks for you careful review and valuable comments.

> Note, I really dont have a lot of experience on FreeBSD. I relied heavily
> on others who added freebsd support to tell me what made sense, and I'll do
> the same for you.
That is ok for me.

> Thank you for your efforts here... There are definitely some things we need
> to re-work to better support different "distros" throughout the code base.
> We'll see these same sorts of things as aixtools is working on getting better
> aix support in.
>
> One such example is your moving generate_fallback_config to the distro object.
I agree. Azure supports both Linux and FreeBSD, so for the same data source, there are two distros and different distros sometimes have different native commands. So, we need to consider how to support them in an elegant way.

> 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.

> 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.

> - 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?

>
> - resizefs: it seems like you made some changes for this
> in an attempt to make it work, but then disabled it in your
> config. If it isn't going to work, lets not make ancillary changes there.
> these include
> tests/unit...

Read more...

Revision history for this message
Scott Moser (smoser) wrote :
Download full text (4.4 KiB)

> 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
...

Read more...

Revision history for this message
Hongjiang Zhang (redriver) wrote :
Download full text (4.8 KiB)

> > 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.
>
Ok. I see. Anyway, I observed it does not work as expected with default user.
I'll investigate it.

> > > - 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.
> > > ...

Read more...

Revision history for this message
Hongjiang Zhang (redriver) wrote :

> Hi,
>
> I have been doing some testing on FreeBSD.
>
> The current growpart seems to work fine with FreeBSD 11.0 with GPT/UFS - the
> partition gets resized. The issue is with filesystem resize in resizefs. Its a
> simple fix, before you can modify the MBR you must first set
> kern.geom.debugflags=16, then when calling growfs you must pass the '-y' flag.
> You should then set debugflags back to it's original value.
>
> Basically this, I did a quick hack to test and seems ok:
>
> current_debugflags = sysctl -n kern.geom.debugflags
> sysctl kern.geom.debugflags=16
> growfs -y /dev/vtbd0p2
> sysctl kern.geom.debugflags=current_debugflags
My problem is growfs always reports error since the disk is already full expanded.

# growfs -y /dev/da0p2
growfs: requested size 28GB is not larger than the current filesystem size 28GB
# echo $?
1

My disk is formatted to be UFS:
# gpart show
=> 40 62914480 da0 GPT (30G)
        40 1024 1 freebsd-boot (512K)
      1064 58719232 2 freebsd-ufs (28G)
  58720296 3145728 3 freebsd-swap (1.5G)
  61866024 1048496 - free - (512M)

=> 63 146800577 da1 MBR (70G)
         63 1985 - free - (993K)
       2048 146796544 1 ntfs (70G)
  146798592 2048 - free - (1.0M)

Revision history for this message
Jake Smith (c0by) wrote :

> > Hi,
> >
> > I have been doing some testing on FreeBSD.
> >
> > The current growpart seems to work fine with FreeBSD 11.0 with GPT/UFS - the
> > partition gets resized. The issue is with filesystem resize in resizefs. Its
> a
> > simple fix, before you can modify the MBR you must first set
> > kern.geom.debugflags=16, then when calling growfs you must pass the '-y'
> flag.
> > You should then set debugflags back to it's original value.
> >
> > Basically this, I did a quick hack to test and seems ok:
> >
> > current_debugflags = sysctl -n kern.geom.debugflags
> > sysctl kern.geom.debugflags=16
> > growfs -y /dev/vtbd0p2
> > sysctl kern.geom.debugflags=current_debugflags
> My problem is growfs always reports error since the disk is already full
> expanded.
>
> # growfs -y /dev/da0p2
> growfs: requested size 28GB is not larger than the current filesystem size
> 28GB
> # echo $?
> 1
>
> My disk is formatted to be UFS:
> # gpart show
> => 40 62914480 da0 GPT (30G)
> 40 1024 1 freebsd-boot (512K)
> 1064 58719232 2 freebsd-ufs (28G)
> 58720296 3145728 3 freebsd-swap (1.5G)
> 61866024 1048496 - free - (512M)
>
> => 63 146800577 da1 MBR (70G)
> 63 1985 - free - (993K)
> 2048 146796544 1 ntfs (70G)
> 146798592 2048 - free - (1.0M)

Not sure what the most elegant solution would be to avoid exit(1) when there is nothing to do...

Just thinking out loud here:

1.
Can resizefs be called to ignore exit status 1? Could be dangerous?

2.
Is there a way to pass back from growpart to see if gpart size did anything, if so then run growfs?

3.
You could do a dry run first and check the exit status before continuing:
# growfs -N -y /dev/da0p2

4.
Could we look at the current size of the file system and compare to gpart, (there is probably a better way to get the fs size):

# dumpfs -m /
# newfs command for / (/dev/da0p2)
newfs -O 2 -U -a 4 -b 32768 -d 32768 -e 4096 -f 4096 -g 16384 -h 64 -i 8192 -j -k 5240 -m 8 -o time -s 83885872 /dev/da0p2

Grab -s <size> round it and compare against

# gpart show
=> 40 83886000 da0 MBR (466G)
         40 128 1 freebsd-boot (64K)
        168 83886000 2 freebsd-ufs (40.0G)

Revision history for this message
Hongjiang Zhang (redriver) wrote :

> > > Hi,
> > >
> > > I have been doing some testing on FreeBSD.
> > >
> > > The current growpart seems to work fine with FreeBSD 11.0 with GPT/UFS -
> the
> > > partition gets resized. The issue is with filesystem resize in resizefs.
> Its
> > a
> > > simple fix, before you can modify the MBR you must first set
> > > kern.geom.debugflags=16, then when calling growfs you must pass the '-y'
> > flag.
> > > You should then set debugflags back to it's original value.
> > >
> > > Basically this, I did a quick hack to test and seems ok:
> > >
> > > current_debugflags = sysctl -n kern.geom.debugflags
> > > sysctl kern.geom.debugflags=16
> > > growfs -y /dev/vtbd0p2
> > > sysctl kern.geom.debugflags=current_debugflags
> > My problem is growfs always reports error since the disk is already full
> > expanded.
> >
> > # growfs -y /dev/da0p2
> > growfs: requested size 28GB is not larger than the current filesystem size
> > 28GB
> > # echo $?
> > 1
> >
> > My disk is formatted to be UFS:
> > # gpart show
> > => 40 62914480 da0 GPT (30G)
> > 40 1024 1 freebsd-boot (512K)
> > 1064 58719232 2 freebsd-ufs (28G)
> > 58720296 3145728 3 freebsd-swap (1.5G)
> > 61866024 1048496 - free - (512M)
> >
> > => 63 146800577 da1 MBR (70G)
> > 63 1985 - free - (993K)
> > 2048 146796544 1 ntfs (70G)
> > 146798592 2048 - free - (1.0M)
>
> Not sure what the most elegant solution would be to avoid exit(1) when there
> is nothing to do...
>
> Just thinking out loud here:
>
> 1.
> Can resizefs be called to ignore exit status 1? Could be dangerous?
>
> 2.
> Is there a way to pass back from growpart to see if gpart size did anything,
> if so then run growfs?
>
> 3.
> You could do a dry run first and check the exit status before continuing:
> # growfs -N -y /dev/da0p2
>
> 4.
> Could we look at the current size of the file system and compare to gpart,
> (there is probably a better way to get the fs size):
>
> # dumpfs -m /
> # newfs command for / (/dev/da0p2)
> newfs -O 2 -U -a 4 -b 32768 -d 32768 -e 4096 -f 4096 -g 16384 -h 64 -i 8192 -j
> -k 5240 -m 8 -o time -s 83885872 /dev/da0p2
>
> Grab -s <size> round it and compare against
>
> # gpart show
> => 40 83886000 da0 MBR (466G)
> 40 128 1 freebsd-boot (64K)
> 168 83886000 2 freebsd-ufs (40.0G)
Firstly, I appreciate your help to resolve this issue. I prefer the 4th solution. Thanks.

Revision history for this message
Jake Smith (c0by) wrote :

Just checked the code, you will need to round the output from gpart to the nearest whole block, see this example:

# gpart show md0
=> 34 297086 md0 GPT (145M)
      34 297086 1 freebsd-ufs (145M)

# dumpfs -m /mnt
# newfs command for /mnt (/dev/md0p1)
newfs -O 2 -U -a 4 -b 32768 -d 32768 -e 4096 -f 4096 -g 16384 -h 64 -i 8192 -k 368 -m 8 -o time -s 297080 /dev/md0p1

So gpart reports 297086 but dumpfs reports 297080, making your code think it needs to do a resize.

Should be able to round the number using the fragment size (-f <frag-size>) like this, assuming a sector size of 512:

>>> gpart = 297086
>>> frag = 4096
>>> (gpart) - (gpart) % (frag / 512)
297080.0

Revision history for this message
Hongjiang Zhang (redriver) wrote :

> Just checked the code, you will need to round the output from gpart to the
> nearest whole block, see this example:
>
> # gpart show md0
> => 34 297086 md0 GPT (145M)
> 34 297086 1 freebsd-ufs (145M)
>
> # dumpfs -m /mnt
> # newfs command for /mnt (/dev/md0p1)
> newfs -O 2 -U -a 4 -b 32768 -d 32768 -e 4096 -f 4096 -g 16384 -h 64 -i 8192 -k
> 368 -m 8 -o time -s 297080 /dev/md0p1
>
> So gpart reports 297086 but dumpfs reports 297080, making your code think it
> needs to do a resize.
>
> Should be able to round the number using the fragment size (-f <frag-size>)
> like this, assuming a sector size of 512:
>
> >>> gpart = 297086
> >>> frag = 4096
> >>> (gpart) - (gpart) % (frag / 512)
> 297080.0
Thanks. I'll apply it to my patch.

Revision history for this message
Hongjiang Zhang (redriver) wrote :

> Just checked the code, you will need to round the output from gpart to the
> nearest whole block, see this example:
>
> # gpart show md0
> => 34 297086 md0 GPT (145M)
> 34 297086 1 freebsd-ufs (145M)
>
> # dumpfs -m /mnt
> # newfs command for /mnt (/dev/md0p1)
> newfs -O 2 -U -a 4 -b 32768 -d 32768 -e 4096 -f 4096 -g 16384 -h 64 -i 8192 -k
> 368 -m 8 -o time -s 297080 /dev/md0p1
>
> So gpart reports 297086 but dumpfs reports 297080, making your code think it
> needs to do a resize.
>
> Should be able to round the number using the fragment size (-f <frag-size>)
> like this, assuming a sector size of 512:
>
> >>> gpart = 297086
> >>> frag = 4096
> >>> (gpart) - (gpart) % (frag / 512)
> 297080.0
Another question, when I install cloud-init on FreeBSD, the cloud.cfg is used and I have to manually replace it with cloud.cfg-freebsd. Is it better select cloud configuration according to --init-system option. I want to implement like the following:

if (the value of --init-system option equal to sysvinit_freebsd)
then
   replace cloud.cfg with cloud.cfg-FreeBSD
fi

But I also find tools/build-on-FreeBSD contains some logic similar. My question is where build-on-FreeBSD is invoked? My cloud-init installation is "python setup.py --init-system=sysvinit_freebsd", does I miss anything?

Revision history for this message
Hongjiang Zhang (redriver) wrote :

one more issue I found is the user information provided by Azure cannot override the default one. The reason is: Stages.Modules.cfg depends on helpers.ConfigMerger to merge the cfg. But the internal function helpers._read_cfg says env_configs overrides datasource_configs. See the comments below. It also mentions the input config overrides env_config, but on Azure, the user information comes from datasource, how can I get input config ahead of datasource?

The quick fix is move "cfgs.extend(self._get_datasource_configs())" ahead of "cfgs.extend(self._get_env_configs())".

I also wonder why Ubuntu on Azure does not encounter this issue. It looks like Ubuntu on Azure invokes cloudinitlocal, but I'm still confused how Ubuntu can override env_configs.

    def _read_cfg(self):
        # Input config files override
        # env config files which
        # override instance configs
        # which override datasource
        # configs which override
        # base configuration
        cfgs = []
        if self._fns:
            for c_fn in self._fns:
                try:
                    cfgs.append(util.read_conf(c_fn))
                except Exception:
                    util.logexc(LOG, "Failed loading of configuration from %s",
                                c_fn)

        cfgs.extend(self._get_env_configs())
        cfgs.extend(self._get_instance_configs())
        cfgs.extend(self._get_datasource_configs())
        if self._base_cfg:
            cfgs.append(self._base_cfg)
        return util.mergemanydict(cfgs)

Revision history for this message
Scott Moser (smoser) wrote :

I'm confused because Ubuntu's cloud.cfg is exactly what is in
config/cloud.cfg in trunk. which has:

  users:
     - default
  ...

  system_info:
     # This will affect which distro class gets used
     distro: ubuntu
     # Default user name + that default users groups (if added/used)
     default_user:
       name: ubuntu
       lock_passwd: True
       gecos: Ubuntu
       groups: [adm, audio, cdrom, dialout, dip, floppy, lxd, netdev, plugdev, sudo, video]
       sudo: ["ALL=(ALL) NOPASSWD:ALL"]
       shell: /bin/bash

Thats almost identical to what config/cloud.cfg-freebsd has in your
branch.

I think that comment is actually wrong. If you look at how that method
works, it basically builds an array of configs (dictionaries), with
environment config (from environment CFG_ENV_NAME) first, then isntance
configs, then datasource, then base. Then calls mergemanydict with that.

So basically:
  util.mergemanydict([
     {'foo': 'from enviroment'},
     {'foo': 'from instance, no user override'},
     {'system_info': {'default_user': {'name': 'smoser-from-ds'}}},
     {'system_info': {'default_user': {'name': 'ubuntu'}}},
  ])

Calling that returns:
{'foo': 'from enviroment',
 'system_info': {'default_user': {'name': 'smoser-from-ds'}}}

So I'm pretty sure the comment is just wrong. Its probably because
'mergemanydict' has very odd usage in that precedence goes to the *first*
value, not the last.

Revision history for this message
Hongjiang Zhang (redriver) wrote :
Download full text (4.0 KiB)

> I'm confused because Ubuntu's cloud.cfg is exactly what is in
> config/cloud.cfg in trunk. which has:
>
> users:
> - default
> ...
>
> system_info:
> # This will affect which distro class gets used
> distro: ubuntu
> # Default user name + that default users groups (if added/used)
> default_user:
> name: ubuntu
> lock_passwd: True
> gecos: Ubuntu
> groups: [adm, audio, cdrom, dialout, dip, floppy, lxd, netdev, plugdev,
> sudo, video]
> sudo: ["ALL=(ALL) NOPASSWD:ALL"]
> shell: /bin/bash
>
> Thats almost identical to what config/cloud.cfg-freebsd has in your
> branch.
>
> I think that comment is actually wrong. If you look at how that method
> works, it basically builds an array of configs (dictionaries), with
> environment config (from environment CFG_ENV_NAME) first, then isntance
> configs, then datasource, then base. Then calls mergemanydict with that.
>
> So basically:
> util.mergemanydict([
> {'foo': 'from enviroment'},
> {'foo': 'from instance, no user override'},
> {'system_info': {'default_user': {'name': 'smoser-from-ds'}}},
> {'system_info': {'default_user': {'name': 'ubuntu'}}},
> ])
>
> Calling that returns:
> {'foo': 'from enviroment',
> 'system_info': {'default_user': {'name': 'smoser-from-ds'}}}
>
> So I'm pretty sure the comment is just wrong. Its probably because
> 'mergemanydict' has very odd usage in that precedence goes to the *first*
> value, not the last.
Ok. Let's ignore that comment. I debug this code snippet and still found it there.
I added a test function in helper.py, and specify two input files '/usr/local/etc/cloud/cloud.cfg' and 'ds.dat'
The content of ds.dat is as following.

{'disk_setup': {'ephemeral0': {'table_type': 'gpt', 'layout': [100], 'overwrite': True}}, 'fs_setup': [{'device': 'ephemeral0.1', 'replace_fs': 'ntfs', 'filesystem': 'freebsd-ufs'}], 'system_info': {'default_user': {'passwd': '$6$WSLBpwonNkC0Yi2b$w0xih6LR1X.TSyjn2mYSNiHITd.2oqD/j6CdoskUoXhH04/N8bvi6GB3smdZ4JT5hMrbT1svqtkkQeYY/B2fZ/', 'name': u'azureuser', 'lock_passwd': False}}, 'ssh_pwauth': True}

The output indicates that the username is still "freebsd" even though the "passwd" field was updated. So, in my environment, there is something different from the environment on Ubuntu.

{"ssh_pwauth": true, "users": ["default"], "disable_root": false, "disk_setup": {"ephemeral0": {"table_type": "gpt", "layout": [100], "overwrite": true}}, "system_info": {"default_user": {"shell": "/bin/tcsh", "gecos": "FreeBSD", "name": "freebsd", "groups": ["wheel"], "passwd": "$6$WSLBpwonNkC0Yi2b$w0xih6LR1X.TSyjn2mYSNiHITd.2oqD/j6CdoskUoXhH04/N8bvi6GB3smdZ4JT5hMrbT1svqtkkQeYY/B2fZ/", "sudo": ["ALL=(ALL) NOPASSWD:ALL"], "lock_passwd": true}, "distro": "freebsd"}, "syslog_fix_perms": "root:wheel", "fs_setup": [{"device": "ephemeral0.1", "replace_fs": "ntfs", "filesystem": "freebsd-ufs"}], "cloud_init_modules": ["seed_random", "bootcmd", "growpart", "resizefs", "set_hostname", "update_hostname", "users-groups", "ssh"], "preserve_hostname": false, "cloud_final_modules": ["rightscale_userdata", "scripts-vendor", "scripts-per-once", "scripts-per-boot", "sc...

Read more...

Revision history for this message
Hongjiang Zhang (redriver) wrote :

> I'm confused because Ubuntu's cloud.cfg is exactly what is in
> config/cloud.cfg in trunk. which has:
>
> users:
> - default
> ...
>
> system_info:
> # This will affect which distro class gets used
> distro: ubuntu
> # Default user name + that default users groups (if added/used)
> default_user:
> name: ubuntu
> lock_passwd: True
> gecos: Ubuntu
> groups: [adm, audio, cdrom, dialout, dip, floppy, lxd, netdev, plugdev,
> sudo, video]
> sudo: ["ALL=(ALL) NOPASSWD:ALL"]
> shell: /bin/bash
>
> Thats almost identical to what config/cloud.cfg-freebsd has in your
> branch.
>
> I think that comment is actually wrong. If you look at how that method
> works, it basically builds an array of configs (dictionaries), with
> environment config (from environment CFG_ENV_NAME) first, then isntance
> configs, then datasource, then base. Then calls mergemanydict with that.
>
> So basically:
> util.mergemanydict([
> {'foo': 'from enviroment'},
> {'foo': 'from instance, no user override'},
> {'system_info': {'default_user': {'name': 'smoser-from-ds'}}},
> {'system_info': {'default_user': {'name': 'ubuntu'}}},
> ])
>
> Calling that returns:
> {'foo': 'from enviroment',
> 'system_info': {'default_user': {'name': 'smoser-from-ds'}}}
>
> So I'm pretty sure the comment is just wrong. Its probably because
> 'mergemanydict' has very odd usage in that precedence goes to the *first*
> value, not the last.
Sorry, you are right, Ubuntu's cloud.cfg also specifies default user "ubuntu". That is not the root cause why cloud-init does not work on FreeBSD.
New finding is the helper.py will not load cloud.cfg as env_configs because only FreeBSD defines the CLOUD_CFG, which tells helpers.py to load env_configs.
I grep "CLOUD_CFG" from systemd and sysvinit and find:
 grep "CLOUD_CFG" sysvinit/* -rn
sysvinit/freebsd/cloudconfig:10:export CLOUD_CFG=/usr/local/etc/cloud/cloud.cfg
sysvinit/freebsd/cloudfinal:10:export CLOUD_CFG=/usr/local/etc/cloud/cloud.cfg
sysvinit/freebsd/cloudinit:10:export CLOUD_CFG=/usr/local/etc/cloud/cloud.cfg
sysvinit/freebsd/cloudinitlocal:10:export CLOUD_CFG=/usr/local/etc/cloud/cloud.cfg

in the helpers.py, I observed:

CFG_ENV_NAME = "CLOUD_CFG"
...
    def _get_env_configs(self):
        e_cfgs = []
        if CFG_ENV_NAME in os.environ:
            e_fn = os.environ[CFG_ENV_NAME]
            try:
                e_cfgs.append(util.read_conf(e_fn))
            except Exception:
                util.logexc(LOG, 'Failed loading of env. config from %s',
                            e_fn)
        return e_cfgs

I don't know why FreeBSD needs to export this environment, but none of the other distros need it.

Revision history for this message
Hongjiang Zhang (redriver) wrote :

I have finished all the needed fixes.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

There is a lot of good stuff here, thank you for your work and your patience.
Some good things:
 - the generate_fallback_config() is good, thanks.
 - the unit tests added for that and for the resize.

There are some things that still need fixing though, and I have some in-line
comments to help elaborate.
a.) You've fixed some general things inside of the DataSourceAzureNet only.
    Ie, you made devent2deev run through the datasource, which means
    that that code wont work running freebsd on Ec2. Better to have
    logic in it that does the right thing both places.

b.) I think we can simplify the '_can_skip_resize_*' things a bit.:w

c.) is your '_can_skip_resize_ufs' for performance? or is it actually needed?

review: Needs Fixing
Revision history for this message
Hongjiang Zhang (redriver) :
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Hongjiang Zhang (redriver) wrote :

> There is a lot of good stuff here, thank you for your work and your patience.
> Some good things:
> - the generate_fallback_config() is good, thanks.
> - the unit tests added for that and for the resize.
>
> There are some things that still need fixing though, and I have some in-line
> comments to help elaborate.
> a.) You've fixed some general things inside of the DataSourceAzureNet only.
> Ie, you made devent2deev run through the datasource, which means
> that that code wont work running freebsd on Ec2. Better to have
> logic in it that does the right thing both places.
When I change devent2dev, I have considered other platforms. I have put a generic implementation of get_mount_info in sources/__init__.py. So, for other platform, for example, EC2, it will invoke the util.get_mount_info. See:
    def get_mount_info(self, path, log=LOG):
        return util.get_mount_info(path, log)

Why this function cannot be put to util?
FreeBSD on Azure uses glabel to label /dev/da0p2 with 'rootfs', so if we want to find '/' partition, we should first find /dev/label/rootfs through '/', and then map it to /dev/da0p2.

> b.) I think we can simplify the '_can_skip_resize_*' things a bit.:w
Yes, I have changed it per your suggestion.

> c.) is your '_can_skip_resize_ufs' for performance? or is it actually needed?
It is used to check whether resize is necessary. On Azure, the partition has already fully covered the whole disk. It will report error in log if we resize it by force. This pre-check wants to avoid such error.

Revision history for this message
Scott Moser (smoser) wrote :

> > There are some things that still need fixing though, and I have some in-line
> > comments to help elaborate.
> > a.) You've fixed some general things inside of the DataSourceAzureNet only.
> > Ie, you made devent2deev run through the datasource, which means
> > that that code wont work running freebsd on Ec2. Better to have
> > logic in it that does the right thing both places.
> When I change devent2dev, I have considered other platforms. I have put a
> generic implementation of get_mount_info in sources/__init__.py. So, for other
> platform, for example, EC2, it will invoke the util.get_mount_info. See:
> def get_mount_info(self, path, log=LOG):
> return util.get_mount_info(path, log)

devent2dev basically is supposed to turn either a filesystem location ("/home")
or a device into a device that contains that filesystem. Input of '/dev/sda'
should return '/dev/sda'. Input of "/home" should return the device that
/home's filesystem is on (what should be rezied).

Your made a change to resize_devices in cloudinit/config/cc_growpart.py to
take a 'cloud'.

  - resize_devices then calls devent2dev(devent, cloud)
  - devent2dev calls cloud.datasource.get_mount_info(devent)
This change:
- result = util.get_mount_info(devent)
+ result = cloud.datasource.get_mount_info(devent)

I'd have preferred for 'util.get_mount_info' to have code that says:
   if is_FreeBSD:
      # get the device one way
   else:
      # get it for linux.

Instead, since you're going through 'datasource', your fix will only work
on Azure.

> > c.) is your '_can_skip_resize_ufs' for performance? or is it actually
> needed?
> It is used to check whether resize is necessary. On Azure, the partition has
> already fully covered the whole disk. It will report error in log if we resize
> it by force. This pre-check wants to avoid such error.

OK, thats fine.

Revision history for this message
Scott Moser (smoser) wrote :

I think you might be able to not change anything in the get_mount_info path, but rather just fix a bug in 'parse_mount(path)'.

--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -2056,6 +2056,8 @@ def parse_mount(path):
     mount_locs = mountoutput.splitlines()
     for line in mount_locs:
         m = re.search(r'^(/dev/[\S]+) on (/.*) \((.+), .+, (.+)\)$', line)
+ if not m:
+ continue
         devpth = m.group(1)
         mount_point = m.group(2)
         fs_type = m.group(3)

then, in my testing on a freebsd instance in digital ocean :
$ python -c 'from cloudinit import util; print(util.get_mount_info("/"))'
('/dev/gpt/rootfs', 'ufs', '/')

Actually, even without the change it works for me with 'mount' output like:
$ mount
/dev/gpt/rootfs on / (ufs, local, soft-updates)
devfs on /dev (devfs, local, multilabel)
fdescfs on /dev/fd (fdescfs)
/dev/vtbd1 on /var/lib/cloud/seed/config_drive (cd9660, local, read-only)

Note, that it does not work unless you give it a mount point as input. Ie, util.get_mount_info("/etc") will not work while it does work on linux. We could fix that though, by just looking through all the mount points and finding the one that 'path' is on.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Hongjiang Zhang (redriver) wrote :

I have done the fix. I also fixed the issue you mentioned for get_mount_info by providing a path.

> I think you might be able to not change anything in the get_mount_info path,
> but rather just fix a bug in 'parse_mount(path)'.
>
> --- a/cloudinit/util.py
> +++ b/cloudinit/util.py
> @@ -2056,6 +2056,8 @@ def parse_mount(path):
> mount_locs = mountoutput.splitlines()
> for line in mount_locs:
> m = re.search(r'^(/dev/[\S]+) on (/.*) \((.+), .+, (.+)\)$', line)
> + if not m:
> + continue
> devpth = m.group(1)
> mount_point = m.group(2)
> fs_type = m.group(3)
>
>
> then, in my testing on a freebsd instance in digital ocean :
> $ python -c 'from cloudinit import util; print(util.get_mount_info("/"))'
> ('/dev/gpt/rootfs', 'ufs', '/')
>
> Actually, even without the change it works for me with 'mount' output like:
> $ mount
> /dev/gpt/rootfs on / (ufs, local, soft-updates)
> devfs on /dev (devfs, local, multilabel)
> fdescfs on /dev/fd (fdescfs)
> /dev/vtbd1 on /var/lib/cloud/seed/config_drive (cd9660, local, read-only)
>
>
> Note, that it does not work unless you give it a mount point as input. Ie,
> util.get_mount_info("/etc") will not work while it does work on linux. We
> could fix that though, by just looking through all the mount points and
> finding the one that 'path' is on.

Revision history for this message
Scott Moser (smoser) wrote :

Hi,
Sorry for the slow reply.
We are getting closer.
I have some comments inline.

review: Needs Fixing
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Hongjiang Zhang (redriver) wrote :

> Hi,
> Sorry for the slow reply.
> We are getting closer.
> I have some comments inline.
I have removed 'cloud' parameter from devent2dev, and I added more check in util.get_mount_info to specially handle the case for FreeBSD on Azure.

Is there any other mechanism for me to easily check whether it is running on Hyper-V?

Revision history for this message
Hongjiang Zhang (redriver) wrote :

> Hi,
> Sorry for the slow reply.
> We are getting closer.
> I have some comments inline.
I have removed 'cloud' parameter from devent2dev, and I added more check in util.get_mount_info to specially handle the case for FreeBSD on Azure.

Is there any other mechanism for me to easily check whether it is running on Hyper-V?

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

there are som ecomments in line.
I have a mp i will submit to you to cleanup some others.

Revision history for this message
Scott Moser (smoser) wrote :
Revision history for this message
Scott Moser (smoser) wrote :

Other than that and I think it looks really good.
thank you for adding the unit tests.

Revision history for this message
Hongjiang Zhang (redriver) wrote :
Download full text (5.2 KiB)

> diff --git a/cloudinit/util.py b/cloudinit/util.py index
> 3301957..ef84e32 100644
> --- a/cloudinit/util.py
> +++ b/cloudinit/util.py
> @@ -2089,6 +2144,8 @@ def get_mount_info(path, log=LOG):
> #
> # So use /proc/$$/mountinfo to find the device underlying the
> # input path.
> + if is_FreeBSD_on_hyperv():

>but why "freebsd on hyperv"
>why are you not just checking "is this freebsd".
>what is specific to Azure here?

FreeBSD on Azure used a special label on disk. If we want to
get the /dev/daXXX from mount information, we need to handle
it specially.

> + return get_mount_info_freebsd_on_Azure(path, log)
> mountinfo_path = '/proc/%s/mountinfo' % os.getpid()
> if os.path.exists(mountinfo_path):
> lines = load_file(mountinfo_path).splitlines()
> diff --git a/tests/unittests/test_handler/test_handler_resizefs.py
> b/tests/unittests/test_handler/test_handler_resizefs.py
> new file mode 100644
> index 0000000..b5384e4
> --- /dev/null
> +++ b/tests/unittests/test_handler/test_handler_resizefs.py
> @@ -0,0 +1,73 @@
> +# This file is part of cloud-init. See LICENSE file for license information.
> +
> +from cloudinit.config import cc_resizefs
> +
> +import unittest
> +
> +try:
> + from unittest import mock
> +except ImportError:
> + import mock
> +
> +
> +class TestResizefs(unittest.TestCase):
> + def setUp(self):
> + super(TestResizefs, self).setUp()
> + self.name = "resizefs"
> +
> + @mock.patch('cloudinit.config.cc_resizefs._get_dumpfs_output')
> + @mock.patch('cloudinit.config.cc_resizefs._get_gpart_output')
> + def test_skip_ufs_resize(self, gpart_out, dumpfs_out):
> + fs_type = "ufs"
> + resize_what = "/"
> + devpth = "/dev/da0p2"
> + dumpfs_out.return_value = "# newfs command for / "\
> + "(/dev/label/rootfs)\n" \
> + "newfs -O 2 -U -a 4 -b "\
> + "32768 -d 32768 -e 4096 "\
> + "-f 4096 -g 16384 -h 64 "\
> + "-i 8192 -j -k 6408 -m 8 "\
> + "-o time -s 58719232 "\
> + "/dev/label/rootfs\n"
> + gpart_out.return_value = """
> +=> 40 62914480 da0 GPT (30G)
> + 40 1024 1 freebsd-boot (512K)
> + 1064 58719232 2 freebsd-ufs (28G)
> + 58720296 3145728 3 freebsd-swap (1.5G)
> + 61866024 1048496 - free - (512M)
> +"""
> + res = cc_resizefs.can_skip_resize(fs_type,
> + resize_what,
> + devpth)
> + self.assertTrue(res)
> +
> + @mock.patch('cloudinit.config.cc_resizefs._get_dumpfs_output')
> + @mock.patch('cloudinit.config.cc_resizefs._get_gpart_output')
> + def test_skip_ufs_resize_roundup(self, gpart_out, dumpfs_out):
> + fs_type = "ufs"
> + resize_what = "/"
> + devpth = "/dev/da0p2"
> + dumpfs_out.return_value = "# newfs command for / "\
> + ...

Read more...

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Hongjiang Zhang (redriver) wrote :

Hi Scott,

I have manually merged your modifications to my branch and fixed a unit test issue.
Do you think it is ok for merging?

Thanks
Hongjiang Zhang

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Scott Moser
Sent: Thursday, April 13, 2017 2:51 AM
To: <email address hidden>
Subject: Re: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

Other than that and I think it looks really good.
thank you for adding the unit tests.

--
https://na01.safelinks.protection.outlook.com/?url=https:%2F%2Fcode.launchpad.net%2F~redriver%2Fcloud-init%2F%2Bgit%2Fcloud-init%2F%2Bmerge%2F314895&data=02%7C01%7Chonzhan%40microsoft.com%7Ca62135a0b9514c7e130608d481d4cf8c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636276198391371945&sdata=qGDZNXUPvGHGs3%2ByT685EVs8jwnZQ12YIWBT%2BBrMPb8%3D&reserved=0
You are the owner of ~redriver/cloud-init:frbsd-azure-branch.

Revision history for this message
Scott Moser (smoser) wrote :

On Fri, 14 Apr 2017, Hongjiang Zhang wrote:

> Hi Scott,
>
> I have manually merged your modifications to my branch and fixed a unit test issue.
> Do you think it is ok for merging?

The only thing I had left was that I'm still confused as what why we need
azure specific code in that code path.

I have to think about it some more and probably go play with it a little
on freebsd.

>
> Thanks
> Hongjiang Zhang
>
>
> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Scott Moser
> Sent: Thursday, April 13, 2017 2:51 AM
> To: <email address hidden>
> Subject: Re: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master
>
> Other than that and I think it looks really good.
> thank you for adding the unit tests.
>
> --
> https://na01.safelinks.protection.outlook.com/?url=https:%2F%2Fcode.launchpad.net%2F~redriver%2Fcloud-init%2F%2Bgit%2Fcloud-init%2F%2Bmerge%2F314895&data=02%7C01%7Chonzhan%40microsoft.com%7Ca62135a0b9514c7e130608d481d4cf8c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636276198391371945&sdata=qGDZNXUPvGHGs3%2ByT685EVs8jwnZQ12YIWBT%2BBrMPb8%3D&reserved=0
> You are the owner of ~redriver/cloud-init:frbsd-azure-branch.
>

Revision history for this message
Hongjiang Zhang (redriver) wrote :

The reason is FreeBSD on Azure have more than 1 disks attached, and the rootfs located in one disk, for example, /dev/da0, but I cannot directly write something below:
/dev/da0 / ufs rw 1 1
Because the rootfs may be switched to /dev/da1. The solution is label the disk. For FreeBSD on Azure, I labeled /dev/da0 with "rootfs",
so for get_mount_info, I have to find "rootfs" first, and then find the corresponding /dev/da0.

But on Linux, there is a /proc/$$/mountinfo to help find the device. For Azure, there is also /dev/disk/cloud/azure_root on Linux.
Both make the get_mount_info easy to implement.

-----Original Message-----
From: Scott Moser [mailto:<email address hidden>] On Behalf Of Scott Moser
Sent: Friday, April 14, 2017 10:46 AM
To: Hongjiang Zhang <email address hidden>
Cc: <email address hidden>; Kylie Liang <email address hidden>; Josh Poulson <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

On Fri, 14 Apr 2017, Hongjiang Zhang wrote:

> Hi Scott,
>
> I have manually merged your modifications to my branch and fixed a unit test issue.
> Do you think it is ok for merging?

The only thing I had left was that I'm still confused as what why we need azure specific code in that code path.

I have to think about it some more and probably go play with it a little on freebsd.

>
> Thanks
> Hongjiang Zhang
>
>
> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf
> Of Scott Moser
> Sent: Thursday, April 13, 2017 2:51 AM
> To: <email address hidden>
> Subject: Re: [Merge] ~redriver/cloud-init:frbsd-azure-branch into
> cloud-init:master
>
> Other than that and I think it looks really good.
> thank you for adding the unit tests.
>
> --
> https://na01.safelinks.protection.outlook.com/?url=https:%2F%2Fcode.la
> unchpad.net%2F~redriver%2Fcloud-init%2F%2Bgit%2Fcloud-init%2F%2Bmerge%
> 2F314895&data=02%7C01%7Chonzhan%40microsoft.com%7Ca62135a0b9514c7e1306
> 08d481d4cf8c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636276198391
> 371945&sdata=qGDZNXUPvGHGs3%2ByT685EVs8jwnZQ12YIWBT%2BBrMPb8%3D&reserv
> ed=0 You are the owner of ~redriver/cloud-init:frbsd-azure-branch.
>

Revision history for this message
Hongjiang Zhang (redriver) wrote :

Hello Scott,

I'm not sure whether you are still confused about why we need Azure specific code, or is there anything I can do to help make any progress on this merge?

Thanks
Hongjiang Zhang

-----Original Message-----
From: Hongjiang Zhang
Sent: Friday, April 14, 2017 10:58 AM
To: Scott Moser <email address hidden>
Cc: <email address hidden>; Kylie Liang <email address hidden>; Josh Poulson <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

The reason is FreeBSD on Azure have more than 1 disks attached, and the rootfs located in one disk, for example, /dev/da0, but I cannot directly write something below:
/dev/da0 / ufs rw 1 1
Because the rootfs may be switched to /dev/da1. The solution is label the disk. For FreeBSD on Azure, I labeled /dev/da0 with "rootfs", so for get_mount_info, I have to find "rootfs" first, and then find the corresponding /dev/da0.

But on Linux, there is a /proc/$$/mountinfo to help find the device. For Azure, there is also /dev/disk/cloud/azure_root on Linux.
Both make the get_mount_info easy to implement.

-----Original Message-----
From: Scott Moser [mailto:<email address hidden>] On Behalf Of Scott Moser
Sent: Friday, April 14, 2017 10:46 AM
To: Hongjiang Zhang <email address hidden>
Cc: <email address hidden>; Kylie Liang <email address hidden>; Josh Poulson <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

On Fri, 14 Apr 2017, Hongjiang Zhang wrote:

> Hi Scott,
>
> I have manually merged your modifications to my branch and fixed a unit test issue.
> Do you think it is ok for merging?

The only thing I had left was that I'm still confused as what why we need azure specific code in that code path.

I have to think about it some more and probably go play with it a little on freebsd.

>
> Thanks
> Hongjiang Zhang
>
>
> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf
> Of Scott Moser
> Sent: Thursday, April 13, 2017 2:51 AM
> To: <email address hidden>
> Subject: Re: [Merge] ~redriver/cloud-init:frbsd-azure-branch into
> cloud-init:master
>
> Other than that and I think it looks really good.
> thank you for adding the unit tests.
>
> --
> https://na01.safelinks.protection.outlook.com/?url=https:%2F%2Fcode.la
> unchpad.net%2F~redriver%2Fcloud-init%2F%2Bgit%2Fcloud-init%2F%2Bmerge%
> 2F314895&data=02%7C01%7Chonzhan%40microsoft.com%7Ca62135a0b9514c7e1306
> 08d481d4cf8c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636276198391
> 371945&sdata=qGDZNXUPvGHGs3%2ByT685EVs8jwnZQ12YIWBT%2BBrMPb8%3D&reserv
> ed=0 You are the owner of ~redriver/cloud-init:frbsd-azure-branch.
>

c1cf3ad... by Hongjiang Zhang

Remove Azure specific check

Take the get_mount_info for Azure as a common case on FreeBSD.
This was only triggered if the device info '/dev/xxx' is illegal.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Hongjiang Zhang (redriver) wrote :
Download full text (5.4 KiB)

By the way, I removed the Azure specific check for "get_mount_info".
get_mount_info_freebsd() was invoked only if the normal parse_mount failed on FreeBSD platform.
What do you think?

--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -565,6 +565,10 @@ def is_ipv4(instr):
     return len(toks) == 4

+def is_FreeBSD():
+ return system_info()['platform'].startswith('FreeBSD')
+
+
 def get_cfg_option_bool(yobj, key, default=False):
     if key not in yobj:
         return default
@@ -2055,11 +2059,56 @@ def parse_mtab(path):
     return None

+def find_freebsd_part(label_part):
+ if label_part.startswith("/dev/label/"):
+ target_label = label_part[5:]
+ (label_part, err) = subp(['glabel', 'status', '-s'])
+ for labels in label_part.split("\n"):
+ items = labels.split()
+ if len(items) > 0 and items[0].startswith(target_label):
+ label_part = items[2]
+ break
+ label_part = str(label_part)
+ return label_part
+
+
+def get_path_dev_freebsd(path, mnt_list):
+ path_found = None
+ for line in mnt_list.split("\n"):
+ items = line.split()
+ if (len(items) > 2 and os.path.exists(items[1] + path)):
+ path_found = line
+ break
+ return path_found
+
+
+def get_mount_info_freebsd(path, log=LOG):
+ (result, err) = subp(['mount', '-p', path], rcs=[0, 1])
+ if len(err):
+ # find a path if the input is not a mounting point
+ (mnt_list, err) = subp(['mount', '-p'])
+ path_found = get_path_dev_freebsd(path, mnt_list)
+ if (path_found is None):
+ return None
+ result = path_found
+ ret = result.split()
+ label_part = find_freebsd_part(ret[0])
+ return "/dev/" + label_part, ret[2], ret[1]
+
+
 def parse_mount(path):
     (mountoutput, _err) = subp("mount")
     mount_locs = mountoutput.splitlines()
     for line in mount_locs:
         m = re.search(r'^(/dev/[\S]+) on (/.*) \((.+), .+, (.+)\)$', line)
+ if not m:
+ continue
+ # check whether the dev refers to a label on FreeBSD
+ # for example, if dev is '/dev/label/rootfs', we should
+ # continue finding the real device like '/dev/da0'.
+ devm = re.search('^(/dev/.+)p([0-9])$', m.group(1))
+ if (not devm and is_FreeBSD()):
+ return get_mount_info_freebsd(path)
         devpth = m.group(1)

-----Original Message-----
From: Hongjiang Zhang
Sent: Tuesday, April 25, 2017 11:38 AM
To: 'Scott Moser' <email address hidden>
Cc: '<email address hidden>' <email address hidden>; Kylie Liang <email address hidden>; Josh Poulson <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

Hello Scott,

I'm not sure whether you are still confused about why we need Azure specific code, or is there anything I can do to help make any progress on this merge?

Thanks
Hongjiang Zhang

-----Original Message-----
From: Hongjiang Zhang
Sent: Friday, April 14, 2017 10:58 AM
To: Scott Moser <email address hidden>
Cc: <email address hidden>; Kylie Liang <email address hidden>; Josh Poulson <email address hidden>
Subj...

Read more...

Revision history for this message
Kylie Liang (kyliel) wrote :
Download full text (3.2 KiB)

Hi Scott,

Thank you for your effort. Could you please help take minutes to see any else we should do to get the patch merged? Feel free let us know your thoughts. It is our highest priority to get this work for BSD users. Your support is appreciated. Thank you.

Thanks,
Kylie Liang

-----Original Message-----
From: Hongjiang Zhang
Sent: Tuesday, April 25, 2017 11:38 AM
To: Scott Moser <email address hidden>
Cc: <email address hidden>; Kylie Liang <email address hidden>; Josh Poulson <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

Hello Scott,

I'm not sure whether you are still confused about why we need Azure specific code, or is there anything I can do to help make any progress on this merge?

Thanks
Hongjiang Zhang

-----Original Message-----
From: Hongjiang Zhang
Sent: Friday, April 14, 2017 10:58 AM
To: Scott Moser <email address hidden>
Cc: <email address hidden>; Kylie Liang <email address hidden>; Josh Poulson <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

The reason is FreeBSD on Azure have more than 1 disks attached, and the rootfs located in one disk, for example, /dev/da0, but I cannot directly write something below:
/dev/da0 / ufs rw 1 1
Because the rootfs may be switched to /dev/da1. The solution is label the disk. For FreeBSD on Azure, I labeled /dev/da0 with "rootfs", so for get_mount_info, I have to find "rootfs" first, and then find the corresponding /dev/da0.

But on Linux, there is a /proc/$$/mountinfo to help find the device. For Azure, there is also /dev/disk/cloud/azure_root on Linux.
Both make the get_mount_info easy to implement.

-----Original Message-----
From: Scott Moser [mailto:<email address hidden>] On Behalf Of Scott Moser
Sent: Friday, April 14, 2017 10:46 AM
To: Hongjiang Zhang <email address hidden>
Cc: <email address hidden>; Kylie Liang <email address hidden>; Josh Poulson <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

On Fri, 14 Apr 2017, Hongjiang Zhang wrote:

> Hi Scott,
>
> I have manually merged your modifications to my branch and fixed a unit test issue.
> Do you think it is ok for merging?

The only thing I had left was that I'm still confused as what why we need azure specific code in that code path.

I have to think about it some more and probably go play with it a little on freebsd.

>
> Thanks
> Hongjiang Zhang
>
>
> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On Behalf
> Of Scott Moser
> Sent: Thursday, April 13, 2017 2:51 AM
> To: <email address hidden>
> Subject: Re: [Merge] ~redriver/cloud-init:frbsd-azure-branch into
> cloud-init:master
>
> Other than that and I think it looks really good.
> thank you for adding the unit tests.
>
> --
> https://na01.safelinks.protection.outlook.com/?url=https:%2F%2Fcode.la
> unchpad.net%2F~redriver%2Fcloud-init%2F%2Bgit%2Fcloud-init%2F%2Bmerge%
> 2F314895&data=02%7C01%7Chonzhan%40microsoft.com%7Ca62135a0b9514c7e1306
> 08d481d4cf8c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636276198391
> 371945&sdata=q...

Read more...

1b1e0a8... by Hongjiang Zhang

fix the issue of 'ssh does not exist in /etc/rc.d'

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Hongjiang Zhang (redriver) wrote :
Download full text (5.4 KiB)

Hi Scott,

I have removed the Azure specific check for "get_mount_info".
get_mount_info_freebsd() was invoked only if the normal parse_mount failed on FreeBSD platform.
What do you think?

--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -565,6 +565,10 @@ def is_ipv4(instr):
     return len(toks) == 4

+def is_FreeBSD():
+ return system_info()['platform'].startswith('FreeBSD')
+
+
 def get_cfg_option_bool(yobj, key, default=False):
     if key not in yobj:
         return default
@@ -2055,11 +2059,56 @@ def parse_mtab(path):
     return None

+def find_freebsd_part(label_part):
+ if label_part.startswith("/dev/label/"):
+ target_label = label_part[5:]
+ (label_part, err) = subp(['glabel', 'status', '-s'])
+ for labels in label_part.split("\n"):
+ items = labels.split()
+ if len(items) > 0 and items[0].startswith(target_label):
+ label_part = items[2]
+ break
+ label_part = str(label_part)
+ return label_part
+
+
+def get_path_dev_freebsd(path, mnt_list):
+ path_found = None
+ for line in mnt_list.split("\n"):
+ items = line.split()
+ if (len(items) > 2 and os.path.exists(items[1] + path)):
+ path_found = line
+ break
+ return path_found
+
+
+def get_mount_info_freebsd(path, log=LOG):
+ (result, err) = subp(['mount', '-p', path], rcs=[0, 1])
+ if len(err):
+ # find a path if the input is not a mounting point
+ (mnt_list, err) = subp(['mount', '-p'])
+ path_found = get_path_dev_freebsd(path, mnt_list)
+ if (path_found is None):
+ return None
+ result = path_found
+ ret = result.split()
+ label_part = find_freebsd_part(ret[0])
+ return "/dev/" + label_part, ret[2], ret[1]
+
+
 def parse_mount(path):
     (mountoutput, _err) = subp("mount")
     mount_locs = mountoutput.splitlines()
     for line in mount_locs:
         m = re.search(r'^(/dev/[\S]+) on (/.*) \((.+), .+, (.+)\)$', line)
+ if not m:
+ continue
+ # check whether the dev refers to a label on FreeBSD
+ # for example, if dev is '/dev/label/rootfs', we should
+ # continue finding the real device like '/dev/da0'.
+ devm = re.search('^(/dev/.+)p([0-9])$', m.group(1))
+ if (not devm and is_FreeBSD()):
+ return get_mount_info_freebsd(path)
         devpth = m.group(1)

-----Original Message-----
From: Hongjiang Zhang
Sent: Tuesday, April 25, 2017 11:38 AM
To: 'Scott Moser' <email address hidden>
Cc: '<email address hidden>' <email address hidden>; Kylie Liang <email address hidden>; Josh Poulson <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

Hello Scott,

I'm not sure whether you are still confused about why we need Azure specific code, or is there anything I can do to help make any progress on this merge?

Thanks
Hongjiang Zhang

-----Original Message-----
From: Hongjiang Zhang
Sent: Friday, April 14, 2017 10:58 AM
To: Scott Moser <email address hidden>
Cc: <email address hidden>; Kylie Liang <email address hidden>; Josh Poulson <email address hidden>
S...

Read more...

Revision history for this message
Hongjiang Zhang (redriver) wrote :
Download full text (6.1 KiB)

Hi Scott,

I don't want to bother you again and again, but I really hope you can take 3~5 minutes to go through my patch, because it has taken at least 3 months on review since merge request was created.

Your main concern is why I need Azure specific code in get_mount_info, and I have removed this dependence.

Do you have any other concern?

Thanks
Hongjiang Zhang

-----Original Message-----
From: Hongjiang Zhang
Sent: Tuesday, May 2, 2017 10:26 AM
To: 'Scott Moser' <email address hidden>; '<email address hidden>' <email address hidden>
Cc: '<email address hidden>' <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

Hi Scott,

I have removed the Azure specific check for "get_mount_info".
get_mount_info_freebsd() was invoked only if the normal parse_mount failed on FreeBSD platform.
What do you think?

--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -565,6 +565,10 @@ def is_ipv4(instr):
     return len(toks) == 4

+def is_FreeBSD():
+ return system_info()['platform'].startswith('FreeBSD')
+
+
 def get_cfg_option_bool(yobj, key, default=False):
     if key not in yobj:
         return default
@@ -2055,11 +2059,56 @@ def parse_mtab(path):
     return None

+def find_freebsd_part(label_part):
+ if label_part.startswith("/dev/label/"):
+ target_label = label_part[5:]
+ (label_part, err) = subp(['glabel', 'status', '-s'])
+ for labels in label_part.split("\n"):
+ items = labels.split()
+ if len(items) > 0 and items[0].startswith(target_label):
+ label_part = items[2]
+ break
+ label_part = str(label_part)
+ return label_part
+
+
+def get_path_dev_freebsd(path, mnt_list):
+ path_found = None
+ for line in mnt_list.split("\n"):
+ items = line.split()
+ if (len(items) > 2 and os.path.exists(items[1] + path)):
+ path_found = line
+ break
+ return path_found
+
+
+def get_mount_info_freebsd(path, log=LOG):
+ (result, err) = subp(['mount', '-p', path], rcs=[0, 1])
+ if len(err):
+ # find a path if the input is not a mounting point
+ (mnt_list, err) = subp(['mount', '-p'])
+ path_found = get_path_dev_freebsd(path, mnt_list)
+ if (path_found is None):
+ return None
+ result = path_found
+ ret = result.split()
+ label_part = find_freebsd_part(ret[0])
+ return "/dev/" + label_part, ret[2], ret[1]
+
+
 def parse_mount(path):
     (mountoutput, _err) = subp("mount")
     mount_locs = mountoutput.splitlines()
     for line in mount_locs:
         m = re.search(r'^(/dev/[\S]+) on (/.*) \((.+), .+, (.+)\)$', line)
+ if not m:
+ continue
+ # check whether the dev refers to a label on FreeBSD
+ # for example, if dev is '/dev/label/rootfs', we should
+ # continue finding the real device like '/dev/da0'.
+ devm = re.search('^(/dev/.+)p([0-9])$', m.group(1))
+ if (not devm and is_FreeBSD()):
+ return get_mount_info_freebsd(path)
         devpth = m.group(1)

-----Original Message-----
From: Hongjiang Zhang
Sent: Tuesday, April 25, 2...

Read more...

Revision history for this message
Scott Moser (smoser) wrote :
Download full text (7.0 KiB)

On Thu, 4 May 2017, Hongjiang Zhang wrote:

> Hi Scott,
>
> I don't want to bother you again and again, but I really hope you can take 3~5 minutes to go through my patch, because it has taken at least 3 months on review since merge request was created.
>
> Your main concern is why I need Azure specific code in get_mount_info, and I have removed this dependence.
>
> Do you have any other concern?
>
> Thanks
> Hongjiang Zhang

Hongjiang,
I'm really sorry for being so unresponsive.
I really do not mind you bothering me and appreciate your persistence.

I grabbed [1] and made some changes. I think that was your latest.
I pushed that to [2].

Please take a look, and integrate my changes. Then, please propose that
branch and write a good commit message that describes all the fixes you've
made.

I'll give it another look tomorrow. We are really close.

Thank you.

[1] https://code.launchpad.net/~redriver/cloud-init/+git/cloud-init/+ref/cl_on_Azure_0.7.9-fixups
[2] https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+ref/cl_on_Azure_0.7.9-fixups
>
> -----Original Message-----
> From: Hongjiang Zhang
> Sent: Tuesday, May 2, 2017 10:26 AM
> To: 'Scott Moser' <email address hidden>; '<email address hidden>' <email address hidden>
> Cc: '<email address hidden>' <email address hidden>
> Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master
>
> Hi Scott,
>
> I have removed the Azure specific check for "get_mount_info".
> get_mount_info_freebsd() was invoked only if the normal parse_mount failed on FreeBSD platform.
> What do you think?
>
> --- a/cloudinit/util.py
> +++ b/cloudinit/util.py
> @@ -565,6 +565,10 @@ def is_ipv4(instr):
> return len(toks) == 4
>
>
> +def is_FreeBSD():
> + return system_info()['platform'].startswith('FreeBSD')
> +
> +
> def get_cfg_option_bool(yobj, key, default=False):
> if key not in yobj:
> return default
> @@ -2055,11 +2059,56 @@ def parse_mtab(path):
> return None
>
>
> +def find_freebsd_part(label_part):
> + if label_part.startswith("/dev/label/"):
> + target_label = label_part[5:]
> + (label_part, err) = subp(['glabel', 'status', '-s'])
> + for labels in label_part.split("\n"):
> + items = labels.split()
> + if len(items) > 0 and items[0].startswith(target_label):
> + label_part = items[2]
> + break
> + label_part = str(label_part)
> + return label_part
> +
> +
> +def get_path_dev_freebsd(path, mnt_list):
> + path_found = None
> + for line in mnt_list.split("\n"):
> + items = line.split()
> + if (len(items) > 2 and os.path.exists(items[1] + path)):
> + path_found = line
> + break
> + return path_found
> +
> +
> +def get_mount_info_freebsd(path, log=LOG):
> + (result, err) = subp(['mount', '-p', path], rcs=[0, 1])
> + if len(err):
> + # find a path if the input is not a mounting point
> + (mnt_list, err) = subp(['mount', '-p'])
> + path_found = get_path_dev_freebsd(path, mnt_list)
> + if (path_found is None):
> + return None
> + result = path_found
> + ...

Read more...

Revision history for this message
Hongjiang Zhang (redriver) wrote :
Download full text (8.8 KiB)

Hi Scott,

The branch cl_on_Azure_0.7.9 was created recently and used to patch on freebsd ports: cloud-init-azure (see the ports code review: https://reviews.freebsd.org/D10566) since the merge request (314895) to head is still under review.

I hope you can give some comments about another merge request: https://code.launchpad.net/~redriver/cloud-init/+git/cloud-init/+merge/314895 This is the main branch I have been working on for almost 3 months.

Since I did not see the hope to complete that merge request of 314895, I have to create another branch: cl_on_Azure_0.7.9 for FreeBSD cloud-init ports.

Now, the cloud-init-azure ports is almost ready, but I still hope you can take some time on reviewing https://code.launchpad.net/~redriver/cloud-init/+git/cloud-init/+merge/314895

Thanks
Hongjiang Zhang

-----Original Message-----
From: Scott Moser [mailto:<email address hidden>] On Behalf Of Scott Moser
Sent: Thursday, May 4, 2017 11:56 AM
To: Hongjiang Zhang <email address hidden>
Cc: <email address hidden>; Josh Poulson <email address hidden>; Kylie Liang <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

On Thu, 4 May 2017, Hongjiang Zhang wrote:

> Hi Scott,
>
> I don't want to bother you again and again, but I really hope you can take 3~5 minutes to go through my patch, because it has taken at least 3 months on review since merge request was created.
>
> Your main concern is why I need Azure specific code in get_mount_info, and I have removed this dependence.
>
> Do you have any other concern?
>
> Thanks
> Hongjiang Zhang

Hongjiang,
I'm really sorry for being so unresponsive.
I really do not mind you bothering me and appreciate your persistence.

I grabbed [1] and made some changes. I think that was your latest.
I pushed that to [2].

Please take a look, and integrate my changes. Then, please propose that branch and write a good commit message that describes all the fixes you've made.

I'll give it another look tomorrow. We are really close.

Thank you.

[1] https://na01.safelinks.protection.outlook.com/?url=https:%2F%2Fcode.launchpad.net%2F~redriver%2Fcloud-init%2F%2Bgit%2Fcloud-init%2F%2Bref%2Fcl_on_Azure_0.7.9-fixups&data=02%7C01%7Chonzhan%40microsoft.com%7C8d3cb29db2d9482d62c208d492a172d7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636294669492915135&sdata=v%2FIQi1ZJnqAdMCN%2Bz4NTX1fjIIN9i9MItvrwYBJPhNI%3D&reserved=0
[2] https://na01.safelinks.protection.outlook.com/?url=https:%2F%2Fcode.launchpad.net%2F~smoser%2Fcloud-init%2F%2Bgit%2Fcloud-init%2F%2Bref%2Fcl_on_Azure_0.7.9-fixups&data=02%7C01%7Chonzhan%40microsoft.com%7C8d3cb29db2d9482d62c208d492a172d7%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636294669492915135&sdata=mw7O6ukLZl8%2FLXRlmCFQAqFTIa7L2VnOz8xky%2BtHNoA%3D&reserved=0
>
> -----Original Message-----
> From: Hongjiang Zhang
> Sent: Tuesday, May 2, 2017 10:26 AM
> To: 'Scott Moser' <email address hidden>; '<email address hidden>'
> <email address hidden>
> Cc: '<email address hidden>' <email address hidden>
> Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into
> cloud-init:master
>
> Hi Scott,
>
> I have removed the Azure ...

Read more...

Revision history for this message
Hongjiang Zhang (redriver) wrote :
Download full text (10.0 KiB)

Hi Scott,

Sorry for making you confused. Thanks for your reviewing on the branch cl_on_Azure_0.7.9, which is totally for FreeBSD ports, and that patch mainly comes from merge request of 314895.

I hope the 1st step is to make cloud-init head work for FreeBSD on Azure, and the 2nd step is to make FreeBSD ports work.

Could you please focus on my merge request of 314895?

>diff --git a/requirements.txt b/requirements.txt
>index 2559b31..0c4951f 100644
>--- a/requirements.txt
>+++ b/requirements.txt
>@@ -28,7 +28,7 @@ configobj>=5.0.2
> pyyaml
>
> # The new main entrypoint uses argparse instead of optparse
>-# argparse
>+argparse
>
> # Requests handles ssl correctly!
> requests

I removed "argparse" because I found FreeBSD ports will fail to run. The reason is python 2.7 has made argparse builtin library. FreeBSD currently python version is 2.7, so, that is a FreeBSD specific change.
That is why I create FreeBSD ports to hold that change.

Thanks
Hongjiang Zhang

-----Original Message-----
From: Hongjiang Zhang
Sent: Thursday, May 4, 2017 12:49 PM
To: 'Scott Moser' <email address hidden>
Cc: <email address hidden>; Josh Poulson <email address hidden>; Kylie Liang <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

Hi Scott,

The branch cl_on_Azure_0.7.9 was created recently and used to patch on freebsd ports: cloud-init-azure (see the ports code review: https://reviews.freebsd.org/D10566) since the merge request (314895) to head is still under review.

I hope you can give some comments about another merge request: https://code.launchpad.net/~redriver/cloud-init/+git/cloud-init/+merge/314895 This is the main branch I have been working on for almost 3 months.

Since I did not see the hope to complete that merge request of 314895, I have to create another branch: cl_on_Azure_0.7.9 for FreeBSD cloud-init ports.

Now, the cloud-init-azure ports is almost ready, but I still hope you can take some time on reviewing https://code.launchpad.net/~redriver/cloud-init/+git/cloud-init/+merge/314895

Thanks
Hongjiang Zhang

-----Original Message-----
From: Scott Moser [mailto:<email address hidden>] On Behalf Of Scott Moser
Sent: Thursday, May 4, 2017 11:56 AM
To: Hongjiang Zhang <email address hidden>
Cc: <email address hidden>; Josh Poulson <email address hidden>; Kylie Liang <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

On Thu, 4 May 2017, Hongjiang Zhang wrote:

> Hi Scott,
>
> I don't want to bother you again and again, but I really hope you can take 3~5 minutes to go through my patch, because it has taken at least 3 months on review since merge request was created.
>
> Your main concern is why I need Azure specific code in get_mount_info, and I have removed this dependence.
>
> Do you have any other concern?
>
> Thanks
> Hongjiang Zhang

Hongjiang,
I'm really sorry for being so unresponsive.
I really do not mind you bothering me and appreciate your persistence.

I grabbed [1] and made some changes. I think that was your latest.
I pushed that to [2].

Please take a look, and integrate my changes. Then, please propose that ...

77669d5... by Hongjiang Zhang

modify according to Scott comments

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Hongjiang Zhang (redriver) wrote :
Download full text (10.5 KiB)

Hi Scott,

I did not propose the branch of cl_on_Azure_0.7.9. Instead, I manually merged your diff to my existing merge request (https://code.launchpad.net/~redriver/cloud-init/+git/cloud-init/+merge/314895)

Thanks
Hongjiang Zhang

-----Original Message-----
From: Hongjiang Zhang
Sent: Thursday, May 4, 2017 1:09 PM
To: 'Scott Moser' <email address hidden>
Cc: '<email address hidden>' <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

Hi Scott,

Sorry for making you confused. Thanks for your reviewing on the branch cl_on_Azure_0.7.9, which is totally for FreeBSD ports, and that patch mainly comes from merge request of 314895.

I hope the 1st step is to make cloud-init head work for FreeBSD on Azure, and the 2nd step is to make FreeBSD ports work.

Could you please focus on my merge request of 314895?

>diff --git a/requirements.txt b/requirements.txt index 2559b31..0c4951f
>100644
>--- a/requirements.txt
>+++ b/requirements.txt
>@@ -28,7 +28,7 @@ configobj>=5.0.2
> pyyaml
>
> # The new main entrypoint uses argparse instead of optparse -#
>argparse
>+argparse
>
> # Requests handles ssl correctly!
> requests

I removed "argparse" because I found FreeBSD ports will fail to run. The reason is python 2.7 has made argparse builtin library. FreeBSD currently python version is 2.7, so, that is a FreeBSD specific change.
That is why I create FreeBSD ports to hold that change.

Thanks
Hongjiang Zhang

-----Original Message-----
From: Hongjiang Zhang
Sent: Thursday, May 4, 2017 12:49 PM
To: 'Scott Moser' <email address hidden>
Cc: <email address hidden>; Josh Poulson <email address hidden>; Kylie Liang <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

Hi Scott,

The branch cl_on_Azure_0.7.9 was created recently and used to patch on freebsd ports: cloud-init-azure (see the ports code review: https://reviews.freebsd.org/D10566) since the merge request (314895) to head is still under review.

I hope you can give some comments about another merge request: https://code.launchpad.net/~redriver/cloud-init/+git/cloud-init/+merge/314895 This is the main branch I have been working on for almost 3 months.

Since I did not see the hope to complete that merge request of 314895, I have to create another branch: cl_on_Azure_0.7.9 for FreeBSD cloud-init ports.

Now, the cloud-init-azure ports is almost ready, but I still hope you can take some time on reviewing https://code.launchpad.net/~redriver/cloud-init/+git/cloud-init/+merge/314895

Thanks
Hongjiang Zhang

-----Original Message-----
From: Scott Moser [mailto:<email address hidden>] On Behalf Of Scott Moser
Sent: Thursday, May 4, 2017 11:56 AM
To: Hongjiang Zhang <email address hidden>
Cc: <email address hidden>; Josh Poulson <email address hidden>; Kylie Liang <email address hidden>
Subject: RE: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

On Thu, 4 May 2017, Hongjiang Zhang wrote:

> Hi Scott,
>
> I don't want to bother you again and again, but I really hope you can take 3~5 minutes to go through my patch, because it has taken at least 3 months o...

0ff61d4... by Hongjiang Zhang

remove Linux specific installation on FreeBSD

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

I've merged this.
The diff from the merged commit (0a71d5a870b416f2c86c8bc196004bb3fc0768a0)
to your tip (0ff61d4) when rebased is just:

diff --git a/cloudinit/distros/freebsd.py b/cloudinit/distros/freebsd.py
index d9f31bfd..bad112fe 100644
--- a/cloudinit/distros/freebsd.py
+++ b/cloudinit/distros/freebsd.py
@@ -272,7 +272,7 @@ class Distro(distros.Distro):
         cmd = ['ifconfig', '-l']
         (nics, err) = util.subp(cmd, rcs=[0, 1])
         if len(err):
- LOG.warn("Error running %s: %s", cmd, err)
+ LOG.warning("Error running %s: %s", cmd, err)
             return None
         return nics

@@ -281,7 +281,7 @@ class Distro(distros.Distro):
         cmd = ['ifconfig', ifname]
         (if_result, err) = util.subp(cmd, rcs=[0, 1])
         if len(err):
- LOG.warn("Error running %s: %s", cmd, err)
+ LOG.warning("Error running %s: %s", cmd, err)
             return None
         return if_result

@@ -290,7 +290,7 @@ class Distro(distros.Distro):
         cmd = ['ifconfig', '-l', 'ether']
         (nics, err) = util.subp(cmd, rcs=[0, 1])
         if len(err):
- LOG.warn("Error running %s: %s", cmd, err)
+ LOG.warning("Error running %s: %s", cmd, err)
             return None
         return nics

diff --git a/setup.py b/setup.py
index 46d05219..4616599b 100755
--- a/setup.py
+++ b/setup.py
@@ -173,8 +173,9 @@ else:
             [f for f in glob('doc/examples/seed/*') if is_f(f)]),
     ]
     if os.uname()[0] != 'FreeBSD':
- data_files.append([
- (ETC + '/NetworkManager/dispatcher.d/', ['tools/hook-network-manager']),
+ data_files.extend([
+ (ETC + '/NetworkManager/dispatcher.d/',
+ ['tools/hook-network-manager']),
             (ETC + '/dhcp/dhclient-exit-hooks.d/', ['tools/hook-dhclient']),
             (LIB + '/udev/rules.d', [f for f in glob('udev/*.rules')])
         ])

review: Approve
Revision history for this message
Hongjiang Zhang (redriver) wrote :

Hi Scott,

Great to see it is merged finally!

I appreciated all your review and help in the past 2 months.

Thanks
Hongjiang Zhang

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Scott Moser
Sent: Thursday, May 11, 2017 1:05 AM
To: <email address hidden>
Subject: Re: [Merge] ~redriver/cloud-init:frbsd-azure-branch into cloud-init:master

Review: Approve

I've merged this.
The diff from the merged commit (0a71d5a870b416f2c86c8bc196004bb3fc0768a0)
to your tip (0ff61d4) when rebased is just:

diff --git a/cloudinit/distros/freebsd.py b/cloudinit/distros/freebsd.py index d9f31bfd..bad112fe 100644
--- a/cloudinit/distros/freebsd.py
+++ b/cloudinit/distros/freebsd.py
@@ -272,7 +272,7 @@ class Distro(distros.Distro):
         cmd = ['ifconfig', '-l']
         (nics, err) = util.subp(cmd, rcs=[0, 1])
         if len(err):
- LOG.warn("Error running %s: %s", cmd, err)
+ LOG.warning("Error running %s: %s", cmd, err)
             return None
         return nics

@@ -281,7 +281,7 @@ class Distro(distros.Distro):
         cmd = ['ifconfig', ifname]
         (if_result, err) = util.subp(cmd, rcs=[0, 1])
         if len(err):
- LOG.warn("Error running %s: %s", cmd, err)
+ LOG.warning("Error running %s: %s", cmd, err)
             return None
         return if_result

@@ -290,7 +290,7 @@ class Distro(distros.Distro):
         cmd = ['ifconfig', '-l', 'ether']
         (nics, err) = util.subp(cmd, rcs=[0, 1])
         if len(err):
- LOG.warn("Error running %s: %s", cmd, err)
+ LOG.warning("Error running %s: %s", cmd, err)
             return None
         return nics

diff --git a/setup.py b/setup.py
index 46d05219..4616599b 100755
--- a/setup.py
+++ b/setup.py
@@ -173,8 +173,9 @@ else:
             [f for f in glob('doc/examples/seed/*') if is_f(f)]),
     ]
     if os.uname()[0] != 'FreeBSD':
- data_files.append([
- (ETC + '/NetworkManager/dispatcher.d/', ['tools/hook-network-manager']),
+ data_files.extend([
+ (ETC + '/NetworkManager/dispatcher.d/',
+ ['tools/hook-network-manager']),
             (ETC + '/dhcp/dhclient-exit-hooks.d/', ['tools/hook-dhclient']),
             (LIB + '/udev/rules.d', [f for f in glob('udev/*.rules')])
         ])

--
https://na01.safelinks.protection.outlook.com/?url=https:%2F%2Fcode.launchpad.net%2F~redriver%2Fcloud-init%2F%2Bgit%2Fcloud-init%2F%2Bmerge%2F314895&data=02%7C01%7Chonzhan%40microsoft.com%7C6ecd4fba7ef945442b5008d497c6a74e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636300326842539159&sdata=LLwO0W1%2F0bm4EoNr%2BFlbUZmPEphENyRUv1cF1IafsN8%3D&reserved=0
You are the owner of ~redriver/cloud-init:frbsd-azure-branch.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/config/cc_resizefs.py b/cloudinit/config/cc_resizefs.py
2index 60e3ab5..ceee952 100644
3--- a/cloudinit/config/cc_resizefs.py
4+++ b/cloudinit/config/cc_resizefs.py
5@@ -33,7 +33,10 @@ disabled altogether by setting ``resize_rootfs`` to ``false``.
6 """
7
8 import errno
9+import getopt
10 import os
11+import re
12+import shlex
13 import stat
14
15 from cloudinit.settings import PER_ALWAYS
16@@ -58,6 +61,62 @@ def _resize_ufs(mount_point, devpth):
17 return ('growfs', devpth)
18
19
20+def _get_dumpfs_output(mount_point):
21+ dumpfs_res, err = util.subp(['dumpfs', '-m', mount_point])
22+ return dumpfs_res
23+
24+
25+def _get_gpart_output(part):
26+ gpart_res, err = util.subp(['gpart', 'show', part])
27+ return gpart_res
28+
29+
30+def _can_skip_resize_ufs(mount_point, devpth):
31+ # extract the current fs sector size
32+ """
33+ # dumpfs -m /
34+ # newfs command for / (/dev/label/rootfs)
35+ newfs -O 2 -U -a 4 -b 32768 -d 32768 -e 4096 -f 4096 -g 16384
36+ -h 64 -i 8192 -j -k 6408 -m 8 -o time -s 58719232 /dev/label/rootf
37+ """
38+ cur_fs_sz = None
39+ frag_sz = None
40+ dumpfs_res = _get_dumpfs_output(mount_point)
41+ for line in dumpfs_res.splitlines():
42+ if not line.startswith('#'):
43+ newfs_cmd = shlex.split(line)
44+ opt_value = 'O:Ua:s:b:d:e:f:g:h:i:jk:m:o:'
45+ optlist, args = getopt.getopt(newfs_cmd[1:], opt_value)
46+ for o, a in optlist:
47+ if o == "-s":
48+ cur_fs_sz = int(a)
49+ if o == "-f":
50+ frag_sz = int(a)
51+ # check the current partition size
52+ """
53+ # gpart show /dev/da0
54+=> 40 62914480 da0 GPT (30G)
55+ 40 1024 1 freebsd-boot (512K)
56+ 1064 58719232 2 freebsd-ufs (28G)
57+ 58720296 3145728 3 freebsd-swap (1.5G)
58+ 61866024 1048496 - free - (512M)
59+ """
60+ expect_sz = None
61+ m = re.search('^(/dev/.+)p([0-9])$', devpth)
62+ gpart_res = _get_gpart_output(m.group(1))
63+ for line in gpart_res.splitlines():
64+ if re.search(r"freebsd-ufs", line):
65+ fields = line.split()
66+ expect_sz = int(fields[1])
67+ # Normalize the gpart sector size,
68+ # because the size is not exactly the same as fs size.
69+ normal_expect_sz = (expect_sz - expect_sz % (frag_sz / 512))
70+ if normal_expect_sz == cur_fs_sz:
71+ return True
72+ else:
73+ return False
74+
75+
76 # Do not use a dictionary as these commands should be able to be used
77 # for multiple filesystem types if possible, e.g. one command for
78 # ext2, ext3 and ext4.
79@@ -68,9 +127,40 @@ RESIZE_FS_PREFIXES_CMDS = [
80 ('ufs', _resize_ufs),
81 ]
82
83+RESIZE_FS_PRECHECK_CMDS = {
84+ 'ufs': _can_skip_resize_ufs
85+}
86+
87 NOBLOCK = "noblock"
88
89
90+def rootdev_from_cmdline(cmdline):
91+ found = None
92+ for tok in cmdline.split():
93+ if tok.startswith("root="):
94+ found = tok[5:]
95+ break
96+ if found is None:
97+ return None
98+
99+ if found.startswith("/dev/"):
100+ return found
101+ if found.startswith("LABEL="):
102+ return "/dev/disk/by-label/" + found[len("LABEL="):]
103+ if found.startswith("UUID="):
104+ return "/dev/disk/by-uuid/" + found[len("UUID="):]
105+
106+ return "/dev/" + found
107+
108+
109+def can_skip_resize(fs_type, resize_what, devpth):
110+ fstype_lc = fs_type.lower()
111+ for i, func in RESIZE_FS_PRECHECK_CMDS.items():
112+ if fstype_lc.startswith(i):
113+ return func(resize_what, devpth)
114+ return False
115+
116+
117 def handle(name, cfg, _cloud, log, args):
118 if len(args) != 0:
119 resize_root = args[0]
120@@ -139,6 +229,11 @@ def handle(name, cfg, _cloud, log, args):
121 return
122
123 resizer = None
124+ if can_skip_resize(fs_type, resize_what, devpth):
125+ log.debug("Skip resize filesystem type %s for %s",
126+ fs_type, resize_what)
127+ return
128+
129 fstype_lc = fs_type.lower()
130 for (pfix, root_cmd) in RESIZE_FS_PREFIXES_CMDS:
131 if fstype_lc.startswith(pfix):
132diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
133index 28650b8..f56c0cf 100755
134--- a/cloudinit/distros/__init__.py
135+++ b/cloudinit/distros/__init__.py
136@@ -155,6 +155,9 @@ class Distro(object):
137 ns, header=header, render_hwaddress=True)
138 return self.apply_network(contents, bring_up=bring_up)
139
140+ def generate_fallback_config(self):
141+ return net.generate_fallback_config()
142+
143 def apply_network_config(self, netconfig, bring_up=False):
144 # apply network config netconfig
145 # This method is preferred to apply_network which only takes
146diff --git a/cloudinit/distros/freebsd.py b/cloudinit/distros/freebsd.py
147index 183e445..d9f31bf 100644
148--- a/cloudinit/distros/freebsd.py
149+++ b/cloudinit/distros/freebsd.py
150@@ -30,6 +30,7 @@ class Distro(distros.Distro):
151 login_conf_fn_bak = '/etc/login.conf.orig'
152 resolv_conf_fn = '/etc/resolv.conf'
153 ci_sudoers_fn = '/usr/local/etc/sudoers.d/90-cloud-init-users'
154+ default_primary_nic = 'hn0'
155
156 def __init__(self, name, cfg, paths):
157 distros.Distro.__init__(self, name, cfg, paths)
158@@ -38,6 +39,8 @@ class Distro(distros.Distro):
159 # should only happen say once per instance...)
160 self._runner = helpers.Runners(paths)
161 self.osfamily = 'freebsd'
162+ self.ipv4_pat = re.compile(r"\s+inet\s+\d+[.]\d+[.]\d+[.]\d+")
163+ cfg['ssh_svcname'] = 'sshd'
164
165 # Updates a key in /etc/rc.conf.
166 def updatercconf(self, key, value):
167@@ -183,7 +186,6 @@ class Distro(distros.Distro):
168 "gecos": '-c',
169 "primary_group": '-g',
170 "groups": '-G',
171- "passwd": '-h',
172 "shell": '-s',
173 "inactive": '-E',
174 }
175@@ -193,19 +195,11 @@ class Distro(distros.Distro):
176 "no_log_init": '--no-log-init',
177 }
178
179- redact_opts = ['passwd']
180-
181 for key, val in kwargs.items():
182 if (key in adduser_opts and val and
183 isinstance(val, six.string_types)):
184 adduser_cmd.extend([adduser_opts[key], val])
185
186- # Redact certain fields from the logs
187- if key in redact_opts:
188- log_adduser_cmd.extend([adduser_opts[key], 'REDACTED'])
189- else:
190- log_adduser_cmd.extend([adduser_opts[key], val])
191-
192 elif key in adduser_flags and val:
193 adduser_cmd.append(adduser_flags[key])
194 log_adduser_cmd.append(adduser_flags[key])
195@@ -226,19 +220,21 @@ class Distro(distros.Distro):
196 except Exception as e:
197 util.logexc(LOG, "Failed to create user %s", name)
198 raise e
199+ # Set the password if it is provided
200+ # For security consideration, only hashed passwd is assumed
201+ passwd_val = kwargs.get('passwd', None)
202+ if passwd_val is not None:
203+ self.set_passwd(name, passwd_val, hashed=True)
204
205 def set_passwd(self, user, passwd, hashed=False):
206- cmd = ['pw', 'usermod', user]
207-
208 if hashed:
209- cmd.append('-H')
210+ hash_opt = "-H"
211 else:
212- cmd.append('-h')
213-
214- cmd.append('0')
215+ hash_opt = "-h"
216
217 try:
218- util.subp(cmd, passwd, logstring="chpasswd for %s" % user)
219+ util.subp(['pw', 'usermod', user, hash_opt, '0'],
220+ data=passwd, logstring="chpasswd for %s" % user)
221 except Exception as e:
222 util.logexc(LOG, "Failed to set password for %s", user)
223 raise e
224@@ -271,6 +267,255 @@ class Distro(distros.Distro):
225 keys = set(kwargs['ssh_authorized_keys']) or []
226 ssh_util.setup_user_keys(keys, name, options=None)
227
228+ @staticmethod
229+ def get_ifconfig_list():
230+ cmd = ['ifconfig', '-l']
231+ (nics, err) = util.subp(cmd, rcs=[0, 1])
232+ if len(err):
233+ LOG.warn("Error running %s: %s", cmd, err)
234+ return None
235+ return nics
236+
237+ @staticmethod
238+ def get_ifconfig_ifname_out(ifname):
239+ cmd = ['ifconfig', ifname]
240+ (if_result, err) = util.subp(cmd, rcs=[0, 1])
241+ if len(err):
242+ LOG.warn("Error running %s: %s", cmd, err)
243+ return None
244+ return if_result
245+
246+ @staticmethod
247+ def get_ifconfig_ether():
248+ cmd = ['ifconfig', '-l', 'ether']
249+ (nics, err) = util.subp(cmd, rcs=[0, 1])
250+ if len(err):
251+ LOG.warn("Error running %s: %s", cmd, err)
252+ return None
253+ return nics
254+
255+ @staticmethod
256+ def get_interface_mac(ifname):
257+ if_result = Distro.get_ifconfig_ifname_out(ifname)
258+ for item in if_result.splitlines():
259+ if item.find('ether ') != -1:
260+ mac = str(item.split()[1])
261+ if mac:
262+ return mac
263+
264+ @staticmethod
265+ def get_devicelist():
266+ nics = Distro.get_ifconfig_list()
267+ return nics.split()
268+
269+ @staticmethod
270+ def get_ipv6():
271+ ipv6 = []
272+ nics = Distro.get_devicelist()
273+ for nic in nics:
274+ if_result = Distro.get_ifconfig_ifname_out(nic)
275+ for item in if_result.splitlines():
276+ if item.find("inet6 ") != -1 and item.find("scopeid") == -1:
277+ ipv6.append(nic)
278+ return ipv6
279+
280+ def get_ipv4(self):
281+ ipv4 = []
282+ nics = Distro.get_devicelist()
283+ for nic in nics:
284+ if_result = Distro.get_ifconfig_ifname_out(nic)
285+ for item in if_result.splitlines():
286+ print(item)
287+ if self.ipv4_pat.match(item):
288+ ipv4.append(nic)
289+ return ipv4
290+
291+ def is_up(self, ifname):
292+ if_result = Distro.get_ifconfig_ifname_out(ifname)
293+ pat = "^" + ifname
294+ for item in if_result.splitlines():
295+ if re.match(pat, item):
296+ flags = item.split('<')[1].split('>')[0]
297+ if flags.find("UP") != -1:
298+ return True
299+
300+ def _get_current_rename_info(self, check_downable=True):
301+ """Collect information necessary for rename_interfaces."""
302+ names = Distro.get_devicelist()
303+ bymac = {}
304+ for n in names:
305+ bymac[Distro.get_interface_mac(n)] = {
306+ 'name': n, 'up': self.is_up(n), 'downable': None}
307+
308+ if check_downable:
309+ nics_with_addresses = set()
310+ ipv6 = self.get_ipv6()
311+ ipv4 = self.get_ipv4()
312+ for bytes_out in (ipv6, ipv4):
313+ for i in ipv6:
314+ nics_with_addresses.update(i)
315+ for i in ipv4:
316+ nics_with_addresses.update(i)
317+
318+ for d in bymac.values():
319+ d['downable'] = (d['up'] is False or
320+ d['name'] not in nics_with_addresses)
321+
322+ return bymac
323+
324+ def _rename_interfaces(self, renames):
325+ if not len(renames):
326+ LOG.debug("no interfaces to rename")
327+ return
328+
329+ current_info = self._get_current_rename_info()
330+
331+ cur_bymac = {}
332+ for mac, data in current_info.items():
333+ cur = data.copy()
334+ cur['mac'] = mac
335+ cur_bymac[mac] = cur
336+
337+ def update_byname(bymac):
338+ return dict((data['name'], data)
339+ for data in bymac.values())
340+
341+ def rename(cur, new):
342+ util.subp(["ifconfig", cur, "name", new], capture=True)
343+
344+ def down(name):
345+ util.subp(["ifconfig", name, "down"], capture=True)
346+
347+ def up(name):
348+ util.subp(["ifconfig", name, "up"], capture=True)
349+
350+ ops = []
351+ errors = []
352+ ups = []
353+ cur_byname = update_byname(cur_bymac)
354+ tmpname_fmt = "cirename%d"
355+ tmpi = -1
356+
357+ for mac, new_name in renames:
358+ cur = cur_bymac.get(mac, {})
359+ cur_name = cur.get('name')
360+ cur_ops = []
361+ if cur_name == new_name:
362+ # nothing to do
363+ continue
364+
365+ if not cur_name:
366+ errors.append("[nic not present] Cannot rename mac=%s to %s"
367+ ", not available." % (mac, new_name))
368+ continue
369+
370+ if cur['up']:
371+ msg = "[busy] Error renaming mac=%s from %s to %s"
372+ if not cur['downable']:
373+ errors.append(msg % (mac, cur_name, new_name))
374+ continue
375+ cur['up'] = False
376+ cur_ops.append(("down", mac, new_name, (cur_name,)))
377+ ups.append(("up", mac, new_name, (new_name,)))
378+
379+ if new_name in cur_byname:
380+ target = cur_byname[new_name]
381+ if target['up']:
382+ msg = "[busy-target] Error renaming mac=%s from %s to %s."
383+ if not target['downable']:
384+ errors.append(msg % (mac, cur_name, new_name))
385+ continue
386+ else:
387+ cur_ops.append(("down", mac, new_name, (new_name,)))
388+
389+ tmp_name = None
390+ while tmp_name is None or tmp_name in cur_byname:
391+ tmpi += 1
392+ tmp_name = tmpname_fmt % tmpi
393+
394+ cur_ops.append(("rename", mac, new_name, (new_name, tmp_name)))
395+ target['name'] = tmp_name
396+ cur_byname = update_byname(cur_bymac)
397+ if target['up']:
398+ ups.append(("up", mac, new_name, (tmp_name,)))
399+
400+ cur_ops.append(("rename", mac, new_name, (cur['name'], new_name)))
401+ cur['name'] = new_name
402+ cur_byname = update_byname(cur_bymac)
403+ ops += cur_ops
404+
405+ opmap = {'rename': rename, 'down': down, 'up': up}
406+ if len(ops) + len(ups) == 0:
407+ if len(errors):
408+ LOG.debug("unable to do any work for renaming of %s", renames)
409+ else:
410+ LOG.debug("no work necessary for renaming of %s", renames)
411+ else:
412+ LOG.debug("achieving renaming of %s with ops %s",
413+ renames, ops + ups)
414+
415+ for op, mac, new_name, params in ops + ups:
416+ try:
417+ opmap.get(op)(*params)
418+ except Exception as e:
419+ errors.append(
420+ "[unknown] Error performing %s%s for %s, %s: %s" %
421+ (op, params, mac, new_name, e))
422+ if len(errors):
423+ raise Exception('\n'.join(errors))
424+
425+ def apply_network_config_names(self, netcfg):
426+ renames = []
427+ for ent in netcfg.get('config', {}):
428+ if ent.get('type') != 'physical':
429+ continue
430+ mac = ent.get('mac_address')
431+ name = ent.get('name')
432+ if not mac:
433+ continue
434+ renames.append([mac, name])
435+ return self._rename_interfaces(renames)
436+
437+ @classmethod
438+ def generate_fallback_config(self):
439+ nics = Distro.get_ifconfig_ether()
440+ if nics is None:
441+ LOG.debug("Fail to get network interfaces")
442+ return None
443+ potential_interfaces = nics.split()
444+ connected = []
445+ for nic in potential_interfaces:
446+ pat = "^" + nic
447+ if_result = Distro.get_ifconfig_ifname_out(nic)
448+ for item in if_result.split("\n"):
449+ if re.match(pat, item):
450+ flags = item.split('<')[1].split('>')[0]
451+ if flags.find("RUNNING") != -1:
452+ connected.append(nic)
453+ if connected:
454+ potential_interfaces = connected
455+ names = list(sorted(potential_interfaces))
456+ default_pri_nic = Distro.default_primary_nic
457+ if default_pri_nic in names:
458+ names.remove(default_pri_nic)
459+ names.insert(0, default_pri_nic)
460+ target_name = None
461+ target_mac = None
462+ for name in names:
463+ mac = Distro.get_interface_mac(name)
464+ if mac:
465+ target_name = name
466+ target_mac = mac
467+ break
468+ if target_mac and target_name:
469+ nconf = {'config': [], 'version': 1}
470+ nconf['config'].append(
471+ {'type': 'physical', 'name': target_name,
472+ 'mac_address': target_mac, 'subnets': [{'type': 'dhcp'}]})
473+ return nconf
474+ else:
475+ return None
476+
477 def _write_network(self, settings):
478 entries = net_util.translate_network(settings)
479 nameservers = []
480diff --git a/cloudinit/settings.py b/cloudinit/settings.py
481index dbafead..411960d 100644
482--- a/cloudinit/settings.py
483+++ b/cloudinit/settings.py
484@@ -39,7 +39,7 @@ CFG_BUILTIN = {
485 ],
486 'def_log_file': '/var/log/cloud-init.log',
487 'log_cfgs': [],
488- 'syslog_fix_perms': ['syslog:adm', 'root:adm'],
489+ 'syslog_fix_perms': ['syslog:adm', 'root:adm', 'root:wheel'],
490 'system_info': {
491 'paths': {
492 'cloud_dir': '/var/lib/cloud',
493diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
494index 04358b7..5254e18 100644
495--- a/cloudinit/sources/DataSourceAzure.py
496+++ b/cloudinit/sources/DataSourceAzure.py
497@@ -10,6 +10,7 @@ import crypt
498 from functools import partial
499 import os
500 import os.path
501+import re
502 import time
503 from xml.dom import minidom
504 import xml.etree.ElementTree as ET
505@@ -32,19 +33,160 @@ BOUNCE_COMMAND = [
506 # azure systems will always have a resource disk, and 66-azure-ephemeral.rules
507 # ensures that it gets linked to this path.
508 RESOURCE_DISK_PATH = '/dev/disk/cloud/azure_resource'
509+DEFAULT_PRIMARY_NIC = 'eth0'
510+LEASE_FILE = '/var/lib/dhcp/dhclient.eth0.leases'
511+DEFAULT_FS = 'ext4'
512+
513+
514+def find_storvscid_from_sysctl_pnpinfo(sysctl_out, deviceid):
515+ # extract the 'X' from dev.storvsc.X. if deviceid matches
516+ """
517+ dev.storvsc.1.%pnpinfo:
518+ classid=32412632-86cb-44a2-9b5c-50d1417354f5
519+ deviceid=00000000-0001-8899-0000-000000000000
520+ """
521+ for line in sysctl_out.splitlines():
522+ if re.search(r"pnpinfo", line):
523+ fields = line.split()
524+ if len(fields) >= 3:
525+ columns = fields[2].split('=')
526+ if (len(columns) >= 2 and
527+ columns[0] == "deviceid" and
528+ columns[1].startswith(deviceid)):
529+ comps = fields[0].split('.')
530+ return comps[2]
531+ return None
532+
533+
534+def find_busdev_from_disk(camcontrol_out, disk_drv):
535+ # find the scbusX from 'camcontrol devlist -b' output
536+ # if disk_drv matches the specified disk driver, i.e. blkvsc1
537+ """
538+ scbus0 on ata0 bus 0
539+ scbus1 on ata1 bus 0
540+ scbus2 on blkvsc0 bus 0
541+ scbus3 on blkvsc1 bus 0
542+ scbus4 on storvsc2 bus 0
543+ scbus5 on storvsc3 bus 0
544+ scbus-1 on xpt0 bus 0
545+ """
546+ for line in camcontrol_out.splitlines():
547+ if re.search(disk_drv, line):
548+ items = line.split()
549+ return items[0]
550+ return None
551+
552+
553+def find_dev_from_busdev(camcontrol_out, busdev):
554+ # find the daX from 'camcontrol devlist' output
555+ # if busdev matches the specified value, i.e. 'scbus2'
556+ """
557+ <Msft Virtual CD/ROM 1.0> at scbus1 target 0 lun 0 (cd0,pass0)
558+ <Msft Virtual Disk 1.0> at scbus2 target 0 lun 0 (da0,pass1)
559+ <Msft Virtual Disk 1.0> at scbus3 target 1 lun 0 (da1,pass2)
560+ """
561+ for line in camcontrol_out.splitlines():
562+ if re.search(busdev, line):
563+ items = line.split('(')
564+ if len(items) == 2:
565+ dev_pass = items[1].split(',')
566+ return dev_pass[0]
567+ return None
568+
569+
570+def get_dev_storvsc_sysctl():
571+ try:
572+ sysctl_out, err = util.subp(['sysctl', 'dev.storvsc'])
573+ except util.ProcessExecutionError:
574+ LOG.debug("Fail to execute sysctl dev.storvsc")
575+ return None
576+ return sysctl_out
577+
578+
579+def get_camcontrol_dev_bus():
580+ try:
581+ camcontrol_b_out, err = util.subp(['camcontrol', 'devlist', '-b'])
582+ except util.ProcessExecutionError:
583+ LOG.debug("Fail to execute camcontrol devlist -b")
584+ return None
585+ return camcontrol_b_out
586+
587+
588+def get_camcontrol_dev():
589+ try:
590+ camcontrol_out, err = util.subp(['camcontrol', 'devlist'])
591+ except util.ProcessExecutionError:
592+ LOG.debug("Fail to execute camcontrol devlist")
593+ return None
594+ return camcontrol_out
595+
596+
597+def get_resource_disk_on_freebsd(port_id):
598+ g0 = "00000000"
599+ if port_id > 1:
600+ g0 = "00000001"
601+ port_id = port_id - 2
602+ g1 = "000" + str(port_id)
603+ g0g1 = "{0}-{1}".format(g0, g1)
604+ """
605+ search 'X' from
606+ 'dev.storvsc.X.%pnpinfo:
607+ classid=32412632-86cb-44a2-9b5c-50d1417354f5
608+ deviceid=00000000-0001-8899-0000-000000000000'
609+ """
610+ sysctl_out = get_dev_storvsc_sysctl()
611+
612+ storvscid = find_storvscid_from_sysctl_pnpinfo(sysctl_out, g0g1)
613+ if not storvscid:
614+ LOG.debug("Fail to find storvsc id from sysctl")
615+ return None
616+
617+ camcontrol_b_out = get_camcontrol_dev_bus()
618+ camcontrol_out = get_camcontrol_dev()
619+ # try to find /dev/XX from 'blkvsc' device
620+ blkvsc = "blkvsc{0}".format(storvscid)
621+ scbusx = find_busdev_from_disk(camcontrol_b_out, blkvsc)
622+ if scbusx:
623+ devname = find_dev_from_busdev(camcontrol_out, scbusx)
624+ if devname is None:
625+ LOG.debug("Fail to find /dev/daX")
626+ return None
627+ return devname
628+ # try to find /dev/XX from 'storvsc' device
629+ storvsc = "storvsc{0}".format(storvscid)
630+ scbusx = find_busdev_from_disk(camcontrol_b_out, storvsc)
631+ if scbusx:
632+ devname = find_dev_from_busdev(camcontrol_out, scbusx)
633+ if devname is None:
634+ LOG.debug("Fail to find /dev/daX")
635+ return None
636+ return devname
637+ return None
638+
639+# update the FreeBSD specific information
640+if util.is_FreeBSD():
641+ DEFAULT_PRIMARY_NIC = 'hn0'
642+ LEASE_FILE = '/var/db/dhclient.leases.hn0'
643+ DEFAULT_FS = 'freebsd-ufs'
644+ res_disk = get_resource_disk_on_freebsd(1)
645+ if res_disk is not None:
646+ LOG.debug("resource disk is not None")
647+ RESOURCE_DISK_PATH = "/dev/" + res_disk
648+ else:
649+ LOG.debug("resource disk is None")
650
651 BUILTIN_DS_CONFIG = {
652 'agent_command': AGENT_START_BUILTIN,
653 'data_dir': "/var/lib/waagent",
654 'set_hostname': True,
655 'hostname_bounce': {
656- 'interface': 'eth0',
657+ 'interface': DEFAULT_PRIMARY_NIC,
658 'policy': True,
659 'command': BOUNCE_COMMAND,
660 'hostname_command': 'hostname',
661 },
662 'disk_aliases': {'ephemeral0': RESOURCE_DISK_PATH},
663- 'dhclient_lease_file': '/var/lib/dhcp/dhclient.eth0.leases',
664+ 'dhclient_lease_file': LEASE_FILE,
665 }
666
667 BUILTIN_CLOUD_CONFIG = {
668@@ -53,7 +195,7 @@ BUILTIN_CLOUD_CONFIG = {
669 'layout': [100],
670 'overwrite': True},
671 },
672- 'fs_setup': [{'filesystem': 'ext4',
673+ 'fs_setup': [{'filesystem': DEFAULT_FS,
674 'device': 'ephemeral0.1',
675 'replace_fs': 'ntfs'}],
676 }
677@@ -190,7 +332,11 @@ class DataSourceAzureNet(sources.DataSource):
678 for cdev in candidates:
679 try:
680 if cdev.startswith("/dev/"):
681- ret = util.mount_cb(cdev, load_azure_ds_dir)
682+ if util.is_FreeBSD():
683+ ret = util.mount_cb(cdev, load_azure_ds_dir,
684+ mtype="udf", sync=False)
685+ else:
686+ ret = util.mount_cb(cdev, load_azure_ds_dir)
687 else:
688 ret = load_azure_ds_dir(cdev)
689
690@@ -218,11 +364,12 @@ class DataSourceAzureNet(sources.DataSource):
691 LOG.debug("using files cached in %s", ddir)
692
693 # azure / hyper-v provides random data here
694- seed = util.load_file("/sys/firmware/acpi/tables/OEM0",
695- quiet=True, decode=False)
696- if seed:
697- self.metadata['random_seed'] = seed
698-
699+ if not util.is_FreeBSD():
700+ seed = util.load_file("/sys/firmware/acpi/tables/OEM0",
701+ quiet=True, decode=False)
702+ if seed:
703+ self.metadata['random_seed'] = seed
704+ # TODO. find the seed on FreeBSD platform
705 # now update ds_cfg to reflect contents pass in config
706 user_ds_cfg = util.get_cfg_by_path(self.cfg, DS_CFG_PATH, {})
707 self.ds_cfg = util.mergemanydict([user_ds_cfg, self.ds_cfg])
708@@ -633,8 +780,19 @@ def encrypt_pass(password, salt_id="$6$"):
709 def list_possible_azure_ds_devs():
710 # return a sorted list of devices that might have a azure datasource
711 devlist = []
712- for fstype in ("iso9660", "udf"):
713- devlist.extend(util.find_devs_with("TYPE=%s" % fstype))
714+ if util.is_FreeBSD():
715+ cdrom_dev = "/dev/cd0"
716+ try:
717+ util.subp(["mount", "-o", "ro", "-t", "udf", cdrom_dev,
718+ "/mnt/cdrom/secure"])
719+ except util.ProcessExecutionError:
720+ LOG.debug("Fail to mount cd")
721+ return devlist
722+ util.subp(["umount", "/mnt/cdrom/secure"])
723+ devlist.append(cdrom_dev)
724+ else:
725+ for fstype in ("iso9660", "udf"):
726+ devlist.extend(util.find_devs_with("TYPE=%s" % fstype))
727
728 devlist.sort(reverse=True)
729 return devlist
730diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py
731index 6e01aa4..e22409d 100644
732--- a/cloudinit/sources/helpers/azure.py
733+++ b/cloudinit/sources/helpers/azure.py
734@@ -29,6 +29,14 @@ def cd(newdir):
735 os.chdir(prevdir)
736
737
738+def _get_dhcp_endpoint_option_name():
739+ if util.is_FreeBSD():
740+ azure_endpoint = "option-245"
741+ else:
742+ azure_endpoint = "unknown-245"
743+ return azure_endpoint
744+
745+
746 class AzureEndpointHttpClient(object):
747
748 headers = {
749@@ -235,8 +243,9 @@ class WALinuxAgentShim(object):
750 leases = []
751 content = util.load_file(fallback_lease_file)
752 LOG.debug("content is %s", content)
753+ option_name = _get_dhcp_endpoint_option_name()
754 for line in content.splitlines():
755- if 'unknown-245' in line:
756+ if option_name in line:
757 # Example line from Ubuntu
758 # option unknown-245 a8:3f:81:10;
759 leases.append(line.strip(' ').split(' ', 2)[-1].strip(';\n"'))
760diff --git a/cloudinit/stages.py b/cloudinit/stages.py
761index f7191b0..ad55782 100644
762--- a/cloudinit/stages.py
763+++ b/cloudinit/stages.py
764@@ -624,7 +624,7 @@ class Init(object):
765 return (None, loc)
766 if ncfg:
767 return (ncfg, loc)
768- return (net.generate_fallback_config(), "fallback")
769+ return (self.distro.generate_fallback_config(), "fallback")
770
771 def apply_network_config(self, bring_up):
772 netcfg, src = self._find_networking_config()
773diff --git a/cloudinit/util.py b/cloudinit/util.py
774index 22af99d..27a9833 100644
775--- a/cloudinit/util.py
776+++ b/cloudinit/util.py
777@@ -565,6 +565,10 @@ def is_ipv4(instr):
778 return len(toks) == 4
779
780
781+def is_FreeBSD():
782+ return system_info()['platform'].startswith('FreeBSD')
783+
784+
785 def get_cfg_option_bool(yobj, key, default=False):
786 if key not in yobj:
787 return default
788@@ -2055,11 +2059,56 @@ def parse_mtab(path):
789 return None
790
791
792+def find_freebsd_part(label_part):
793+ if label_part.startswith("/dev/label/"):
794+ target_label = label_part[5:]
795+ (label_part, err) = subp(['glabel', 'status', '-s'])
796+ for labels in label_part.split("\n"):
797+ items = labels.split()
798+ if len(items) > 0 and items[0].startswith(target_label):
799+ label_part = items[2]
800+ break
801+ label_part = str(label_part)
802+ return label_part
803+
804+
805+def get_path_dev_freebsd(path, mnt_list):
806+ path_found = None
807+ for line in mnt_list.split("\n"):
808+ items = line.split()
809+ if (len(items) > 2 and os.path.exists(items[1] + path)):
810+ path_found = line
811+ break
812+ return path_found
813+
814+
815+def get_mount_info_freebsd(path, log=LOG):
816+ (result, err) = subp(['mount', '-p', path], rcs=[0, 1])
817+ if len(err):
818+ # find a path if the input is not a mounting point
819+ (mnt_list, err) = subp(['mount', '-p'])
820+ path_found = get_path_dev_freebsd(path, mnt_list)
821+ if (path_found is None):
822+ return None
823+ result = path_found
824+ ret = result.split()
825+ label_part = find_freebsd_part(ret[0])
826+ return "/dev/" + label_part, ret[2], ret[1]
827+
828+
829 def parse_mount(path):
830 (mountoutput, _err) = subp("mount")
831 mount_locs = mountoutput.splitlines()
832 for line in mount_locs:
833 m = re.search(r'^(/dev/[\S]+) on (/.*) \((.+), .+, (.+)\)$', line)
834+ if not m:
835+ continue
836+ # check whether the dev refers to a label on FreeBSD
837+ # for example, if dev is '/dev/label/rootfs', we should
838+ # continue finding the real device like '/dev/da0'.
839+ devm = re.search('^(/dev/.+)p([0-9])$', m.group(1))
840+ if (not devm and is_FreeBSD()):
841+ return get_mount_info_freebsd(path)
842 devpth = m.group(1)
843 mount_point = m.group(2)
844 fs_type = m.group(3)
845@@ -2336,7 +2385,8 @@ def read_dmi_data(key):
846 uname_arch = os.uname()[4]
847 if not (uname_arch == "x86_64" or
848 (uname_arch.startswith("i") and uname_arch[2:] == "86") or
849- uname_arch == 'aarch64'):
850+ uname_arch == 'aarch64' or
851+ uname_arch == 'amd64'):
852 LOG.debug("dmidata is not supported on %s", uname_arch)
853 return None
854
855diff --git a/config/cloud.cfg-freebsd b/config/cloud.cfg-freebsd
856index be664f5..d666c39 100644
857--- a/config/cloud.cfg-freebsd
858+++ b/config/cloud.cfg-freebsd
859@@ -5,7 +5,7 @@ syslog_fix_perms: root:wheel
860
861 # This should not be required, but leave it in place until the real cause of
862 # not beeing able to find -any- datasources is resolved.
863-datasource_list: ['ConfigDrive', 'OpenStack', 'Ec2']
864+datasource_list: ['ConfigDrive', 'Azure', 'OpenStack', 'Ec2']
865
866 # A set of users which may be applied and/or used by various modules
867 # when a 'default' entry is found it will reference the 'default_user'
868diff --git a/setup.py b/setup.py
869index 32a44d9..46d0521 100755
870--- a/setup.py
871+++ b/setup.py
872@@ -89,7 +89,6 @@ LIB = "/lib"
873 if os.uname()[0] == 'FreeBSD':
874 USR = "/usr/local"
875 USR_LIB_EXEC = "/usr/local/lib"
876- ETC = "/usr/local/etc"
877 elif os.path.isfile('/etc/redhat-release'):
878 USR_LIB_EXEC = "/usr/libexec"
879
880@@ -164,8 +163,6 @@ else:
881 (ETC + '/cloud', glob('config/*.cfg')),
882 (ETC + '/cloud/cloud.cfg.d', glob('config/cloud.cfg.d/*')),
883 (ETC + '/cloud/templates', glob('templates/*')),
884- (ETC + '/NetworkManager/dispatcher.d/', ['tools/hook-network-manager']),
885- (ETC + '/dhcp/dhclient-exit-hooks.d/', ['tools/hook-dhclient']),
886 (USR_LIB_EXEC + '/cloud-init', ['tools/ds-identify',
887 'tools/uncloud-init',
888 'tools/write-ssh-key-fingerprints']),
889@@ -174,8 +171,13 @@ else:
890 [f for f in glob('doc/examples/*') if is_f(f)]),
891 (USR + '/share/doc/cloud-init/examples/seed',
892 [f for f in glob('doc/examples/seed/*') if is_f(f)]),
893- (LIB + '/udev/rules.d', [f for f in glob('udev/*.rules')]),
894 ]
895+ if os.uname()[0] != 'FreeBSD':
896+ data_files.append([
897+ (ETC + '/NetworkManager/dispatcher.d/', ['tools/hook-network-manager']),
898+ (ETC + '/dhcp/dhclient-exit-hooks.d/', ['tools/hook-dhclient']),
899+ (LIB + '/udev/rules.d', [f for f in glob('udev/*.rules')])
900+ ])
901 # Use a subclass for install that handles
902 # adding on the right init system configuration files
903 cmdclass = {
904diff --git a/sysvinit/freebsd/cloudconfig b/sysvinit/freebsd/cloudconfig
905index 01bc061..e4064fa 100755
906--- a/sysvinit/freebsd/cloudconfig
907+++ b/sysvinit/freebsd/cloudconfig
908@@ -7,24 +7,14 @@
909 . /etc/rc.subr
910
911 PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
912-export CLOUD_CFG=/usr/local/etc/cloud/cloud.cfg
913
914 name="cloudconfig"
915 command="/usr/local/bin/cloud-init"
916 start_cmd="cloudconfig_start"
917 stop_cmd=":"
918 rcvar="cloudinit_enable"
919-start_precmd="cloudinit_override"
920 start_cmd="cloudconfig_start"
921
922-cloudinit_override()
923-{
924- # If there exist sysconfig/defaults variable override files use it...
925- if [ -f /etc/defaults/cloud-init ]; then
926- . /etc/defaults/cloud-init
927- fi
928-}
929-
930 cloudconfig_start()
931 {
932 echo "${command} starting"
933diff --git a/sysvinit/freebsd/cloudfinal b/sysvinit/freebsd/cloudfinal
934index 1b487aa..b6894c3 100755
935--- a/sysvinit/freebsd/cloudfinal
936+++ b/sysvinit/freebsd/cloudfinal
937@@ -7,24 +7,14 @@
938 . /etc/rc.subr
939
940 PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
941-export CLOUD_CFG=/usr/local/etc/cloud/cloud.cfg
942
943 name="cloudfinal"
944 command="/usr/local/bin/cloud-init"
945 start_cmd="cloudfinal_start"
946 stop_cmd=":"
947 rcvar="cloudinit_enable"
948-start_precmd="cloudinit_override"
949 start_cmd="cloudfinal_start"
950
951-cloudinit_override()
952-{
953- # If there exist sysconfig/defaults variable override files use it...
954- if [ -f /etc/defaults/cloud-init ]; then
955- . /etc/defaults/cloud-init
956- fi
957-}
958-
959 cloudfinal_start()
960 {
961 echo -n "${command} starting"
962diff --git a/sysvinit/freebsd/cloudinit b/sysvinit/freebsd/cloudinit
963index 862eeab..3326300 100755
964--- a/sysvinit/freebsd/cloudinit
965+++ b/sysvinit/freebsd/cloudinit
966@@ -7,24 +7,14 @@
967 . /etc/rc.subr
968
969 PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
970-export CLOUD_CFG=/usr/local/etc/cloud/cloud.cfg
971
972 name="cloudinit"
973 command="/usr/local/bin/cloud-init"
974 start_cmd="cloudinit_start"
975 stop_cmd=":"
976 rcvar="cloudinit_enable"
977-start_precmd="cloudinit_override"
978 start_cmd="cloudinit_start"
979
980-cloudinit_override()
981-{
982- # If there exist sysconfig/defaults variable override files use it...
983- if [ -f /etc/defaults/cloud-init ]; then
984- . /etc/defaults/cloud-init
985- fi
986-}
987-
988 cloudinit_start()
989 {
990 echo -n "${command} starting"
991diff --git a/sysvinit/freebsd/cloudinitlocal b/sysvinit/freebsd/cloudinitlocal
992index fb342a0..11a5eb1 100755
993--- a/sysvinit/freebsd/cloudinitlocal
994+++ b/sysvinit/freebsd/cloudinitlocal
995@@ -1,30 +1,20 @@
996 #!/bin/sh
997
998 # PROVIDE: cloudinitlocal
999-# REQUIRE: mountcritlocal
1000+# REQUIRE: ldconfig mountcritlocal
1001 # BEFORE: NETWORKING FILESYSTEMS cloudinit cloudconfig cloudfinal
1002
1003 . /etc/rc.subr
1004
1005 PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
1006-export CLOUD_CFG=/usr/local/etc/cloud/cloud.cfg
1007
1008 name="cloudinitlocal"
1009 command="/usr/local/bin/cloud-init"
1010 start_cmd="cloudlocal_start"
1011 stop_cmd=":"
1012 rcvar="cloudinit_enable"
1013-start_precmd="cloudinit_override"
1014 start_cmd="cloudlocal_start"
1015
1016-cloudinit_override()
1017-{
1018- # If there exist sysconfig/defaults variable override files use it...
1019- if [ -f /etc/defaults/cloud-init ]; then
1020- . /etc/defaults/cloud-init
1021- fi
1022-}
1023-
1024 cloudlocal_start()
1025 {
1026 echo -n "${command} starting"
1027diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
1028index 8d22bb5..e6b0dcb 100644
1029--- a/tests/unittests/test_datasource/test_azure.py
1030+++ b/tests/unittests/test_datasource/test_azure.py
1031@@ -3,6 +3,8 @@
1032 from cloudinit import helpers
1033 from cloudinit.util import b64e, decode_binary, load_file
1034 from cloudinit.sources import DataSourceAzure
1035+from cloudinit.util import find_freebsd_part
1036+from cloudinit.util import get_path_dev_freebsd
1037
1038 from ..helpers import TestCase, populate_dir, mock, ExitStack, PY26, SkipTest
1039
1040@@ -95,6 +97,41 @@ class TestAzureDataSource(TestCase):
1041 for module, name, new in patches:
1042 self.patches.enter_context(mock.patch.object(module, name, new))
1043
1044+ def _get_mockds(self):
1045+ mod = DataSourceAzure
1046+ sysctl_out = "dev.storvsc.3.%pnpinfo: "\
1047+ "classid=ba6163d9-04a1-4d29-b605-72e2ffb1dc7f "\
1048+ "deviceid=f8b3781b-1e82-4818-a1c3-63d806ec15bb\n"
1049+ sysctl_out += "dev.storvsc.2.%pnpinfo: "\
1050+ "classid=ba6163d9-04a1-4d29-b605-72e2ffb1dc7f "\
1051+ "deviceid=f8b3781a-1e82-4818-a1c3-63d806ec15bb\n"
1052+ sysctl_out += "dev.storvsc.1.%pnpinfo: "\
1053+ "classid=32412632-86cb-44a2-9b5c-50d1417354f5 "\
1054+ "deviceid=00000000-0001-8899-0000-000000000000\n"
1055+ camctl_devbus = """
1056+scbus0 on ata0 bus 0
1057+scbus1 on ata1 bus 0
1058+scbus2 on blkvsc0 bus 0
1059+scbus3 on blkvsc1 bus 0
1060+scbus4 on storvsc2 bus 0
1061+scbus5 on storvsc3 bus 0
1062+scbus-1 on xpt0 bus 0
1063+ """
1064+ camctl_dev = """
1065+<Msft Virtual CD/ROM 1.0> at scbus1 target 0 lun 0 (cd0,pass0)
1066+<Msft Virtual Disk 1.0> at scbus2 target 0 lun 0 (da0,pass1)
1067+<Msft Virtual Disk 1.0> at scbus3 target 1 lun 0 (da1,pass2)
1068+ """
1069+ self.apply_patches([
1070+ (mod, 'get_dev_storvsc_sysctl', mock.MagicMock(
1071+ return_value=sysctl_out)),
1072+ (mod, 'get_camcontrol_dev_bus', mock.MagicMock(
1073+ return_value=camctl_devbus)),
1074+ (mod, 'get_camcontrol_dev', mock.MagicMock(
1075+ return_value=camctl_dev))
1076+ ])
1077+ return mod
1078+
1079 def _get_ds(self, data, agent_command=None):
1080
1081 def dsdevs():
1082@@ -177,6 +214,34 @@ class TestAzureDataSource(TestCase):
1083 return
1084 raise AssertionError("XML is the same")
1085
1086+ def test_get_resource_disk(self):
1087+ ds = self._get_mockds()
1088+ dev = ds.get_resource_disk_on_freebsd(1)
1089+ self.assertEqual("da1", dev)
1090+
1091+ @mock.patch('cloudinit.util.subp')
1092+ def test_find_freebsd_part_on_Azure(self, mock_subp):
1093+ glabel_out = '''
1094+gptid/fa52d426-c337-11e6-8911-00155d4c5e47 N/A da0p1
1095+ label/rootfs N/A da0p2
1096+ label/swap N/A da0p3
1097+'''
1098+ mock_subp.return_value = (glabel_out, "")
1099+ res = find_freebsd_part("/dev/label/rootfs")
1100+ self.assertEqual("da0p2", res)
1101+
1102+ def test_get_path_dev_freebsd_on_Azure(self):
1103+ mnt_list = '''
1104+/dev/label/rootfs / ufs rw 1 1
1105+devfs /dev devfs rw,multilabel 0 0
1106+fdescfs /dev/fd fdescfs rw 0 0
1107+/dev/da1s1 /mnt/resource ufs rw 2 2
1108+'''
1109+ with mock.patch.object(os.path, 'exists',
1110+ return_value=True):
1111+ res = get_path_dev_freebsd('/etc', mnt_list)
1112+ self.assertNotEqual(res, None)
1113+
1114 def test_basic_seed_dir(self):
1115 odata = {'HostName': "myhost", 'UserName': "myuser"}
1116 data = {'ovfcontent': construct_valid_ovf_env(data=odata),
1117diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py
1118index aafdebd..b2d2971 100644
1119--- a/tests/unittests/test_datasource/test_azure_helper.py
1120+++ b/tests/unittests/test_datasource/test_azure_helper.py
1121@@ -3,7 +3,6 @@
1122 import os
1123
1124 from cloudinit.sources.helpers import azure as azure_helper
1125-
1126 from ..helpers import ExitStack, mock, TestCase
1127
1128
1129@@ -72,10 +71,11 @@ class TestFindEndpoint(TestCase):
1130
1131 @staticmethod
1132 def _build_lease_content(encoded_address):
1133+ endpoint = azure_helper._get_dhcp_endpoint_option_name()
1134 return '\n'.join([
1135 'lease {',
1136 ' interface "eth0";',
1137- ' option unknown-245 {0};'.format(encoded_address),
1138+ ' option {0} {1};'.format(endpoint, encoded_address),
1139 '}'])
1140
1141 def test_from_dhcp_client(self):
1142diff --git a/tests/unittests/test_datasource/test_cloudstack.py b/tests/unittests/test_datasource/test_cloudstack.py
1143index e93d28d..1d3d2f1 100644
1144--- a/tests/unittests/test_datasource/test_cloudstack.py
1145+++ b/tests/unittests/test_datasource/test_cloudstack.py
1146@@ -15,6 +15,11 @@ class TestCloudStackPasswordFetching(TestCase):
1147 mod_name = 'cloudinit.sources.DataSourceCloudStack'
1148 self.patches.enter_context(mock.patch('{0}.ec2'.format(mod_name)))
1149 self.patches.enter_context(mock.patch('{0}.uhelp'.format(mod_name)))
1150+ default_gw = "192.201.20.0"
1151+ mod_name = 'cloudinit.sources.DataSourceCloudStack.get_default_gateway'
1152+ get_default_gw = mock.MagicMock(return_value=default_gw)
1153+ self.patches.enter_context(
1154+ mock.patch(mod_name, get_default_gw))
1155
1156 def _set_password_server_response(self, response_string):
1157 subp = mock.MagicMock(return_value=(response_string, ''))
1158diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py
1159index 8837066..1e10a33 100644
1160--- a/tests/unittests/test_distros/test_netconfig.py
1161+++ b/tests/unittests/test_distros/test_netconfig.py
1162@@ -178,6 +178,20 @@ class WriteBuffer(object):
1163
1164 class TestNetCfgDistro(TestCase):
1165
1166+ frbsd_ifout = """\
1167+hn0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
1168+ options=51b<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,TSO4,LRO>
1169+ ether 00:15:5d:4c:73:00
1170+ inet6 fe80::215:5dff:fe4c:7300%hn0 prefixlen 64 scopeid 0x2
1171+ inet 10.156.76.127 netmask 0xfffffc00 broadcast 10.156.79.255
1172+ nd6 options=23<PERFORMNUD,ACCEPT_RTADV,AUTO_LINKLOCAL>
1173+ media: Ethernet autoselect (10Gbase-T <full-duplex>)
1174+ status: active
1175+"""
1176+
1177+ def setUp(self):
1178+ super(TestNetCfgDistro, self).setUp()
1179+
1180 def _get_distro(self, dname, renderers=None):
1181 cls = distros.fetch(dname)
1182 cfg = settings.CFG_BUILTIN
1183@@ -251,6 +265,7 @@ class TestNetCfgDistro(TestCase):
1184
1185 def test_apply_network_config_v1_to_netplan_ub(self):
1186 renderers = ['netplan']
1187+ devlist = ['eth0', 'lo']
1188 ub_distro = self._get_distro('ubuntu', renderers=renderers)
1189 with ExitStack() as mocks:
1190 write_bufs = {}
1191@@ -272,6 +287,9 @@ class TestNetCfgDistro(TestCase):
1192 mock.patch.object(util, 'subp', return_value=(0, 0)))
1193 mocks.enter_context(
1194 mock.patch.object(os.path, 'isfile', return_value=False))
1195+ mocks.enter_context(
1196+ mock.patch("cloudinit.net.netplan.get_devicelist",
1197+ return_value=devlist))
1198
1199 ub_distro.apply_network_config(V1_NET_CFG, False)
1200
1201@@ -285,6 +303,7 @@ class TestNetCfgDistro(TestCase):
1202
1203 def test_apply_network_config_v2_passthrough_ub(self):
1204 renderers = ['netplan']
1205+ devlist = ['eth0', 'lo']
1206 ub_distro = self._get_distro('ubuntu', renderers=renderers)
1207 with ExitStack() as mocks:
1208 write_bufs = {}
1209@@ -306,7 +325,10 @@ class TestNetCfgDistro(TestCase):
1210 mock.patch.object(util, 'subp', return_value=(0, 0)))
1211 mocks.enter_context(
1212 mock.patch.object(os.path, 'isfile', return_value=False))
1213-
1214+ # FreeBSD does not have '/sys/class/net' file,
1215+ # so we need mock here.
1216+ mocks.enter_context(
1217+ mock.patch.object(os, 'listdir', return_value=devlist))
1218 ub_distro.apply_network_config(V2_NET_CFG, False)
1219
1220 self.assertEqual(len(write_bufs), 1)
1221@@ -328,6 +350,29 @@ class TestNetCfgDistro(TestCase):
1222 for (k, v) in b1.items():
1223 self.assertEqual(v, b2[k])
1224
1225+ @mock.patch('cloudinit.distros.freebsd.Distro.get_ifconfig_list')
1226+ @mock.patch('cloudinit.distros.freebsd.Distro.get_ifconfig_ifname_out')
1227+ def test_get_ip_nic_freebsd(self, ifname_out, iflist):
1228+ frbsd_distro = self._get_distro('freebsd')
1229+ iflist.return_value = "lo0 hn0"
1230+ ifname_out.return_value = self.frbsd_ifout
1231+ res = frbsd_distro.get_ipv4()
1232+ self.assertEqual(res, ['lo0', 'hn0'])
1233+ res = frbsd_distro.get_ipv6()
1234+ self.assertEqual(res, [])
1235+
1236+ @mock.patch('cloudinit.distros.freebsd.Distro.get_ifconfig_ether')
1237+ @mock.patch('cloudinit.distros.freebsd.Distro.get_ifconfig_ifname_out')
1238+ @mock.patch('cloudinit.distros.freebsd.Distro.get_interface_mac')
1239+ def test_generate_fallback_config_freebsd(self, mac, ifname_out, if_ether):
1240+ frbsd_distro = self._get_distro('freebsd')
1241+
1242+ if_ether.return_value = 'hn0'
1243+ ifname_out.return_value = self.frbsd_ifout
1244+ mac.return_value = '00:15:5d:4c:73:00'
1245+ res = frbsd_distro.generate_fallback_config()
1246+ self.assertIsNotNone(res)
1247+
1248 def test_simple_write_rh(self):
1249 rh_distro = self._get_distro('rhel')
1250
1251diff --git a/tests/unittests/test_handler/test_handler_resizefs.py b/tests/unittests/test_handler/test_handler_resizefs.py
1252new file mode 100644
1253index 0000000..52591b8
1254--- /dev/null
1255+++ b/tests/unittests/test_handler/test_handler_resizefs.py
1256@@ -0,0 +1,59 @@
1257+# This file is part of cloud-init. See LICENSE file for license information.
1258+
1259+from cloudinit.config import cc_resizefs
1260+
1261+import textwrap
1262+import unittest
1263+
1264+try:
1265+ from unittest import mock
1266+except ImportError:
1267+ import mock
1268+
1269+
1270+class TestResizefs(unittest.TestCase):
1271+ def setUp(self):
1272+ super(TestResizefs, self).setUp()
1273+ self.name = "resizefs"
1274+
1275+ @mock.patch('cloudinit.config.cc_resizefs._get_dumpfs_output')
1276+ @mock.patch('cloudinit.config.cc_resizefs._get_gpart_output')
1277+ def test_skip_ufs_resize(self, gpart_out, dumpfs_out):
1278+ fs_type = "ufs"
1279+ resize_what = "/"
1280+ devpth = "/dev/da0p2"
1281+ dumpfs_out.return_value = (
1282+ "# newfs command for / (/dev/label/rootfs)\n"
1283+ "newfs -O 2 -U -a 4 -b 32768 -d 32768 -e 4096 "
1284+ "-f 4096 -g 16384 -h 64 -i 8192 -j -k 6408 -m 8 "
1285+ "-o time -s 58719232 /dev/label/rootfs\n")
1286+ gpart_out.return_value = textwrap.dedent("""\
1287+ => 40 62914480 da0 GPT (30G)
1288+ 40 1024 1 freebsd-boot (512K)
1289+ 1064 58719232 2 freebsd-ufs (28G)
1290+ 58720296 3145728 3 freebsd-swap (1.5G)
1291+ 61866024 1048496 - free - (512M)
1292+ """)
1293+ res = cc_resizefs.can_skip_resize(fs_type, resize_what, devpth)
1294+ self.assertTrue(res)
1295+
1296+ @mock.patch('cloudinit.config.cc_resizefs._get_dumpfs_output')
1297+ @mock.patch('cloudinit.config.cc_resizefs._get_gpart_output')
1298+ def test_skip_ufs_resize_roundup(self, gpart_out, dumpfs_out):
1299+ fs_type = "ufs"
1300+ resize_what = "/"
1301+ devpth = "/dev/da0p2"
1302+ dumpfs_out.return_value = (
1303+ "# newfs command for / (/dev/label/rootfs)\n"
1304+ "newfs -O 2 -U -a 4 -b 32768 -d 32768 -e 4096 "
1305+ "-f 4096 -g 16384 -h 64 -i 8192 -j -k 368 -m 8 "
1306+ "-o time -s 297080 /dev/label/rootfs\n")
1307+ gpart_out.return_value = textwrap.dedent("""\
1308+ => 34 297086 da0 GPT (145M)
1309+ 34 297086 1 freebsd-ufs (145M)
1310+ """)
1311+ res = cc_resizefs.can_skip_resize(fs_type, resize_what, devpth)
1312+ self.assertTrue(res)
1313+
1314+
1315+# vi: ts=4 expandtab
1316diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
1317index 89e7536..5d2dd03 100644
1318--- a/tests/unittests/test_net.py
1319+++ b/tests/unittests/test_net.py
1320@@ -1120,14 +1120,14 @@ class TestNetplanPostcommands(CiTestCase):
1321 render_target = 'netplan.yaml'
1322 renderer = netplan.Renderer(
1323 {'netplan_path': render_target, 'postcmds': True})
1324- renderer.render_network_state(render_dir, ns)
1325-
1326 expected = [
1327 mock.call(['netplan', 'generate'], capture=True),
1328 mock.call(['udevadm', 'test-builtin', 'net_setup_link',
1329 '/sys/class/net/lo'], capture=True),
1330 ]
1331- mock_subp.assert_has_calls(expected)
1332+ with mock.patch.object(os.path, 'islink', return_value=True):
1333+ renderer.render_network_state(render_dir, ns)
1334+ mock_subp.assert_has_calls(expected)
1335
1336
1337 class TestEniNetworkStateToEni(CiTestCase):
1338diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
1339index 5d21b4b..189caca 100644
1340--- a/tests/unittests/test_util.py
1341+++ b/tests/unittests/test_util.py
1342@@ -596,7 +596,8 @@ class TestSubp(helpers.TestCase):
1343 def test_subp_capture_stderr(self):
1344 data = b'hello world'
1345 (out, err) = util.subp(self.stdin2err, capture=True,
1346- decode=False, data=data)
1347+ decode=False, data=data,
1348+ update_env={'LC_ALL': 'C'})
1349 self.assertEqual(err, data)
1350 self.assertEqual(out, b'')
1351
1352diff --git a/tools/build-on-freebsd b/tools/build-on-freebsd
1353index 8436498..ccc10b4 100755
1354--- a/tools/build-on-freebsd
1355+++ b/tools/build-on-freebsd
1356@@ -10,9 +10,7 @@ depschecked=/tmp/c-i.dependencieschecked
1357 pkgs="
1358 dmidecode
1359 e2fsprogs
1360- gpart
1361 py27-Jinja2
1362- py27-argparse
1363 py27-boto
1364 py27-cheetah
1365 py27-configobj
1366@@ -38,7 +36,7 @@ python setup.py build
1367 python setup.py install -O1 --skip-build --prefix /usr/local/ --init-system sysvinit_freebsd
1368
1369 # Install the correct config file:
1370-cp config/cloud.cfg-freebsd /usr/local/etc/cloud/cloud.cfg
1371+cp config/cloud.cfg-freebsd /etc/cloud/cloud.cfg
1372
1373 # Enable cloud-init in /etc/rc.conf:
1374 sed -i.bak -e "/cloudinit_enable=.*/d" /etc/rc.conf

Subscribers

People subscribed via source and target branches