| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-07-19 | |||
| 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? | |
| 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? | |
| 19:38:10 | sean-k-mooney | yes before xena that was teh only way to set hostname | |
| 19:38:20 | sean-k-mooney | it was an internal atribute on the instance | |
| 19:38:24 | dansmith | ah | |
| 19:38:54 | sean-k-mooney | we added an api to set that in repsonce to the issues raised with ^ | |
| 19:38:59 | dansmith | okay so, this is even less of a problem I think.. if they specify the hostname, we can just take it as-is and if not, then we keep the existing display->filter->hostname behavior right? | |
| 19:39:03 | sean-k-mooney | https://specs.openstack.org/openstack/nova-specs/specs/xena/implemented/configurable-instance-hostnames.html | |
| 19:39:30 | sean-k-mooney | well we do not allow FQDNs via that api | |
| 19:39:42 | sean-k-mooney | e.g. if you pass --hostname it must not be an fqdn | |
| 19:39:54 | dansmith | ack, so that requires a microversion then | |
| 19:40:04 | sean-k-mooney | to allow it to be an fqdn yes | |
| 19:40:14 | sean-k-mooney | if we want to allwo that | |
| 19:40:15 | dansmith | all sounds fine to me, and isn't going to break anyone | |
| 19:40:32 | sean-k-mooney | well that is what the --domain spec was trying to do | |
| 19:40:34 | johnsom | dansmith Yeah, that was what I was thinking. The display name is always going to be a mess. But the hostname field is pretty clear it can't be crazy and should be good for pass through | |
| 19:40:44 | sean-k-mooney | add a domain filed to not continute to overload hostname | |
| 19:40:58 | sean-k-mooney | since hostnanme is used for dns_name and that cant be an fqdn today | |
| 19:41:05 | dansmith | johnsom: right and since we have the displayname->hostname(ifunset) part, we can keep the mangling if you don't otherwise set it to something... but if you do, it better be right | |
| 19:41:06 | sean-k-mooney | well ok it kind of can | |
| 19:41:36 | dansmith | "it" being hostname in my statement above | |
| 19:41:47 | sean-k-mooney | dansmith: so the propsoal is to just relax instanstnce.hostname if you pass it explictly | |
| 19:41:58 | sean-k-mooney | that might need a db migration | |
| 19:42:09 | sean-k-mooney | i would hae to check the column size | |
| 19:42:11 | johnsom | Nope, I looked, the field is fine in the DB | |
| 19:42:13 | dansmith | sean-k-mooney: sounds like that's all that needs to happen? | |
| 19:42:18 | johnsom | It's 255 already | |
| 19:42:22 | sean-k-mooney | oh ok | |
| 19:42:27 | sean-k-mooney | we are limiting it to 63 | |
| 19:42:29 | sean-k-mooney | in the api | |
| 19:42:50 | sean-k-mooney | so we would just need to drop the extra vlaidation on hostname when its passed explcitly | |
| 19:43:00 | sean-k-mooney | and live with the fact it can be an fqdn | |
| 19:43:01 | dansmith | for the new microversion yeah | |
| 19:43:14 | sean-k-mooney | and document that it will be passed as is to neutron | |
| 19:43:25 | dansmith | I don't love it, but I like it a lot better than us taking multiple things and constructing FQDNs | |
| 19:43:49 | sean-k-mooney | ya we dissucsed having --fqdn when --hostname was added | |
| 19:43:55 | sean-k-mooney | but we did not want to support fqdns | |
| 19:44:13 | sean-k-mooney | but i guess that is a less invasive change | |
| 19:44:20 | dansmith | okay now hold up, | |
| 19:44:24 | dansmith | what about the multi-create case? | |
| 19:44:46 | sean-k-mooney | we will sufix the fqdn | |
| 19:44:47 | dansmith | that would require the parsing of the hostname in nova to insert the index | |
| 19:44:59 | sean-k-mooney | well or that | |
| 19:45:17 | sean-k-mooney | or make the sufic a prefix | |
| 19:45:18 | dansmith | suffixing the fqdn won't yield anything valid, so no point in doing that | |
| 19:45:27 | dansmith | yeah 1-$hostname would work I think | |
| 19:45:36 | sean-k-mooney | we could change that in the microversion | |