Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-01-13
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...
16:52:04 rfolco dims, ah ok. thanks for the ping.
16:52:10 mriedem so you could run the api_db sync, it passes b/c no flavors, then you run simple_cell_setup which only creates cell0, and then you register your computes - you won't have any cell mappings for those
16:52:17 mriedem bauzas: yeah i think that's part of the bug fix
16:52:20 mriedem as noted above
16:52:47 mriedem melwitt: i'm pretty sure we are dumping warnings in the logs if we don't find a cell mapping and fallback to the main db
16:52:53 bauzas mriedem: yeah, I will change that
16:53:08 melwitt yeah, I can't remember how that got decided. probably the argument of "error condition vs not"
16:53:24 mriedem melwitt: the automatic compute node / cell mapping registration would fix this...but it's just not something we've been serious about adding
16:54:08 mriedem thinking about gradually increasing levels of pain, which has been our MO,
16:54:12 melwitt mriedem: yeah. it's not straightforward how to do that because, how do you make assumptions about what cell to auto map new hosts to? usually that's deliberate decision on the part of the operator
16:54:28 mriedem we could do the fallback with a warning in ocata, then make that an error in pike
16:54:40 dtp afk a bit
16:54:47 mriedem melwitt: right you'd need to probably flag some cell mapping as the 'default' to auto-register with
16:55:19 mriedem so in ocata we have the fallback mechanism if we don't find a cell mapping,
16:55:34 melwitt yeah, that's what I had in mind. new stuff goes into some "staging cell" and then after that you have to re-map. though I don't even think we have a way to re-map
16:55:38 mriedem which buys us some time to consider an auto-registration in pike, or remove the fallback and make it an error
16:56:05 mriedem melwitt: we'd probably have to write a REST API for remapping, or nova-manage CLI
16:56:19 melwitt yeah, probably nova-manage

Earlier   Later