Issue Type: | Bug |
---|---|
Priority: | 3 - Elevated |
Status: | Resolved |
Created at: | 2016-05-10T04:37:33.000Z |
Updated at: | 2017-02-25T01:10:06.000Z |
Created by: | Michael Zeller |
---|---|
Reported by: | Michael Zeller |
Assigned to: | Marsell K |
Fixed: A fix for this issue is checked into the tree and tested.
(Resolution Date: 2017-02-25T01:10:06.000Z)
2017-03-02 KENYA OPTION (Release Date: 2017-03-02)
On my home SDC standup I noticed 'triton ssh alias' wasn't working because the primaryIp is not set.
I managed to dig up this thread from the mailing list:
https://www.listbox.com/member/archive/247449/2016/01/sort/subj/page/4/entry/4:85/20160127170534:1458CF52-C542-11E5-BFC2-B5E6F010038B/
Looking at the code it seems like we should check nic.primary rather than nic_tag. https://github.com/joyent/sdc-cloudapi/blob/master/lib/machines.js#L168
Making this change locally makes cloudapi/triton work as expected. What I am not sure about is previous versions of napi reporting the primary field.
Discussed with Cody. Just using the "primary:true" NIC isn't what we want.
tl;dr: The "primaryIp" spec should be: If there is a NIC with primary: true, and it has a non-RFC1918 (private) IPv4 address, select that. Otherwise, pick an IPv4 address from the first NIC that has non-RFC1918 addresses.
Some other notes:
@accountid:557058:a348ec04-3066-457c-8197-c6cfc6e56d48 Want to volunteer to poke at a patch for this? Else you can drop yourself as the assignee.
Discussion on #joyent:
bdha
10:14
Finally get back to this bug where primaryIp is returning a non-primary interface:
10:14
https://gist.github.com/bdha/952c8fc7395b4151cfc4b3bcc74f178f
10:15
Anyone have suggestions on what to kick?
trentm
10:19
bdha: any of PUBAPI-1238, PUBAPI-1286, or PUBAPI-1291 :|
bdha
10:22
hm.
10:22
What if all of my NICs are RFC1918?
10:22
(re 1291 specifically)
10:23
I think you should rely on the NIC tagged as primary, personally, but maybe my use case is weird.
trentm
10:24
given the name "primaryIp", I can't disagree with you
bdha
10:24
"The "primaryIp" spec should be: If there is a NIC with primary: true, and it has a non-RFC1918 (private) IPv4 address, select that. Otherwise, pick an IPv4 address from the first NIC that has non-RFC1918 addresses."
10:24
^-- Then I disagree with that.
trentm
10:24
yah
Long discussion today on this: https://jabber.joyent.com/logs/mib@conference.joyent.com/2016/09/02.html#18:24:02.139839
An attempted summary of ^^:
Another hit of this on sdc-discuss: https://www.listbox.com/member/archive/247449/2016/11/sort/time_rev/page/1/entry/2:91/20161124223923:BF3E5B1E-B2C0-11E6-B8B2-8A430559514B/
A proper fix seems like it could get a bit involved. This is a possible short-term improvement: https://cr.joyent.us/#/c/965/
It still checks for the external nic_tag first, since I'd rather not break anything that currently works using that, but then next checks for a nic set as primary.
^^^ I think we should just do the nic.primary check as we've discussed, instead of a middle ground, more complex change.
It would be nice to take a look at all the current VMs in JPC to see if/how many would get a changed 'primaryIp' by just changing to using nic.primary -- to get a feel for potential compat.
However, thanks for pushing on this Marsell!
I discussed with Cody a bit more to try to cut to a concensus. Notes and a proposal:
History: 'primaryIp' was added to the cloudapi machine object waaay back (see PUBAPI-400, PROV-1333 for the original, and then PUBAPI-548 to revive it in the re-written cloudapi for SDC 7) to enable the user to tell which IP to use for ssh'ing in. Commonly in JPC one would get two IPs for a zone (one on an 'internal' DC-only network and one on the 'external', i.e. a public IP). The machine object would return both IPs in an 'ips' array as simple strings, so there wasn't a way to distinguish internal from external from just that data. Both then and now, it isn't documented whether there is a stable and logical order of the IPs in the 'ips' array. For reasons not given on the tickets above a 'primary_ip' (later "primaryIp") field was added to give the "primary" IP to cloudapi users (notably at the time, the portal).
Now, coming ipv6 support (RFD 11, RFD 32) and CNS (RFD 1) provide wrinkles. Providing a simple 'primaryIp' attribute and having tooling (e.g. command examples in portal, 'triton ssh ...') use that is problematic: Should 'primaryIp' return an IPv6 address if the primary NIC has one? If the primary NIC has both IPv4 and IPv6 address, which should it prefer? A problem with preferring IPv6 is that it could mean the tooling breaks for a user on an IPv4-only host. With CNS a potentially better answer is that tooling use the CNS hostname. However, there are multiple 'dns_names' provided and (at least currently, IIUC the comments from Alex) some can be for non-primary networks and not typically routable and the order in 'dns_names' isn't specified.
Proposal:
tl;dr: We should change "MACHINE.primaryIp" to be the IPv4 address of the NIC with primary=true. We should look at backward compatibility. Everything else is a separate ticket.
I would love if we could just treat primaryIp = primary, but I am leery about removing the prior 'external' check. While looking at JPC is a good idea, that covers one installation of many. Part of cloudapi's job is to contain all the gungy crap so that internal systems can evolve freely while we present a fairly stable external API.
If we go with the primaryIp = primary in all cases, then I'd prefer we bumped the API version, keep CR 965 for version 8.0.0 and older, and use primaryIp = primary for anything newer.
sdc-cloudapi commit 2a296b0 (branch master, by Marsell Kukuljevic)
PUBAPI-1291: primaryIp's logic could be better
Reviewed by: Trent Mick <trent.mick@joyent.com>