Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-02-10
14:50:46 openstackgerrit Merged openstack/nova-specs master: Re-propose "Secure Boot support for KVM & QEMU guests" for Ussuri https://review.opendev.org/693844
14:53:16 gmann elod: thanks
14:54:10 lyarwood gmann: just waiting for CI before I ack it btw
14:54:31 openstackgerrit Balazs Gibizer proposed openstack/nova master: Reproduce bug 1862633 https://review.opendev.org/706867
14:54:31 openstack bug 1862633 in OpenStack Compute (nova) "unshelve leak allocation if update port fails" [Medium,Triaged] https://launchpad.net/bugs/1862633 - Assigned to Balazs Gibizer (balazs-gibizer)
14:54:31 openstackgerrit Balazs Gibizer proposed openstack/nova master: Clean up allocation if unshelve fails due to neutron https://review.opendev.org/706868
14:56:40 gmann lyarwood: ok, thanks. i did not backport to ocata but i can see open backport for nova ocata which will have same issue. should I backport this fix there too ?
14:57:13 lyarwood gmann: if it's an easy cherry pick sure
14:57:23 gmann lyarwood: ok
15:03:00 openstackgerrit Ghanshyam Mann proposed openstack/nova stable/queens: Use stable constraint for Tempest pinned stable branches https://review.opendev.org/706714
15:04:39 openstackgerrit Ghanshyam Mann proposed openstack/nova stable/pike: Use stable constraint for Tempest pinned stable branches https://review.opendev.org/706715
15:05:37 openstackgerrit Ghanshyam Mann proposed openstack/nova stable/ocata: Use stable constraint for Tempest pinned stable branches https://review.opendev.org/706872
15:06:13 gmann lyarwood: done ^^. updated with cherry-pick -x
15:07:38 Sundar gibi: Re. https://review.opendev.org/#/c/631244/61/nova/tests/functional/test_servers.py@7621, I have a question. Please LMK when you have a few min.
15:08:50 gibi Sundar: hi! I'm available now
15:09:02 dansmith efried: I'm thinking we should do a release of train now that the hidden instances fix is in, given its criticality
15:12:32 efried dansmith: fine by me. You proposing?
15:12:57 dansmith efried: I can yea, I was just looking to see when we last did it
15:13:40 Sundar gibi: The Cyborg fixture itself is a mock, and is returning pre-fabricated data. Any queries to it will only return the prefabricated data. Specifically, fake_get_arqs_for_instance will return a single bound ARQ in the current implementation, and hence the first 2 assertions will always be true.
15:13:57 Sundar Did you have something else in mind?
15:14:43 gibi Sundar: is this mean that there is no state stored in the fixture that is changed by nova during the boot?
15:15:34 efried Sundar: Re: blocking unsupported operations: If that's the only objection, I feel like we could get around it by making the blockers error 500 rather than 400. We're allowed to "fix a 500" without a microversion if I understand the rules correctly.
15:15:56 efried But if that's not the case, meh. I've backed down from this argument before, won't make a big deal of it now.
15:16:13 Sundar gibi: The only two variables that are from the test case are the host name and device_rp_uuid. I could assert for those.
15:16:28 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add test coverage of existing attach_interfaces policies https://review.opendev.org/705126
15:16:55 gibi Sundar: yes, those are the thing that is stored in the fixture in the bindings_by_instance
15:17:10 gibi Sundar: asserting only for device_rp_uuid and hostname works for me
15:17:10 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Introduce scope_types in os-attach-interfaces https://review.opendev.org/705799
15:17:11 Sundar efried: Good. sean-k-mooney, dansmith, gibi: Are we all good if we block the unsupported ops with HTTP 500 as efried said?
15:17:22 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add new default roles in os-atttach-inerfaces policies https://review.opendev.org/706672
15:17:38 dansmith Sundar: sorry, I'm working on something else, but 500 does not seem appropriate to me
15:17:42 efried rosmaita: Looking.
15:18:02 Sundar gibi: Sure, thanks.
15:18:08 rosmaita efried: ty
15:18:31 dansmith isn't 401/403 the right thing here? tell the user they're not allowed, which could be for any reason which may change in the future (when we allow it or implement it)
15:18:43 efried dansmith: my reasoning is, if you try it before we've coded it up, you're going to get a 500 anyway; it'll just be with some really obscure and hard-to-understand error message. We're just making the 500 understandable as a courtesy before we actually add the support.
15:18:46 gibi efried, Sundar: for qos we used HTTP 400 for rejecting unsupported moves, and we fixed those 400 without new microversion
15:19:08 Sundar dansmith: If it is 400, do we need a microversion change? We are not changing anything, just clarifying what happens with this new feature i.e. accelerator support.
15:19:10 efried okay, I thought that was the objection, that you can't put in explicit blockers and then unblock without a microversion.
15:20:03 dansmith efried: to me 500 means either server-side code needs fixing, or some infra failure in the backend. and converting 500s to 400s is allowed without a microversion because they're all bugfixes
15:20:07 efried I agree strictly it would be best to add support with new microversions, since that's the only way the feature would be discoverable. That makes sense. So... why are we opposed to that strategy in the first place? Just because microversions are a bunch of paperwork?
15:20:51 dansmith making them all 40x now with no microversion is fine with me
15:21:19 dansmith I think the microversion purists would expect a 400->200 to be a microversion because otherwise people can't know whether or not they should try a thing
15:21:33 efried exactly
15:21:43 Sundar dansmith, efried: Agreed. We could do it now without a microversion change. Unblocking in the future will need a microversion change, since it is a change in semantics.
15:22:09 dansmith I'd much rather do the paperwork than cheat with 500
15:22:15 efried agreed
15:22:35 gibi I'm OK to have 400 -> 200 with microversion, I just remember that I was asked not to do that for qos
15:22:37 efried even leaving it "unsupported" in some way now and then "fixing" without a microversion seems like cheating.
15:23:00 efried gibi: do you remember why?
15:23:06 gibi trying to find it...
15:23:15 dansmith to me,
15:23:38 dansmith a thing that doesn't work because of some subtle detail returning "you can't do that right now" and then later returning "okay now you can" is not a huge violation
15:23:52 dansmith it's an operation that you can do normally, but can't for some policy reason
15:24:07 dansmith so I've never really had a problem with enabling a thing to work by implementing a detail,
15:24:20 dansmith because I think a lot of client code that does this is ignorant of the fact that makes the instance special
15:25:19 gibi efried: http://lists.openstack.org/pipermail/openstack-discuss/2019-January/001881.html
15:26:03 dansmith yup, that ^ :)
15:26:38 gibi mriedem was OK with that too http://lists.openstack.org/pipermail/openstack-discuss/2019-January/001887.html
15:28:44 efried gibi, Sundar: ack, if we decided on this and set a precedent with the qos feature, so be it. (I feel like the API-SIG might have, ahem, kept the discussion alive a bit longer, had they been involved.)
15:29:09 efried rosmaita, lyarwood: I'm going to need a little help here https://review.opendev.org/#/c/706298/
15:29:35 rosmaita efried: i'm all yours
15:30:11 efried Changing a conf opt default doesn't seem a) wise, b) effective, especially if you were planning to backport this (were you?)
15:30:54 efried I also need to understand a bit better which operations are supported/unsupported today and how they break.
15:31:30 rosmaita yes, was trying to backport
15:31:45 rosmaita but to answer your second question
15:31:51 efried The patch says we don't support "direct booting" of an instance created from encrypted volume. Do we support *anything* from such an image?
15:32:06 rosmaita yes, if you boot from volume
15:32:16 efried like, does that code path exist for backup/restore or shelve/unshelve?
15:32:19 lyarwood efried: nope, we've never supported booting from an encrypted image with cinder_encryption_key_* set in any of the in-tree virt drivers.
15:32:41 lyarwood efried: these are encrypted images created by cinder, so outside of Nova's normal flows with encrypted volumes.
15:33:05 lyarwood efried: shelve/unshelve shouldn't create images for boot from volume instances
15:33:13 efried right right.
15:33:39 dansmith notice how he says "shouldn't" ?
15:33:40 Sundar dansmith, efried: The 400s are supposed to be client error. Is this really not an unsupported operation on the server side? Or, are we taking the line that the client should have known about the restriction, and not made the request in the first place, and so it is a client error?
15:34:23 dansmith Sundar: but "permission denied" is a 40x error.. it doesn't mean the client did something wrong, it means the client shouldn't try that thing again without circumstances having changed
15:34:49 dansmith doesn't "always" mean.. I should say
15:35:14 efried lyarwood, rosmaita: And the objection to blocking this at the API level is that we don't want to rip function from 3p drivers that might have figured out a way to support it?
15:35:40 efried lyarwood, rosmaita: are we talking about 3p nova virt drivers or 3p cinder storage drivers? Or would it have to be a combination of both for it to work?
15:36:06 rosmaita efried: i think we probably should block at api layer, it's just that we don't
15:36:35 efried I'm about to agree with that, just want to confirm ---^
15:36:35 rosmaita at least short term, if you really want to implement this functionality
15:36:50 lyarwood efried: 3p nova virt drivers
15:37:01 Sundar dansmith: I am fine with that interpretation. This is what I was doing in https://review.opendev.org/#/c/674726/. So I am going to bring back that patch with some changes in the list of supported ops.
15:37:31 rosmaita efried: the config opt change is a quick short term fix that won't require operators to do an upgrade to address this
15:37:54 lyarwood rosmaita: I still don't get the usecase tbh
15:38:10 lyarwood rosmaita: they boot something that doesn't work and then snapshot it?
15:38:28 lyarwood rosmaita: but yeah this is a quick and easy fix to avoid someone doing something like that
15:38:29 rosmaita lyarwood: hopefully it is low probability
15:38:50 rosmaita but i could see someone doing a script that boots, and snapshots immediately for some reason
15:39:08 rosmaita and then when a useless image is deleted, the problem happens
15:39:57 rosmaita efried: if a config value change backport isn't allowed, maybe we could just backport the "known issues" part of the release note
15:40:02 lyarwood rosmaita: anything is possible I guess
15:40:20 efried okay, so putting my dansmith hat on (it's red, for multiple reasons), I don't think we worry about accommodating 3p virt drivers in situations like this. I usually insist we send a courtesy email to openstack-discuss when we make interface changes that could break 3p drivers; but that's about all we do.
15:40:57 lyarwood okay well in that case lets block it in the API fully and backport that
15:41:16 efried rosmaita: I don't know for sure that backporting config default changes is disallowed, but it sounds like something that would be.
15:41:25 efried but also, I just don't see it doing much good.
15:41:51 efried because you're only going to "help" people who didn't already have the value set, and who were trying to do this craziness in the first place.
15:42:02 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Fix os-create-backup policy to be admin_or_owner https://review.opendev.org/706727
15:42:33 rosmaita i guess i can just send something to the operators list

Earlier   Later