| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-02-12 | |||
| 15:59:22 | stephenfin | Looking | |
| 16:00:39 | sean-k-mooney | i just responed to alex_xu coment regarding the service object version bump. i think we can use a comptue capablity trait instead and avoid the need for that | |
| 16:00:52 | sean-k-mooney | but excpetion for that point im good with it | |
| 16:16:41 | openstackgerrit | Merged openstack/nova stable/queens: Use stable constraint for Tempest pinned stable branches https://review.opendev.org/706714 | |
| 16:21:14 | openstackgerrit | Lee Yarwood proposed openstack/nova stable/queens: Block deleting compute services with in-progress migrations https://review.opendev.org/699718 | |
| 16:33:19 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Skip to run all integration jobs for policy-only changes. https://review.opendev.org/707268 | |
| 16:33:29 | gmann | stephenfin: melwitt updated ^^ | |
| 16:34:26 | melwitt | gmann: do you have a Depends-On change that shows what gets run on a policy change? I think that's the best way to check whether it's doing what we want before +2ing | |
| 16:34:48 | gmann | melwitt: ok, i can do that. | |
| 16:35:52 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Add new default roles in os-console-output policies https://review.opendev.org/707041 | |
| 16:36:17 | gmann | melwitt: ^^ done. let's see once gate finish | |
| 16:36:48 | melwitt | thanks gmann | |
| 16:37:09 | melwitt | gmann: oh, you have a typo Depednds-On | |
| 16:37:46 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Add new default roles in os-console-output policies https://review.opendev.org/707041 | |
| 16:37:57 | gmann | ah sorry, updated | |
| 16:40:35 | Sundar | sean-k-mooney: Do you think https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L655 is a good place to delete the old ARQs? | |
| 16:42:56 | gmann | melwitt: I am not sure if default GET AZ to admin is right things. can you check this - https://review.opendev.org/#/c/706683/ | |
| 16:43:35 | melwitt | gmann: ok, will look | |
| 16:43:39 | gmann | It is currently admin-or-owner which is open for everyone due to bug or that is expected and admin-or-owner in policy is just misleading | |
| 16:43:42 | gmann | melwitt: thanks | |
| 16:48:09 | melwitt | gmann: I think it should be admin_or_owner, is it possible to just fix the bug? AZ is user-facing but usually only project members would be able to see which AZ in a instance is in (not everybody from other projects) | |
| 17:08:24 | sean-k-mooney | Sundar: looking | |
| 17:08:25 | gmann | melwitt: but issue is we do not know the owner while GET AZ. | |
| 17:08:54 | melwitt | oh ... /me looks again | |
| 17:08:59 | gmann | melwitt: this is list and detail API - https://github.com/openstack/nova/blob/1fcd74730d343b7cee12a0a50ea537dc4ff87f65/nova/api/openstack/compute/availability_zone.py#L108 | |
| 17:09:43 | sean-k-mooney | am we could. althoh i belive we will do a cleaup of the failed spawn before that so you might be able to do it earlier. | |
| 17:10:01 | sean-k-mooney | that is proably the latest i would delete and recreate the ARQ | |
| 17:10:52 | sean-k-mooney | Sundar: you could also do it here https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L694 | |
| 17:11:03 | sean-k-mooney | which i think is where i would expect it to happen | |
| 17:11:50 | melwitt | gmann: ohhhh this is just giving a list of all the AZs that exist, not the AZ of any particular instance. yeah, sorry, let me comment again | |
| 17:11:56 | gmann | melwitt: "which AZ is instance is in" is admin-or-owner which is via GET /servers API | |
| 17:12:01 | gmann | melwitt: yeah | |
| 17:12:04 | Sundar | sean-k-mooney: Ok, got it. Thanks. | |
| 17:12:55 | sean-k-mooney | im not 100% if you want to be in our out side of the if but you likely should be doing the clean up in that exeption handeling code https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L674-L699 | |
| 17:14:36 | melwitt | gmann: yeah, I agree it should be allowing everyone to see AZ list by default. I added a comment | |
| 17:15:34 | gmann | melwitt: ok. detail make sense to be admin-only as it has host info | |
| 17:15:44 | melwitt | gmann: right, agreed | |
| 17:16:03 | gmann | thanks. I will modify accordingly | |
| 17:29:49 | stephenfin | sean-k-mooney: You can only specify the number of instance CPUs via the flavor, right? | |
| 17:29:53 | stephenfin | i.e. not via the image | |
| 17:30:12 | sean-k-mooney | yes | |
| 17:32:46 | sean-k-mooney | technically you could do evial things with a cyborg device profile but if you put VCPU or PCPU int eh device profile your on your own | |
| 17:35:05 | sean-k-mooney | *evil | |
| 17:38:59 | sean-k-mooney | lyarwood: by the way did you want me to do anything with the backprots for the numa inplace rebuild patches | |
| 17:39:31 | sean-k-mooney | lyarwood: you pinged my earlier but i was not sure if you already fixt them up or if there was still stuff pending | |
| 17:39:53 | stephenfin | huaqiang: Reviewed https://review.opendev.org/#/c/668656/. You need to drop the image metadata option and avoid the need to mix placement'y syntax extra specs with legacy extra specs. Do that and I'm +2 | |
| 17:39:56 | stephenfin | sean-k-mooney, efried, alex_xu: ^ | |
| 17:40:24 | sean-k-mooney | what do you think about the need for the service bump | |
| 17:40:45 | stephenfin | vs. a capability? | |
| 17:40:45 | sean-k-mooney | e.g. do you think we could use a compute capablity trait isntead | |
| 17:40:49 | sean-k-mooney | ya | |
| 17:40:59 | sean-k-mooney | i would be fine with leaving that to impleenation to decied | |
| 17:41:04 | sean-k-mooney | but i think the trait would work | |
| 17:41:43 | sean-k-mooney | there may be another reason for the service version bump that i missed | |
| 17:41:55 | stephenfin | I used a capability for the PCPU work | |
| 17:42:01 | stephenfin | so yeah, I'd probably do the same again | |
| 17:42:35 | stephenfin | commit f329ee5a3b6 | |
| 17:42:43 | sean-k-mooney | using the capablity trait means we can support rolling upgrades too rather then requiring all nodes to be upgraded | |
| 17:43:10 | sean-k-mooney | just the contoler and the host it lands on will need to be ussuri | |
| 17:44:59 | stephenfin | Exactly | |
| 17:45:22 | sean-k-mooney | there is a usecase for keeping the image property by the way | |
| 17:45:32 | sean-k-mooney | we support setting the realtime mask in the image | |
| 17:45:44 | stephenfin | We do, but I think that's wrong | |
| 17:45:45 | sean-k-mooney | so for parity we could support the pinned cpus there too | |
| 17:45:49 | sean-k-mooney | why | |
| 17:45:54 | stephenfin | For the same reasons | |
| 17:46:07 | stephenfin | You boot an N core instance | |
| 17:46:11 | sean-k-mooney | the idea is the vnf vendor will package teh image and expres the requirements in the image properties | |
| 17:46:21 | stephenfin | Right, but they can't express the number of CPUs | |
| 17:46:29 | stephenfin | which is probably also required | |
| 17:46:43 | sean-k-mooney | well the can express a min numa of cpus in the image | |
| 17:47:06 | stephenfin | Via the topology stuff? | |
| 17:47:08 | sean-k-mooney | the same way they can express a min amount of ram or disk | |
| 17:47:10 | stephenfin | That's a bit of hack | |
| 17:47:17 | sean-k-mooney | no | |
| 17:47:23 | sean-k-mooney | one sec | |
| 17:47:27 | sean-k-mooney | medtadef time | |
| 17:47:55 | sean-k-mooney | actully im not sure this is even a metadef it migh be directly on the image | |
| 17:48:30 | stephenfin | Hmm, we do also have 'hw_numa_cpus' there | |
| 17:48:41 | stephenfin | That's wrong too, IMO :( | |
| 17:49:00 | stephenfin | I wonder if we should do this just to be compatible even if it's not a great design though | |
| 17:49:54 | stephenfin | We have a 'img_bittorrent' field on the 'ImageMetaProps' object. Good Lord | |
| 17:50:43 | sean-k-mooney | oh looks like it only has min disk and min ram | |
| 17:50:45 | sean-k-mooney | https://docs.openstack.org/python-openstackclient/pike/cli/command-objects/image.html#image-create | |
| 17:50:59 | sean-k-mooney | stephenfin: its by design | |
| 17:51:12 | sean-k-mooney | again the intent was to allow the image to descibr the toplogy | |
| 17:51:20 | sean-k-mooney | and the flavor to describe the quantity | |
| 17:51:45 | stephenfin | sean-k-mooney: But remember, that's a not a simple list | |
| 17:51:58 | stephenfin | or actually, maybe it is for the image metadata | |
| 17:52:45 | stephenfin | what happens if you create an N node instance that lists M CPUs in the 'hw_numa_cpus' image metadata option? | |
| 17:52:50 | sean-k-mooney | stephenfin: i agree that we should not mix it with the resouces:* syntax | |
| 17:53:28 | stephenfin | yeah, I really don't think we need the image metadata option. If anything, I'd be looking to deprecate the likes of 'hw_numa_cpus' | |
| 17:53:52 | sean-k-mooney | but if we are seting hw_cpu_policy=mixed in the image seting hw_pinned_cpus=1-5 i think is ok | |
| 17:54:07 | sean-k-mooney | stephenfin: i dont think that we shoudl do that | |
| 17:54:22 | sean-k-mooney | we could but its there for a reason | |
| 17:54:27 | stephenfin | so if we boot using a flavor with 4 cores, what happens? | |
| 17:54:30 | sean-k-mooney | partly to prevent falvor explosion | |
| 17:54:30 | stephenfin | or with 8 cores? | |
| 17:55:06 | sean-k-mooney | with 4 course it woudl be rejected by the numa constraint in the api | |
| 17:55:07 | stephenfin | by having image explosion instead? | |