Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Refactor vsphere provider. #6490
Conversation
wallyworld
approved these changes
Oct 24, 2016
Please indicate what QA was done. How was it verified that the connect leak was fixed. What functional testing was done to ensure no regressions.
| @@ -89,7 +96,8 @@ func (m *ovaImportManager) importOva(ecfg *environConfig, instSpec *instanceSpec | ||
| return nil, errors.New(spec.Error[0].LocalizedMessage) | ||
| } | ||
| s := &spec.ImportSpec.(*types.VirtualMachineImportSpec).ConfigSpec | ||
| - s.NumCPUs = int(*instSpec.hwc.CpuCores) | ||
| + numCpus := int32(*instSpec.hwc.CpuCores) |
wallyworld
Oct 24, 2016
Owner
Doesn't the fact that CpuCores is a pointer imply that it is optional and the pointer may be nil?
A nil check seems like it sis required here, same with Mem, CpuPower etc below.
| @@ -200,12 +243,23 @@ func (c *client) Refresh(v *mo.VirtualMachine) error { | ||
| } | ||
| func (c *client) getVm(name string) (*mo.VirtualMachine, error) { |
hoenirvili
Oct 24, 2016
Contributor
Why not switch the name of this func simply vm instead of getVm. In go, no notions of getters and setters exist, like in other languages for example, Java. A more Idiomatic way is:
When you are making a "getter" for a thing is much nicer to name it thing rather than getTing If you want to make a set function that alternates the state of thing name it setTing.
More on this subject: https://golang.org/doc/effective_go.html#Getters
perrito666
Oct 24, 2016
Contributor
Well this is out of the scope of the present change but since its a free rename i did it.
perrito666
added some commits
Oct 20, 2016
|
!!build!! |
|
!!try!! |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
perrito666 commentedOct 24, 2016
•
Edited 1 time
-
perrito666
Oct 24, 2016
The provider for vsphere was using the wrong aproach to
connect never logging out and it was leaking 45 conn/min
Now all connections login/logout per request.
QA Steps
Open the VSPhere Management Console.
Click on vCenter Inventory List --> Resources -- vCenter Servers --> your server ip
Go the right and click on Manage --> Sessions
Bootstrap a vsphere provider juju and deploy a few services.
There should NOT be 45 new open/idle processes every minute, instead numbers of 1 or 2 is expected.