Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-09-24
20:20:20 mriedem or, we need to lowercase aggregate members everywhere...?
20:20:20 dansmith mriedem: yes I think we should revert rajesh's patch and fix the NotFound on original add
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 jaypipes what are you guys talking about?
20:21:49 dansmith you won't have ever found COMPUTE0
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 dansmith so we won't add the aggregate_host
20:22:45 jaypipes is it?
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 dansmith melwitt: this is making the api consistent with the database, if your database is PG
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 mriedem https://bugs.launchpad.net/nova/+bug/1793747
20:26:01 openstackgerrit Merged openstack/python-novaclient stable/pike: import zuul job settings from project-config https://review.openstack.org/601395
20:26:01 dansmith right
20:26:03 openstackgerrit Merged openstack/python-novaclient stable/ocata: import zuul job settings from project-config https://review.openstack.org/601391
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: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 dansmith right
20:26:29 mriedem so this isn't a microversion
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
20:35:44 dansmith thoughts?
20:35:51 mriedem agree on the api being explicitly
20:35:53 mriedem *explicit
20:38:15 melwitt yeah it sounds like fail fast would be better
20:40:56 openstackgerrit Jack Ding proposed openstack/nova master: Correct instance port binding for rebuilds/reboots https://review.openstack.org/603844
20:47:08 jaypipes yeah, I definitely won't be reviewing 504885. Tired of approving these patches and then needing them to be reverted.
20:47:25 jaypipes I'll just stick to reviewing anything that isn't in the API layer I guess.
20:47:41 jaypipes since our API layer is so friggin eggshells.
20:49:26 openstackgerrit Dan Smith proposed openstack/nova master: Enforce case-sensitive hostnames in aggregate host add https://review.openstack.org/604906
20:49:36 dansmith mriedem: all yours if that fails other tests
20:50:15 openstackgerrit Dan Smith proposed openstack/nova master: Enforce case-sensitive hostnames in aggregate host add https://review.openstack.org/604906
20:55:00 mriedem ack
21:05:42 melwitt reminder to everyone: forum topic submission deadline is Wed Sep 26
21:06:05 melwitt https://etherpad.openstack.org/p/nova-forum-stein
21:15:37 openstackgerrit Dan Smith proposed openstack/nova master: Enforce case-sensitive hostnames in aggregate host add https://review.openstack.org/604906

Earlier   Later