| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-01-25 | |||
| 23:24:18 | dansmith | okay | |
| 23:24:30 | bauzas | but in case it works, could you please look at the change ? | |
| 23:24:49 | bauzas | I'll review your own change tomorrow morning | |
| 23:25:34 | dansmith | cripes I keep failing this one test, but nothing has changed since it was passing | |
| 23:25:34 | dansmith | grr | |
| 23:26:00 | bauzas | which one ? | |
| 23:27:03 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova: Updated from global requirements https://review.openstack.org/424828 | |
| 23:28:30 | dansmith | bauzas: go to sleep | |
| 23:28:37 | openstackgerrit | Sylvain Bauza proposed openstack/nova: Scheduler calling the Placement API https://review.openstack.org/417961 | |
| 23:28:37 | openstackgerrit | Sylvain Bauza proposed openstack/nova: Fix server group functional test by using all filters https://review.openstack.org/425185 | |
| 23:28:48 | bauzas | voila... all the changes are now rebased | |
| 23:29:34 | bauzas | dansmith: I'll at least wait a few mins to see if we have at least one grenade job done | |
| 23:32:18 | dansmith | oh jeez | |
| 23:36:22 | openstackgerrit | Dan Smith proposed openstack/nova: Move instance creation to conductor https://review.openstack.org/319379 | |
| #openstack-nova - 2017-01-26 | |||
| 00:44:36 | openstackgerrit | Takashi NATSUME proposed openstack/nova: Enable cold migration with target host(1/2) https://review.openstack.org/408955 | |
| 00:44:49 | openstackgerrit | Takashi NATSUME proposed openstack/nova: Enable cold migration with target host(1/2) https://review.openstack.org/408955 | |
| 00:45:59 | openstackgerrit | Takashi NATSUME proposed openstack/nova: Enable cold migration with target host(2/2) https://review.openstack.org/408964 | |
| 00:46:58 | openstackgerrit | Takashi NATSUME proposed openstack/nova: api-ref: Add parameters in cold migrate action https://review.openstack.org/410042 | |
| 00:47:26 | openstackgerrit | Takashi NATSUME proposed openstack/python-novaclient: Microversion 2.42 - Enable cold migration with target host https://review.openstack.org/406707 | |
| 00:47:54 | openstackgerrit | Takashi NATSUME proposed openstack/nova: Add functional tests for cold migration to same host https://review.openstack.org/414926 | |
| 00:58:08 | alex_xu | gmann: johnthetubaguy if we think 403 is behaviour change. Then this is a behaviour change https://review.openstack.org/#/c/415142/33/nova/api/openstack/compute/servers.py@1230 ? | |
| 00:58:15 | alex_xu | Kevin_Zheng: ^ | |
| 00:59:57 | alex_xu | gmann: johnthetubaguy but expose host and node to the user is kind of bug fix. it isn't same case with all_tenants which is filter we support it public | |
| 01:00:09 | alex_xu | s/bug fix/bug/ | |
| 01:05:01 | dansmith | melwitt: this is good again: https://review.openstack.org/#/c/319379/ | |
| 01:05:17 | dansmith | melwitt: I broke it in a very subtle way when fixing johnthetubaguy's comment, but it's good now | |
| 01:13:09 | mriedem | did bauzas get the grenade thing figured out? | |
| 01:13:47 | mriedem | dansmith: simple change needed in https://review.openstack.org/#/c/424243/ and then i'm +2 | |
| 01:14:08 | dansmith | mriedem: we're waiting for a run I think | |
| 01:16:07 | gmann | alex_xu: | |
| 01:16:15 | gmann | alex_xu: humm, | |
| 01:17:22 | dansmith | mriedem: | |
| 01:17:34 | dansmith | gah | |
| 01:17:53 | mriedem | _LW _MFER | |
| 01:18:13 | openstackgerrit | Dan Smith proposed openstack/nova: Only warn about hostmappings during ocata upgrade https://review.openstack.org/424243 | |
| 01:18:18 | dansmith | there | |
| 01:21:05 | gmann | alex_xu: IMO, that as bug fix makes it valid | |
| 01:24:10 | oomichi | alex_xu: Does it mean the patch changes 403 to 400 ? | |
| 01:26:13 | oomichi | alex_xu: we are discussing error status code changes on https://review.openstack.org/#/c/421846 . I am ok to allow error status code changes anyways | |
| 01:26:30 | openstackgerrit | Dan Smith proposed openstack/nova: Only warn about hostmappings during ocata upgrade https://review.openstack.org/424243 | |
| 01:29:33 | gmann | oomichi: policy change for all_tenant will change 403->200 | |
| 01:29:53 | gmann | oomichi: so its kinnda behavior change | |
| 01:30:37 | gmann | oomichi: and on sort key change it is bug fix (not expose host and node to non admin user) and 200->400 | |
| 01:31:02 | gmann | alex_xu: correct me if am wrong on sort key which used to me 200 now? | |
| 01:32:54 | oomichi | gmann: humm, 200->400 seems against on changing policy: "The following types of changes are generally **not** considered acceptable:" | |
| 01:33:18 | oomichi | "A change such that a request which was successful before now results in an error response" | |
| 01:33:49 | oomichi | if that is not security bug | |
| 01:34:56 | oomichi | gmann: "not expose host and node to non admin user" seems a little bit security thing, which means abstraction doesn't work correctly for non-admin users | |
| 01:35:08 | mriedem | melwitt: this is an easy one https://review.openstack.org/#/c/424243/ | |
| 01:35:09 | oomichi | but I am not sure at this time | |
| 01:37:52 | gmann | oomichi: yea that seems security bug for me too. but johnthetubaguy alex_xu mriedem can give better feedback on this - last paragraph in https://specs.openstack.org/openstack/nova-specs/specs/ocata/approved/add-whitelist-for-server-list-filter-sort-parameters.html#proposed-change | |
| 01:39:24 | oomichi | gmann: yeah I see "For the non-admin user, the sort key ‘host’ and ‘node’ will be excluded. Those two columns are about the cloud internal. It can’t be leaked to the end user." | |
| 01:41:56 | oomichi | gmann: if we consider it as security issue, the bug should be fixed on stable branches. But we need different patch for fixing it because it is difficult to backport the whole patches related to the bp | |
| 01:43:03 | gmann | oomichi: hummm | |
| 01:46:07 | gmann | alex_xu: johnthetubaguy oomichi on policy all_tenant changes, should we first change the spec ? - https://specs.openstack.org/openstack/nova-specs/specs/ocata/approved/add-whitelist-for-server-list-filter-sort-parameters.html#rest-api-impact | |
| 02:04:44 | melwitt | dansmith: I left a comment on https://review.openstack.org/#/c/319379/ it looks like the IBM CI failure might be legit, OrphanedObjectError raised from L1790 | |
| 02:05:12 | melwitt | I don't yet understand how the instance could be orphaned at that point | |
| 02:19:04 | alex_xu | oomichi: yea, just as gmann said, this probably is a fix for key we didn't want to expose. I feel that is different with all_tenants | |
| 02:20:00 | alex_xu | gmann: i'm not sure about the spec. but give th time i'm not sure we can get all thing done. maybe worth a email also to get widely suggestion | |
| 02:20:14 | mriedem | dansmith: bauzas: jaypipes: well it looks like the scheduler + placement stuff is failing grenade multinode because of this test: | |
| 02:20:15 | mriedem | https://github.com/openstack/tempest/blob/8bdd42766a7f02180d07e6ab75f0914e5d876a91/tempest/scenario/test_server_multinode.py#L49 | |
| 02:20:16 | gmann | alex_xu: i did not fully understood your comment about capability discovery on this https://review.openstack.org/#/c/415330/24 | |
| 02:20:26 | mriedem | i believe because we have 2 computes, but one is newton and one is ocata, and the newton one doesn't have placement configured, | |
| 02:20:45 | mriedem | so when the scheduler tries to place an instance on the newton compute, the scheduler pukes because there isn't a resource provider for that node | |
| 02:21:22 | gmann | alex_xu: you mean 403->200 is ok to change and discover with capabilities of all_tenant ? | |
| 02:21:36 | gmann | alex_xu: correct me if other way around | |
| 02:21:41 | alex_xu | gmann: whether the user is allowed to execute on a API, that should be discovered by capabilities discovery https://review.openstack.org/386555 | |
| 02:21:49 | owalsh | dansmith, mriedem, melwitt: big thanks for https://review.openstack.org/424243 | |
| 02:22:33 | dansmith | mriedem: with my backported change the newton one should keep trying | |
| 02:22:42 | dansmith | mriedem: this: https://review.openstack.org/#/c/419217/ | |
| 02:22:47 | dansmith | owalsh: np | |
| 02:23:03 | alex_xu | gmann: no, I agree with johnthetubaguy. We change a enforce from hard to soft, that is a change on the API contract. so yes, we need microversion | |
| 02:23:11 | mriedem | dansmith: but do we ever configure the newton subnode in grenade with placement creds? | |
| 02:23:32 | mriedem | dansmith: nope http://logs.openstack.org/61/417961/26/check/gate-grenade-dsvm-neutron-multinode-ubuntu-xenial/15545e4/logs/subnode-2/etc/nova/nova.conf.txt.gz | |
| 02:23:41 | dansmith | mriedem: oh right, damn | |
| 02:24:08 | dansmith | mriedem: so it fails because we don't have resources for multiple computes, right? | |
| 02:24:23 | dansmith | mriedem: that's farther than it was getting before, right? so that | |
| 02:24:25 | gmann | alex_xu: yea, so we all are on same page. thanks | |
| 02:24:27 | dansmith | mriedem: is progress | |
| 02:24:37 | alex_xu | gmann: yea | |
| 02:25:00 | gmann | alex_xu: i ll push spec + patch updates and both can be reviewed in parallel | |
| 02:25:10 | alex_xu | gmann: thanks! | |
| 02:25:21 | mriedem | dansmith: correct, this test uses AZs to specifically place an instance on each compute node | |
| 02:25:32 | mriedem | so the forced_host thing or whatever | |
| 02:26:29 | mriedem | we could change the test to not be a smoke test and it wouldn't be run with grenade, but i could see people freaking out about that | |
| 02:27:08 | dansmith | mriedem: well, it's either that or backport cred setup to newton devstack right? | |
| 02:27:18 | mriedem | cred setup is in newton devstack, | |
| 02:27:29 | mriedem | it's just grenade isn't configuring placement creds into nova.conf on the subnode | |
| 02:27:58 | dansmith | oh | |
| 02:28:27 | mriedem | the other alternative, which you're not going to probably like, is for ocata we fallback to the compute node get_all and drop that in pike | |
| 02:28:40 | mriedem | because ocata computes will have placement setup for grenade from ocata->pike | |
| 02:28:40 | dansmith | mriedem: I guess it's an important thing to check that the subnode is still live after the upgrade, assuming proper config | |
| 02:29:06 | dansmith | mriedem: seems strange to push this hard and then not try to fix grenade at the last minute | |
| 02:33:40 | mriedem | yeah i know, i'm not sure how hard this is to make grenade config the newton subnode for placement right now | |
| 02:34:08 | dansmith | melwitt: can you see my response to you on my patch just now? | |
| 02:34:28 | mriedem | i have an emergency toenail situation to deal with that's much easier to fix, will check grenade after that | |
| 02:34:36 | melwitt | dansmith: yeah just opened it. replying | |
| 02:41:06 | dansmith | melwitt: might be easier to chat here | |
| 02:41:45 | melwitt | dansmith: okay. that's setting the _context on the BuildRequest object though, not the BuildRequest.instance object | |
| 02:42:18 | melwitt | I don't see anywhere in _load_instance that it sets instance._context | |