| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-07-19 | |||
| 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 | sean-k-mooney | ok because that is what we used to do | |
| 19:18:35 | johnsom | Yep | |
| 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 | sean-k-mooney | and break people on upgrade | |
| 19:23:31 | johnsom | That works for me and would solve the customer issue | |
| 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? | |
| 19:25:07 | sean-k-mooney | yes | |
| 19:26:50 | johnsom | They would start getting ERROR instances if the hostname is bogus instead of having the hostname switched around on them. Which seems like the right answer to me. APIs that magically change the data input to something else are ... unpleasant | |
| 19:27:52 | sean-k-mooney | johnsom: im pretty sure you review this by the way in the past if not appolgies but the mangaleing was discussed at leant on the mainile list | |
| 19:28:17 | sean-k-mooney | and it was chosen to go that appoch since we already did it for unicode and we were following the rfc for mangeling rules | |
| 19:28:46 | sean-k-mooney | and we explictly asks operator if they were depenidng on the fqdns in the hostname at the time | |
| 19:28:52 | dansmith | johnsom: agree, and unless we let people opt into the old behavior with a microversion, we're already changing that behavior underneath them | |
| 19:29:22 | sean-k-mooney | that is an option | |
| 19:29:25 | dansmith | no, | |
| 19:29:29 | dansmith | I mean with the previous change | |
| 19:29:35 | sean-k-mooney | disable the mangeling in new microverion | |
| 19:29:42 | dansmith | going from more mangling to less mangling is less disruptive, I'm sure | |
| 19:29:58 | sean-k-mooney | it will go from 200 to 400 | |
| 19:30:08 | dansmith | no, because we won't know until too late, right? | |
| 19:30:21 | dansmith | but as I said above, there's a discussion to be had on the microversion requirement | |
| 19:30:26 | sean-k-mooney | actully right it will not change the respocne and go form active to errror | |
| 19:30:30 | dansmith | ...right | |
| 19:34:25 | sean-k-mooney | https://github.com/openstack/nova/commit/9046f0fff4be424eda25401a3f9b8752964de775 that was the change we did 2 years ago to adress https://bugs.launchpad.net/nova/+bug/1581977 | |
| 19:35:02 | sean-k-mooney | https://lists.openstack.org/pipermail/openstack-discuss/2020-November/019113.html was the mailing list thread | |
| 19:37:45 | dansmith | sean-k-mooney: that's because we set hostname from display name if hostname isn't set specifically right? | |