| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-09-24 | |||
| 20:13:46 | jaypipes | dansmith: where are we mangling it internally other than the host manager's host aggregate state internal map? | |
| 20:13:58 | dansmith | exactly there | |
| 20:14:03 | dansmith | that's the problem right? | |
| 20:14:12 | jaypipes | dansmith: and how exactly would PG vs. MySQL "solve" this problem? | |
| 20:14:19 | dansmith | jaypipes: mriedem said that not me | |
| 20:14:25 | dansmith | I don't think it would | |
| 20:14:45 | dansmith | unless PG honors case, but rejects duplicates that differ only by case | |
| 20:14:56 | jaypipes | "<dansmith> jaypipes: yeah we should totes just depend on our backend database ignoring case for us :)" | |
| 20:14:58 | mriedem | PG is case sensitive by default | |
| 20:15:07 | openstackgerrit | Merged openstack/python-novaclient stable/queens: Switch to stestr https://review.openstack.org/601933 | |
| 20:15:08 | openstackgerrit | Merged openstack/python-novaclient stable/queens: import zuul job settings from project-config https://review.openstack.org/601400 | |
| 20:15:09 | mriedem | so fat fingering COMPUTE0 should result in HostNotFound | |
| 20:15:20 | dansmith | mriedem: I don't think it would if we're not checking | |
| 20:15:38 | mriedem | 1709260 | |
| 20:15:39 | mriedem | oops | |
| 20:15:44 | dansmith | or maybe you mean we're "checking" by just looking it up? | |
| 20:15:45 | mriedem | mapping = objects.HostMapping.get_by_host(context, host_name) | |
| 20:15:47 | mriedem | yes | |
| 20:15:49 | dansmith | gotcha | |
| 20:15:55 | dansmith | then yeah, mysql _is_ hurting us here | |
| 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 | |