| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-01-09 | |||
| 18:27:44 | efried | sean-k-mooney: trying to tackle the "Multiple networks found" CI errors that are making 80% of our CI runs fail. | |
| 18:27:45 | sean-k-mooney | ok in some cases where you are using tempest without floating ip we still need to be able to specify a network for the tempset clietn to be abel to ssh in on | |
| 18:27:52 | sean-k-mooney | i assume that will still work | |
| 18:28:16 | ralonsoh | sean-k-mooney, yeah, from the neutron channel | |
| 18:28:17 | ralonsoh | http://paste.openstack.org/show/788211/ | |
| 18:28:18 | efried | I think that's what we're trying to find out | |
| 18:28:31 | efried | possibly "can be made to work" | |
| 18:30:06 | sean-k-mooney | well its more we support using tempest on real clouds for interop testing in that case we suppro specify a public network where you vm can directly create ports instead of using fip | |
| 18:30:30 | sean-k-mooney | im jsut making sure that if we create network for the compute test we can still support that when we need too | |
| 18:30:46 | sean-k-mooney | i can move to the neuton channel by the way if that makes more sense | |
| 18:31:50 | sean-k-mooney | i dont in prinsipal see an issue with createing a network for each trest although i know that some clould like rackspace used to have neutron but not allow tenant to call the api | |
| 18:35:07 | efried | See, I don't understand any of that, and don't especially want to take the time to learn it. I just want to make our CI not fail for reasons unrelated to the code being tested. | |
| 18:35:21 | sean-k-mooney | https://github.com/openstack/tempest/blob/97052fae121174b9d6daa51684c03f730dc74683/tempest/config.py#L303-L309 | |
| 18:35:32 | sean-k-mooney | im refering to this config option by the way | |
| 18:36:21 | efried | dansmith: added a comment in https://review.opendev.org/#/c/701795/ | |
| 18:36:21 | efried | I like where this is going, thank you. | |
| 18:36:26 | sean-k-mooney | and there is also a seperate config for ssh verification | |
| 18:36:28 | sean-k-mooney | https://github.com/openstack/tempest/blob/97052fae121174b9d6daa51684c03f730dc74683/tempest/config.py#L831-L834 | |
| 18:36:32 | dansmith | efried: yep, replying | |
| 18:37:51 | efried | sean-k-mooney: I did notice that we're doing this explicit network setup in a different method, prepare_instance_network, based on some other conf options. | |
| 18:38:38 | efried | but understanding what those conf options mean, and why certain tests would want to do that but others wouldn't... again, deeper than I want to go. | |
| 18:39:57 | sean-k-mooney | well to be clear i have not fully groked what ralonsoh is change yet | |
| 18:40:23 | ralonsoh | groked? | |
| 18:40:26 | sean-k-mooney | i just know that there are some case where always createing neutron network might be an issue | |
| 18:40:45 | sean-k-mooney | ralonsoh: formed a complete understanding of | |
| 18:40:58 | sean-k-mooney | https://en.wikipedia.org/wiki/Grok | |
| 18:42:19 | ralonsoh | sean-k-mooney, maybe we can do the opposite: in those cases where the network creation is an issue, we can disable it | |
| 18:43:37 | sean-k-mooney | well the idea fo the fixed network was so you could execute test in an enve where the network were precreated. in that case you wont have the multiple network isue anyway | |
| 18:43:56 | sean-k-mooney | so if that config is defiend you would be able to jsut not create teh network and use the one set in the config | |
| 18:44:14 | ralonsoh | agree | |
| 18:45:40 | sean-k-mooney | for the ssh verificaiton im also wondering if you need to create a router too? instead of an isolated network? | |
| 19:00:03 | sean-k-mooney | efried: ralonsoh i left a comment on the review. i think in general this would make sense just would need someone more familar to check this wont break things | |
| 19:01:10 | sean-k-mooney | another example of when you need to use fixed netowrkisn is usign tempest with ironic and flat netorkign. so its not just for rackspace. | |
| 19:01:43 | sean-k-mooney | that said you can deploy ironic with neutron netorks too but i still dont know how common that is | |
| 19:16:07 | dustinc | dansmith: Is there a chance that you can check this out and maybe wf+1? It is just a small update to the Provider Config File spec. https://review.opendev.org/#/c/693414/ | |
| 20:14:37 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: Add recreate test for bug 1855927 https://review.opendev.org/701756 | |
| 20:14:37 | openstack | bug 1855927 in OpenStack Compute (nova) "_poll_unconfirmed_resizes may not retry later if confirm_resize fails in API" [Low,In progress] https://launchpad.net/bugs/1855927 - Assigned to Matt Riedemann (mriedem) | |
| 20:14:37 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/train: Ensure source service is up before resizing/migrating https://review.opendev.org/701757 | |
| 20:41:29 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Add new default roles in os-agents policies https://review.opendev.org/701648 | |
| 20:42:53 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Pass the actual target in os-agents policy https://review.opendev.org/701649 | |
| 20:51:18 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Add test coverage of existing os-assisted_volume_snapshots policies https://review.opendev.org/701835 | |
| 20:58:52 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Introduce scope_types in os-assisted_volume_snapshots policy https://review.opendev.org/701837 | |
| 21:02:47 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Add new default roles in os-assisted_volume_snapshots policies https://review.opendev.org/701840 | |
| 21:08:38 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Pass the actual target in os-assisted_volume_snapshots policy https://review.opendev.org/701841 | |
| 21:40:02 | dansmith | efried: still around? | |
| 21:40:07 | efried | yes | |
| 21:41:01 | dansmith | efried: I think you're missing my point on the defaulting stuff, and mashing together the default_fn thing with the object mix-in | |
| 21:41:24 | dansmith | they're not really related, as you'll note the mixin has nothing to do with the default_fn thing | |
| 21:41:51 | efried | yeah, I get that they could be separate. I'm just not sure there's a reason to do that. | |
| 21:41:54 | dansmith | the latter is just syntactic sugar in most cases, aside from the fact that the latest patch avoids copying everything all the time | |
| 21:42:09 | dansmith | they're wholly separate | |
| 21:42:29 | dansmith | default_fn has nothing to do with whether or not it's applied implicitly at init time | |
| 21:42:39 | efried | I understand that | |
| 21:43:00 | dansmith | okay | |
| 21:43:13 | efried | but still think there's value in smashing them together. | |
| 21:43:37 | dansmith | do you understand that I don't want to mix default-on-init into objects that are representative of row-based data structures on the other side of an rpc pipe? | |
| 21:43:55 | efried | Yes. | |
| 21:44:07 | efried | um, I think I do | |
| 21:44:12 | efried | I don't think I'm suggesting otherwise. | |
| 21:44:20 | dansmith | your latest reply seems to disagree | |
| 21:44:55 | efried | as long as we're talking about object as opposed to class | |
| 21:45:47 | efried | I don't think a class def of a field needs to be tied to every case where an instance of that class is used in a parent object | |
| 21:46:23 | efried | like with nullable, where we're saying a particular instance of that class is tied to a db column that's either nullable or not. | |
| 21:46:47 | efried | (I get that that's not all it says, but that was its impetus) | |
| 21:47:02 | dansmith | um | |
| 21:47:30 | dansmith | maybe you're focusing on objects that are included as fields on another object specifically? | |
| 21:47:41 | efried | yes, that's exactly what I'm doing. | |
| 21:48:16 | dansmith | hang on, ups | |
| 21:49:30 | dansmith | so, I the inner object getting the behavior we want is cool, but not if it breaks the outer objects or the whole system | |
| 21:49:58 | dansmith | and making a single field default-on-init is what I don't want to do | |
| 21:50:21 | dansmith | which is why keying the default-on-init on the default_fn=<callable> isn't okay, IMHO | |
| 21:51:56 | efried | I thought "making a single field default-on-init" is exactly what we *did* want to do. It indicates that we know that field isn't backed by a db column, among other things, and we want the convenience of being able to count on it being set without having to default it explicitly or have it autovivify. | |
| 21:53:17 | dansmith | no, it's precisely what I don't want | |
| 21:54:02 | dansmith | I just finished reading your reply and replied, that we definitely don't need a subclass-per-type like you surmised | |
| 21:54:11 | efried | I'm playing with it now, and I think it's helping me understand the distinction you're making. | |
| 21:54:19 | efried | gimme a couple minutes... | |
| 21:56:44 | dansmith | right, so RequestSpec could become fully ephemeral IMHO | |
| 21:57:27 | efried | okay, so really what the bottom patch does is make __init__(): obj_set_defaults() sanctioned behavior | |
| 21:57:35 | dansmith | which means RequestSpec(NovaObject) becomes RequestSpec(EphemeralObject), and your default request_level_params gets applied at start | |
| 21:57:38 | dansmith | yes | |
| 21:57:51 | dansmith | categorically if inheriting from the mixin, that's the point | |
| 21:58:19 | dansmith | "this object is one of those that doesn't represent database columns and therefore can behave defaultly" | |
| 21:58:33 | dansmith | basically, anything that does not have NovaPersistentObject in its MRO would be okay to make ephemeral I think | |
| 21:58:48 | efried | ...and RequestLevelParams itself would also inherit from EphemeralObject, so that its root_required/root_forbidden fields woul default at that same time? | |
| 21:58:55 | dansmith | that ^ is our mixin for hard database affinity, although we couldn't be quite that sweeping | |
| 21:59:00 | dansmith | yes | |
| 22:01:22 | efried | We could do this just as easily in nova; but we want to do it in ovo to kind of send a message about preferred usage?? | |
| 22:02:10 | dansmith | I guess we could do the init defaulting part yeah | |
| 22:02:18 | efried | Could do it in nova as a stopgap until we can suck in the ovo release | |
| 22:02:41 | dansmith | yup | |
| 22:02:48 | efried | and I can see making an infrastructure that poisons use of obj_set_defaults in __init__ unless inheriting from Ephemeral | |
| 22:03:05 | efried | ...to enforce the paradigm | |
| 22:03:07 | dansmith | I thought it was going to involve a little more mucking with obj_set_defaults() than it did, but I was also planning both of these together | |
| 22:03:35 | dansmith | efried: we can test for it in test_objects I think | |
| 22:03:44 | dansmith | efried: instantiate every object (like we do) and make sure that unless it inherits from that mixin, nothing is set after init | |
| 22:04:41 | efried | well, that doesn't prove obj_set_defaults wasn't invoked. I was thinking about making obj_set_defaults set a flag or something, then we can check for that flag in the test | |
| 22:05:05 | dansmith | well, we have (or had)objects in the past that didn't use set_defaults, but just set them explicitly in __init_ | |
| 22:05:20 | efried | Is that also a bad thing? | |
| 22:05:21 | dansmith | just "self.foo = None" or something in __init__ | |
| 22:05:32 | efried | I would think that would be okay. | |