Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-09-24
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
20:30:56 melwitt it was confusing thinking about it, but the proposal was to make the key column case sensitive by changing the collation, but that would break anyone who was relying on the case insensitive behavior. and then I got fatigued thinking about it
20:31:30 dansmith yep,
20:31:31 melwitt so I was trying to determine whether there would be any similar thing here
20:31:38 dansmith the difference here is that a hostname with a different name is not legit
20:31:42 dansmith in an aggregate mapping
20:31:56 dansmith since metadata is free-form, it is legit (although silly) to use two keys that differ only by case
20:32:09 dansmith er, "hostname with a different case"
20:32:32 melwitt ok, I see
20:33:06 dansmith mriedem: I can stack the check on top of this if you'll write the tests when I run out of time
20:35:04 dansmith mriedem: we could also just use the name from the mapping instead of the one they asked for,
20:35:18 dansmith which means they won't get a NotFound, and we'll add the mapping they intended
20:35:27 dansmith but they may fail similarly later if they keep using the wrong name
20:35:34 dansmith so I tend to err on the side of fail fast,
20:35:41 dansmith especially since PG will already cause us to behave that way

Earlier   Later