| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-16 | |||
| 16:55:49 | bauzas | but I think it's okay to make the request transformation to be "config-driven" | |
| 16:56:15 | efried | So the operator needs to change to the new config in such a way that it *exactly* matches what we inferred, right? | |
| 16:56:27 | bauzas | so, the operator would basically tell when he's okay to count PCPUs (ie. probably after the whole nodes config change) | |
| 16:56:59 | efried | Otherwise we don't just need a reshape (move allocations) - we would also possibly need to re-pin guests to different physical processors and such. | |
| 16:57:06 | bauzas | efried: no, I'm saying that existing config will report VCPUs anyway | |
| 16:57:20 | bauzas | (including options that were asking for pinned cpu)s | |
| 16:57:41 | bauzas | efried: only new config option (explicitely cpu_dedicated_set) will trigger a reshape | |
| 16:57:55 | efried | yes, I get that bauzas, what I'm saying is, we're going to *infer* VCPU/PCPU counts and pinsets based on legacy conf options; but then the operator wants to cut over to using the new conf options. | |
| 16:58:24 | bauzas | no, I don't want us to infer VCPU and PCPU based on those options because they are errorprone | |
| 16:58:28 | bauzas | efried: ^ | |
| 16:58:38 | bauzas | those options being the legacy ones | |
| 16:58:49 | efried | oh, that's the basis for the PCPU spec as written at PS24 anyway. Haven't checked since then... | |
| 16:59:39 | bauzas | that's exactly why I'm saying "don't touch anything until operator explicitely says 'I want cpu_dedicated_set') | |
| 16:59:53 | bauzas | old world = VCPU | |
| 17:00:03 | bauzas | new world = VCPU and PCPU | |
| 17:00:30 | bauzas | for the request, trigger the request option when you consider having enough hosts to sustain PCPU requests | |
| 17:00:59 | bauzas | anyway, I need to bail out | |
| 17:01:24 | bauzas | kids aren't in town, and I promised some evening to my spouse | |
| 17:01:25 | efried | I think I see. Did you comment accordingly on the spec? | |
| 17:01:33 | bauzas | efried: I think so | |
| 17:01:36 | efried | okay. | |
| 17:03:01 | sean-k-mooney | efried: yes the cpu spec currently does infer but i agree with bauzas that we should not | |
| 17:04:02 | sean-k-mooney | efried: i think not supporting inference form the old config values would remove much of the upgrade impact or atleast help us too. | |
| 17:05:04 | efried | I'm good with that. Let's get it reflected in the spec | |
| 17:05:33 | bauzas | I just provided a comment trying to summarize my thoughts | |
| 17:05:45 | bauzas | this said, calling it a day | |
| 17:05:50 | sean-k-mooney | o/ | |
| 17:06:23 | efried | imacdonn: I agree with your assessment in https://bugs.launchpad.net/nova/+bug/1824435 | |
| 17:06:24 | openstack | Launchpad bug 1824435 in OpenStack Compute (nova) stein "fill_virtual_interface_list migration fails on second attempt" [High,Triaged] | |
| 17:06:32 | efried | (I wanted to say that, but not pollute the bug with it) | |
| 17:06:52 | mriedem | dansmith: you want to check my rolling upgrade validation logic that i dumped in gibi's spec https://review.openstack.org/#/c/652608/4/specs/train/approved/server-move-operations-with-ports-having-resource-request.rst@190 ? | |
| 17:10:22 | dansmith | mriedem: I probably have to read the whole spec to make sense of that huh? | |
| 17:11:04 | imacdonn | efried, which one? i.e. do you think we need to address (2) or would fixing (1) obviate that ? | |
| 17:11:37 | efried | imacdonn: fixing (1) would obviate. But that assumes we can do so. | |
| 17:12:05 | imacdonn | efried, right ... so now I'm trying to understand why the row is being created in the first place | |
| 17:12:05 | efried | imacdonn: IMO we should fix (1) and change (2) to raise an explicit exception to "guarantee" it. | |
| 17:12:09 | efried | ++ | |
| 17:12:48 | efried | imacdonn: Is it possible for the rows to differ in any material way? | |
| 17:13:01 | efried | (i.e. a way that makes a difference to the outcome) | |
| 17:13:05 | mriedem | dansmith: not really, it's just the usual "how could this fail during an upgrade" | |
| 17:13:18 | mriedem | dansmith: he needs to pass new parameters to compute rpc api methods, | |
| 17:13:30 | dansmith | well, I read it and seemed like I needed to understand more, so I'm reading the wholething now | |
| 17:13:33 | mriedem | which could be (1) stein computes that don't handle those or (2) rpc pinned so we pop those parameters | |
| 17:13:36 | mriedem | ok | |
| 17:13:47 | imacdonn | efried, I'm assuming that _security_group_get_by_names() is used elsewhere (or at least intended to be reusable), so probably should consider all possible use-cases, if we tackle that one | |
| 17:14:45 | efried | used in two places | |
| 17:15:19 | mriedem | imacdonn: efried: honestly i'm not sure how much relevance that code even has anymore if you're using neutron | |
| 17:15:36 | imacdonn | mriedem, I was wondering about that | |
| 17:16:31 | mriedem | i think it at least means if you're using neutron, every project that ever created an instance in nova has a 'default' security_groups table record that is never used or cleaned up | |
| 17:16:33 | mriedem | vestigial | |
| 17:17:12 | imacdonn | that seems plausible | |
| 17:22:48 | dansmith | mriedem: see if my words help at all | |
| 17:26:47 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop handling cells v1 in '/os-hypervisors' API https://review.openstack.org/651292 | |
| 17:26:47 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove '/os-cells' REST APIs https://review.openstack.org/651291 | |
| 17:26:48 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'nova-manage cell' commands https://review.openstack.org/651294 | |
| 17:26:48 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop handling cells v1 in '/os-servers' API https://review.openstack.org/651293 | |
| 17:26:49 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove old-style cell v1 instance listing https://review.openstack.org/651296 | |
| 17:26:49 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop handling cells v1 for console authentication https://review.openstack.org/651295 | |
| 17:26:50 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'instance_fault_create_at_top' https://review.openstack.org/651298 | |
| 17:26:50 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'bdm_(update_or_create|destroy)_at_top' https://review.openstack.org/651297 | |
| 17:26:51 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'get_keypair_at_top' https://review.openstack.org/651300 | |
| 17:26:51 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'instance_info_cache_update_at_top' https://review.openstack.org/651299 | |
| 17:26:52 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'instance_update_from_api' https://review.openstack.org/651302 | |
| 17:26:52 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'instance_update_at_top', 'instance_destroy_at_top' https://review.openstack.org/651301 | |
| 17:26:53 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop handling cells v1 for instance naming https://review.openstack.org/651304 | |
| 17:26:53 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop handling 'update_cells' on 'BandwidthUsage.create' https://review.openstack.org/651303 | |
| 17:26:54 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop handling 'InstanceUnknownCell' exception https://review.openstack.org/651307 | |
| 17:26:54 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove cells code https://review.openstack.org/651306 | |
| 17:26:55 | openstackgerrit | Stephen Finucane proposed openstack/nova master: db: Remove cell APIs https://review.openstack.org/651309 | |
| 17:26:55 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove unnecessary wrapper https://review.openstack.org/651308 | |
| 17:27:08 | mriedem | dansmith: replied but yeah i think so | |
| 17:27:59 | dansmith | cool | |
| 17:28:42 | sean-k-mooney | mriedem: so related but sperate form gibis spec should we jsut use teh livemigration multiple port bindign workflow for all move operations | |
| 17:29:04 | sean-k-mooney | mriedem: i mentioned it in gibi's spec but it would simplfy things to have a singel common code path | |
| 17:29:53 | mriedem | given the issues NewBruce is hitting idk | |
| 17:30:36 | mriedem | moving all move operations to that model would be a big change i think, and likely not something we should block gibi's spec on | |
| 17:30:53 | sean-k-mooney | oh im not suggesting we should block on it | |
| 17:31:11 | sean-k-mooney | its jsut some/alot of the compleix would be reduced | |
| 17:31:37 | sean-k-mooney | but NewBruce issue is concering ill grant that | |
| 17:35:28 | francoisp_ | alex_xu, hi, would you have time to look at https://review.openstack.org/#/c/648123/6 - thanks! | |
| 18:05:11 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Do not create default security group during instance create if using Neutron https://review.openstack.org/653065 | |
| 18:05:12 | mriedem | imacdonn: let's see what blows up ^ | |
| 18:09:19 | imacdonn | mriedem: hmm .. wouldn't use_neutron be true in most cases, when the migration is being run ? | |
| 18:10:37 | mriedem | yes use_neutron is the default and what 99% of deployments are probably using at this point | |
| 18:11:10 | imacdonn | mriedem: I haz the dumb ... how would this solve the problem? | |
| 18:12:01 | mriedem | we don't hit the problem code if you're using neutron | |
| 18:12:17 | imacdonn | but .. I am using neutron, and I do hit the prpblem | |
| 18:12:25 | mriedem | with this patch | |
| 18:12:26 | mriedem | ? | |
| 18:12:48 | imacdonn | no, but the patch only makes a difference if you're not using neutron | |
| 18:12:51 | imacdonn | (?) | |
| 18:13:03 | mriedem | if not NEUTRON: create default sec group | |
| 18:13:14 | imacdonn | oh wait, I had it upside-down | |
| 18:14:35 | imacdonn | yeah OK ... I was about to propose making _security_group_ensure_default() return None if the context has no project_id (which does make the migration work for me) | |
| 18:16:31 | imacdonn | wonder if the migration hits https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L1740 | |
| 18:19:01 | imacdonn | mriedem: ^ I think it will | |
| 18:22:09 | imacdonn | mriedem: confirmed that I still hit the problem with your change, for above reason | |
| 18:24:50 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Do not create default security group during instance create if using Neutron https://review.openstack.org/653065 | |
| 18:24:52 | mriedem | try this ^ | |
| 18:28:18 | imacdonn | mriedem: that seems to work (at least allow the migration to work) | |