| *** hiro-kobayashi has joined #openstack-blazar | 01:06 | |
| *** masahito has joined #openstack-blazar | 01:29 | |
| *** nicolasbock has quit IRC | 03:05 | |
| *** masahito has quit IRC | 03:06 | |
| *** masahito has joined #openstack-blazar | 07:03 | |
| *** hiro-kobayashi has quit IRC | 08:07 | |
| openstackgerrit | Masahito Muroi proposed openstack/blazar master: Support update reservation in instance reservation https://review.openstack.org/512988 | 08:40 |
|---|---|---|
| openstackgerrit | Masahito Muroi proposed openstack/blazar master: Add resource_properties column to instance_reservations table https://review.openstack.org/527342 | 08:40 |
| openstackgerrit | Masahito Muroi proposed openstack/blazar master: Support resource_properties key in the instance reservation https://review.openstack.org/527343 | 08:40 |
| *** masahito has quit IRC | 08:41 | |
| *** hiro-kobayashi has joined #openstack-blazar | 08:47 | |
| -openstackstatus- NOTICE: Our CI system Zuul is currently not accessible. Wait with approving changes and rechecks until it's back online. Currently waiting for an admin to investigate. | 08:47 | |
| *** masahito has joined #openstack-blazar | 08:58 | |
| *** bertys has joined #openstack-blazar | 08:58 | |
| *** priteau has joined #openstack-blazar | 09:00 | |
| *** masahito has quit IRC | 09:01 | |
| *** masahito has joined #openstack-blazar | 09:01 | |
| -openstackstatus- NOTICE: Zuul is back online, looks like a temporary network problem. | 09:07 | |
| *** bertys has quit IRC | 09:34 | |
| *** bertys_ has joined #openstack-blazar | 09:54 | |
| masahito | hi, everyone there? | 10:01 |
| priteau | o/ | 10:01 |
| hiro-kobayashi | o/ | 10:02 |
| masahito | Looks like most of us is here. | 10:02 |
| masahito | This is the review meeting day1. So mainly reviewing big changes or blueprints. | 10:03 |
| masahito | I see five big changes in review board. | 10:04 |
| masahito | 1. state-machine | 10:04 |
| masahito | 2. resource-monitoring | 10:04 |
| masahito | 3. deploy-api-in-wsgi | 10:04 |
| masahito | 4. flavor-extra-specs | 10:05 |
| masahito | 5. multi-freepools | 10:05 |
| masahito | Does someone have different patches for the review? | 10:05 |
| hiro-kobayashi | no | 10:06 |
| masahito | okay. let's start from #1 | 10:06 |
| masahito | There is only one patch for the state-machine, right? https://review.openstack.org/#/c/512198/ | 10:07 |
| openstackgerrit | Pierre Riteau proposed openstack/blazar master: Add a status module https://review.openstack.org/512198 | 10:07 |
| hiro-kobayashi | And this small patch: https://review.openstack.org/#/c/526235/ | 10:08 |
| openstackgerrit | Pierre Riteau proposed openstack/blazar master: Add a status module https://review.openstack.org/512198 | 10:09 |
| hiro-kobayashi | priteau: thanks for revising the commit message. | 10:09 |
| masahito | This patch LGTM. thanks for the big change. | 10:09 |
| priteau | I am reviewing the patch, fixing typos as I go along | 10:09 |
| hiro-kobayashi | Thanks for reviewing! Any comment or question? | 10:10 |
| masahito | Just one question, why the create_lease method doesn't have lease_status decorator? | 10:11 |
| hiro-kobayashi | Because a lease does not exist at the beginning of the create_lease(). | 10:11 |
| priteau | Why is one decorator using "result_in=status.lease.STABLE" and the other "result_in=(status.lease.ERROR,)"? | 10:12 |
| hiro-kobayashi | After the create_lease() completes, the status management starts. | 10:12 |
| masahito | oh, I see. | 10:12 |
| hiro-kobayashi | priteau: status.lease.STABLE is a tuple of statuses. | 10:13 |
| hiro-kobayashi | It's defined at the LeaseStatus class of status.py | 10:14 |
| priteau | I see | 10:14 |
| masahito | sorry, one more comment for the test_state.py | 10:15 |
| masahito | Please replace self.assertEqual(True/False, ret) with self.assertTrue/False(ret) | 10:16 |
| hiro-kobayashi | Oh, okay | 10:16 |
| masahito | I saw lots of patches for replacing this before and don't want to see these patch on the review. | 10:17 |
| masahito | another comment? If nothing, I write down the comments on the patch set. | 10:19 |
| masahito | #2 resource-monitoring | 10:21 |
| masahito | I can see the five patches. | 10:22 |
| priteau | I am still reading the code, I have at least one other comment | 10:22 |
| priteau | Will add it to the patch | 10:22 |
| openstackgerrit | Hiroaki Kobayashi proposed openstack/blazar master: Add a status module https://review.openstack.org/512198 | 10:22 |
| masahito | priteau: can we move to next? | 10:25 |
| priteau | Yes | 10:25 |
| masahito | okay | 10:25 |
| masahito | patches for resource-monitoring | 10:26 |
| masahito | 1. https://review.openstack.org/#/c/517135/ | 10:26 |
| masahito | 2. https://review.openstack.org/#/c/517316/ | 10:26 |
| masahito | 3. https://review.openstack.org/#/c/519963/ | 10:26 |
| masahito | 4. https://review.openstack.org/#/c/524054/4 | 10:26 |
| masahito | 5. https://review.openstack.org/#/c/524069/ | 10:26 |
| hiro-kobayashi | and 2 small patches for the dashboard: | 10:27 |
| hiro-kobayashi | https://review.openstack.org/#/c/526272/ | 10:27 |
| masahito | hiro-kobayashi: Just a question. All of the fives are ready to review? | 10:27 |
| hiro-kobayashi | https://review.openstack.org/#/c/526276/ | 10:27 |
| hiro-kobayashi | masahito: Yes. It works in my env. | 10:27 |
| hiro-kobayashi | 2 more tasks remains: | 10:28 |
| masahito | hiro-kobayashi: got it. | 10:28 |
| hiro-kobayashi | 1. add heal_reservations() for instance plugin. | 10:29 |
| masahito | First one is updating db schema. | 10:29 |
| hiro-kobayashi | 2. send notification | 10:29 |
| hiro-kobayashi | masahito: yes. It adds three fields as described in the blueprint. | 10:30 |
| hiro-kobayashi | sorry, 4 fields | 10:30 |
| masahito | priteau: do you have comments for the patch? I'm thinking it's ready to merge. | 10:32 |
| priteau | yes I do | 10:32 |
| priteau | Posted | 10:32 |
| masahito | Second one is about base monitor plugin. | 10:34 |
| hiro-kobayashi | The db migration script was auto-generated and I don't know why it's text. | 10:34 |
| hiro-kobayashi | I'll fix it, anyway. | 10:35 |
| masahito | Thanks for the update the patch. For me, it's ready to go. | 10:37 |
| priteau | I have some comments | 10:38 |
| hiro-kobayashi | priteau: Thanks! I'll fix them. | 10:42 |
| masahito | Third one is just add query for reservable field | 10:43 |
| hiro-kobayashi | masahito: How do you think about my comment for this patch? | 10:44 |
| masahito | As I commented, it's good to encapsulate the ['reservable == 1'] in utils or somewhere. | 10:45 |
| masahito | No, I don't propose to add the logic into convert_requirement. I'm proposing to introduce a different method only for this. | 10:47 |
| hiro-kobayashi | ok | 10:47 |
| masahito | Because when we support another resource the flag name could be changed. | 10:48 |
| hiro-kobayashi | How about db_api.reservable_host_get_all_by_queries()? | 10:48 |
| masahito | It's better than what I propose. | 10:50 |
| masahito | priteau: okay to move to next? | 10:52 |
| priteau | I am reviewing https://review.openstack.org/#/c/524069/ now | 10:53 |
| bertys_ | I have just posted comment for https://review.openstack.org/#/c/524069/ | 10:53 |
| masahito | got it | 10:54 |
| hiro-kobayashi | bertys_: thanks for youre comment! | 10:56 |
| hiro-kobayashi | I want to discuss it. | 10:56 |
| bertys_ | ok | 10:56 |
| hiro-kobayashi | In my proposal, I haven't implemented resource recovery. | 10:57 |
| hiro-kobayashi | I mean that if host A fails and it's reservable field is set 'False', then it will never back to 'reservable = True' | 10:58 |
| openstackgerrit | Pierre Riteau proposed openstack/blazar master: Move wsgi app class to api directory https://review.openstack.org/521422 | 11:00 |
| hiro-kobayashi | Should blazar support resource recovery? | 11:01 |
| openstackgerrit | Pierre Riteau proposed openstack/blazar master: Move wsgi app class to api directory https://review.openstack.org/521422 | 11:01 |
| bertys_ | Hmm, I am checking now https://review.openstack.org/#/c/492533/ | 11:01 |
| bertys_ | I would say yes at least for the "disabled case" | 11:04 |
| hiro-kobayashi | bertys_: make sense. But resource recovery is complicated and I don't have a clear blueprint for that. | 11:04 |
| hiro-kobayashi | If it's needed, I will add work items to the resource-monitoring blueprint. | 11:05 |
| hiro-kobayashi | or should it dealt with by another blueprint? | 11:06 |
| bertys_ | hiro-kobayashi: yes I know. | 11:06 |
| bertys_ | hiro-kobayashi: I will add this to the resource-monitoring blueprint | 11:07 |
| hiro-kobayashi | bertys_: thanks | 11:07 |
| bertys_ | s/will/would | 11:07 |
| hiro-kobayashi | okay | 11:07 |
| masahito | hiro-kobayashi: question for the poloing monitor plugin | 11:09 |
| masahito | why the heal_reservation is class method? and some methods are changed to staticmethod or classmethod? | 11:09 |
| hiro-kobayashi | masahito: Because they are used without instance. | 11:11 |
| hiro-kobayashi | They are called by PhysicalHostMonitorPlugin | 11:12 |
| masahito | But PhysicalHostMonitorPlugin isn't instantiated unless HostPlugin is instantiated. | 11:13 |
| hiro-kobayashi | right. but I don't have a good idea how to provide these instances to the PhysicalHostMonitorPlugin? | 11:15 |
| masahito | I found the reason you did. I add comment about it. | 11:16 |
| hiro-kobayashi | thanks | 11:17 |
| masahito | okay then someone has other comments for resource-moniroting? | 11:17 |
| priteau | I left them on Gerrit | 11:19 |
| hiro-kobayashi | Thanks all for comments! I'll check them and update patches. | 11:19 |
| masahito | then move on to #3, deploy-api-in-wsgi | 11:21 |
| masahito | there're three patches | 11:21 |
| masahito | 1. https://review.openstack.org/#/c/521422/ | 11:21 |
| priteau | I requested the first patch to be merged | 11:21 |
| masahito | 2. https://review.openstack.org/#/c/521423/ | 11:21 |
| masahito | 3. https://review.openstack.org/#/c/521424/ | 11:22 |
| masahito | right. | 11:22 |
| priteau | Will do the second one zuul has merged | 11:22 |
| masahito | thanks | 11:22 |
| priteau | The third doesn't pass Zuul | 11:23 |
| masahito | third one got -1 by Zuul | 11:23 |
| masahito | yes. | 11:23 |
| masahito | The error looks not related to the change. | 11:24 |
| masahito | It's from tempest failure. | 11:24 |
| priteau | Once the second has merged, I will rebase the third patch and that will give a chance for Zuul to rerun | 11:25 |
| masahito | hiro-kobayashi: Do you think we need install doc for wsgi deployment? | 11:25 |
| masahito | priteau: thanks. | 11:25 |
| priteau | Doc and release note | 11:25 |
| hiro-kobayashi | masahito: Yes, it's better. | 11:25 |
| masahito | hiro-kobayashi: if so, I should change Implement tag to Partially tag. | 11:25 |
| masahito | priteau: nice catch. I completely forgot releasenote. | 11:26 |
| masahito | I'll add that. | 11:26 |
| priteau | Actually we have to write Queens release notes for a lot of patches | 11:26 |
| hiro-kobayashi | thanks | 11:26 |
| hiro-kobayashi | priteau: Yes, l'll keep it in mind | 11:27 |
| masahito | any comments for api-wsgi? | 11:27 |
| priteau | No comment from me, although I haven't tried it | 11:28 |
| priteau | I will take care of doing the W+! | 11:28 |
| priteau | W+1 | 11:28 |
| masahito | thanks :-) | 11:29 |
| masahito | next is flavor-extra-specs. | 11:29 |
| masahito | there're two patches. | 11:29 |
| masahito | 1. https://review.openstack.org/#/c/527342/ | 11:30 |
| masahito | 2. https://review.openstack.org/#/c/527343/ | 11:30 |
| masahito | First one is add resource_properties column into instance_reservation column | 11:30 |
| masahito | Second one is adding resource_properties key for instance reservation API | 11:31 |
| priteau | We should be careful about merging 1. because there is another DB migration in resource-monitoring | 11:31 |
| *** nicolasbock has joined #openstack-blazar | 11:31 | |
| priteau | I am not sure if they can refer to the same down_revision = '6bfd1c23aa18' | 11:31 |
| priteau | The would make the history branch out rather than be a single line | 11:32 |
| masahito | I added W-1 for second one b/c its tests aren't updated well. | 11:32 |
| hiro-kobayashi | priteau: I think it should be updated. | 11:32 |
| priteau | Depends which one gets merged first | 11:33 |
| hiro-kobayashi | s/should/must/ | 11:33 |
| hiro-kobayashi | yes | 11:33 |
| masahito | right. If needed, I'll rebase it on resource-monitoring patch. | 11:33 |
| hiro-kobayashi | masahito: flavor-extra-specs is smaller, so I think it can be merged faster | 11:34 |
| masahito | resource-monitoring has higher priority for the cycle. This change shouldn't barrier it. | 11:34 |
| hiro-kobayashi | Just a one line change and I'm okay to fix it if your patch will be merged faster :-) | 11:35 |
| masahito | ummm, | 11:35 |
| openstackgerrit | Pierre Riteau proposed openstack/blazar master: Support resource_properties key in instance reservation https://review.openstack.org/527343 | 11:36 |
| masahito | okay, then how about first W+1 first merged? like first come first serve | 11:37 |
| hiro-kobayashi | agree | 11:37 |
| openstackgerrit | Pierre Riteau proposed openstack/blazar master: Support resource_properties key in instance reservation https://review.openstack.org/527343 | 11:37 |
| bertys_ | I can give more tasks to hiro-kobayashi, e.g. write tempest test for resource monitoring BP!? | 11:38 |
| bertys_ | Just kidding... | 11:38 |
| hiro-kobayashi | oops. | 11:38 |
| hiro-kobayashi | Actually, it's better to have it. | 11:39 |
| masahito | before moving last one, I'd like to ask everyone resource_properties should be required param or optional param for the instance reservation API. | 11:39 |
| priteau | masahito: In https://review.openstack.org/#/c/527343/ you say "adds a 'resource_properties' key for instance reservation as a required parameter", but I don't see where it is made required? | 11:39 |
| bertys_ | hiro-kobayashi: Thanks for all your work again and sorry for asking you too many tasks | 11:40 |
| hiro-kobayashi | masahito: I think it should be optional. However, it's required for the host reservation. I we would be better to redefine required/optional paramters. | 11:41 |
| masahito | L170 in instance_plugin.py | 11:41 |
| hiro-kobayashi | bertys_: np ;-) your comments improves blazar codebase! | 11:42 |
| masahito | The pickup_host assures input values for the reservation always have 'resource_propeties' key | 11:42 |
| masahito | The reserve_resource() calls pickup_host() with requested value directly. | 11:44 |
| masahito | I noticed I need to add 'resource_properties' in marshall_attributes... if it's a required key. | 11:44 |
| masahito | hiro-kobayashi: thanks for your idea. Introducing a new required key has big impact for deployers and users. | 11:45 |
| masahito | then, move on to last one. | 11:46 |
| masahito | last one is spec not codes. | 11:47 |
| masahito | https://review.openstack.org/#/c/524168/ | 11:47 |
| masahito | The discussion is we can define az info in computeextracapability instead of az support in computehost. | 11:48 |
| hiro-kobayashi | As I commented, +1 for ComputeExtraCapability | 11:49 |
| hiro-kobayashi | Blazar should automatically store az as the ComputeExtraCapability when it creates a host | 11:50 |
| priteau | Would this mean that there is no API change? | 11:50 |
| masahito | If we use ComputeExtraCapability, we don't need to work the BP | 11:50 |
| masahito | priteau: yes. | 11:50 |
| masahito | hiro-kobayashi: I suppose operators add the az information if needed. | 11:51 |
| priteau | One problem with this approach is that we don't yet support removing extra capabilities, so you couldn't remove the AZ ifno | 11:52 |
| hiro-kobayashi | masahito: I understood. It's better to automate it, isn't it? | 11:52 |
| masahito | If it's automated, it should be in computehost table. | 11:53 |
| priteau | +1 masahito | 11:53 |
| hiro-kobayashi | okay | 11:54 |
| masahito | From my understand, computehost table store info quered from Nova. meaning operators can't update the info. | 11:54 |
| masahito | ComputeExtraCapability stores additional info which operators can handle | 11:54 |
| hiro-kobayashi | Make sense. | 11:55 |
| masahito | okay. I'll add the ComputeExtraCapability approach as another alternative. | 11:56 |
| hiro-kobayashi | How do you think about my second comment? It's about az which host reservation creates. | 11:56 |
| hiro-kobayashi | It is for preventing failures in case users specifies az when they launch instances. | 11:57 |
| masahito | sorry, I'm not clear the comment. | 11:58 |
| hiro-kobayashi | I mean, if a user specifies 'az-A' in the resource properties, he/she can specify 'az-A' when he/she launches instances. And it fails. | 11:58 |
| masahito | It's about when users launch their instance? or when they reserve instance? | 11:58 |
| hiro-kobayashi | Because reserved hosts are in the 'blazar_*' az. | 11:59 |
| hiro-kobayashi | masahito: when users launch their instance. | 11:59 |
| masahito | My proposal is doing following steps: | 12:00 |
| masahito | 1. Blazar stores original az into computehost table | 12:00 |
| masahito | 2. An user can specify reservations with the original az | 12:01 |
| masahito | 3. In the reserved time, the az info stored in computehost hasn't been changed to track original az. | 12:02 |
| masahito | but move to blazar_* az in Nova side. | 12:03 |
| masahito | So another user can create a reservation with the original az while the host is in blazar_* az in Nova API. | 12:04 |
| hiro-kobayashi | masahito: about the final step, I think it should not set blazar_* az to the aggregate. | 12:04 |
| hiro-kobayashi | az is just a metadata of host aggregate, right? | 12:05 |
| masahito | yes. | 12:05 |
| hiro-kobayashi | I don't think it doesn't need to set blazar_* az | 12:06 |
| hiro-kobayashi | sorry, I think it doesn't need to set blazar_* az | 12:06 |
| priteau | However having the AZ set by Blazar can be useful for users to see which reservation each instance is using | 12:06 |
| priteau | Otherwise, you may not remember which reservation was selected when you launched the instance | 12:07 |
| hiro-kobayashi | priteau: Yes. There is a possibility that users specifies original az when they launch instances on the reserved hosts. | 12:09 |
| masahito | so my plan is keeping the original az info only in blazar db. | 12:09 |
| hiro-kobayashi | In that case, the instance creation fails. | 12:09 |
| masahito | Should we change blazar_* az name like blazar_<reservation_id>_original_az? | 12:11 |
| hiro-kobayashi | Anyway, I'll write a concrete situation on gerrit ;-0 | 12:11 |
| hiro-kobayashi | masahito: No. My proposal was just setting the original az. But priteau said the blazar_* az is useful. | 12:12 |
| priteau | Maybe we could have both? | 12:12 |
| priteau | blazar_<reservation_id>;nova_<original_az> | 12:13 |
| hiro-kobayashi | Multiple az is not allowed, isn't it? | 12:13 |
| hiro-kobayashi | oh, sorry, misunderstood. | 12:13 |
| masahito | In that case nova only returns first one as its az. | 12:13 |
| masahito | It looks like the discussion point is whether the user can see the original az from both Nova and Blazar or only from Blazar. | 12:15 |
| hiro-kobayashi | yes, it looks. | 12:16 |
| masahito | For more good discussion, I'll add it on the spec. | 12:16 |
| hiro-kobayashi | Thanks! | 12:16 |
| masahito | We ended the today's review topics! | 12:18 |
| masahito | Great discussion and great progress! | 12:18 |
| masahito | Thanks all. | 12:18 |
| priteau | Thanks everyone | 12:18 |
| hiro-kobayashi | Thanks! | 12:19 |
| masahito | thanks everyone. | 12:19 |
| priteau | Looks like there might be issues with Zuul, https://review.openstack.org/#/c/521422/ has been running for more than one hour | 12:19 |
| priteau | I will recheck if it returns a timeout | 12:19 |
| hiro-kobayashi | oh | 12:20 |
| hiro-kobayashi | thanks, priteau! | 12:20 |
| masahito | Looks like zuul has a trouble again. | 12:20 |
| masahito | http://zuulv3.openstack.org/ | 12:20 |
| masahito | All jobs get Failure. | 12:21 |
| masahito | priteau bertys_: btw, I forgot asking you next weekly meeting. | 12:21 |
| masahito | IIRC, next week is holiday week in Euro, isn't it? | 12:21 |
| masahito | If so, we can skip the next meeting. | 12:22 |
| bertys_ | I would be available next week. I am OoO between Dec 25 and Jan 5 | 12:24 |
| masahito | bertys_: got it. btw, what's OoO? | 12:26 |
| bertys_ | out of office | 12:26 |
| masahito | oh, I see. | 12:26 |
| priteau | masahito: I should be able to join the next meeting | 12:28 |
| priteau | But not the one of the 26 | 12:28 |
| bertys_ | thanks all, have to go now. Have a great day/evening | 12:29 |
| *** bertys_ has quit IRC | 12:30 | |
| hiro-kobayashi | so 19th will be the last meeting in 2017? | 12:30 |
| hiro-kobayashi | bertys_: thanks! bye | 12:30 |
| *** chandankumar has joined #openstack-blazar | 12:30 | |
| hiro-kobayashi | have a nice day! | 12:30 |
| masahito | hiro-kobayashi: meybe | 12:30 |
| chandankumar | masahito: hello | 12:30 |
| masahito | okay, let's finish today's meeting | 12:30 |
| masahito | thanks all. | 12:30 |
| masahito | chandankumar: hello | 12:30 |
| chandankumar | masahito: will i abandon this patch https://review.openstack.org/#/c/524581/ related to tempest plugin | 12:31 |
| chandankumar | masahito: i have got some spare time do you need any help on tempest plugin split work. | 12:31 |
| *** hiro-kobayashi has quit IRC | 12:31 | |
| masahito | chandankumar: if you can, it's very helpful for us! | 12:33 |
| chandankumar | masahito: i am taking over this patch then https://review.openstack.org/#/c/428067/ | 12:39 |
| masahito | chandankumar: thank you! | 12:40 |
| *** masahito has quit IRC | 12:51 | |
| *** masahito has joined #openstack-blazar | 13:17 | |
| *** masahito has quit IRC | 13:35 | |
| -openstackstatus- NOTICE: We're currently seeing an elevated rate of timeouts in jobs and the zuulv3.openstack.org dashboard is intermittently unresponsive, please stand by while we troubleshoot the issues. | 14:38 | |
| *** nicolasbock has quit IRC | 18:08 | |
| *** tonyb_ has quit IRC | 19:52 | |
| *** tonyb has joined #openstack-blazar | 19:52 | |
| -openstackstatus- NOTICE: The zuul scheduler has been restarted after lengthy troubleshooting for a memory consumption issue; earlier changes have been reenqueued but if you notice jobs not running for a new or approved change you may want to leave a recheck comment or a new approval vote | 20:14 | |
| *** openstackgerrit has quit IRC | 20:34 | |
| *** priteau has quit IRC | 23:15 | |
| *** chandankumar has quit IRC | 23:22 | |
| *** chandankumar has joined #openstack-blazar | 23:24 | |
| *** chandankumar has quit IRC | 23:29 | |
| *** chandankumar has joined #openstack-blazar | 23:35 | |
| *** openstackgerrit has joined #openstack-blazar | 23:54 | |
| openstackgerrit | Merged openstack/blazar master: Move wsgi app class to api directory https://review.openstack.org/521422 | 23:54 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!