Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-01-13
16:24:26 mriedem alaski: is your perspective on that model biased to how it was done with cells v1?
16:24:26 mwhahaha alaski: sure, just trying to understand this better
16:25:06 mriedem an empty cell shouldn't break anything, no
16:25:07 mdbooth cfriesen: It's a reasonable hook. Possible other storage could be cleverer under some circumstances, but Rbd requires it.
16:25:17 alaski mriedem: very likely, I'm irreparably damaged from v1
16:25:27 mriedem alaski: you're safe now, it's OK :)
16:25:38 openstackgerrit Sven Anderson proposed openstack/nova: Fix suggested database migration command https://review.openstack.org/420060
16:25:43 cfriesen mdbooth: agreed, it's fine. I just didn't understand what was going on.
16:26:01 mriedem ok so i've updated https://wiki.openstack.org/wiki/Nova-Cells-v2#Nova_Cells_v2 with bugs, todos and open questoins
16:26:03 mriedem *questions
16:26:27 alaski note that I'm not trying to argue for not allowing an empty cell, just explaining why that assumption was in my head when writing code which likely affected the direction
16:27:11 dtp dansmith melwitt i'm at your disposal today
16:27:35 mriedem alaski: yup
16:28:02 dims mriedem : bauzas : https://review.openstack.org/#/c/419476/ gets the full cinder/rally test working. that test does a nova boot/attach/detach/delete etc. should be easy to review as well (almost all code protected with six.PY3 checks so wont regress existing stuff too)
16:28:03 melwitt dtp: dansmith is on PTO today FYI
16:28:10 dtp ok
16:29:44 alaski o/
16:29:54 melwitt so things used to work prior to that patch because the first run of simple_cell_setup created cell0 and then failed when it found no compute nodes, then the retry found cell0 and returned 0
16:30:15 melwitt so it continued on without creating any cell mapping
16:30:54 bauzas melwitt: yup
16:33:04 dtp melwitt i'll just be reviewing your and dan's changes under your blueprints. lemme know if there's something else i should do
16:35:14 dtp if i don't respond immdediately, i'm trapped in a meeting
16:36:13 openstackgerrit Balazs Gibizer proposed openstack/nova: handle uninited fields in notification payload https://review.openstack.org/415857
16:36:44 mriedem melwitt: so the question right now is shoud this be a 1 or a 0 https://github.com/openstack/nova/blob/3ec43d81c324d8229bbb4b5db301175886a049b6/nova/cmd/manage.py#L1114
16:37:42 mriedem that's also called from the map_cell_and_hosts CLI https://github.com/openstack/nova/blob/3ec43d81c324d8229bbb4b5db301175886a049b6/nova/cmd/manage.py#L1316
16:37:56 mriedem note the comment in there about what the return codes mean (0 everything is done, 1 means you have more to do)
16:38:13 melwitt yeah. I think the 1 is supposed to be a signal that "hey, I didn't create any cell mappings other than cell0"
16:38:17 mriedem i think i took issue with that a bit during a review because i consider >0 == error
16:38:34 sdague which is the standard unixism
16:38:38 melwitt yeah
16:38:50 sdague set -e on bash, and this will blow up any script running it
16:39:22 bauzas mriedem: I think that if we wanna have simple_cell_setup idempotent, it's acceptable to have it return 0 if you have no hosts
16:39:53 mriedem right i tend to think that simple_cell_setup should not return >0 unless there is a real error
16:39:57 melwitt that's what I'm not sure of. I thought the command means "create cell mappings" and if there are no hosts, it doesn't
16:39:58 bauzas I agree
16:40:07 bauzas well, I agree with mriedem
16:40:23 mriedem map_cell_and_hosts *should* return 1 if there are no computes
16:40:33 owalsh mriedem: what's worse is that unhandled exceptions will return 1
16:40:35 mriedem because you're specifically telling it to map hosts to cell mappings and if there are no computes, it's an error
16:40:44 melwitt mriedem: okay, I see what you're getting at
16:40:46 mriedem owalsh: well it depends on what the exception is
16:40:47 bauzas mriedem: I agree (again)
16:41:13 bauzas people could put simple_cells_setup in a cron and leave it run
16:41:20 mriedem we also need to, at some point in the near future, probably do an audit of the various commands and figure out in which scenarios you'd use them
16:41:24 mriedem because we have a glut of CLIs for this right now
16:41:29 owalsh mriedem: anything uncaught that subclasses Exception it caugh in __main__
16:41:42 mriedem owalsh: i don't see a problem with that
16:42:00 bauzas mriedem: melwitt: fancy leaving me submit a patch for changing the return code to 0 ?
16:42:02 mriedem owalsh: if we can't connect to the database and it raises an exception, then i think we should return a non-0 exit code
16:42:05 dims mriedem : "IBM PowerKVM" CI uses github to pull stuff? "GIT_BASE=git://github.com" (http://dal05.objectstorage.softlayer.net/v1/AUTH_3d8e6ecb-f597-448c-8ec2-164e9f710dd6/pkvmci/nova/76/419476/11/check/tempest-dsvm-full-xenial/2649398/console.html)
16:42:18 bauzas mriedem: yup, if we can't connect, it's a very different situation
16:42:20 mriedem dims: ask mmedvede
16:42:23 bauzas something is messed up
16:42:27 owalsh mriedem: yees, but having 1 == on computes, 2 == something else doesn't communicate any info
16:42:35 owalsh s/on/no/
16:42:51 dims mmedvede : looks like "IBM PowerKVM CI" got busted when github went down
16:43:02 openstack bug 1656276 in tripleo "Error running nova-manage cell_v2 simple_cell_setup when configuring nova with puppet-nova" [Critical,Triaged] https://launchpad.net/bugs/1656276
16:43:02 mriedem bauzas: i think that's something we should at least put up a patch for yeah for bug 1656276 - make simple_cell_setup return 0 if map_cell_and_hosts returns None, and change map_cell_and_hosts (the CLI) to return 1 if there is nothing mapped
16:43:28 mriedem owalsh: i'm saying i don't like a non-0 exit code communicating non-error conditions,
16:43:29 bauzas mriedem: yup, and you reported a bug too ?
16:43:39 mriedem i.e. 1 == no computes
16:43:44 mriedem bauzas: that's bug 1656276
16:43:50 mriedem it's what tripleo is hitting
16:44:07 rfolco dims, looking
16:44:13 mmedvede dims: yeah, we had problems pulling from git.o.o at some point
16:44:17 openstack Launchpad bug 1656017 in OpenStack Compute (nova) "nova-manage cell_v2 map_cell0 always returns a non-0 exit code" [Medium,Confirmed]
16:44:17 bauzas mriedem: I was thinking of https://bugs.launchpad.net/nova/+bug/1656017
16:44:18 mmedvede that is the reason
16:44:26 mriedem bauzas: that's different
16:44:35 bauzas mriedem: but yeah, the patch I'm writing on is against 1656276
16:44:35 dims mmedvede : interesting, talked to our infra folks?
16:44:43 bauzas mriedem: ie. changing what we return
16:44:56 mriedem dtp: you could tackle bug 1656017 if you want
16:45:06 mriedem dtp: that fix should be pretty straight-forward
16:45:17 mmedvede dims: it appeared to be a problem with our internal network, so I did not discuss with infra
16:45:19 dtp ok, i'll check it out
16:45:28 dims mmedvede : ack. thanks
16:45:29 mriedem bauzas: melwitt: the only reservation i have is that simpe_cell_setup will now return 0 for an incomplete setup
16:45:41 mriedem bauzas: melwitt: and we need to understand the implications of that
16:45:48 mmedvede dims: do you think there are inherent problems with using github vs git.o.o?
16:46:00 melwitt mriedem: yeah, that's what I was thinking about
16:46:04 mriedem it might just mean, print a message saying you need to run map_cell_and_hosts or discover_hosts after you've added compute nodes
16:46:06 bauzas mriedem: I know a bit of puppet world and idempotence, and I think it's reasonable
16:46:42 mmedvede dims: ok, I see your comment above now re our CI going down with github
16:46:42 bauzas I mean, it's a reasonable assertion that an idempotent script can only provide a portion of what it can modify on the first run
16:46:43 dims mmedvede : just surprised. you may get a stronger reaction from infra folks if you tell them :)
16:46:52 owalsh mriedem: yea, I don't like it either, but if we do want to communicate a specific condition in this way then non-zero, but not 1
16:46:52 dims mmedvede : right
16:46:59 bauzas but eventually, after X runs, it would do all required
16:47:25 mriedem bauzas: melwitt: note that with the nova-status upgrade check command,
16:47:35 mriedem we don't consider it a failure if you don't have any computes yet https://github.com/openstack/nova/blob/3ec43d81c324d8229bbb4b5db301175886a049b6/nova/cmd/status.py#L168
16:47:39 mriedem but you have cell mappings and cell0
16:47:48 mriedem which is specifically for the fresh install case
16:47:58 melwitt mriedem: right, that's what I was trying to work backward from
16:49:21 melwitt mriedem: I like your idea about printing a message if simple_cell_setup finds no hosts, with a reminder what to do once you have compute hosts. the thing is the incomplete setup, things actually work fine for now because we have a bunch of fallbacks if no cell mappings, we run the old way
16:50:26 rfolco dims, stronger reaction means they disagree ?
16:50:59 dims rfolco : i think they would prefer direct git.openstack.org for sure
16:51:28 mriedem melwitt: hmm, the db schema migration will fail if you don't have cell mappings,
16:51:38 mriedem melwitt: but if you run that before adding flavors, then it's a noop
16:51:57 bauzas mriedem: melwitt: interesting, we return 0 in map_cell_and_hosts even if we have no hosts...

Earlier   Later