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
diff --git a/cloudinit/config/cc_resizefs.py b/cloudinit/config/cc_resizefs.py
index 60e3ab5..ceee952 100644
--- a/cloudinit/config/cc_resizefs.py
+++ b/cloudinit/config/cc_resizefs.py
@@ -33,7 +33,10 @@ disabled altogether by setting ``resize_rootfs`` to ``false``.
33"""33"""
3434
35import errno35import errno
36import getopt
36import os37import os
38import re
39import shlex
37import stat40import stat
3841
39from cloudinit.settings import PER_ALWAYS42from cloudinit.settings import PER_ALWAYS
@@ -58,6 +61,62 @@ def _resize_ufs(mount_point, devpth):
58 return ('growfs', devpth)61 return ('growfs', devpth)
5962
6063
64def _get_dumpfs_output(mount_point):
65 dumpfs_res, err = util.subp(['dumpfs', '-m', mount_point])
66 return dumpfs_res
67
68
69def _get_gpart_output(part):
70 gpart_res, err = util.subp(['gpart', 'show', part])
71 return gpart_res
72
73
74def _can_skip_resize_ufs(mount_point, devpth):
75 # extract the current fs sector size
76 """
77 # dumpfs -m /
78 # newfs command for / (/dev/label/rootfs)
79 newfs -O 2 -U -a 4 -b 32768 -d 32768 -e 4096 -f 4096 -g 16384
80 -h 64 -i 8192 -j -k 6408 -m 8 -o time -s 58719232 /dev/label/rootf
81 """
82 cur_fs_sz = None
83 frag_sz = None
84 dumpfs_res = _get_dumpfs_output(mount_point)
85 for line in dumpfs_res.splitlines():
86 if not line.startswith('#'):
87 newfs_cmd = shlex.split(line)
88 opt_value = 'O:Ua:s:b:d:e:f:g:h:i:jk:m:o:'
89 optlist, args = getopt.getopt(newfs_cmd[1:], opt_value)
90 for o, a in optlist:
91 if o == "-s":
92 cur_fs_sz = int(a)
93 if o == "-f":
94 frag_sz = int(a)
95 # check the current partition size
96 """
97 # gpart show /dev/da0
98=> 40 62914480 da0 GPT (30G)
99 40 1024 1 freebsd-boot (512K)
100 1064 58719232 2 freebsd-ufs (28G)
101 58720296 3145728 3 freebsd-swap (1.5G)
102 61866024 1048496 - free - (512M)
103 """
104 expect_sz = None
105 m = re.search('^(/dev/.+)p([0-9])$', devpth)
106 gpart_res = _get_gpart_output(m.group(1))
107 for line in gpart_res.splitlines():
108 if re.search(r"freebsd-ufs", line):
109 fields = line.split()
110 expect_sz = int(fields[1])
111 # Normalize the gpart sector size,
112 # because the size is not exactly the same as fs size.
113 normal_expect_sz = (expect_sz - expect_sz % (frag_sz / 512))
114 if normal_expect_sz == cur_fs_sz:
115 return True
116 else:
117 return False
118
119
61# Do not use a dictionary as these commands should be able to be used120# Do not use a dictionary as these commands should be able to be used
62# for multiple filesystem types if possible, e.g. one command for121# for multiple filesystem types if possible, e.g. one command for
63# ext2, ext3 and ext4.122# ext2, ext3 and ext4.
@@ -68,9 +127,40 @@ RESIZE_FS_PREFIXES_CMDS = [
68 ('ufs', _resize_ufs),127 ('ufs', _resize_ufs),
69]128]
70129
130RESIZE_FS_PRECHECK_CMDS = {
131 'ufs': _can_skip_resize_ufs
132}
133
71NOBLOCK = "noblock"134NOBLOCK = "noblock"
72135
73136
137def rootdev_from_cmdline(cmdline):
138 found = None
139 for tok in cmdline.split():
140 if tok.startswith("root="):
141 found = tok[5:]
142 break
143 if found is None:
144 return None
145
146 if found.startswith("/dev/"):
147 return found
148 if found.startswith("LABEL="):
149 return "/dev/disk/by-label/" + found[len("LABEL="):]
150 if found.startswith("UUID="):
151 return "/dev/disk/by-uuid/" + found[len("UUID="):]
152
153 return "/dev/" + found
154
155
156def can_skip_resize(fs_type, resize_what, devpth):
157 fstype_lc = fs_type.lower()
158 for i, func in RESIZE_FS_PRECHECK_CMDS.items():
159 if fstype_lc.startswith(i):
160 return func(resize_what, devpth)
161 return False
162
163
74def handle(name, cfg, _cloud, log, args):164def handle(name, cfg, _cloud, log, args):
75 if len(args) != 0:165 if len(args) != 0:
76 resize_root = args[0]166 resize_root = args[0]
@@ -139,6 +229,11 @@ def handle(name, cfg, _cloud, log, args):
139 return229 return
140230
141 resizer = None231 resizer = None
232 if can_skip_resize(fs_type, resize_what, devpth):
233 log.debug("Skip resize filesystem type %s for %s",
234 fs_type, resize_what)
235 return
236
142 fstype_lc = fs_type.lower()237 fstype_lc = fs_type.lower()
143 for (pfix, root_cmd) in RESIZE_FS_PREFIXES_CMDS:238 for (pfix, root_cmd) in RESIZE_FS_PREFIXES_CMDS:
144 if fstype_lc.startswith(pfix):239 if fstype_lc.startswith(pfix):
diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
index 28650b8..f56c0cf 100755
--- a/cloudinit/distros/__init__.py
+++ b/cloudinit/distros/__init__.py
@@ -155,6 +155,9 @@ class Distro(object):
155 ns, header=header, render_hwaddress=True)155 ns, header=header, render_hwaddress=True)
156 return self.apply_network(contents, bring_up=bring_up)156 return self.apply_network(contents, bring_up=bring_up)
157157
158 def generate_fallback_config(self):
159 return net.generate_fallback_config()
160
158 def apply_network_config(self, netconfig, bring_up=False):161 def apply_network_config(self, netconfig, bring_up=False):
159 # apply network config netconfig162 # apply network config netconfig
160 # This method is preferred to apply_network which only takes163 # This method is preferred to apply_network which only takes
diff --git a/cloudinit/distros/freebsd.py b/cloudinit/distros/freebsd.py
index 183e445..d9f31bf 100644
--- a/cloudinit/distros/freebsd.py
+++ b/cloudinit/distros/freebsd.py
@@ -30,6 +30,7 @@ class Distro(distros.Distro):
30 login_conf_fn_bak = '/etc/login.conf.orig'30 login_conf_fn_bak = '/etc/login.conf.orig'
31 resolv_conf_fn = '/etc/resolv.conf'31 resolv_conf_fn = '/etc/resolv.conf'
32 ci_sudoers_fn = '/usr/local/etc/sudoers.d/90-cloud-init-users'32 ci_sudoers_fn = '/usr/local/etc/sudoers.d/90-cloud-init-users'
33 default_primary_nic = 'hn0'
3334
34 def __init__(self, name, cfg, paths):35 def __init__(self, name, cfg, paths):
35 distros.Distro.__init__(self, name, cfg, paths)36 distros.Distro.__init__(self, name, cfg, paths)
@@ -38,6 +39,8 @@ class Distro(distros.Distro):
38 # should only happen say once per instance...)39 # should only happen say once per instance...)
39 self._runner = helpers.Runners(paths)40 self._runner = helpers.Runners(paths)
40 self.osfamily = 'freebsd'41 self.osfamily = 'freebsd'
42 self.ipv4_pat = re.compile(r"\s+inet\s+\d+[.]\d+[.]\d+[.]\d+")
43 cfg['ssh_svcname'] = 'sshd'
4144
42 # Updates a key in /etc/rc.conf.45 # Updates a key in /etc/rc.conf.
43 def updatercconf(self, key, value):46 def updatercconf(self, key, value):
@@ -183,7 +186,6 @@ class Distro(distros.Distro):
183 "gecos": '-c',186 "gecos": '-c',
184 "primary_group": '-g',187 "primary_group": '-g',
185 "groups": '-G',188 "groups": '-G',
186 "passwd": '-h',
187 "shell": '-s',189 "shell": '-s',
188 "inactive": '-E',190 "inactive": '-E',
189 }191 }
@@ -193,19 +195,11 @@ class Distro(distros.Distro):
193 "no_log_init": '--no-log-init',195 "no_log_init": '--no-log-init',
194 }196 }
195197
196 redact_opts = ['passwd']
197
198 for key, val in kwargs.items():198 for key, val in kwargs.items():
199 if (key in adduser_opts and val and199 if (key in adduser_opts and val and
200 isinstance(val, six.string_types)):200 isinstance(val, six.string_types)):
201 adduser_cmd.extend([adduser_opts[key], val])201 adduser_cmd.extend([adduser_opts[key], val])
202202
203 # Redact certain fields from the logs
204 if key in redact_opts:
205 log_adduser_cmd.extend([adduser_opts[key], 'REDACTED'])
206 else:
207 log_adduser_cmd.extend([adduser_opts[key], val])
208
209 elif key in adduser_flags and val:203 elif key in adduser_flags and val:
210 adduser_cmd.append(adduser_flags[key])204 adduser_cmd.append(adduser_flags[key])
211 log_adduser_cmd.append(adduser_flags[key])205 log_adduser_cmd.append(adduser_flags[key])
@@ -226,19 +220,21 @@ class Distro(distros.Distro):
226 except Exception as e:220 except Exception as e:
227 util.logexc(LOG, "Failed to create user %s", name)221 util.logexc(LOG, "Failed to create user %s", name)
228 raise e222 raise e
223 # Set the password if it is provided
224 # For security consideration, only hashed passwd is assumed
225 passwd_val = kwargs.get('passwd', None)
226 if passwd_val is not None:
227 self.set_passwd(name, passwd_val, hashed=True)
229228
230 def set_passwd(self, user, passwd, hashed=False):229 def set_passwd(self, user, passwd, hashed=False):
231 cmd = ['pw', 'usermod', user]
232
233 if hashed:230 if hashed:
234 cmd.append('-H')231 hash_opt = "-H"
235 else:232 else:
236 cmd.append('-h')233 hash_opt = "-h"
237
238 cmd.append('0')
239234
240 try:235 try:
241 util.subp(cmd, passwd, logstring="chpasswd for %s" % user)236 util.subp(['pw', 'usermod', user, hash_opt, '0'],
237 data=passwd, logstring="chpasswd for %s" % user)
242 except Exception as e:238 except Exception as e:
243 util.logexc(LOG, "Failed to set password for %s", user)239 util.logexc(LOG, "Failed to set password for %s", user)
244 raise e240 raise e
@@ -271,6 +267,255 @@ class Distro(distros.Distro):
271 keys = set(kwargs['ssh_authorized_keys']) or []267 keys = set(kwargs['ssh_authorized_keys']) or []
272 ssh_util.setup_user_keys(keys, name, options=None)268 ssh_util.setup_user_keys(keys, name, options=None)
273269
270 @staticmethod
271 def get_ifconfig_list():
272 cmd = ['ifconfig', '-l']
273 (nics, err) = util.subp(cmd, rcs=[0, 1])
274 if len(err):
275 LOG.warn("Error running %s: %s", cmd, err)
276 return None
277 return nics
278
279 @staticmethod
280 def get_ifconfig_ifname_out(ifname):
281 cmd = ['ifconfig', ifname]
282 (if_result, err) = util.subp(cmd, rcs=[0, 1])
283 if len(err):
284 LOG.warn("Error running %s: %s", cmd, err)
285 return None
286 return if_result
287
288 @staticmethod
289 def get_ifconfig_ether():
290 cmd = ['ifconfig', '-l', 'ether']
291 (nics, err) = util.subp(cmd, rcs=[0, 1])
292 if len(err):
293 LOG.warn("Error running %s: %s", cmd, err)
294 return None
295 return nics
296
297 @staticmethod
298 def get_interface_mac(ifname):
299 if_result = Distro.get_ifconfig_ifname_out(ifname)
300 for item in if_result.splitlines():
301 if item.find('ether ') != -1:
302 mac = str(item.split()[1])
303 if mac:
304 return mac
305
306 @staticmethod
307 def get_devicelist():
308 nics = Distro.get_ifconfig_list()
309 return nics.split()
310
311 @staticmethod
312 def get_ipv6():
313 ipv6 = []
314 nics = Distro.get_devicelist()
315 for nic in nics:
316 if_result = Distro.get_ifconfig_ifname_out(nic)
317 for item in if_result.splitlines():
318 if item.find("inet6 ") != -1 and item.find("scopeid") == -1:
319 ipv6.append(nic)
320 return ipv6
321
322 def get_ipv4(self):
323 ipv4 = []
324 nics = Distro.get_devicelist()
325 for nic in nics:
326 if_result = Distro.get_ifconfig_ifname_out(nic)
327 for item in if_result.splitlines():
328 print(item)
329 if self.ipv4_pat.match(item):
330 ipv4.append(nic)
331 return ipv4
332
333 def is_up(self, ifname):
334 if_result = Distro.get_ifconfig_ifname_out(ifname)
335 pat = "^" + ifname
336 for item in if_result.splitlines():
337 if re.match(pat, item):
338 flags = item.split('<')[1].split('>')[0]
339 if flags.find("UP") != -1:
340 return True
341
342 def _get_current_rename_info(self, check_downable=True):
343 """Collect information necessary for rename_interfaces."""
344 names = Distro.get_devicelist()
345 bymac = {}
346 for n in names:
347 bymac[Distro.get_interface_mac(n)] = {
348 'name': n, 'up': self.is_up(n), 'downable': None}
349
350 if check_downable:
351 nics_with_addresses = set()
352 ipv6 = self.get_ipv6()
353 ipv4 = self.get_ipv4()
354 for bytes_out in (ipv6, ipv4):
355 for i in ipv6:
356 nics_with_addresses.update(i)
357 for i in ipv4:
358 nics_with_addresses.update(i)
359
360 for d in bymac.values():
361 d['downable'] = (d['up'] is False or
362 d['name'] not in nics_with_addresses)
363
364 return bymac
365
366 def _rename_interfaces(self, renames):
367 if not len(renames):
368 LOG.debug("no interfaces to rename")
369 return
370
371 current_info = self._get_current_rename_info()
372
373 cur_bymac = {}
374 for mac, data in current_info.items():
375 cur = data.copy()
376 cur['mac'] = mac
377 cur_bymac[mac] = cur
378
379 def update_byname(bymac):
380 return dict((data['name'], data)
381 for data in bymac.values())
382
383 def rename(cur, new):
384 util.subp(["ifconfig", cur, "name", new], capture=True)
385
386 def down(name):
387 util.subp(["ifconfig", name, "down"], capture=True)
388
389 def up(name):
390 util.subp(["ifconfig", name, "up"], capture=True)
391
392 ops = []
393 errors = []
394 ups = []
395 cur_byname = update_byname(cur_bymac)
396 tmpname_fmt = "cirename%d"
397 tmpi = -1
398
399 for mac, new_name in renames:
400 cur = cur_bymac.get(mac, {})
401 cur_name = cur.get('name')
402 cur_ops = []
403 if cur_name == new_name:
404 # nothing to do
405 continue
406
407 if not cur_name:
408 errors.append("[nic not present] Cannot rename mac=%s to %s"
409 ", not available." % (mac, new_name))
410 continue
411
412 if cur['up']:
413 msg = "[busy] Error renaming mac=%s from %s to %s"
414 if not cur['downable']:
415 errors.append(msg % (mac, cur_name, new_name))
416 continue
417 cur['up'] = False
418 cur_ops.append(("down", mac, new_name, (cur_name,)))
419 ups.append(("up", mac, new_name, (new_name,)))
420
421 if new_name in cur_byname:
422 target = cur_byname[new_name]
423 if target['up']:
424 msg = "[busy-target] Error renaming mac=%s from %s to %s."
425 if not target['downable']:
426 errors.append(msg % (mac, cur_name, new_name))
427 continue
428 else:
429 cur_ops.append(("down", mac, new_name, (new_name,)))
430
431 tmp_name = None
432 while tmp_name is None or tmp_name in cur_byname:
433 tmpi += 1
434 tmp_name = tmpname_fmt % tmpi
435
436 cur_ops.append(("rename", mac, new_name, (new_name, tmp_name)))
437 target['name'] = tmp_name
438 cur_byname = update_byname(cur_bymac)
439 if target['up']:
440 ups.append(("up", mac, new_name, (tmp_name,)))
441
442 cur_ops.append(("rename", mac, new_name, (cur['name'], new_name)))
443 cur['name'] = new_name
444 cur_byname = update_byname(cur_bymac)
445 ops += cur_ops
446
447 opmap = {'rename': rename, 'down': down, 'up': up}
448 if len(ops) + len(ups) == 0:
449 if len(errors):
450 LOG.debug("unable to do any work for renaming of %s", renames)
451 else:
452 LOG.debug("no work necessary for renaming of %s", renames)
453 else:
454 LOG.debug("achieving renaming of %s with ops %s",
455 renames, ops + ups)
456
457 for op, mac, new_name, params in ops + ups:
458 try:
459 opmap.get(op)(*params)
460 except Exception as e:
461 errors.append(
462 "[unknown] Error performing %s%s for %s, %s: %s" %
463 (op, params, mac, new_name, e))
464 if len(errors):
465 raise Exception('\n'.join(errors))
466
467 def apply_network_config_names(self, netcfg):
468 renames = []
469 for ent in netcfg.get('config', {}):
470 if ent.get('type') != 'physical':
471 continue
472 mac = ent.get('mac_address')
473 name = ent.get('name')
474 if not mac:
475 continue
476 renames.append([mac, name])
477 return self._rename_interfaces(renames)
478
479 @classmethod
480 def generate_fallback_config(self):
481 nics = Distro.get_ifconfig_ether()
482 if nics is None:
483 LOG.debug("Fail to get network interfaces")
484 return None
485 potential_interfaces = nics.split()
486 connected = []
487 for nic in potential_interfaces:
488 pat = "^" + nic
489 if_result = Distro.get_ifconfig_ifname_out(nic)
490 for item in if_result.split("\n"):
491 if re.match(pat, item):
492 flags = item.split('<')[1].split('>')[0]
493 if flags.find("RUNNING") != -1:
494 connected.append(nic)
495 if connected:
496 potential_interfaces = connected
497 names = list(sorted(potential_interfaces))
498 default_pri_nic = Distro.default_primary_nic
499 if default_pri_nic in names:
500 names.remove(default_pri_nic)
501 names.insert(0, default_pri_nic)
502 target_name = None
503 target_mac = None
504 for name in names:
505 mac = Distro.get_interface_mac(name)
506 if mac:
507 target_name = name
508 target_mac = mac
509 break
510 if target_mac and target_name:
511 nconf = {'config': [], 'version': 1}
512 nconf['config'].append(
513 {'type': 'physical', 'name': target_name,
514 'mac_address': target_mac, 'subnets': [{'type': 'dhcp'}]})
515 return nconf
516 else:
517 return None
518
274 def _write_network(self, settings):519 def _write_network(self, settings):
275 entries = net_util.translate_network(settings)520 entries = net_util.translate_network(settings)
276 nameservers = []521 nameservers = []
diff --git a/cloudinit/settings.py b/cloudinit/settings.py
index dbafead..411960d 100644
--- a/cloudinit/settings.py
+++ b/cloudinit/settings.py
@@ -39,7 +39,7 @@ CFG_BUILTIN = {
39 ],39 ],
40 'def_log_file': '/var/log/cloud-init.log',40 'def_log_file': '/var/log/cloud-init.log',
41 'log_cfgs': [],41 'log_cfgs': [],
42 'syslog_fix_perms': ['syslog:adm', 'root:adm'],42 'syslog_fix_perms': ['syslog:adm', 'root:adm', 'root:wheel'],
43 'system_info': {43 'system_info': {
44 'paths': {44 'paths': {
45 'cloud_dir': '/var/lib/cloud',45 'cloud_dir': '/var/lib/cloud',
diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index 04358b7..5254e18 100644
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -10,6 +10,7 @@ import crypt
10from functools import partial10from functools import partial
11import os11import os
12import os.path12import os.path
13import re
13import time14import time
14from xml.dom import minidom15from xml.dom import minidom
15import xml.etree.ElementTree as ET16import xml.etree.ElementTree as ET
@@ -32,19 +33,160 @@ BOUNCE_COMMAND = [
32# azure systems will always have a resource disk, and 66-azure-ephemeral.rules33# azure systems will always have a resource disk, and 66-azure-ephemeral.rules
33# ensures that it gets linked to this path.34# ensures that it gets linked to this path.
34RESOURCE_DISK_PATH = '/dev/disk/cloud/azure_resource'35RESOURCE_DISK_PATH = '/dev/disk/cloud/azure_resource'
36DEFAULT_PRIMARY_NIC = 'eth0'
37LEASE_FILE = '/var/lib/dhcp/dhclient.eth0.leases'
38DEFAULT_FS = 'ext4'
39
40
41def find_storvscid_from_sysctl_pnpinfo(sysctl_out, deviceid):
42 # extract the 'X' from dev.storvsc.X. if deviceid matches
43 """
44 dev.storvsc.1.%pnpinfo:
45 classid=32412632-86cb-44a2-9b5c-50d1417354f5
46 deviceid=00000000-0001-8899-0000-000000000000
47 """
48 for line in sysctl_out.splitlines():
49 if re.search(r"pnpinfo", line):
50 fields = line.split()
51 if len(fields) >= 3:
52 columns = fields[2].split('=')
53 if (len(columns) >= 2 and
54 columns[0] == "deviceid" and
55 columns[1].startswith(deviceid)):
56 comps = fields[0].split('.')
57 return comps[2]
58 return None
59
60
61def find_busdev_from_disk(camcontrol_out, disk_drv):
62 # find the scbusX from 'camcontrol devlist -b' output
63 # if disk_drv matches the specified disk driver, i.e. blkvsc1
64 """
65 scbus0 on ata0 bus 0
66 scbus1 on ata1 bus 0
67 scbus2 on blkvsc0 bus 0
68 scbus3 on blkvsc1 bus 0
69 scbus4 on storvsc2 bus 0
70 scbus5 on storvsc3 bus 0
71 scbus-1 on xpt0 bus 0
72 """
73 for line in camcontrol_out.splitlines():
74 if re.search(disk_drv, line):
75 items = line.split()
76 return items[0]
77 return None
78
79
80def find_dev_from_busdev(camcontrol_out, busdev):
81 # find the daX from 'camcontrol devlist' output
82 # if busdev matches the specified value, i.e. 'scbus2'
83 """
84 <Msft Virtual CD/ROM 1.0> at scbus1 target 0 lun 0 (cd0,pass0)
85 <Msft Virtual Disk 1.0> at scbus2 target 0 lun 0 (da0,pass1)
86 <Msft Virtual Disk 1.0> at scbus3 target 1 lun 0 (da1,pass2)
87 """
88 for line in camcontrol_out.splitlines():
89 if re.search(busdev, line):
90 items = line.split('(')
91 if len(items) == 2:
92 dev_pass = items[1].split(',')
93 return dev_pass[0]
94 return None
95
96
97def get_dev_storvsc_sysctl():
98 try:
99 sysctl_out, err = util.subp(['sysctl', 'dev.storvsc'])
100 except util.ProcessExecutionError:
101 LOG.debug("Fail to execute sysctl dev.storvsc")
102 return None
103 return sysctl_out
104
105
106def get_camcontrol_dev_bus():
107 try:
108 camcontrol_b_out, err = util.subp(['camcontrol', 'devlist', '-b'])
109 except util.ProcessExecutionError:
110 LOG.debug("Fail to execute camcontrol devlist -b")
111 return None
112 return camcontrol_b_out
113
114
115def get_camcontrol_dev():
116 try:
117 camcontrol_out, err = util.subp(['camcontrol', 'devlist'])
118 except util.ProcessExecutionError:
119 LOG.debug("Fail to execute camcontrol devlist")
120 return None
121 return camcontrol_out
122
123
124def get_resource_disk_on_freebsd(port_id):
125 g0 = "00000000"
126 if port_id > 1:
127 g0 = "00000001"
128 port_id = port_id - 2
129 g1 = "000" + str(port_id)
130 g0g1 = "{0}-{1}".format(g0, g1)
131 """
132 search 'X' from
133 'dev.storvsc.X.%pnpinfo:
134 classid=32412632-86cb-44a2-9b5c-50d1417354f5
135 deviceid=00000000-0001-8899-0000-000000000000'
136 """
137 sysctl_out = get_dev_storvsc_sysctl()
138
139 storvscid = find_storvscid_from_sysctl_pnpinfo(sysctl_out, g0g1)
140 if not storvscid:
141 LOG.debug("Fail to find storvsc id from sysctl")
142 return None
143
144 camcontrol_b_out = get_camcontrol_dev_bus()
145 camcontrol_out = get_camcontrol_dev()
146 # try to find /dev/XX from 'blkvsc' device
147 blkvsc = "blkvsc{0}".format(storvscid)
148 scbusx = find_busdev_from_disk(camcontrol_b_out, blkvsc)
149 if scbusx:
150 devname = find_dev_from_busdev(camcontrol_out, scbusx)
151 if devname is None:
152 LOG.debug("Fail to find /dev/daX")
153 return None
154 return devname
155 # try to find /dev/XX from 'storvsc' device
156 storvsc = "storvsc{0}".format(storvscid)
157 scbusx = find_busdev_from_disk(camcontrol_b_out, storvsc)
158 if scbusx:
159 devname = find_dev_from_busdev(camcontrol_out, scbusx)
160 if devname is None:
161 LOG.debug("Fail to find /dev/daX")
162 return None
163 return devname
164 return None
165
166# update the FreeBSD specific information
167if util.is_FreeBSD():
168 DEFAULT_PRIMARY_NIC = 'hn0'
169 LEASE_FILE = '/var/db/dhclient.leases.hn0'
170 DEFAULT_FS = 'freebsd-ufs'
171 res_disk = get_resource_disk_on_freebsd(1)
172 if res_disk is not None:
173 LOG.debug("resource disk is not None")
174 RESOURCE_DISK_PATH = "/dev/" + res_disk
175 else:
176 LOG.debug("resource disk is None")
35177
36BUILTIN_DS_CONFIG = {178BUILTIN_DS_CONFIG = {
37 'agent_command': AGENT_START_BUILTIN,179 'agent_command': AGENT_START_BUILTIN,
38 'data_dir': "/var/lib/waagent",180 'data_dir': "/var/lib/waagent",
39 'set_hostname': True,181 'set_hostname': True,
40 'hostname_bounce': {182 'hostname_bounce': {
41 'interface': 'eth0',183 'interface': DEFAULT_PRIMARY_NIC,
42 'policy': True,184 'policy': True,
43 'command': BOUNCE_COMMAND,185 'command': BOUNCE_COMMAND,
44 'hostname_command': 'hostname',186 'hostname_command': 'hostname',
45 },187 },
46 'disk_aliases': {'ephemeral0': RESOURCE_DISK_PATH},188 'disk_aliases': {'ephemeral0': RESOURCE_DISK_PATH},
47 'dhclient_lease_file': '/var/lib/dhcp/dhclient.eth0.leases',189 'dhclient_lease_file': LEASE_FILE,
48}190}
49191
50BUILTIN_CLOUD_CONFIG = {192BUILTIN_CLOUD_CONFIG = {
@@ -53,7 +195,7 @@ BUILTIN_CLOUD_CONFIG = {
53 'layout': [100],195 'layout': [100],
54 'overwrite': True},196 'overwrite': True},
55 },197 },
56 'fs_setup': [{'filesystem': 'ext4',198 'fs_setup': [{'filesystem': DEFAULT_FS,
57 'device': 'ephemeral0.1',199 'device': 'ephemeral0.1',
58 'replace_fs': 'ntfs'}],200 'replace_fs': 'ntfs'}],
59}201}
@@ -190,7 +332,11 @@ class DataSourceAzureNet(sources.DataSource):
190 for cdev in candidates:332 for cdev in candidates:
191 try:333 try:
192 if cdev.startswith("/dev/"):334 if cdev.startswith("/dev/"):
193 ret = util.mount_cb(cdev, load_azure_ds_dir)335 if util.is_FreeBSD():
336 ret = util.mount_cb(cdev, load_azure_ds_dir,
337 mtype="udf", sync=False)
338 else:
339 ret = util.mount_cb(cdev, load_azure_ds_dir)
194 else:340 else:
195 ret = load_azure_ds_dir(cdev)341 ret = load_azure_ds_dir(cdev)
196342
@@ -218,11 +364,12 @@ class DataSourceAzureNet(sources.DataSource):
218 LOG.debug("using files cached in %s", ddir)364 LOG.debug("using files cached in %s", ddir)
219365
220 # azure / hyper-v provides random data here366 # azure / hyper-v provides random data here
221 seed = util.load_file("/sys/firmware/acpi/tables/OEM0",367 if not util.is_FreeBSD():
222 quiet=True, decode=False)368 seed = util.load_file("/sys/firmware/acpi/tables/OEM0",
223 if seed:369 quiet=True, decode=False)
224 self.metadata['random_seed'] = seed370 if seed:
225371 self.metadata['random_seed'] = seed
372 # TODO. find the seed on FreeBSD platform
226 # now update ds_cfg to reflect contents pass in config373 # now update ds_cfg to reflect contents pass in config
227 user_ds_cfg = util.get_cfg_by_path(self.cfg, DS_CFG_PATH, {})374 user_ds_cfg = util.get_cfg_by_path(self.cfg, DS_CFG_PATH, {})
228 self.ds_cfg = util.mergemanydict([user_ds_cfg, self.ds_cfg])375 self.ds_cfg = util.mergemanydict([user_ds_cfg, self.ds_cfg])
@@ -633,8 +780,19 @@ def encrypt_pass(password, salt_id="$6$"):
633def list_possible_azure_ds_devs():780def list_possible_azure_ds_devs():
634 # return a sorted list of devices that might have a azure datasource781 # return a sorted list of devices that might have a azure datasource
635 devlist = []782 devlist = []
636 for fstype in ("iso9660", "udf"):783 if util.is_FreeBSD():
637 devlist.extend(util.find_devs_with("TYPE=%s" % fstype))784 cdrom_dev = "/dev/cd0"
785 try:
786 util.subp(["mount", "-o", "ro", "-t", "udf", cdrom_dev,
787 "/mnt/cdrom/secure"])
788 except util.ProcessExecutionError:
789 LOG.debug("Fail to mount cd")
790 return devlist
791 util.subp(["umount", "/mnt/cdrom/secure"])
792 devlist.append(cdrom_dev)
793 else:
794 for fstype in ("iso9660", "udf"):
795 devlist.extend(util.find_devs_with("TYPE=%s" % fstype))
638796
639 devlist.sort(reverse=True)797 devlist.sort(reverse=True)
640 return devlist798 return devlist
diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py
index 6e01aa4..e22409d 100644
--- a/cloudinit/sources/helpers/azure.py
+++ b/cloudinit/sources/helpers/azure.py
@@ -29,6 +29,14 @@ def cd(newdir):
29 os.chdir(prevdir)29 os.chdir(prevdir)
3030
3131
32def _get_dhcp_endpoint_option_name():
33 if util.is_FreeBSD():
34 azure_endpoint = "option-245"
35 else:
36 azure_endpoint = "unknown-245"
37 return azure_endpoint
38
39
32class AzureEndpointHttpClient(object):40class AzureEndpointHttpClient(object):
3341
34 headers = {42 headers = {
@@ -235,8 +243,9 @@ class WALinuxAgentShim(object):
235 leases = []243 leases = []
236 content = util.load_file(fallback_lease_file)244 content = util.load_file(fallback_lease_file)
237 LOG.debug("content is %s", content)245 LOG.debug("content is %s", content)
246 option_name = _get_dhcp_endpoint_option_name()
238 for line in content.splitlines():247 for line in content.splitlines():
239 if 'unknown-245' in line:248 if option_name in line:
240 # Example line from Ubuntu249 # Example line from Ubuntu
241 # option unknown-245 a8:3f:81:10;250 # option unknown-245 a8:3f:81:10;
242 leases.append(line.strip(' ').split(' ', 2)[-1].strip(';\n"'))251 leases.append(line.strip(' ').split(' ', 2)[-1].strip(';\n"'))
diff --git a/cloudinit/stages.py b/cloudinit/stages.py
index f7191b0..ad55782 100644
--- a/cloudinit/stages.py
+++ b/cloudinit/stages.py
@@ -624,7 +624,7 @@ class Init(object):
624 return (None, loc)624 return (None, loc)
625 if ncfg:625 if ncfg:
626 return (ncfg, loc)626 return (ncfg, loc)
627 return (net.generate_fallback_config(), "fallback")627 return (self.distro.generate_fallback_config(), "fallback")
628628
629 def apply_network_config(self, bring_up):629 def apply_network_config(self, bring_up):
630 netcfg, src = self._find_networking_config()630 netcfg, src = self._find_networking_config()
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 22af99d..27a9833 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -565,6 +565,10 @@ def is_ipv4(instr):
565 return len(toks) == 4565 return len(toks) == 4
566566
567567
568def is_FreeBSD():
569 return system_info()['platform'].startswith('FreeBSD')
570
571
568def get_cfg_option_bool(yobj, key, default=False):572def get_cfg_option_bool(yobj, key, default=False):
569 if key not in yobj:573 if key not in yobj:
570 return default574 return default
@@ -2055,11 +2059,56 @@ def parse_mtab(path):
2055 return None2059 return None
20562060
20572061
2062def find_freebsd_part(label_part):
2063 if label_part.startswith("/dev/label/"):
2064 target_label = label_part[5:]
2065 (label_part, err) = subp(['glabel', 'status', '-s'])
2066 for labels in label_part.split("\n"):
2067 items = labels.split()
2068 if len(items) > 0 and items[0].startswith(target_label):
2069 label_part = items[2]
2070 break
2071 label_part = str(label_part)
2072 return label_part
2073
2074
2075def get_path_dev_freebsd(path, mnt_list):
2076 path_found = None
2077 for line in mnt_list.split("\n"):
2078 items = line.split()
2079 if (len(items) > 2 and os.path.exists(items[1] + path)):
2080 path_found = line
2081 break
2082 return path_found
2083
2084
2085def get_mount_info_freebsd(path, log=LOG):
2086 (result, err) = subp(['mount', '-p', path], rcs=[0, 1])
2087 if len(err):
2088 # find a path if the input is not a mounting point
2089 (mnt_list, err) = subp(['mount', '-p'])
2090 path_found = get_path_dev_freebsd(path, mnt_list)
2091 if (path_found is None):
2092 return None
2093 result = path_found
2094 ret = result.split()
2095 label_part = find_freebsd_part(ret[0])
2096 return "/dev/" + label_part, ret[2], ret[1]
2097
2098
2058def parse_mount(path):2099def parse_mount(path):
2059 (mountoutput, _err) = subp("mount")2100 (mountoutput, _err) = subp("mount")
2060 mount_locs = mountoutput.splitlines()2101 mount_locs = mountoutput.splitlines()
2061 for line in mount_locs:2102 for line in mount_locs:
2062 m = re.search(r'^(/dev/[\S]+) on (/.*) \((.+), .+, (.+)\)$', line)2103 m = re.search(r'^(/dev/[\S]+) on (/.*) \((.+), .+, (.+)\)$', line)
2104 if not m:
2105 continue
2106 # check whether the dev refers to a label on FreeBSD
2107 # for example, if dev is '/dev/label/rootfs', we should
2108 # continue finding the real device like '/dev/da0'.
2109 devm = re.search('^(/dev/.+)p([0-9])$', m.group(1))
2110 if (not devm and is_FreeBSD()):
2111 return get_mount_info_freebsd(path)
2063 devpth = m.group(1)2112 devpth = m.group(1)
2064 mount_point = m.group(2)2113 mount_point = m.group(2)
2065 fs_type = m.group(3)2114 fs_type = m.group(3)
@@ -2336,7 +2385,8 @@ def read_dmi_data(key):
2336 uname_arch = os.uname()[4]2385 uname_arch = os.uname()[4]
2337 if not (uname_arch == "x86_64" or2386 if not (uname_arch == "x86_64" or
2338 (uname_arch.startswith("i") and uname_arch[2:] == "86") or2387 (uname_arch.startswith("i") and uname_arch[2:] == "86") or
2339 uname_arch == 'aarch64'):2388 uname_arch == 'aarch64' or
2389 uname_arch == 'amd64'):
2340 LOG.debug("dmidata is not supported on %s", uname_arch)2390 LOG.debug("dmidata is not supported on %s", uname_arch)
2341 return None2391 return None
23422392
diff --git a/config/cloud.cfg-freebsd b/config/cloud.cfg-freebsd
index be664f5..d666c39 100644
--- a/config/cloud.cfg-freebsd
+++ b/config/cloud.cfg-freebsd
@@ -5,7 +5,7 @@ syslog_fix_perms: root:wheel
55
6# This should not be required, but leave it in place until the real cause of6# This should not be required, but leave it in place until the real cause of
7# not beeing able to find -any- datasources is resolved.7# not beeing able to find -any- datasources is resolved.
8datasource_list: ['ConfigDrive', 'OpenStack', 'Ec2']8datasource_list: ['ConfigDrive', 'Azure', 'OpenStack', 'Ec2']
99
10# A set of users which may be applied and/or used by various modules10# A set of users which may be applied and/or used by various modules
11# when a 'default' entry is found it will reference the 'default_user'11# when a 'default' entry is found it will reference the 'default_user'
diff --git a/setup.py b/setup.py
index 32a44d9..46d0521 100755
--- a/setup.py
+++ b/setup.py
@@ -89,7 +89,6 @@ LIB = "/lib"
89if os.uname()[0] == 'FreeBSD':89if os.uname()[0] == 'FreeBSD':
90 USR = "/usr/local"90 USR = "/usr/local"
91 USR_LIB_EXEC = "/usr/local/lib"91 USR_LIB_EXEC = "/usr/local/lib"
92 ETC = "/usr/local/etc"
93elif os.path.isfile('/etc/redhat-release'):92elif os.path.isfile('/etc/redhat-release'):
94 USR_LIB_EXEC = "/usr/libexec"93 USR_LIB_EXEC = "/usr/libexec"
9594
@@ -164,8 +163,6 @@ else:
164 (ETC + '/cloud', glob('config/*.cfg')),163 (ETC + '/cloud', glob('config/*.cfg')),
165 (ETC + '/cloud/cloud.cfg.d', glob('config/cloud.cfg.d/*')),164 (ETC + '/cloud/cloud.cfg.d', glob('config/cloud.cfg.d/*')),
166 (ETC + '/cloud/templates', glob('templates/*')),165 (ETC + '/cloud/templates', glob('templates/*')),
167 (ETC + '/NetworkManager/dispatcher.d/', ['tools/hook-network-manager']),
168 (ETC + '/dhcp/dhclient-exit-hooks.d/', ['tools/hook-dhclient']),
169 (USR_LIB_EXEC + '/cloud-init', ['tools/ds-identify',166 (USR_LIB_EXEC + '/cloud-init', ['tools/ds-identify',
170 'tools/uncloud-init',167 'tools/uncloud-init',
171 'tools/write-ssh-key-fingerprints']),168 'tools/write-ssh-key-fingerprints']),
@@ -174,8 +171,13 @@ else:
174 [f for f in glob('doc/examples/*') if is_f(f)]),171 [f for f in glob('doc/examples/*') if is_f(f)]),
175 (USR + '/share/doc/cloud-init/examples/seed',172 (USR + '/share/doc/cloud-init/examples/seed',
176 [f for f in glob('doc/examples/seed/*') if is_f(f)]),173 [f for f in glob('doc/examples/seed/*') if is_f(f)]),
177 (LIB + '/udev/rules.d', [f for f in glob('udev/*.rules')]),
178 ]174 ]
175 if os.uname()[0] != 'FreeBSD':
176 data_files.append([
177 (ETC + '/NetworkManager/dispatcher.d/', ['tools/hook-network-manager']),
178 (ETC + '/dhcp/dhclient-exit-hooks.d/', ['tools/hook-dhclient']),
179 (LIB + '/udev/rules.d', [f for f in glob('udev/*.rules')])
180 ])
179 # Use a subclass for install that handles181 # Use a subclass for install that handles
180 # adding on the right init system configuration files182 # adding on the right init system configuration files
181 cmdclass = {183 cmdclass = {
diff --git a/sysvinit/freebsd/cloudconfig b/sysvinit/freebsd/cloudconfig
index 01bc061..e4064fa 100755
--- a/sysvinit/freebsd/cloudconfig
+++ b/sysvinit/freebsd/cloudconfig
@@ -7,24 +7,14 @@
7. /etc/rc.subr7. /etc/rc.subr
88
9PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"9PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
10export CLOUD_CFG=/usr/local/etc/cloud/cloud.cfg
1110
12name="cloudconfig"11name="cloudconfig"
13command="/usr/local/bin/cloud-init"12command="/usr/local/bin/cloud-init"
14start_cmd="cloudconfig_start"13start_cmd="cloudconfig_start"
15stop_cmd=":"14stop_cmd=":"
16rcvar="cloudinit_enable"15rcvar="cloudinit_enable"
17start_precmd="cloudinit_override"
18start_cmd="cloudconfig_start"16start_cmd="cloudconfig_start"
1917
20cloudinit_override()
21{
22 # If there exist sysconfig/defaults variable override files use it...
23 if [ -f /etc/defaults/cloud-init ]; then
24 . /etc/defaults/cloud-init
25 fi
26}
27
28cloudconfig_start()18cloudconfig_start()
29{19{
30 echo "${command} starting"20 echo "${command} starting"
diff --git a/sysvinit/freebsd/cloudfinal b/sysvinit/freebsd/cloudfinal
index 1b487aa..b6894c3 100755
--- a/sysvinit/freebsd/cloudfinal
+++ b/sysvinit/freebsd/cloudfinal
@@ -7,24 +7,14 @@
7. /etc/rc.subr7. /etc/rc.subr
88
9PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"9PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
10export CLOUD_CFG=/usr/local/etc/cloud/cloud.cfg
1110
12name="cloudfinal"11name="cloudfinal"
13command="/usr/local/bin/cloud-init"12command="/usr/local/bin/cloud-init"
14start_cmd="cloudfinal_start"13start_cmd="cloudfinal_start"
15stop_cmd=":"14stop_cmd=":"
16rcvar="cloudinit_enable"15rcvar="cloudinit_enable"
17start_precmd="cloudinit_override"
18start_cmd="cloudfinal_start"16start_cmd="cloudfinal_start"
1917
20cloudinit_override()
21{
22 # If there exist sysconfig/defaults variable override files use it...
23 if [ -f /etc/defaults/cloud-init ]; then
24 . /etc/defaults/cloud-init
25 fi
26}
27
28cloudfinal_start()18cloudfinal_start()
29{19{
30 echo -n "${command} starting"20 echo -n "${command} starting"
diff --git a/sysvinit/freebsd/cloudinit b/sysvinit/freebsd/cloudinit
index 862eeab..3326300 100755
--- a/sysvinit/freebsd/cloudinit
+++ b/sysvinit/freebsd/cloudinit
@@ -7,24 +7,14 @@
7. /etc/rc.subr7. /etc/rc.subr
88
9PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"9PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
10export CLOUD_CFG=/usr/local/etc/cloud/cloud.cfg
1110
12name="cloudinit"11name="cloudinit"
13command="/usr/local/bin/cloud-init"12command="/usr/local/bin/cloud-init"
14start_cmd="cloudinit_start"13start_cmd="cloudinit_start"
15stop_cmd=":"14stop_cmd=":"
16rcvar="cloudinit_enable"15rcvar="cloudinit_enable"
17start_precmd="cloudinit_override"
18start_cmd="cloudinit_start"16start_cmd="cloudinit_start"
1917
20cloudinit_override()
21{
22 # If there exist sysconfig/defaults variable override files use it...
23 if [ -f /etc/defaults/cloud-init ]; then
24 . /etc/defaults/cloud-init
25 fi
26}
27
28cloudinit_start()18cloudinit_start()
29{19{
30 echo -n "${command} starting"20 echo -n "${command} starting"
diff --git a/sysvinit/freebsd/cloudinitlocal b/sysvinit/freebsd/cloudinitlocal
index fb342a0..11a5eb1 100755
--- a/sysvinit/freebsd/cloudinitlocal
+++ b/sysvinit/freebsd/cloudinitlocal
@@ -1,30 +1,20 @@
1#!/bin/sh1#!/bin/sh
22
3# PROVIDE: cloudinitlocal3# PROVIDE: cloudinitlocal
4# REQUIRE: mountcritlocal 4# REQUIRE: ldconfig mountcritlocal
5# BEFORE: NETWORKING FILESYSTEMS cloudinit cloudconfig cloudfinal5# BEFORE: NETWORKING FILESYSTEMS cloudinit cloudconfig cloudfinal
66
7. /etc/rc.subr7. /etc/rc.subr
88
9PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"9PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
10export CLOUD_CFG=/usr/local/etc/cloud/cloud.cfg
1110
12name="cloudinitlocal"11name="cloudinitlocal"
13command="/usr/local/bin/cloud-init"12command="/usr/local/bin/cloud-init"
14start_cmd="cloudlocal_start"13start_cmd="cloudlocal_start"
15stop_cmd=":"14stop_cmd=":"
16rcvar="cloudinit_enable"15rcvar="cloudinit_enable"
17start_precmd="cloudinit_override"
18start_cmd="cloudlocal_start"16start_cmd="cloudlocal_start"
1917
20cloudinit_override()
21{
22 # If there exist sysconfig/defaults variable override files use it...
23 if [ -f /etc/defaults/cloud-init ]; then
24 . /etc/defaults/cloud-init
25 fi
26}
27
28cloudlocal_start()18cloudlocal_start()
29{19{
30 echo -n "${command} starting"20 echo -n "${command} starting"
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 8d22bb5..e6b0dcb 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -3,6 +3,8 @@
3from cloudinit import helpers3from cloudinit import helpers
4from cloudinit.util import b64e, decode_binary, load_file4from cloudinit.util import b64e, decode_binary, load_file
5from cloudinit.sources import DataSourceAzure5from cloudinit.sources import DataSourceAzure
6from cloudinit.util import find_freebsd_part
7from cloudinit.util import get_path_dev_freebsd
68
7from ..helpers import TestCase, populate_dir, mock, ExitStack, PY26, SkipTest9from ..helpers import TestCase, populate_dir, mock, ExitStack, PY26, SkipTest
810
@@ -95,6 +97,41 @@ class TestAzureDataSource(TestCase):
95 for module, name, new in patches:97 for module, name, new in patches:
96 self.patches.enter_context(mock.patch.object(module, name, new))98 self.patches.enter_context(mock.patch.object(module, name, new))
9799
100 def _get_mockds(self):
101 mod = DataSourceAzure
102 sysctl_out = "dev.storvsc.3.%pnpinfo: "\
103 "classid=ba6163d9-04a1-4d29-b605-72e2ffb1dc7f "\
104 "deviceid=f8b3781b-1e82-4818-a1c3-63d806ec15bb\n"
105 sysctl_out += "dev.storvsc.2.%pnpinfo: "\
106 "classid=ba6163d9-04a1-4d29-b605-72e2ffb1dc7f "\
107 "deviceid=f8b3781a-1e82-4818-a1c3-63d806ec15bb\n"
108 sysctl_out += "dev.storvsc.1.%pnpinfo: "\
109 "classid=32412632-86cb-44a2-9b5c-50d1417354f5 "\
110 "deviceid=00000000-0001-8899-0000-000000000000\n"
111 camctl_devbus = """
112scbus0 on ata0 bus 0
113scbus1 on ata1 bus 0
114scbus2 on blkvsc0 bus 0
115scbus3 on blkvsc1 bus 0
116scbus4 on storvsc2 bus 0
117scbus5 on storvsc3 bus 0
118scbus-1 on xpt0 bus 0
119 """
120 camctl_dev = """
121<Msft Virtual CD/ROM 1.0> at scbus1 target 0 lun 0 (cd0,pass0)
122<Msft Virtual Disk 1.0> at scbus2 target 0 lun 0 (da0,pass1)
123<Msft Virtual Disk 1.0> at scbus3 target 1 lun 0 (da1,pass2)
124 """
125 self.apply_patches([
126 (mod, 'get_dev_storvsc_sysctl', mock.MagicMock(
127 return_value=sysctl_out)),
128 (mod, 'get_camcontrol_dev_bus', mock.MagicMock(
129 return_value=camctl_devbus)),
130 (mod, 'get_camcontrol_dev', mock.MagicMock(
131 return_value=camctl_dev))
132 ])
133 return mod
134
98 def _get_ds(self, data, agent_command=None):135 def _get_ds(self, data, agent_command=None):
99136
100 def dsdevs():137 def dsdevs():
@@ -177,6 +214,34 @@ class TestAzureDataSource(TestCase):
177 return214 return
178 raise AssertionError("XML is the same")215 raise AssertionError("XML is the same")
179216
217 def test_get_resource_disk(self):
218 ds = self._get_mockds()
219 dev = ds.get_resource_disk_on_freebsd(1)
220 self.assertEqual("da1", dev)
221
222 @mock.patch('cloudinit.util.subp')
223 def test_find_freebsd_part_on_Azure(self, mock_subp):
224 glabel_out = '''
225gptid/fa52d426-c337-11e6-8911-00155d4c5e47 N/A da0p1
226 label/rootfs N/A da0p2
227 label/swap N/A da0p3
228'''
229 mock_subp.return_value = (glabel_out, "")
230 res = find_freebsd_part("/dev/label/rootfs")
231 self.assertEqual("da0p2", res)
232
233 def test_get_path_dev_freebsd_on_Azure(self):
234 mnt_list = '''
235/dev/label/rootfs / ufs rw 1 1
236devfs /dev devfs rw,multilabel 0 0
237fdescfs /dev/fd fdescfs rw 0 0
238/dev/da1s1 /mnt/resource ufs rw 2 2
239'''
240 with mock.patch.object(os.path, 'exists',
241 return_value=True):
242 res = get_path_dev_freebsd('/etc', mnt_list)
243 self.assertNotEqual(res, None)
244
180 def test_basic_seed_dir(self):245 def test_basic_seed_dir(self):
181 odata = {'HostName': "myhost", 'UserName': "myuser"}246 odata = {'HostName': "myhost", 'UserName': "myuser"}
182 data = {'ovfcontent': construct_valid_ovf_env(data=odata),247 data = {'ovfcontent': construct_valid_ovf_env(data=odata),
diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py
index aafdebd..b2d2971 100644
--- a/tests/unittests/test_datasource/test_azure_helper.py
+++ b/tests/unittests/test_datasource/test_azure_helper.py
@@ -3,7 +3,6 @@
3import os3import os
44
5from cloudinit.sources.helpers import azure as azure_helper5from cloudinit.sources.helpers import azure as azure_helper
6
7from ..helpers import ExitStack, mock, TestCase6from ..helpers import ExitStack, mock, TestCase
87
98
@@ -72,10 +71,11 @@ class TestFindEndpoint(TestCase):
7271
73 @staticmethod72 @staticmethod
74 def _build_lease_content(encoded_address):73 def _build_lease_content(encoded_address):
74 endpoint = azure_helper._get_dhcp_endpoint_option_name()
75 return '\n'.join([75 return '\n'.join([
76 'lease {',76 'lease {',
77 ' interface "eth0";',77 ' interface "eth0";',
78 ' option unknown-245 {0};'.format(encoded_address),78 ' option {0} {1};'.format(endpoint, encoded_address),
79 '}'])79 '}'])
8080
81 def test_from_dhcp_client(self):81 def test_from_dhcp_client(self):
diff --git a/tests/unittests/test_datasource/test_cloudstack.py b/tests/unittests/test_datasource/test_cloudstack.py
index e93d28d..1d3d2f1 100644
--- a/tests/unittests/test_datasource/test_cloudstack.py
+++ b/tests/unittests/test_datasource/test_cloudstack.py
@@ -15,6 +15,11 @@ class TestCloudStackPasswordFetching(TestCase):
15 mod_name = 'cloudinit.sources.DataSourceCloudStack'15 mod_name = 'cloudinit.sources.DataSourceCloudStack'
16 self.patches.enter_context(mock.patch('{0}.ec2'.format(mod_name)))16 self.patches.enter_context(mock.patch('{0}.ec2'.format(mod_name)))
17 self.patches.enter_context(mock.patch('{0}.uhelp'.format(mod_name)))17 self.patches.enter_context(mock.patch('{0}.uhelp'.format(mod_name)))
18 default_gw = "192.201.20.0"
19 mod_name = 'cloudinit.sources.DataSourceCloudStack.get_default_gateway'
20 get_default_gw = mock.MagicMock(return_value=default_gw)
21 self.patches.enter_context(
22 mock.patch(mod_name, get_default_gw))
1823
19 def _set_password_server_response(self, response_string):24 def _set_password_server_response(self, response_string):
20 subp = mock.MagicMock(return_value=(response_string, ''))25 subp = mock.MagicMock(return_value=(response_string, ''))
diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py
index 8837066..1e10a33 100644
--- a/tests/unittests/test_distros/test_netconfig.py
+++ b/tests/unittests/test_distros/test_netconfig.py
@@ -178,6 +178,20 @@ class WriteBuffer(object):
178178
179class TestNetCfgDistro(TestCase):179class TestNetCfgDistro(TestCase):
180180
181 frbsd_ifout = """\
182hn0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
183 options=51b<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,TSO4,LRO>
184 ether 00:15:5d:4c:73:00
185 inet6 fe80::215:5dff:fe4c:7300%hn0 prefixlen 64 scopeid 0x2
186 inet 10.156.76.127 netmask 0xfffffc00 broadcast 10.156.79.255
187 nd6 options=23<PERFORMNUD,ACCEPT_RTADV,AUTO_LINKLOCAL>
188 media: Ethernet autoselect (10Gbase-T <full-duplex>)
189 status: active
190"""
191
192 def setUp(self):
193 super(TestNetCfgDistro, self).setUp()
194
181 def _get_distro(self, dname, renderers=None):195 def _get_distro(self, dname, renderers=None):
182 cls = distros.fetch(dname)196 cls = distros.fetch(dname)
183 cfg = settings.CFG_BUILTIN197 cfg = settings.CFG_BUILTIN
@@ -251,6 +265,7 @@ class TestNetCfgDistro(TestCase):
251265
252 def test_apply_network_config_v1_to_netplan_ub(self):266 def test_apply_network_config_v1_to_netplan_ub(self):
253 renderers = ['netplan']267 renderers = ['netplan']
268 devlist = ['eth0', 'lo']
254 ub_distro = self._get_distro('ubuntu', renderers=renderers)269 ub_distro = self._get_distro('ubuntu', renderers=renderers)
255 with ExitStack() as mocks:270 with ExitStack() as mocks:
256 write_bufs = {}271 write_bufs = {}
@@ -272,6 +287,9 @@ class TestNetCfgDistro(TestCase):
272 mock.patch.object(util, 'subp', return_value=(0, 0)))287 mock.patch.object(util, 'subp', return_value=(0, 0)))
273 mocks.enter_context(288 mocks.enter_context(
274 mock.patch.object(os.path, 'isfile', return_value=False))289 mock.patch.object(os.path, 'isfile', return_value=False))
290 mocks.enter_context(
291 mock.patch("cloudinit.net.netplan.get_devicelist",
292 return_value=devlist))
275293
276 ub_distro.apply_network_config(V1_NET_CFG, False)294 ub_distro.apply_network_config(V1_NET_CFG, False)
277295
@@ -285,6 +303,7 @@ class TestNetCfgDistro(TestCase):
285303
286 def test_apply_network_config_v2_passthrough_ub(self):304 def test_apply_network_config_v2_passthrough_ub(self):
287 renderers = ['netplan']305 renderers = ['netplan']
306 devlist = ['eth0', 'lo']
288 ub_distro = self._get_distro('ubuntu', renderers=renderers)307 ub_distro = self._get_distro('ubuntu', renderers=renderers)
289 with ExitStack() as mocks:308 with ExitStack() as mocks:
290 write_bufs = {}309 write_bufs = {}
@@ -306,7 +325,10 @@ class TestNetCfgDistro(TestCase):
306 mock.patch.object(util, 'subp', return_value=(0, 0)))325 mock.patch.object(util, 'subp', return_value=(0, 0)))
307 mocks.enter_context(326 mocks.enter_context(
308 mock.patch.object(os.path, 'isfile', return_value=False))327 mock.patch.object(os.path, 'isfile', return_value=False))
309328 # FreeBSD does not have '/sys/class/net' file,
329 # so we need mock here.
330 mocks.enter_context(
331 mock.patch.object(os, 'listdir', return_value=devlist))
310 ub_distro.apply_network_config(V2_NET_CFG, False)332 ub_distro.apply_network_config(V2_NET_CFG, False)
311333
312 self.assertEqual(len(write_bufs), 1)334 self.assertEqual(len(write_bufs), 1)
@@ -328,6 +350,29 @@ class TestNetCfgDistro(TestCase):
328 for (k, v) in b1.items():350 for (k, v) in b1.items():
329 self.assertEqual(v, b2[k])351 self.assertEqual(v, b2[k])
330352
353 @mock.patch('cloudinit.distros.freebsd.Distro.get_ifconfig_list')
354 @mock.patch('cloudinit.distros.freebsd.Distro.get_ifconfig_ifname_out')
355 def test_get_ip_nic_freebsd(self, ifname_out, iflist):
356 frbsd_distro = self._get_distro('freebsd')
357 iflist.return_value = "lo0 hn0"
358 ifname_out.return_value = self.frbsd_ifout
359 res = frbsd_distro.get_ipv4()
360 self.assertEqual(res, ['lo0', 'hn0'])
361 res = frbsd_distro.get_ipv6()
362 self.assertEqual(res, [])
363
364 @mock.patch('cloudinit.distros.freebsd.Distro.get_ifconfig_ether')
365 @mock.patch('cloudinit.distros.freebsd.Distro.get_ifconfig_ifname_out')
366 @mock.patch('cloudinit.distros.freebsd.Distro.get_interface_mac')
367 def test_generate_fallback_config_freebsd(self, mac, ifname_out, if_ether):
368 frbsd_distro = self._get_distro('freebsd')
369
370 if_ether.return_value = 'hn0'
371 ifname_out.return_value = self.frbsd_ifout
372 mac.return_value = '00:15:5d:4c:73:00'
373 res = frbsd_distro.generate_fallback_config()
374 self.assertIsNotNone(res)
375
331 def test_simple_write_rh(self):376 def test_simple_write_rh(self):
332 rh_distro = self._get_distro('rhel')377 rh_distro = self._get_distro('rhel')
333378
diff --git a/tests/unittests/test_handler/test_handler_resizefs.py b/tests/unittests/test_handler/test_handler_resizefs.py
334new file mode 100644379new file mode 100644
index 0000000..52591b8
--- /dev/null
+++ b/tests/unittests/test_handler/test_handler_resizefs.py
@@ -0,0 +1,59 @@
1# This file is part of cloud-init. See LICENSE file for license information.
2
3from cloudinit.config import cc_resizefs
4
5import textwrap
6import unittest
7
8try:
9 from unittest import mock
10except ImportError:
11 import mock
12
13
14class TestResizefs(unittest.TestCase):
15 def setUp(self):
16 super(TestResizefs, self).setUp()
17 self.name = "resizefs"
18
19 @mock.patch('cloudinit.config.cc_resizefs._get_dumpfs_output')
20 @mock.patch('cloudinit.config.cc_resizefs._get_gpart_output')
21 def test_skip_ufs_resize(self, gpart_out, dumpfs_out):
22 fs_type = "ufs"
23 resize_what = "/"
24 devpth = "/dev/da0p2"
25 dumpfs_out.return_value = (
26 "# newfs command for / (/dev/label/rootfs)\n"
27 "newfs -O 2 -U -a 4 -b 32768 -d 32768 -e 4096 "
28 "-f 4096 -g 16384 -h 64 -i 8192 -j -k 6408 -m 8 "
29 "-o time -s 58719232 /dev/label/rootfs\n")
30 gpart_out.return_value = textwrap.dedent("""\
31 => 40 62914480 da0 GPT (30G)
32 40 1024 1 freebsd-boot (512K)
33 1064 58719232 2 freebsd-ufs (28G)
34 58720296 3145728 3 freebsd-swap (1.5G)
35 61866024 1048496 - free - (512M)
36 """)
37 res = cc_resizefs.can_skip_resize(fs_type, resize_what, devpth)
38 self.assertTrue(res)
39
40 @mock.patch('cloudinit.config.cc_resizefs._get_dumpfs_output')
41 @mock.patch('cloudinit.config.cc_resizefs._get_gpart_output')
42 def test_skip_ufs_resize_roundup(self, gpart_out, dumpfs_out):
43 fs_type = "ufs"
44 resize_what = "/"
45 devpth = "/dev/da0p2"
46 dumpfs_out.return_value = (
47 "# newfs command for / (/dev/label/rootfs)\n"
48 "newfs -O 2 -U -a 4 -b 32768 -d 32768 -e 4096 "
49 "-f 4096 -g 16384 -h 64 -i 8192 -j -k 368 -m 8 "
50 "-o time -s 297080 /dev/label/rootfs\n")
51 gpart_out.return_value = textwrap.dedent("""\
52 => 34 297086 da0 GPT (145M)
53 34 297086 1 freebsd-ufs (145M)
54 """)
55 res = cc_resizefs.can_skip_resize(fs_type, resize_what, devpth)
56 self.assertTrue(res)
57
58
59# vi: ts=4 expandtab
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 89e7536..5d2dd03 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -1120,14 +1120,14 @@ class TestNetplanPostcommands(CiTestCase):
1120 render_target = 'netplan.yaml'1120 render_target = 'netplan.yaml'
1121 renderer = netplan.Renderer(1121 renderer = netplan.Renderer(
1122 {'netplan_path': render_target, 'postcmds': True})1122 {'netplan_path': render_target, 'postcmds': True})
1123 renderer.render_network_state(render_dir, ns)
1124
1125 expected = [1123 expected = [
1126 mock.call(['netplan', 'generate'], capture=True),1124 mock.call(['netplan', 'generate'], capture=True),
1127 mock.call(['udevadm', 'test-builtin', 'net_setup_link',1125 mock.call(['udevadm', 'test-builtin', 'net_setup_link',
1128 '/sys/class/net/lo'], capture=True),1126 '/sys/class/net/lo'], capture=True),
1129 ]1127 ]
1130 mock_subp.assert_has_calls(expected)1128 with mock.patch.object(os.path, 'islink', return_value=True):
1129 renderer.render_network_state(render_dir, ns)
1130 mock_subp.assert_has_calls(expected)
11311131
11321132
1133class TestEniNetworkStateToEni(CiTestCase):1133class TestEniNetworkStateToEni(CiTestCase):
diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
index 5d21b4b..189caca 100644
--- a/tests/unittests/test_util.py
+++ b/tests/unittests/test_util.py
@@ -596,7 +596,8 @@ class TestSubp(helpers.TestCase):
596 def test_subp_capture_stderr(self):596 def test_subp_capture_stderr(self):
597 data = b'hello world'597 data = b'hello world'
598 (out, err) = util.subp(self.stdin2err, capture=True,598 (out, err) = util.subp(self.stdin2err, capture=True,
599 decode=False, data=data)599 decode=False, data=data,
600 update_env={'LC_ALL': 'C'})
600 self.assertEqual(err, data)601 self.assertEqual(err, data)
601 self.assertEqual(out, b'')602 self.assertEqual(out, b'')
602603
diff --git a/tools/build-on-freebsd b/tools/build-on-freebsd
index 8436498..ccc10b4 100755
--- a/tools/build-on-freebsd
+++ b/tools/build-on-freebsd
@@ -10,9 +10,7 @@ depschecked=/tmp/c-i.dependencieschecked
10pkgs="10pkgs="
11 dmidecode11 dmidecode
12 e2fsprogs12 e2fsprogs
13 gpart
14 py27-Jinja213 py27-Jinja2
15 py27-argparse
16 py27-boto14 py27-boto
17 py27-cheetah15 py27-cheetah
18 py27-configobj16 py27-configobj
@@ -38,7 +36,7 @@ python setup.py build
38python setup.py install -O1 --skip-build --prefix /usr/local/ --init-system sysvinit_freebsd36python setup.py install -O1 --skip-build --prefix /usr/local/ --init-system sysvinit_freebsd
3937
40# Install the correct config file:38# Install the correct config file:
41cp config/cloud.cfg-freebsd /usr/local/etc/cloud/cloud.cfg39cp config/cloud.cfg-freebsd /etc/cloud/cloud.cfg
4240
43# Enable cloud-init in /etc/rc.conf:41# Enable cloud-init in /etc/rc.conf:
44sed -i.bak -e "/cloudinit_enable=.*/d" /etc/rc.conf42sed -i.bak -e "/cloudinit_enable=.*/d" /etc/rc.conf

Subscribers

People subscribed via source and target branches