| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-09-24 | |||
| 20:08:32 | dansmith | jaypipes: I don't | |
| 20:08:42 | dansmith | the aggregate code must not be validating hostnames when you go to add one right? | |
| 20:09:03 | dansmith | in which case maybe the fix is just to make host-add fail if you specify something wrong? | |
| 20:09:04 | jaypipes | dansmith: this isn't about that. this is about the collection of host aggregate states in the scheduler (in Python, not in the DB) | |
| 20:09:27 | dansmith | jaypipes: the original | |
| 20:09:27 | jaypipes | and Python is case-sensitive, as we know. | |
| 20:09:44 | dansmith | fix and the new regression are all about us allowing you to add a host with a non-matching case, | |
| 20:09:58 | dansmith | and then us not also ignoring case when we go to join it up right? | |
| 20:10:20 | dansmith | if we just refuse to let them add non-matching hostnames in the first place, everything else can be consistent right? | |
| 20:10:22 | jaypipes | I need to look (again) at the code. it's a giant ball of turds. | |
| 20:11:02 | jaypipes | dansmith: I don't think this is about them adding non-case-matching hostnames. | |
| 20:11:06 | dansmith | I don't expect to have case ignored. what I do expect is for nova to tell me "that's, like, not a host maan" when I go to add one to an aggregate | |
| 20:11:11 | mriedem | non-matching by looking up the host from the compute_nodes table? | |
| 20:11:31 | dansmith | jaypipes: it is.. the original fix says "accidentally typed COMPUTE0 instead of compute0" | |
| 20:11:52 | jaypipes | dansmith: no, I'm talking about the bug 1793747 | |
| 20:11:52 | openstack | bug 1793747 in OpenStack Compute (nova) "Fails to boot instance using Blazar flavor if compute host names are in uppercase" [High,Triaged] https://launchpad.net/bugs/1793747 - Assigned to Neha Alhat (nehaalhat) | |
| 20:11:58 | dansmith | jaypipes: and the regression is that blazar is taking the mixed-case hostname from the hypervisors api, and using that to add the host to an aggregate | |
| 20:12:04 | jaypipes | dansmith: there's no indication that that bug reporter has used non-matching hostname... | |
| 20:12:05 | dansmith | jaypipes: they're the same thing | |
| 20:12:27 | dansmith | jaypipes: blazar is looking at hypervisors and using that value.. | |
| 20:12:39 | mriedem | fwiw, bug 1709260 wouldn't be possible by default if they were using postgresql :P | |
| 20:12:39 | dansmith | blazar host-create Openstack-VirtualBox | |
| 20:12:40 | openstack | bug 1709260 in OpenStack Compute (nova) queens "Addition of host to host-aggregate should be case -sensitive" [Low,Fix committed] https://launchpad.net/bugs/1709260 - Assigned to Rajesh Tailor (ratailor) | |
| 20:12:55 | jaypipes | dansmith: that's the correct hostname. | |
| 20:13:05 | dansmith | jaypipes: right exactrly | |
| 20:13:12 | dansmith | jaypipes: but we're mangling it internally by lower()ing it | |
| 20:13:16 | dansmith | and they can't see that | |
| 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 | |