Tuesday, 2017-12-12

*** hiro-kobayashi has joined #openstack-blazar01:06
*** masahito has joined #openstack-blazar01:29
*** nicolasbock has quit IRC03:05
*** masahito has quit IRC03:06
*** masahito has joined #openstack-blazar07:03
*** hiro-kobayashi has quit IRC08:07
openstackgerritMasahito Muroi proposed openstack/blazar master: Support update reservation in instance reservation  https://review.openstack.org/51298808:40
openstackgerritMasahito Muroi proposed openstack/blazar master: Add resource_properties column to instance_reservations table  https://review.openstack.org/52734208:40
openstackgerritMasahito Muroi proposed openstack/blazar master: Support resource_properties key in the instance reservation  https://review.openstack.org/52734308:40
*** masahito has quit IRC08:41
*** hiro-kobayashi has joined #openstack-blazar08: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-blazar08:58
*** bertys has joined #openstack-blazar08:58
*** priteau has joined #openstack-blazar09:00
*** masahito has quit IRC09:01
*** masahito has joined #openstack-blazar09:01
-openstackstatus- NOTICE: Zuul is back online, looks like a temporary network problem.09:07
*** bertys has quit IRC09:34
*** bertys_ has joined #openstack-blazar09:54
masahitohi, everyone there?10:01
masahitoLooks like most of us is here.10:02
masahitoThis is the review meeting day1. So mainly reviewing big changes or blueprints.10:03
masahitoI see five big changes in review board.10:04
masahito1. state-machine10:04
masahito2. resource-monitoring10:04
masahito3. deploy-api-in-wsgi10:04
masahito4. flavor-extra-specs10:05
masahito5. multi-freepools10:05
masahitoDoes someone have different patches for the review?10:05
masahitookay. let's start from #110:06
masahitoThere is only one patch for the state-machine, right? https://review.openstack.org/#/c/512198/10:07
openstackgerritPierre Riteau proposed openstack/blazar master: Add a status module  https://review.openstack.org/51219810:07
hiro-kobayashiAnd this small patch: https://review.openstack.org/#/c/526235/10:08
openstackgerritPierre Riteau proposed openstack/blazar master: Add a status module  https://review.openstack.org/51219810:09
hiro-kobayashipriteau: thanks for revising the commit message.10:09
masahitoThis patch LGTM. thanks for the big change.10:09
priteauI am reviewing the patch, fixing typos as I go along10:09
hiro-kobayashiThanks for reviewing! Any comment or question?10:10
masahitoJust one question, why the create_lease method doesn't have lease_status decorator?10:11
hiro-kobayashiBecause a lease does not exist at the beginning of the create_lease().10:11
priteauWhy is one decorator using "result_in=status.lease.STABLE" and the other "result_in=(status.lease.ERROR,)"?10:12
hiro-kobayashiAfter the create_lease() completes, the status management starts.10:12
masahitooh, I see.10:12
hiro-kobayashipriteau: status.lease.STABLE is a tuple of statuses.10:13
hiro-kobayashiIt's defined at the LeaseStatus class of status.py10:14
priteauI see10:14
masahitosorry, one more comment for the test_state.py10:15
masahitoPlease replace self.assertEqual(True/False, ret) with self.assertTrue/False(ret)10:16
hiro-kobayashiOh, okay10:16
masahitoI saw lots of patches for replacing this before and don't want to see these patch on the review.10:17
masahitoanother comment? If nothing, I write down the comments on the patch set.10:19
masahito#2 resource-monitoring10:21
masahitoI can see the five patches.10:22
priteauI am still reading the code, I have at least one other comment10:22
priteauWill add it to the patch10:22
openstackgerritHiroaki Kobayashi proposed openstack/blazar master: Add a status module  https://review.openstack.org/51219810:22
masahitopriteau: can we move to next?10:25
masahitopatches for resource-monitoring10:26
masahito1. https://review.openstack.org/#/c/517135/10:26
masahito2. https://review.openstack.org/#/c/517316/10:26
masahito3. https://review.openstack.org/#/c/519963/10:26
masahito4. https://review.openstack.org/#/c/524054/410:26
masahito5. https://review.openstack.org/#/c/524069/10:26
hiro-kobayashiand 2 small patches for the dashboard:10:27
masahitohiro-kobayashi: Just a question. All of the fives are ready to review?10:27
hiro-kobayashimasahito: Yes. It works in my env.10:27
hiro-kobayashi2 more tasks remains:10:28
masahitohiro-kobayashi: got it.10:28
hiro-kobayashi1. add heal_reservations() for instance plugin.10:29
masahitoFirst one is updating db schema.10:29
hiro-kobayashi2. send notification10:29
hiro-kobayashimasahito: yes. It adds three fields as described in the blueprint.10:30
hiro-kobayashisorry, 4 fields10:30
masahitopriteau: do you have comments for the patch? I'm thinking it's ready to merge.10:32
priteauyes I do10:32
masahitoSecond one is about base monitor plugin.10:34
hiro-kobayashiThe db migration script was auto-generated and I don't know why it's text.10:34
hiro-kobayashiI'll fix it, anyway.10:35
masahitoThanks for the update the patch. For me, it's ready to go.10:37
priteauI have some comments10:38
hiro-kobayashipriteau: Thanks! I'll fix them.10:42
masahitoThird one is just add query for reservable field10:43
hiro-kobayashimasahito: How do you think about my comment for this patch?10:44
masahitoAs I commented, it's good to encapsulate the ['reservable == 1'] in utils or somewhere.10:45
masahitoNo, I don't propose to add the logic into convert_requirement. I'm proposing to introduce a different method only for this.10:47
masahitoBecause when we support another resource the flag name could be changed.10:48
hiro-kobayashiHow about db_api.reservable_host_get_all_by_queries()?10:48
masahitoIt's better than what I propose.10:50
masahitopriteau: okay to move to next?10:52
priteauI am reviewing https://review.openstack.org/#/c/524069/ now10:53
bertys_I have just posted comment for https://review.openstack.org/#/c/524069/10:53
masahitogot it10:54
hiro-kobayashibertys_: thanks for youre comment!10:56
hiro-kobayashiI want to discuss it.10:56
hiro-kobayashiIn my proposal, I haven't implemented resource recovery.10:57
hiro-kobayashiI mean that if host A fails and it's reservable field is set 'False', then it will never back to 'reservable = True'10:58
openstackgerritPierre Riteau proposed openstack/blazar master: Move wsgi app class to api directory  https://review.openstack.org/52142211:00
hiro-kobayashiShould blazar support resource recovery?11:01
openstackgerritPierre Riteau proposed openstack/blazar master: Move wsgi app class to api directory  https://review.openstack.org/52142211: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-kobayashibertys_: make sense. But resource recovery is complicated and I don't have a clear blueprint for that.11:04
hiro-kobayashiIf it's needed, I will add work items to the resource-monitoring blueprint.11:05
hiro-kobayashior 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 blueprint11:07
hiro-kobayashibertys_: thanks11:07
masahitohiro-kobayashi: question for the poloing monitor plugin11:09
masahitowhy the heal_reservation is class method? and some methods are changed to staticmethod or classmethod?11:09
hiro-kobayashimasahito: Because they are used without instance.11:11
hiro-kobayashiThey are called by PhysicalHostMonitorPlugin11:12
masahitoBut PhysicalHostMonitorPlugin isn't instantiated unless HostPlugin is instantiated.11:13
hiro-kobayashiright. but I don't have a good idea how to provide these instances to the PhysicalHostMonitorPlugin?11:15
masahitoI found the reason you did. I add comment about it.11:16
masahitookay then someone has other comments for resource-moniroting?11:17
priteauI left them on Gerrit11:19
hiro-kobayashiThanks all for comments! I'll check them and update patches.11:19
masahitothen move on to #3, deploy-api-in-wsgi11:21
masahitothere're three patches11:21
masahito1. https://review.openstack.org/#/c/521422/11:21
priteauI requested the first patch to be merged11:21
masahito2. https://review.openstack.org/#/c/521423/11:21
masahito3. https://review.openstack.org/#/c/521424/11:22
priteauWill do the second one zuul has merged11:22
priteauThe third doesn't pass Zuul11:23
masahitothird one got -1 by Zuul11:23
masahitoThe error looks not related to the change.11:24
masahitoIt's from tempest failure.11:24
priteauOnce the second has merged, I will rebase the third patch and that will give a chance for Zuul to rerun11:25
masahitohiro-kobayashi: Do you think we need install doc for wsgi deployment?11:25
masahitopriteau: thanks.11:25
priteauDoc and release note11:25
hiro-kobayashimasahito: Yes, it's better.11:25
masahitohiro-kobayashi: if so, I should change Implement tag to Partially tag.11:25
masahitopriteau: nice catch. I completely forgot releasenote.11:26
masahitoI'll add that.11:26
priteauActually we have to write Queens release notes for a lot of patches11:26
hiro-kobayashipriteau: Yes, l'll keep it in mind11:27
masahitoany comments for api-wsgi?11:27
priteauNo comment from me, although I haven't tried it11:28
priteauI will take care of doing the W+!11:28
masahitothanks :-)11:29
masahitonext is flavor-extra-specs.11:29
masahitothere're two patches.11:29
masahito1. https://review.openstack.org/#/c/527342/11:30
masahito2. https://review.openstack.org/#/c/527343/11:30
masahitoFirst one is add resource_properties column into instance_reservation column11:30
masahitoSecond one is adding resource_properties key for instance reservation API11:31
priteauWe should be careful about merging 1. because there is another DB migration in resource-monitoring11:31
*** nicolasbock has joined #openstack-blazar11:31
priteauI am not sure if they can refer to the same down_revision = '6bfd1c23aa18'11:31
priteauThe would make the history branch out rather than be a single line11:32
masahitoI added W-1 for second one b/c its tests aren't updated well.11:32
hiro-kobayashipriteau: I think it should be updated.11:32
priteauDepends which one gets merged first11:33
masahitoright. If needed, I'll rebase it on resource-monitoring patch.11:33
hiro-kobayashimasahito: flavor-extra-specs is smaller, so I think it can be merged faster11:34
masahitoresource-monitoring has higher priority for the cycle. This change shouldn't barrier it.11:34
hiro-kobayashiJust a one line change and I'm okay to fix it if your patch will be merged faster :-)11:35
openstackgerritPierre Riteau proposed openstack/blazar master: Support resource_properties key in instance reservation  https://review.openstack.org/52734311:36
masahitookay, then how about first W+1 first merged? like first come first serve11:37
openstackgerritPierre Riteau proposed openstack/blazar master: Support resource_properties key in instance reservation  https://review.openstack.org/52734311: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-kobayashiActually, it's better to have it.11:39
masahitobefore 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
priteaumasahito: 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 tasks11:40
hiro-kobayashimasahito: 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
masahitoL170 in instance_plugin.py11:41
hiro-kobayashibertys_: np ;-) your comments improves blazar codebase!11:42
masahitoThe pickup_host assures input values for the reservation always have 'resource_propeties' key11:42
masahitoThe reserve_resource() calls pickup_host() with requested value directly.11:44
masahitoI noticed I need to add 'resource_properties' in marshall_attributes... if it's a required key.11:44
masahitohiro-kobayashi: thanks for your idea. Introducing a new required key has big impact for deployers and users.11:45
masahitothen, move on to last one.11:46
masahitolast one is spec not codes.11:47
masahitoThe discussion is we can define az info in computeextracapability instead of az support in computehost.11:48
hiro-kobayashiAs I commented, +1 for ComputeExtraCapability11:49
hiro-kobayashiBlazar should automatically store az as the ComputeExtraCapability when it creates a host11:50
priteauWould this mean that there is no API change?11:50
masahitoIf we use ComputeExtraCapability, we don't need to work the BP11:50
masahitopriteau: yes.11:50
masahitohiro-kobayashi: I suppose operators add the az information if needed.11:51
priteauOne problem with this approach is that we don't yet support removing extra capabilities, so you couldn't remove the AZ ifno11:52
hiro-kobayashimasahito: I understood. It's better to automate it, isn't it?11:52
masahitoIf it's automated, it should be in computehost table.11:53
priteau+1 masahito11:53
masahitoFrom my understand, computehost table store info quered from Nova. meaning operators can't update the info.11:54
masahitoComputeExtraCapability stores additional info which operators can handle11:54
hiro-kobayashiMake sense.11:55
masahitookay. I'll add the ComputeExtraCapability approach as another alternative.11:56
hiro-kobayashiHow do you think about my second comment? It's about az which host reservation creates.11:56
hiro-kobayashiIt is for preventing failures in case users specifies az when they launch instances.11:57
masahitosorry, I'm not clear the comment.11:58
hiro-kobayashiI 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
masahitoIt's about when users launch their instance? or when they reserve instance?11:58
hiro-kobayashiBecause reserved hosts are in the 'blazar_*' az.11:59
hiro-kobayashimasahito: when users launch their instance.11:59
masahitoMy proposal is doing following steps:12:00
masahito1. Blazar stores original az into computehost table12:00
masahito2. An user can specify reservations with the original az12:01
masahito3. In the reserved time, the az info stored in computehost hasn't been changed to track original az.12:02
masahitobut move to blazar_* az in Nova side.12:03
masahitoSo another user can create a reservation with the original az while the host is in blazar_* az in Nova API.12:04
hiro-kobayashimasahito: about the final step, I think it should not set blazar_* az to the aggregate.12:04
hiro-kobayashiaz is just a metadata of host aggregate, right?12:05
hiro-kobayashiI don't think it doesn't need to set blazar_* az12:06
hiro-kobayashisorry, I think it doesn't need to set blazar_* az12:06
priteauHowever having the AZ set by Blazar can be useful for users to see which reservation each instance is using12:06
priteauOtherwise, you may not remember which reservation was selected when you launched the instance12:07
hiro-kobayashipriteau: Yes. There is a possibility that users specifies original az when they launch instances on the reserved hosts.12:09
masahitoso my plan is keeping the original az info only in blazar db.12:09
hiro-kobayashiIn that case, the instance creation fails.12:09
masahitoShould we change blazar_* az name like blazar_<reservation_id>_original_az?12:11
hiro-kobayashiAnyway, I'll write a concrete situation on gerrit ;-012:11
hiro-kobayashimasahito: No. My proposal was just setting the original az. But priteau said the blazar_* az is useful.12:12
priteauMaybe we could have both?12:12
hiro-kobayashiMultiple az is not allowed, isn't it?12:13
hiro-kobayashioh, sorry, misunderstood.12:13
masahitoIn that case nova only returns first one as its az.12:13
masahitoIt 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-kobayashiyes, it looks.12:16
masahitoFor more good discussion, I'll add it on the spec.12:16
masahitoWe ended the today's review topics!12:18
masahitoGreat discussion and great progress!12:18
masahitoThanks all.12:18
priteauThanks everyone12:18
masahitothanks everyone.12:19
priteauLooks like there might be issues with Zuul, https://review.openstack.org/#/c/521422/ has been running for more than one hour12:19
priteauI will recheck if it returns a timeout12:19
hiro-kobayashithanks, priteau!12:20
masahitoLooks like zuul has a trouble again.12:20
masahitoAll jobs get Failure.12:21
masahitopriteau bertys_: btw, I forgot asking you next weekly meeting.12:21
masahitoIIRC, next week is holiday week in Euro, isn't it?12:21
masahitoIf so, we can skip the next meeting.12:22
bertys_I would be available next week. I am OoO between Dec 25 and Jan 512:24
masahitobertys_: got it. btw, what's OoO?12:26
bertys_out of office12:26
masahitooh, I see.12:26
priteaumasahito: I should be able to join the next meeting12:28
priteauBut not the one of the 2612:28
bertys_thanks all, have to go now. Have a great day/evening12:29
*** bertys_ has quit IRC12:30
hiro-kobayashiso 19th will be the last meeting in 2017?12:30
hiro-kobayashibertys_: thanks! bye12:30
*** chandankumar has joined #openstack-blazar12:30
hiro-kobayashihave a nice day!12:30
masahitohiro-kobayashi: meybe12:30
chandankumarmasahito: hello12:30
masahitookay, let's finish today's meeting12:30
masahitothanks all.12:30
masahitochandankumar: hello12:30
chandankumarmasahito: will i abandon this patch https://review.openstack.org/#/c/524581/ related to tempest plugin12:31
chandankumarmasahito: i have got some spare time do you need any help on tempest plugin split work.12:31
*** hiro-kobayashi has quit IRC12:31
masahitochandankumar: if you can, it's very helpful for us!12:33
chandankumarmasahito: i am taking over this patch then https://review.openstack.org/#/c/428067/12:39
masahitochandankumar: thank you!12:40
*** masahito has quit IRC12:51
*** masahito has joined #openstack-blazar13:17
*** masahito has quit IRC13: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 IRC18:08
*** tonyb_ has quit IRC19:52
*** tonyb has joined #openstack-blazar19: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 vote20:14
*** openstackgerrit has quit IRC20:34
*** priteau has quit IRC23:15
*** chandankumar has quit IRC23:22
*** chandankumar has joined #openstack-blazar23:24
*** chandankumar has quit IRC23:29
*** chandankumar has joined #openstack-blazar23:35
*** openstackgerrit has joined #openstack-blazar23:54
openstackgerritMerged openstack/blazar master: Move wsgi app class to api directory  https://review.openstack.org/52142223:54

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!