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 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 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>
> 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%2Bmerg
> > e%
> > 2F314895&data=02%7C01%7Chonzhan%40microsoft.com%7Ca62135a0b9514c7e13
> > 06
> > 08d481d4cf8c%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6362761983
> > 91
> > 371945&sdata=qGDZNXUPvGHGs3%2ByT685EVs8jwnZQ12YIWBT%2BBrMPb8%3D&rese
> > rv
> > ed=0 You are the owner of ~redriver/cloud-init:frbsd-azure-branch.
> >
>
>