VMWware: support to update guest info gc status if enabled#261
Conversation
| self._vmware_cust_conf = Config(cf) | ||
| post_gc_status = self._vmware_cust_conf.post_gc_status | ||
| if post_gc_status: | ||
| set_gc_status("Started") |
There was a problem hiding this comment.
Still looking to have the fix requested in #215 (comment) fixed here.
There was a problem hiding this comment.
Hi OddBloke,
Sorry I misunderstood that you agreed to not move this check into set_gc_status() function. If I still need to move it, then this function will looks like "set_gc_status(gcMsg, vmware_cust_conf)". Each time we set the gc status, we need to parse the vmware cust conf and read boolean "MISC|POST-GC-STATUS". It might be not efficient, do you agree? Thanks.
There was a problem hiding this comment.
Each time we set the gc status, we need to parse the vmware cust conf and read boolean "MISC|POST-GC-STATUS". It might be not efficient, do you agree?
I may not be following the logic through correctly, but it looks to me like ConfigFile takes care of reading the config file a single time, so I don't believe this is an accurate description of the additional work that would be performed. I think we're really just talking about a few additional Python dict operations, which is not a significant cost.
There was a problem hiding this comment.
Address your comments, PTAL, thanks a lot.
OddBloke
left a comment
OddBloke
left a comment
There was a problem hiding this comment.
Thanks for the update, this looks good now! Could we please add some unit tests for set_gc_status? With those added, I think this will be ready to land.
| else: | ||
| return None |
There was a problem hiding this comment.
A nit, but as the above if returns unconditionally, we don't need the else here:
| else: | |
| return None | |
| return None |
There was a problem hiding this comment.
Thanks, the unit test and this comment are addressed. Please check.
|
Request for SRU testing: Hello @xiaofengw-vmware, If you happen to get a chance to validate that this feature behaves as expected we would appreciate any feedback you may have as a comment on the SRU process bug #1881018. Details on testing a the -proposed release of cloud-init are available here if you happen to have a chance to kick off and validate that you patch behaves without causing Tracebacks. Thank you for making cloud-init better, |
3 participants
VMWare: support to update guest info gc status if gc status is enabled in configuration file.