| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-07-19 | |||
| 18:48:17 | sean-k-mooney | johnsom: in there paticaly case it needs to be but in general it may not | |
| 18:48:27 | sean-k-mooney | https://github.com/openstack/nova/blob/93a65f06df67ce39d65827692150c78013c7f6d5/nova/network/neutron.py#L1737= | |
| 18:48:37 | sean-k-mooney | this is where we set the dns_name on the port by the way | |
| 18:48:59 | sean-k-mooney | if that just truncated the hostname neutron and designate woudl work | |
| 18:49:49 | sean-k-mooney | we are sanatising the hostname to workaround that bad request form neutron today | |
| 18:49:52 | johnsom | Yep, just split on the first label | |
| 18:52:06 | sean-k-mooney | so we could revert the normalisation we do right now when setting instance.hostname and truncate there and all uscases that work before wallaby would work again however the downstide of that is if your vm is called ubuntu-20.04 in neutron it will have ubuntu-20 set as the domain name | |
| 18:52:09 | dansmith | johnsom: I know, in the most generic cloud case it would also be something other than your own domain. It just seems like we've got all this overly-complex plumbing of networks now, so we should be able to actually manage network things via the network :) | |
| 18:52:15 | sean-k-mooney | and to make that worse it breaks multi create in that casse | |
| 18:52:35 | dansmith | sean-k-mooney: I really don't think we should be normalizing or truncating or splitting the hostname in nova | |
| 18:52:46 | dansmith | perhaps checking that it's a valid hostname (*maybe*) | |
| 18:52:52 | dansmith | but let the other services deal with it | |
| 18:53:13 | sean-k-mooney | dansmith: well we have been since before essexe | |
| 18:53:27 | sean-k-mooney | there was exsitng code that removed unicode | |
| 18:53:29 | dansmith | sean-k-mooney: and look at where we're at :) | |
| 18:53:35 | sean-k-mooney | and some other sepcial charters | |
| 18:53:49 | sean-k-mooney | so just sayign if we revert this it will still exits | |
| 18:54:03 | sean-k-mooney | dansmith: im not disagreeing that its undeisreable | |
| 18:54:07 | dansmith | again, checking for sanity is not such a big deal, but I'd think we'd want to reject the instance boot, not just sanitize-and-go | |
| 18:54:28 | sean-k-mooney | dansmith: that was also something we discuseed | |
| 18:54:40 | sean-k-mooney | psi was unhappy with that proposal | |
| 18:54:54 | sean-k-mooney | but i think that was our first responce | |
| 18:55:08 | sean-k-mooney | "this is invalide sorry neturon told us so" | |
| 18:56:37 | sean-k-mooney | i dont recall all the details but with queens and without designate vms with numeric TLDs booted | |
| 18:56:45 | sean-k-mooney | and with train and designate it did not | |
| 18:57:49 | sean-k-mooney | nova has been seeting the dns_name form instance.hostname since mitaka | |
| 18:57:58 | dansmith | we have friends that work on designate right? :) | |
| 18:58:03 | sean-k-mooney | so that was either a change in neutron or caused by adding designate | |
| 18:58:06 | johnsom | grin | |
| 18:58:10 | johnsom | You do.... | |
| 18:58:19 | sean-k-mooney | pluarl? | |
| 18:58:26 | dansmith | sean-k-mooney: johnsom is worth at least two | |
| 18:58:31 | sean-k-mooney | :) | |
| 18:58:32 | johnsom | lol | |
| 18:58:50 | dansmith | johnsom: that was a comment of your worth, not your waistline, btw ;P | |
| 18:58:53 | sean-k-mooney | i just tought designate was one of the more understaffed project | |
| 18:59:09 | sean-k-mooney | i think this vlaidation change was in neutron to be honest | |
| 18:59:22 | sean-k-mooney | between train and queens | |
| 18:59:29 | johnsom | There are two full time RH folks, and a couple more cores active. | |
| 18:59:45 | sean-k-mooney | oh ok glad that has improved | |
| 19:00:28 | johnsom | But, as dansmith said, the Designate buck stops with me at the moment, so if we need to fix something on the designate side, assign the bug to me. | |
| 19:02:22 | sean-k-mooney | https://github.com/openstack/neutron-lib/blob/f01b2e9025d33aeff3bf22ea2568bda036878819/neutron_lib/api/validators/dns.py#L59-L92= | |
| 19:02:33 | sean-k-mooney | so that apprently is what does the validation in neutron | |
| 19:03:32 | sean-k-mooney | well it starts here https://github.com/openstack/neutron-lib/blob/f01b2e9025d33aeff3bf22ea2568bda036878819/neutron_lib/api/validators/dns.py#L112= | |
| 19:03:46 | dansmith | I just had to: https://imgur.com/a/spREDAg | |
| 19:04:07 | johnsom | lol | |
| 19:04:24 | johnsom | At least it's a $100 | |
| 19:04:41 | dansmith | inflation, yo | |
| 19:05:34 | sean-k-mooney | https://github.com/openstack/neutron-lib/blob/f01b2e9025d33aeff3bf22ea2568bda036878819/neutron_lib/api/validators/dns.py#L50-L52= | |
| 19:05:44 | sean-k-mooney | that is what was rejecting the numeric tlds | |
| 19:06:32 | sean-k-mooney | and that has been in place since pike | |
| 19:06:51 | sean-k-mooney | so the psi issue was caused by turning on the dns extention | |
| 19:09:57 | sean-k-mooney | it was there in mitak too shich is when we started setting that field https://github.com/openstack/neutron/blob/4d8685da8050df79d9193f91cab572cfc6d67a47/neutron/extensions/dns.py#L130-L133= | |
| 19:10:28 | sean-k-mooney | dansmith: so we could go back to not normalising or do want we don downstream | |
| 19:10:54 | dansmith | sounds like we need to collab with the neutron buck-stop | |
| 19:11:02 | sean-k-mooney | downstream if the tld is numeric we normalise to remove . | |
| 19:11:15 | sean-k-mooney | btu otherwisse we allow the fqdn | |
| 19:11:18 | sean-k-mooney | in hostname | |
| 19:12:20 | sean-k-mooney | so downstream its targeted to just making that one edgecase pass since the change was never backported upstream | |
| 19:12:53 | sean-k-mooney | upstream form wallaby on we replace all '.' with _ or - i cant recall | |
| 19:16:22 | johnsom | The TLD rules are pretty simple, I think it is perfectly acceptable to error to the user. There are two length limitations and the basic regex neutron has. I assume the neutron raise comes to late for nova to communicate that to the user? | |
| 19:16:54 | sean-k-mooney | yes it haapens on the compute node when we are bidning the ports | |
| 19:17:34 | dansmith | not too late to communicate, just too late to reject the request | |
| 19:17:47 | dansmith | we have lots of reasons why the instance goes into error state based on lies you told us earlier | |
| 19:17:52 | sean-k-mooney | without designate this would also work | |
| 19:18:02 | sean-k-mooney | because nova wont try to set the field | |
| 19:18:07 | sean-k-mooney | since the extenion is not enabled | |
| 19:18:11 | dansmith | so if dns is enabled and you gave us something invalid, then failing to wire up would be a fine reason to error the instance | |
| 19:18:35 | johnsom | Yep | |
| 19:18:35 | sean-k-mooney | ok because that is what we used to do | |
| 19:18:45 | sean-k-mooney | before it was reported as a bug and "fixed" | |
| 19:18:53 | sean-k-mooney | in wallaby | |
| 19:19:00 | dansmith | "hacked" | |
| 19:19:15 | dansmith | "swept into the future debt dustbin to screw someone else later" | |
| 19:19:23 | dansmith | but yeah :) | |
| 19:19:48 | sean-k-mooney | well we filed an rfe to add hostname as a sperate top level paramter and did a lot of other work | |
| 19:20:03 | sean-k-mooney | but ya im not happy with the situration we are in currently | |
| 19:20:27 | sean-k-mooney | going back to that old behavior will break some users but fix others | |
| 19:20:38 | sean-k-mooney | depending on if designate is aviaable of not | |
| 19:21:00 | sean-k-mooney | which yes is tecninally detechable via the neutron api | |
| 19:21:20 | sean-k-mooney | by checking the extentions as that is one of the few that is only reported when enabled i belive | |
| 19:21:52 | sean-k-mooney | neutron has a habit of reporting all extesion even if they are not enabled makeing it imposible to determin that | |
| 19:22:07 | johnsom | So, what I am hearing is a proposal: hostname field, remove the FQDN restriction, hand it off to cloud-init single label or FQDN, no hacking on the string (i.e. no . -> -). Pass the string through to neutron. If the domain doesn't match or is rejected, ERROR the instance with "invalid hostname" in the error field. | |
| 19:22:20 | johnsom | Just trying to summarize for clarity | |
| 19:23:11 | dansmith | that's what I'm saying yeah | |
| 19:23:23 | sean-k-mooney | that would regress a fixed bug | |
| 19:23:27 | dansmith | there might be some opinions about whether or not we need to hide that behind a microversion or not I guess | |
| 19:23:31 | johnsom | That works for me and would solve the customer issue | |
| 19:23:31 | sean-k-mooney | and break people on upgrade | |
| 19:23:42 | sean-k-mooney | including breaking psi | |
| 19:23:58 | sean-k-mooney | but if we have an a way to help them fix all invalid hostname we cloud | |
| 19:24:02 | dansmith | sean-k-mooney: it doesn't break them if the neutron thing is fixed right? | |
| 19:24:08 | johnsom | Why would it break on upgrade? you are going from more restrictive to less | |
| 19:24:30 | sean-k-mooney | it wont break exiting vms | |
| 19:24:34 | sean-k-mooney | that we have already normalised | |
| 19:24:43 | johnsom | Right | |
| 19:24:45 | sean-k-mooney | but it will break anyone that started depending on that | |
| 19:24:54 | sean-k-mooney | so custoemr with exsiting heat templates | |
| 19:25:00 | sean-k-mooney | woudl find it breaks | |
| 19:25:02 | dansmith | depending on what specifically? the mangled hostname? | |