Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-10-04
16:17:00 bauzas I'd prefer providing a new filter that would do both image and flavor checks with the same behaviour, and in the meantime deprecate the old two filters
16:17:22 bauzas so we would be super clear that the behaviour is changing
16:17:40 bauzas keep those filters for a couple of releases, and then remove them from the tree
16:17:49 bauzas if people want to keep them out-of-tree, I'm fine
16:18:02 bauzas cfriesen: fancy proposing that ? :)
16:18:14 bauzas after all, it's just filters
16:19:21 cfriesen bauzas: Will propose it internally. What do you think of my proposal for https://review.openstack.org/#/c/381912/ to have the "strictness" of the isolation be scoped to individual keys?
16:19:23 mriedem i thought that's what the new psec was
16:19:34 mriedem yeah that one
16:20:07 cfriesen mriedem: they're just proposing adding a new boolean flag on either the image or flavor to say the matching is strict....but that doesn't factor in that the behaviour of the two filters is different
16:20:20 dansmith edleafe: are you revising your alt hosts thing?
16:20:28 cfriesen mriedem: and I think it'd make more sense to allow strict matching on a per-key basis, specified in the aggregate metadata
16:20:39 edleafe dansmith: which alt hosts thing?
16:20:48 dansmith edleafe: https://review.openstack.org/#/c/486215/12
16:21:34 edleafe dansmith: yeah, I'm working through the whole series to incorporate the changes to the Selection object based on the spec changes
16:21:50 dansmith edleafe: okay cool
16:23:04 bauzas cfriesen: well, I think I said "meh" in my comment
16:23:21 bauzas cfriesen: so, basically, I'm not opiniated
16:23:36 bauzas cfriesen: I tend to avoid having filter behaviours driven by keys
16:23:48 bauzas but looks like we need to be pragmatic
16:24:27 bauzas another option could be a config option, but that would be worst I think for interop
16:24:48 bauzas because two clouds would behave differently
16:25:09 bauzas cfriesen: so, honestly, maybe a key is okay
16:25:20 bauzas I dunno, I need more time to think about that
16:25:33 cfriesen bauzas: I was thinking that we might want to be strictly isolationist for some keys but not for others (as opposed to the all-or-nothing that the current spec proposes)
16:27:36 bauzas when you say "isolationist for a specific *key*", you mean either an aggregate extraspec key for matching the flavor, or an image property?
16:27:40 bauzas cfriesen: ^
16:28:44 bauzas cfriesen: I just wonder how you would express that in the aggregate metadata
16:28:55 bauzas because of the k=v pair
16:29:12 cfriesen bauzas: I was thinking that the aggregate key could be something like '{"strict:os": "windows"}', in which case only instances with image property or flavor extra-spec of "os:windows" would match
16:29:25 cfriesen basically just a "strict:" namespace on the aggregate key
16:29:27 bauzas namespacing ? I don't like that
16:29:35 bauzas we already namespace keys AFAIKK
16:30:06 cfriesen we namespace them on the flavor/image, but not on the aggregate currently I think
16:30:09 bauzas nevermind, we namespace the image properties or the flavor specs
16:30:14 bauzas yeah that
16:30:41 bauzas cfriesen: the problem is that if you do that, you change the API
16:30:42 cfriesen bauzas: the nice thing about that is that it works with existing flavors/images
16:31:01 cfriesen bauzas: we're talking about a new filter anyway
16:31:03 bauzas cfriesen: say I already have aggregates that are tagged and filters
16:31:20 bauzas cfriesen: how can that work in an upgrade way?
16:31:39 bauzas you would default no namespace to the current behaviour ?
16:31:43 cfriesen bauzas: yes
16:31:53 bauzas cfriesen: honesty, I don't like that
16:31:55 cfriesen in the existing filters
16:32:02 bauzas you can create as many aggregates as you want
16:32:10 cfriesen in the new filter we'd want consistent behaviour for both image/flavor
16:32:11 bauzas and a host can be part of 1:N aggs
16:32:51 bauzas so, if you need strict isolation for only a couple of keys, why just not define two aggregates, one containing keys with no strictness, and the other with keys needing to be strict ?
16:32:53 cfriesen bauzas: with strict matching you'd need the flavor/image to match all the strict keys from all the aggregates the host is in
16:33:47 cfriesen bauzas: you're thinking a "strict-match" boolean flag on the aggregate? yeah, that could work.
16:34:00 bauzas I'm just talking of the current proposal
16:34:18 bauzas he proposes to add new keys that are global per-aggregate
16:35:17 bauzas cfriesen: in that case, if you need some keys with strict isolation, and some with not, just define two aggregates and only apply the new metadata tag image_strict_isolation=True to the aggregate containing the keys you want to be strict
16:36:34 mriedem dansmith: looking at https://review.openstack.org/#/c/498950/ - it occurs to me that if prep_resize fails, i don't think we ever set the migration status to 'failed'
16:36:40 mriedem dansmith: which i think is just a latent bug
16:37:26 mriedem if resize_instance fails it will, but we might not get that far
16:38:34 cfriesen bauzas: I guess. Although AggregateInstanceExtraSpecsFilter doesn't do isolation currently, and AggregateImagePropertiesIsolation doesn't ensure that what you specify in the image is present in the aggregate.
16:38:52 mriedem dansmith: oh i know why - because we never had the migration before that point, because the RT always created it
16:39:13 cfriesen bauzas: hence the rationale for a new filter with common behaviour
16:39:25 dansmith mriedem: right, because _prep_resize's with resize_claim would do that right?
16:39:33 mriedem yeah
16:39:39 mriedem but now you're passing down a migration record and not handling errors
16:40:00 dansmith yup
16:40:24 openstackgerrit Rodolfo Alonso Hernandez proposed openstack/os-vif master: Add support for Windows network commands https://review.openstack.org/487405
16:42:07 dansmith the bottom one if this series was just kicked out of the gate anyway, so I'll rebase and freshen since we're like 8 hours from merge anyway
16:46:21 bauzas cfriesen: I don't disagree
16:46:36 mriedem dansmith: still going through this if you want to hold up
16:46:41 mriedem into the conductor stuff now
16:46:46 cfriesen bauzas: will comment on the review
16:46:50 dansmith mriedem: sure
16:48:33 cfriesen bauzas: it occurs to me that combining flavor extra-specs and image properties is tricky...some of that logic is way down in the virt code.
16:53:36 mriedem dansmith: ok done :)
16:54:09 dansmith mriedem: I can't wait to see what gifts you have left for me
16:54:16 mriedem they are bountiful
16:54:27 mriedem i'm going to go pat myself on the back with lunch
16:56:32 mriedem jaypipes: cdent: fyi https://review.openstack.org/#/c/498950/
16:57:25 cdent mriedem: mlph
16:57:37 cdent this shit is too confusing
17:10:08 mriedem cdent: yeah, there are like 10 things that happen outside the scenes of everything...
17:18:37 efried jaypipes ( cdent ) I finished reviewing the series starting at https://review.openstack.org/#/c/470575/ -- is there anything else to look at for NRP or related at the moment?
17:18:46 openstackgerrit Rodolfo Alonso Hernandez proposed openstack/nova-specs master: Network bandwitdh resource provider https://review.openstack.org/502306
17:19:06 cdent efried: have you seen jay’s orm removal stack?
17:19:12 cdent it’s tangentially related
17:19:16 efried What's an orm/
17:19:17 efried ?
17:19:23 efried (But I guess not)
17:19:44 cdent https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:no-orm-resource-providers
17:21:16 efried cdent rgr
17:22:30 cdent efried: you’re already aware of alex’s traits work I think?
17:22:48 efried cdent Ah, sort of, but need to plug into it. Got a starter patch?
17:23:36 cdent it’s currently merge conflict, but: https://review.openstack.org/#/c/489206/7
17:23:59 efried beaut
17:24:49 efried Amago eat, will dig in after.
17:28:30 cfriesen question for someone with more sqlalchemy-foo than I....will the soft_delete() call at https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L5932 affect both the "aggregate" table and the "aggregate_hosts" table? Or just the "aggregate" table?
17:35:52 openstackgerrit priyaduggirala proposed openstack/nova master: Rename parameters in call() of nova/image/glance.py https://review.openstack.org/508533
17:40:02 openstackgerrit Merged openstack/nova master: Read from console ptys using privsep. https://review.openstack.org/489486
17:40:47 openstackgerrit Merged openstack/nova stable/ocata: Fix 500 if list servers called with empty regex pattern https://review.openstack.org/506760
17:42:11 openstackgerrit Dan Smith proposed openstack/nova master: Make allocation cleanup honor new by-migration rules https://review.openstack.org/498948
17:42:11 openstackgerrit Dan Smith proposed openstack/nova master: Pre-create migration object https://review.openstack.org/498950

Earlier   Later