[18:17] <blackboxsw> rharper: just pushed followup on https://github.com/canonical/cloud-init/pull/114 with doc updates and renamed ds config option to ``apply_network_config`` which aligns with both openstack and azure config option for getting stuff from imds
[18:17] <blackboxsw> s/stuff/network config/
[18:18] <blackboxsw> https://paste.ubuntu.com/p/DyvY7vjFyN/ is the successful network config for multi-nics + secondary-ips
[18:18] <blackboxsw> sorry multi-nic only, will add secondary ips to that too (and some ipv6) for a more full success route test
[18:53] <rharper> blackboxsw: ok
[19:42] <Odd_Bloke> rharper: blackboxsw: powersj: Just pushed up my revisions to the code review doc: https://github.com/canonical/cloud-init/pull/160/
[19:42] <rharper> k
[19:43] <blackboxsw> sweet Odd_Bloke will grab that next. wrapping up two other cloud-init pr reviews
[19:43] <blackboxsw> rharper: anything else in progress you are working related to initramfs ? https://github.com/canonical/cloud-init/pull/238
[19:43] <blackboxsw> or is that branch ready for review
[19:43] <rharper> what's 238 ?
[19:44] <rharper> sorry
[19:44] <rharper> heads down in reviewing your 114
[19:44] <rharper> the initramfs.netplan one needs review from xnox/vorlon before we move forward
[19:44] <blackboxsw> np just wondering if you are expecting more work on your #238 (I'll not review it until I get through OddBloke's 160)
[19:44] <blackboxsw> ok thanks rharper
[19:45] <rharper> blackboxsw: ok, finished review 114
[19:59] <powersj> Odd_Bloke, approved
[20:00] <Odd_Bloke> Thanks!
[20:01] <blackboxsw> onto 160 now. closed out https://github.com/canonical/cloud-init/pull/75
[20:02] <blackboxsw> updated FFe bug with secondary nic & secondary ip address context https://bugs.launchpad.net/ubuntu/+source/cloud-init/+bug/1866930
[20:28] <blackboxsw> Odd_Bloke: approved with nits https://github.com/canonical/cloud-init/pull/160#pullrequestreview-374601125
[20:28] <blackboxsw> if you want the nits take 'em otherwise merge at will
[20:36] <Odd_Bloke> blackboxsw: Well, I'm going to want to ping community folks for feedback, just giving core folks a head start. :)
[20:37] <powersj> blackboxsw, did you get another review on ec2 branch?
[20:37] <blackboxsw> powersj: rharper: said he's heads down on it
[20:37] <blackboxsw> right now
[20:37] <rharper> blackboxsw: I finished up 30 minutes ago .. .did you push up again ?
[20:37] <rharper> I left like 20 comments
[20:38] <blackboxsw> powersj: rharper I also validated upgrade path + reboot doesn't rewrite network config with multiple nics if former net config only had eth0 rendered.
[20:38] <blackboxsw> rharper: no didn't see what reviewing OddBloke's but done there.
[20:38] <blackboxsw> will address comments
[20:38] <powersj> rharper, thanks
[20:38] <rharper> np
[21:19] <blackboxsw> rharper: so primary interface discovery on ec2 is really related to find_fallback_nic_on_linux() not truly what ec2 mentioned as device-number: 0 https://github.com/canonical/cloud-init/pull/114/files#r392430307
[21:21] <blackboxsw> as in, there is nothing currently in ec2 net config that truly checks device-number. but I guess you were saying we should document this correlation
[21:24] <blackboxsw> ultimately rharper do you think the route-metric should be set to 100 * nic_metadata['device-number'] +1 and we could drop the nic_idx counter variable?
[21:24] <rharper> blackboxsw: I would be shocked if those don't align
[21:24] <blackboxsw> rharper: I'm almost positive they align, but the code itself doesn't *do* that is what I was raising
[21:24] <blackboxsw> but maybe it should :)
[21:25] <rharper> I feel like it should too, but I'm somewhat worried...
[21:25] <rharper> let me reread the ec2 docs
[21:30] <rharper> blackboxsw: it says, device-index number corresponds to eth<number>   ;   I wonder if that matches 'ifindex' value
[21:31] <blackboxsw> rharper: ssh ubuntu@18.191.148.254
[21:31] <blackboxsw> has 2 nics
[21:32] <rharper> hrm, I wonder why they're called ethX instead of en (predictable names)
[21:32] <rharper> no, ifindex does not relate
[21:33] <rharper> that's xen, I wonder what nitro ones look like
[21:34] <rharper> blackboxsw: well, maybe we can do work on device-id later then
[21:34] <rharper> I'd rather be sure than change it now
[21:35] <blackboxsw> rharper: +1
[21:35] <blackboxsw> waiting on that.
[21:36] <blackboxsw> rharper: clarification for you  https://github.com/canonical/cloud-init/pull/114/files#r392493895
[21:36] <blackboxsw> if we are doing primary nic, it can have secondary addrs so we still need the apply_network_config check before adding secondary ips
[21:37] <blackboxsw> sound ok? (that's current behavior of cloud-init everywhere anyway) primary nic, no secondary ips
[21:39] <blackboxsw> ahh interesting. and I think I have something to fix then
[21:39] <blackboxsw> https://github.com/canonical/cloud-init/pull/114/files#r392419963
[21:39] <blackboxsw> ok I saw our unittest which showed rendered v2 routes that were not under the specific address.
[21:40] <rharper> routes are per interface in v2
[21:41] <rharper> there is no top-level routes in v2
[21:41] <rharper> that will not validate via netplan generate
[21:42] <rharper> blackboxsw: on apply_network_config;  if False, primary gets dhcp4 only, no secondary ; and dhcp6 instead of dhcp4 if ipv6s is non-empty, Yes?  do we do dual-stack v4 + v6 (dhcp4/dhcp6) as well (and all of this is with apply_network_config: False)
[21:42] <rharper> if apply_network_config=True ;; then all of the things everywhere
[21:44] <blackboxsw> 1.  on apply_network_config;  if False, primary gets dhcp4 only, no secondary ; YES
[21:44] <blackboxsw> 2.  and dhcp6 instead of dhcp4 if ipv6s is non-empty, Yes?       No. it gets both dhcp4 and dhcp6
[21:45] <blackboxsw> we bring up dhcp4 always on primary because it is needed anyway to talk to the imds
[21:45] <blackboxsw> so we know it works because we couldn't have getting IMDS networkconfig otherwise
[21:46] <blackboxsw> 3. do we do dual stack ipv4/v6 when apply_network_config == false : YES
[21:46] <blackboxsw> just no secondary ips in that case (or secondary nics)
[21:47] <blackboxsw> rharper: ^ sorry forgot to address me response
[21:47] <blackboxsw> *my8
[21:47] <rharper> ok, on False, agreed, forgot about v4 for imds
[21:50] <blackboxsw> just pushed all I have. scrubbing rharper's review comments to make sure I addressed them all
[21:51] <blackboxsw> ahh right subnet route ordering is wonky https://github.com/canonical/cloud-init/pull/114/files#r392419963
[22:03] <rharper> blackboxsw: one more suggestion
[22:04] <rharper> in convert, I  think if we separate out the logic into two parts,  apply_network_config==False (this is the primary nic dhcp4 + optionally v6, no additional ips) ; and then apply_network_config==True, and this just configures *all* the nics with all features per metadata
[22:05] <rharper> it's easier to see the path that older relases will take, and only the apply_network_config==True path will loop over all interfaces and enable all of the things.
[22:06] <blackboxsw> ahh rharper no prob. that makes sense and simplifies the for loop for all
[22:08] <blackboxsw> adding now
[22:29] <blackboxsw> rharper: just pushed that separation of logic
[22:29] <blackboxsw> 1f35018da2abd253561da82bdc7f577eb1058cb2
[22:29] <blackboxsw> I know we're running out of daylight
[22:29] <blackboxsw> :/