Merge pull request #68402 from saravanan30erd/cloudprovider-ovirt

hack/.golint_failures

@@ -89,7 +89,6 @@ pkg/auth/authorizer/abac
 pkg/capabilities
 pkg/cloudprovider/providers/fake
 pkg/cloudprovider/providers/gce/cloud
-pkg/cloudprovider/providers/ovirt
 pkg/cloudprovider/providers/photon
 pkg/cloudprovider/providers/vsphere
 pkg/cloudprovider/providers/vsphere/vclib

pkg/cloudprovider/providers/ovirt/ovirt.go

@@ -36,24 +36,29 @@ import (
 	cloudprovider "k8s.io/cloud-provider"
 )
 
+// ProviderName is the name of this cloud provider.
 const ProviderName = "ovirt"
 
-type OVirtInstance struct {
+// Instance specifies UUID, name and IP address of the instance.
+type Instance struct {
 	UUID      string
 	Name      string
 	IPAddress string
 }
 
-type OVirtInstanceMap map[string]OVirtInstance
+// InstanceMap provides the map of Ovirt instances.
+type InstanceMap map[string]Instance
 
-type OVirtCloud struct {
+// Cloud is an implementation of the cloud provider interface for Ovirt.
+type Cloud struct {
 	VmsRequest   *url.URL
 	HostsRequest *url.URL
 }
 
-type OVirtApiConfig struct {
+// APIConfig wraps the api settings for the Ovirt.
+type APIConfig struct {
 	Connection struct {
-		ApiEntry string `gcfg:"uri"`
+		APIEntry string `gcfg:"uri"`
 		Username string `gcfg:"username"`
 		Password string `gcfg:"password"`
 	}
@@ -62,21 +67,24 @@ type OVirtApiConfig struct {
 	}
 }
 
-type XmlVmAddress struct {
+// XMLVMAddress is an implementation for the Ovirt instance IP address in xml.
+type XMLVMAddress struct {
 	Address string `xml:"address,attr"`
 }
 
-type XmlVmInfo struct {
+// XMLVMInfo is an implementation for the Ovirt instance details in xml.
+type XMLVMInfo struct {
 	UUID      string         `xml:"id,attr"`
 	Name      string         `xml:"name"`
 	Hostname  string         `xml:"guest_info>fqdn"`
-	Addresses []XmlVmAddress `xml:"guest_info>ips>ip"`
+	Addresses []XMLVMAddress `xml:"guest_info>ips>ip"`
 	State     string         `xml:"status>state"`
 }
 
-type XmlVmsList struct {
+// XMLVmsList is an implementation to provide the list of Ovirt instances.
+type XMLVmsList struct {
 	XMLName xml.Name    `xml:"vms"`
-	Vm      []XmlVmInfo `xml:"vm"`
+	VM      []XMLVMInfo `xml:"vm"`
 }
 
 func init() {
@@ -86,12 +94,12 @@ func init() {
 		})
 }
 
-func newOVirtCloud(config io.Reader) (*OVirtCloud, error) {
+func newOVirtCloud(config io.Reader) (*Cloud, error) {
 	if config == nil {
 		return nil, fmt.Errorf("missing configuration file for ovirt cloud provider")
 	}
 
-	oVirtConfig := OVirtApiConfig{}
+	oVirtConfig := APIConfig{}
 
 	/* defaults */
 	oVirtConfig.Connection.Username = "admin@internal"
@@ -100,11 +108,11 @@ func newOVirtCloud(config io.Reader) (*OVirtCloud, error) {
 		return nil, err
 	}
 
-	if oVirtConfig.Connection.ApiEntry == "" {
+	if oVirtConfig.Connection.APIEntry == "" {
 		return nil, fmt.Errorf("missing ovirt uri in cloud provider configuration")
 	}
 
-	request, err := url.Parse(oVirtConfig.Connection.ApiEntry)
+	request, err := url.Parse(oVirtConfig.Connection.APIEntry)
 	if err != nil {
 		return nil, err
 	}
@@ -113,49 +121,50 @@ func newOVirtCloud(config io.Reader) (*OVirtCloud, error) {
 	request.User = url.UserPassword(oVirtConfig.Connection.Username, oVirtConfig.Connection.Password)
 	request.RawQuery = url.Values{"search": {oVirtConfig.Filters.VmsQuery}}.Encode()
 
-	return &OVirtCloud{VmsRequest: request}, nil
+	return &Cloud{VmsRequest: request}, nil
 }
 
 // Initialize passes a Kubernetes clientBuilder interface to the cloud provider
-func (v *OVirtCloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, stop <-chan struct{}) {
+func (v *Cloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, stop <-chan struct{}) {
 }
 
-func (v *OVirtCloud) Clusters() (cloudprovider.Clusters, bool) {
+// Clusters returns the list of clusters.
+func (v *Cloud) Clusters() (cloudprovider.Clusters, bool) {
 	return nil, false
 }
 
 // ProviderName returns the cloud provider ID.
-func (v *OVirtCloud) ProviderName() string {
+func (v *Cloud) ProviderName() string {
 	return ProviderName
 }
 
 // HasClusterID returns true if the cluster has a clusterID
-func (v *OVirtCloud) HasClusterID() bool {
+func (v *Cloud) HasClusterID() bool {
 	return true
 }
 
 // LoadBalancer returns an implementation of LoadBalancer for oVirt cloud
-func (v *OVirtCloud) LoadBalancer() (cloudprovider.LoadBalancer, bool) {
+func (v *Cloud) LoadBalancer() (cloudprovider.LoadBalancer, bool) {
 	return nil, false
 }
 
 // Instances returns an implementation of Instances for oVirt cloud
-func (v *OVirtCloud) Instances() (cloudprovider.Instances, bool) {
+func (v *Cloud) Instances() (cloudprovider.Instances, bool) {
 	return v, true
 }
 
 // Zones returns an implementation of Zones for oVirt cloud
-func (v *OVirtCloud) Zones() (cloudprovider.Zones, bool) {
+func (v *Cloud) Zones() (cloudprovider.Zones, bool) {
 	return nil, false
 }
 
 // Routes returns an implementation of Routes for oVirt cloud
-func (v *OVirtCloud) Routes() (cloudprovider.Routes, bool) {
+func (v *Cloud) Routes() (cloudprovider.Routes, bool) {
 	return nil, false
 }
 
 // NodeAddresses returns the NodeAddresses of the instance with the specified nodeName.
-func (v *OVirtCloud) NodeAddresses(ctx context.Context, nodeName types.NodeName) ([]v1.NodeAddress, error) {
+func (v *Cloud) NodeAddresses(ctx context.Context, nodeName types.NodeName) ([]v1.NodeAddress, error) {
 	name := mapNodeNameToInstanceName(nodeName)
 	instance, err := v.fetchInstance(name)
 	if err != nil {
@@ -186,7 +195,7 @@ func (v *OVirtCloud) NodeAddresses(ctx context.Context, nodeName types.NodeName)
 // NodeAddressesByProviderID returns the node addresses of an instances with the specified unique providerID
 // This method will not be called from the node that is requesting this ID. i.e. metadata service
 // and other local methods cannot be used here
-func (v *OVirtCloud) NodeAddressesByProviderID(ctx context.Context, providerID string) ([]v1.NodeAddress, error) {
+func (v *Cloud) NodeAddressesByProviderID(ctx context.Context, providerID string) ([]v1.NodeAddress, error) {
 	return []v1.NodeAddress{}, cloudprovider.NotImplemented
 }
 
@@ -198,17 +207,17 @@ func mapNodeNameToInstanceName(nodeName types.NodeName) string {
 
 // InstanceExistsByProviderID returns true if the instance with the given provider id still exists and is running.
 // If false is returned with no error, the instance will be immediately deleted by the cloud controller manager.
-func (v *OVirtCloud) InstanceExistsByProviderID(ctx context.Context, providerID string) (bool, error) {
+func (v *Cloud) InstanceExistsByProviderID(ctx context.Context, providerID string) (bool, error) {
 	return false, cloudprovider.NotImplemented
 }
 
 // InstanceShutdownByProviderID returns true if the instance is in safe state to detach volumes
-func (v *OVirtCloud) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) {
+func (v *Cloud) InstanceShutdownByProviderID(ctx context.Context, providerID string) (bool, error) {
 	return false, cloudprovider.NotImplemented
 }
 
 // InstanceID returns the cloud provider ID of the node with the specified NodeName.
-func (v *OVirtCloud) InstanceID(ctx context.Context, nodeName types.NodeName) (string, error) {
+func (v *Cloud) InstanceID(ctx context.Context, nodeName types.NodeName) (string, error) {
 	name := mapNodeNameToInstanceName(nodeName)
 	instance, err := v.fetchInstance(name)
 	if err != nil {
@@ -222,16 +231,16 @@ func (v *OVirtCloud) InstanceID(ctx context.Context, nodeName types.NodeName) (s
 // InstanceTypeByProviderID returns the cloudprovider instance type of the node with the specified unique providerID
 // This method will not be called from the node that is requesting this ID. i.e. metadata service
 // and other local methods cannot be used here
-func (v *OVirtCloud) InstanceTypeByProviderID(ctx context.Context, providerID string) (string, error) {
+func (v *Cloud) InstanceTypeByProviderID(ctx context.Context, providerID string) (string, error) {
 	return "", cloudprovider.NotImplemented
 }
 
 // InstanceType returns the type of the specified instance.
-func (v *OVirtCloud) InstanceType(ctx context.Context, name types.NodeName) (string, error) {
+func (v *Cloud) InstanceType(ctx context.Context, name types.NodeName) (string, error) {
 	return "", nil
 }
 
-func getInstancesFromXml(body io.Reader) (OVirtInstanceMap, error) {
+func getInstancesFromXML(body io.Reader) (InstanceMap, error) {
 	if body == nil {
 		return nil, fmt.Errorf("ovirt rest-api response body is missing")
 	}
@@ -241,15 +250,15 @@ func getInstancesFromXml(body io.Reader) (OVirtInstanceMap, error) {
 		return nil, err
 	}
 
-	vmlist := XmlVmsList{}
+	vmlist := XMLVmsList{}
 
 	if err := xml.Unmarshal(content, &vmlist); err != nil {
 		return nil, err
 	}
 
-	instances := make(OVirtInstanceMap)
+	instances := make(InstanceMap)
 
-	for _, vm := range vmlist.Vm {
+	for _, vm := range vmlist.VM {
 		// Always return only vms that are up and running
 		if vm.Hostname != "" && strings.ToLower(vm.State) == "up" {
 			address := ""
@@ -257,7 +266,7 @@ func getInstancesFromXml(body io.Reader) (OVirtInstanceMap, error) {
 				address = vm.Addresses[0].Address
 			}
 
-			instances[vm.Hostname] = OVirtInstance{
+			instances[vm.Hostname] = Instance{
 				UUID:      vm.UUID,
 				Name:      vm.Name,
 				IPAddress: address,
@@ -268,7 +277,7 @@ func getInstancesFromXml(body io.Reader) (OVirtInstanceMap, error) {
 	return instances, nil
 }
 
-func (v *OVirtCloud) fetchAllInstances() (OVirtInstanceMap, error) {
+func (v *Cloud) fetchAllInstances() (InstanceMap, error) {
 	response, err := http.Get(v.VmsRequest.String())
 	if err != nil {
 		return nil, err
@@ -276,10 +285,10 @@ func (v *OVirtCloud) fetchAllInstances() (OVirtInstanceMap, error) {
 
 	defer response.Body.Close()
 
-	return getInstancesFromXml(response.Body)
+	return getInstancesFromXML(response.Body)
 }
 
-func (v *OVirtCloud) fetchInstance(name string) (*OVirtInstance, error) {
+func (v *Cloud) fetchInstance(name string) (*Instance, error) {
 	allInstances, err := v.fetchAllInstances()
 	if err != nil {
 		return nil, err
@@ -293,7 +302,8 @@ func (v *OVirtCloud) fetchInstance(name string) (*OVirtInstance, error) {
 	return &instance, nil
 }
 
-func (m *OVirtInstanceMap) ListSortedNames() []string {
+// ListSortedNames returns the list of sorted Ovirt instances name.
+func (m *InstanceMap) ListSortedNames() []string {
 	var names []string
 
 	for k := range *m {
@@ -305,11 +315,12 @@ func (m *OVirtInstanceMap) ListSortedNames() []string {
 	return names
 }
 
-// Implementation of Instances.CurrentNodeName
-func (v *OVirtCloud) CurrentNodeName(ctx context.Context, hostname string) (types.NodeName, error) {
+// CurrentNodeName is implementation of Instances.CurrentNodeName.
+func (v *Cloud) CurrentNodeName(ctx context.Context, hostname string) (types.NodeName, error) {
 	return types.NodeName(hostname), nil
 }
 
-func (v *OVirtCloud) AddSSHKeyToAllInstances(ctx context.Context, user string, keyData []byte) error {
+// AddSSHKeyToAllInstances is currently not implemented.
+func (v *Cloud) AddSSHKeyToAllInstances(ctx context.Context, user string, keyData []byte) error {
 	return cloudprovider.NotImplemented
 }

pkg/cloudprovider/providers/ovirt/ovirt_test.go

@@ -62,14 +62,14 @@ uri = https://localhost:8443/ovirt-engine/api
 func TestOVirtCloudXmlParsing(t *testing.T) {
 	body1 := (io.Reader)(nil)
 
-	_, err1 := getInstancesFromXml(body1)
+	_, err1 := getInstancesFromXML(body1)
 	if err1 == nil {
 		t.Fatalf("An error is expected when body is missing")
 	}
 
 	body2 := strings.NewReader("")
 
-	_, err2 := getInstancesFromXml(body2)
+	_, err2 := getInstancesFromXML(body2)
 	if err2 == nil {
 		t.Fatalf("An error is expected when body is empty")
 	}
@@ -80,7 +80,7 @@ func TestOVirtCloudXmlParsing(t *testing.T) {
 </vms>
 `)
 
-	instances3, err3 := getInstancesFromXml(body3)
+	instances3, err3 := getInstancesFromXML(body3)
 	if err3 != nil {
 		t.Fatalf("Unexpected error listing instances: %s", err3)
 	}
@@ -111,7 +111,7 @@ func TestOVirtCloudXmlParsing(t *testing.T) {
 </vms>
 `)
 
-	instances4, err4 := getInstancesFromXml(body4)
+	instances4, err4 := getInstancesFromXML(body4)
 	if err4 != nil {
 		t.Fatalf("Unexpected error listing instances: %s", err4)
 	}