Conversation
| # Updates a given VM raw entity with vm instance info if exists | ||
| # | ||
| # @param vm [Hash] A hash with vm raw data. | ||
| def populate_server_with_runtime_info(vm) |
There was a problem hiding this comment.
This seems like something we could have in vm entity. Based on responsibilities seems like it would be better place there.
There was a problem hiding this comment.
I'm not sure if VM is the right place for it since it deals with setting values from VitrulaMachineInstance.
I can move this one to Server if you feel it is more right .
|
|
||
| def ready? | ||
| status == 'running' && state == 'Running' && !vmi[:ip_address].blank? | ||
| rescue |
There was a problem hiding this comment.
Any specific error we are expecting here?
There was a problem hiding this comment.
nope, should be:
status == 'running' && state == 'Running' && !ip_address.blank?
30c6370 to
b75f19d
Compare
| new service.get_server(id) | ||
| end | ||
|
|
||
| def bootstrap(new_attributes = {}) |
There was a problem hiding this comment.
Please document potential new_attributes
There was a problem hiding this comment.
will do that in a next PR's version
| :resource_version => metadata[:resourceVersion], | ||
| :uid => metadata[:uid], | ||
| :labels => metadata[:labels], | ||
| :memory => domain[:resources][:requests][:memory], |
There was a problem hiding this comment.
This is the reason why we can reuse. We merged a PR which allows vm not to have memory but here the code would fail. I think it is not good idea to maintain it in 2 places.
There was a problem hiding this comment.
now that should be reside in a single place - see VmAction
It is possible to run create a vm [1] without memory requirement which we need to handle. [1] https://github.com/kubevirt/demo/blob/master/manifests/vm.yaml
dcb0eb2 to
33e254d
Compare
33e254d to
1df6899
Compare
No description provided.