| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-09-24 | |||
| 20:15:58 | dansmith | (IMHO) | |
| 20:16:08 | mriedem | there are lots of things like this in the api | |
| 20:16:12 | dansmith | yup | |
| 20:16:44 | jaypipes | I still don't see how MySQL is hurting us | |
| 20:16:57 | mriedem | because adding the host to the aggregate with the wrong name doesn't puke in the api | |
| 20:17:00 | jaypipes | the user expects a case-insenstive API. | |
| 20:17:10 | dansmith | I don't and I'm a user | |
| 20:17:11 | mriedem | you keep saying that but i'm not sure why | |
| 20:17:13 | dansmith | yeah | |
| 20:17:19 | jaypipes | the user that reported all these bugs. | |
| 20:17:23 | mriedem | heh | |
| 20:17:35 | dansmith | that user would be fine if nova told it it was wrong | |
| 20:17:35 | mriedem | if i create a server with name Foo i don't expect to find it using FOO | |
| 20:17:44 | openstackgerrit | Merged openstack/os-vif stable/queens: import zuul job settings from project-config https://review.openstack.org/601399 | |
| 20:17:46 | openstackgerrit | Merged openstack/os-vif stable/pike: import zuul job settings from project-config https://review.openstack.org/601394 | |
| 20:17:48 | openstackgerrit | Merged openstack/os-vif stable/ocata: import zuul job settings from project-config https://review.openstack.org/601388 | |
| 20:17:58 | dansmith | the original bug even said "accidentally typed" | |
| 20:18:02 | mriedem | yeah i think rajesh's patch / bug report was saying he expected HostNotFound | |
| 20:18:11 | dansmith | like because they haven't properly mapped caps lock to control like everyone should | |
| 20:18:15 | mriedem | "While adding compute0 to host-aggregate, if I provide hostname as "COMPUTE0.example.com", instead of | |
| 20:18:16 | mriedem | throwing HostNotFound error, it is added to host-aggregate." | |
| 20:18:53 | dansmith | face it jaypipes, PG rules and mysql drools | |
| 20:19:02 | jaypipes | perhaps that is because URIs and hostnames are case-insensitive and always have been? ... | |
| 20:19:25 | jaypipes | GOOGLE.com == goOGle.com | |
| 20:19:37 | dansmith | DNS is case insensitive | |
| 20:19:40 | mriedem | so one fix for https://bugs.launchpad.net/nova/+bug/1793747 is to revert https://review.openstack.org/#/q/Iee4b9bbf412adfdc6fdc62ea3429fb960d6ac2a2 right? | |
| 20:19:40 | openstack | Launchpad bug 1793747 in OpenStack Compute (nova) "Fails to boot instance using Blazar flavor if compute host names are in uppercase" [High,Triaged] - Assigned to Neha Alhat (nehaalhat) | |
| 20:19:41 | dansmith | URIs are not, AFAIK, in apache | |
| 20:19:50 | jaypipes | but whatevers, I need to go puke on chef again. | |
| 20:20:20 | dansmith | mriedem: yes I think we should revert rajesh's patch and fix the NotFound on original add | |
| 20:20:20 | mriedem | or, we need to lowercase aggregate members everywhere...? | |
| 20:20:44 | mriedem | how do we fix the not found on original add if the db is case insensitive? | |
| 20:21:05 | dansmith | we just check the result | |
| 20:21:14 | dansmith | if host_name == compute.host | |
| 20:21:15 | dansmith | or whatever | |
| 20:21:16 | jaypipes | dansmith: and you'd need to fix PG manually since it will undoubtedly have allowed 2 records, one for compute0 and one for COMPUTE0 | |
| 20:21:29 | mriedem | jaypipes: mysql manually | |
| 20:21:32 | mriedem | PG won't allow this by default | |
| 20:21:34 | jaypipes | in the host_aggregate_hosts table (or whatever the heck it's named) | |
| 20:21:41 | dansmith | yeah PG is already fine | |
| 20:21:43 | dansmith | no | |
| 20:21:49 | dansmith | you won't have ever found COMPUTE0 | |
| 20:21:49 | jaypipes | what are you guys talking about? | |
| 20:21:51 | dansmith | so we would fail | |
| 20:22:13 | dansmith | jaypipes: we look up the host before we add the aggegate_host | |
| 20:22:20 | jaypipes | PG will allow multiple records to be created, one for agg -> COMPUTE0 and one for agg -> compute0 | |
| 20:22:22 | dansmith | in mysql we find it wrongly, case insensitive | |
| 20:22:27 | dansmith | in PG, we never find it, | |
| 20:22:43 | jaypipes | ah. if that fetch is done in the API layer... | |
| 20:22:45 | jaypipes | is it? | |
| 20:22:45 | dansmith | so we won't add the aggregate_host | |
| 20:22:46 | dansmith | we don't need the constraint to help us in PG, | |
| 20:22:47 | dansmith | because we never get that far | |
| 20:22:48 | dansmith | right | |
| 20:22:58 | dansmith | mriedem says it is | |
| 20:23:09 | dansmith | [13:15:46] <mriedem>mapping = objects.HostMapping.get_by_host(context, host_name) | |
| 20:23:22 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L5289 | |
| 20:24:05 | mriedem | dansmith: so you want to add: if mapping.host == host_name? in the api? | |
| 20:24:09 | mriedem | for the case check? | |
| 20:24:17 | dansmith | yeah | |
| 20:24:26 | mriedem | yeah that seems easy enough | |
| 20:24:28 | dansmith | wait, are you asking if I want to write that patch? | |
| 20:24:35 | dansmith | or just if I like that approach? | |
| 20:24:40 | mriedem | i was confirming that was your idea | |
| 20:24:45 | dansmith | indeed | |
| 20:24:57 | mriedem | i expect you're already half dressed for vacation | |
| 20:25:03 | dansmith | indeed :) | |
| 20:25:05 | mriedem | (un)dressed | |
| 20:25:33 | mriedem | dansmith: are you going to propose the revert at least? | |
| 20:25:37 | melwitt | will this change in anyway cause existing tooling to stop working? that is, do we need a microversion? | |
| 20:25:41 | dansmith | I can if you want | |
| 20:25:52 | mriedem | we already regressed existing tooling | |
| 20:25:59 | mriedem | https://bugs.launchpad.net/nova/+bug/1793747 | |
| 20:25:59 | openstack | Launchpad bug 1793747 in OpenStack Compute (nova) "Fails to boot instance using Blazar flavor if compute host names are in uppercase" [High,Triaged] - Assigned to Neha Alhat (nehaalhat) | |
| 20:25:59 | dansmith | melwitt: this is making the api consistent with the database, if your database is PG | |
| 20:26:01 | dansmith | right | |
| 20:26:01 | openstackgerrit | Merged openstack/python-novaclient stable/pike: import zuul job settings from project-config https://review.openstack.org/601395 | |
| 20:26:03 | openstackgerrit | Merged openstack/python-novaclient stable/ocata: Use generic user for both zuul v2 and v3 https://review.openstack.org/601932 | |
| 20:26:03 | openstackgerrit | Merged openstack/python-novaclient stable/ocata: import zuul job settings from project-config https://review.openstack.org/601391 | |
| 20:26:09 | melwitt | I mean the addition of the case sensitive check, not the revert | |
| 20:26:16 | dansmith | the check is ^ | |
| 20:26:26 | mriedem | shouldn't need to opt out of broken behavior | |
| 20:26:29 | mriedem | so this isn't a microversion | |
| 20:26:29 | dansmith | right | |
| 20:26:45 | melwitt | ok, just wondering if this might somehow break anyone's existing automation | |
| 20:27:19 | dansmith | we already broke it | |
| 20:27:28 | dansmith | this makes it work again | |
| 20:27:36 | dansmith | anyone's automation is going to be using the actual hostname, | |
| 20:27:43 | dansmith | either from the system or from our own API | |
| 20:27:50 | dansmith | those things would have broken silently just now | |
| 20:27:53 | dansmith | this will unbreak them | |
| 20:27:59 | jaypipes | this is partly why I hate even reviewing anything like that patch any more... always seems that they get reverted. | |
| 20:28:00 | dansmith | the combo | |
| 20:28:41 | mriedem | i should have sniffed this out sooner on the backports given i knew about the earlier work from HPE to try and solve the case insensitivity issues in the api | |
| 20:29:02 | mriedem | mdbooth was also working on something related to this recently | |
| 20:29:36 | mriedem | http://lists.openstack.org/pipermail/openstack-dev/2018-August/thread.html#133202 | |
| 20:29:51 | melwitt | yeah, that was the metadata keys case insensitive thing | |
| 20:29:59 | dansmith | mriedem: | |
| 20:30:44 | mriedem | beware then https://review.openstack.org/#/c/504885/ | |
| 20:30:48 | openstackgerrit | Dan Smith proposed openstack/nova master: Revert "Make host_aggregate_map dictionary case-insensitive" https://review.openstack.org/604898 | |