Fix golint for openstack and cinder packages

hack/.golint_failures

@@ -88,7 +88,6 @@ pkg/cloudprovider/providers/aws
 pkg/cloudprovider/providers/fake
 pkg/cloudprovider/providers/gce
 pkg/cloudprovider/providers/gce/cloud
-pkg/cloudprovider/providers/openstack
 pkg/cloudprovider/providers/ovirt
 pkg/cloudprovider/providers/photon
 pkg/cloudprovider/providers/vsphere
@@ -391,7 +390,6 @@ pkg/volume/aws_ebs
 pkg/volume/azure_dd
 pkg/volume/azure_file
 pkg/volume/cephfs
-pkg/volume/cinder
 pkg/volume/configmap
 pkg/volume/empty_dir
 pkg/volume/fc

pkg/cloudprovider/providers/openstack/metadata.go

@@ -33,12 +33,12 @@ import (
 )
 
 const (
-	// metadataUrlTemplate allows building an OpenStack Metadata service URL.
+	// metadataURLTemplate allows building an OpenStack Metadata service URL.
 	// It's a hardcoded IPv4 link-local address as documented in "OpenStack Cloud
 	// Administrator Guide", chapter Compute - Networking with nova-network.
 	//https://docs.openstack.org/nova/latest/admin/networking-nova.html#metadata-service
 	defaultMetadataVersion = "2012-08-10"
-	metadataUrlTemplate    = "http://169.254.169.254/openstack/%s/meta_data.json"
+	metadataURLTemplate    = "http://169.254.169.254/openstack/%s/meta_data.json"
 
 	// metadataID is used as an identifier on the metadata search order configuration.
 	metadataID = "metadataService"
@@ -53,10 +53,10 @@ const (
 	configDriveID = "configDrive"
 )
 
+// ErrBadMetadata is used to indicate a problem parsing data from metadata server
 var ErrBadMetadata = errors.New("invalid OpenStack metadata, got empty uuid")
 
-// There are multiple device types. To keep it simple, we're using a single structure
-// for all device metadata types.
+// DeviceMetadata is a single/simplified data structure for all kinds of device metadata types.
 type DeviceMetadata struct {
 	Type    string `json:"type"`
 	Bus     string `json:"bus,omitempty"`
@@ -65,10 +65,11 @@ type DeviceMetadata struct {
 	// .. and other fields.
 }
 
-// Assumes the "2012-08-10" meta_data.json format.
-//https://docs.openstack.org/nova/latest/user/config-drive.html
+// Metadata has the information fetched from OpenStack metadata service or
+// config drives. Assumes the "2012-08-10" meta_data.json format.
+// See http://docs.openstack.org/user-guide/cli_config_drive.html
 type Metadata struct {
-	Uuid             string           `json:"uuid"`
+	UUID             string           `json:"uuid"`
 	Hostname         string           `json:"hostname"`
 	AvailabilityZone string           `json:"availability_zone"`
 	Devices          []DeviceMetadata `json:"devices,omitempty"`
@@ -84,15 +85,15 @@ func parseMetadata(r io.Reader) (*Metadata, error) {
 		return nil, err
 	}
 
-	if metadata.Uuid == "" {
+	if metadata.UUID == "" {
 		return nil, ErrBadMetadata
 	}
 
 	return &metadata, nil
 }
 
-func getMetadataUrl(metadataVersion string) string {
-	return fmt.Sprintf(metadataUrlTemplate, metadataVersion)
+func getMetadataURL(metadataVersion string) string {
+	return fmt.Sprintf(metadataURLTemplate, metadataVersion)
 }
 
 func getConfigDrivePath(metadataVersion string) string {
@@ -147,16 +148,16 @@ func getMetadataFromConfigDrive(metadataVersion string) (*Metadata, error) {
 
 func getMetadataFromMetadataService(metadataVersion string) (*Metadata, error) {
 	// Try to get JSON from metadata server.
-	metadataUrl := getMetadataUrl(metadataVersion)
-	glog.V(4).Infof("Attempting to fetch metadata from %s", metadataUrl)
-	resp, err := http.Get(metadataUrl)
+	metadataURL := getMetadataURL(metadataVersion)
+	glog.V(4).Infof("Attempting to fetch metadata from %s", metadataURL)
+	resp, err := http.Get(metadataURL)
 	if err != nil {
-		return nil, fmt.Errorf("error fetching %s: %v", metadataUrl, err)
+		return nil, fmt.Errorf("error fetching %s: %v", metadataURL, err)
 	}
 	defer resp.Body.Close()
 
 	if resp.StatusCode != http.StatusOK {
-		err = fmt.Errorf("unexpected status code when reading metadata from %s: %s", metadataUrl, resp.Status)
+		err = fmt.Errorf("unexpected status code when reading metadata from %s: %s", metadataURL, resp.Status)
 		return nil, err
 	}
 

pkg/cloudprovider/providers/openstack/metadata_test.go

@@ -22,7 +22,7 @@ import (
 )
 
 var FakeMetadata = Metadata{
-	Uuid:             "83679162-1378-4288-a2d4-70e13ec132aa",
+	UUID:             "83679162-1378-4288-a2d4-70e13ec132aa",
 	Hostname:         "test",
 	AvailabilityZone: "nova",
 }
@@ -85,8 +85,8 @@ func TestParseMetadata(t *testing.T) {
 		t.Errorf("incorrect hostname: %s", md.Hostname)
 	}
 
-	if md.Uuid != "83679162-1378-4288-a2d4-70e13ec132aa" {
-		t.Errorf("incorrect uuid: %s", md.Uuid)
+	if md.UUID != "83679162-1378-4288-a2d4-70e13ec132aa" {
+		t.Errorf("incorrect uuid: %s", md.UUID)
 	}
 
 	if md.AvailabilityZone != "nova" {

pkg/cloudprovider/providers/openstack/openstack_client.go

@@ -23,6 +23,7 @@ import (
 	"github.com/gophercloud/gophercloud/openstack"
 )
 
+// NewNetworkV2 creates a ServiceClient that may be used with the neutron v2 API
 func (os *OpenStack) NewNetworkV2() (*gophercloud.ServiceClient, error) {
 	network, err := openstack.NewNetworkV2(os.provider, gophercloud.EndpointOpts{
 		Region: os.region,
@@ -33,6 +34,7 @@ func (os *OpenStack) NewNetworkV2() (*gophercloud.ServiceClient, error) {
 	return network, nil
 }
 
+// NewComputeV2 creates a ServiceClient that may be used with the nova v2 API
 func (os *OpenStack) NewComputeV2() (*gophercloud.ServiceClient, error) {
 	compute, err := openstack.NewComputeV2(os.provider, gophercloud.EndpointOpts{
 		Region: os.region,
@@ -43,6 +45,7 @@ func (os *OpenStack) NewComputeV2() (*gophercloud.ServiceClient, error) {
 	return compute, nil
 }
 
+// NewBlockStorageV1 creates a ServiceClient that may be used with the Cinder v1 API
 func (os *OpenStack) NewBlockStorageV1() (*gophercloud.ServiceClient, error) {
 	storage, err := openstack.NewBlockStorageV1(os.provider, gophercloud.EndpointOpts{
 		Region: os.region,
@@ -53,6 +56,7 @@ func (os *OpenStack) NewBlockStorageV1() (*gophercloud.ServiceClient, error) {
 	return storage, nil
 }
 
+// NewBlockStorageV2 creates a ServiceClient that may be used with the Cinder v2 API
 func (os *OpenStack) NewBlockStorageV2() (*gophercloud.ServiceClient, error) {
 	storage, err := openstack.NewBlockStorageV2(os.provider, gophercloud.EndpointOpts{
 		Region: os.region,
@@ -63,6 +67,7 @@ func (os *OpenStack) NewBlockStorageV2() (*gophercloud.ServiceClient, error) {
 	return storage, nil
 }
 
+// NewBlockStorageV3 creates a ServiceClient that may be used with the Cinder v3 API
 func (os *OpenStack) NewBlockStorageV3() (*gophercloud.ServiceClient, error) {
 	storage, err := openstack.NewBlockStorageV3(os.provider, gophercloud.EndpointOpts{
 		Region: os.region,
@@ -73,6 +78,7 @@ func (os *OpenStack) NewBlockStorageV3() (*gophercloud.ServiceClient, error) {
 	return storage, nil
 }
 
+// NewLoadBalancerV2 creates a ServiceClient that may be used with the Neutron LBaaS v2 API
 func (os *OpenStack) NewLoadBalancerV2() (*gophercloud.ServiceClient, error) {
 	var lb *gophercloud.ServiceClient
 	var err error

pkg/cloudprovider/providers/openstack/openstack_instances.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/kubernetes/pkg/cloudprovider"
 )
 
+// Instances encapsulates an implementation of Instances for OpenStack.
 type Instances struct {
 	compute *gophercloud.ServiceClient
 	opts    MetadataOpts
@@ -51,7 +52,7 @@ func (os *OpenStack) Instances() (cloudprovider.Instances, bool) {
 	}, true
 }
 
-// Implementation of Instances.CurrentNodeName
+// CurrentNodeName implements Instances.CurrentNodeName
 // Note this is *not* necessarily the same as hostname.
 func (i *Instances) CurrentNodeName(hostname string) (types.NodeName, error) {
 	md, err := getMetadata(i.opts.SearchOrder)
@@ -61,10 +62,12 @@ func (i *Instances) CurrentNodeName(hostname string) (types.NodeName, error) {
 	return types.NodeName(md.Hostname), nil
 }
 
+// AddSSHKeyToAllInstances is not implemented for OpenStack
 func (i *Instances) AddSSHKeyToAllInstances(user string, keyData []byte) error {
 	return cloudprovider.NotImplemented
 }
 
+// NodeAddresses implements Instances.NodeAddresses
 func (i *Instances) NodeAddresses(name types.NodeName) ([]v1.NodeAddress, error) {
 	glog.V(4).Infof("NodeAddresses(%v) called", name)
 
@@ -212,9 +215,9 @@ func srvInstanceType(srv *servers.Server) (string, error) {
 // See cloudprovider.GetInstanceProviderID and Instances.InstanceID.
 func instanceIDFromProviderID(providerID string) (instanceID string, err error) {
 	// If Instances.InstanceID or cloudprovider.GetInstanceProviderID is changed, the regexp should be changed too.
-	var providerIdRegexp = regexp.MustCompile(`^` + ProviderName + `:///([^/]+)$`)
+	var providerIDRegexp = regexp.MustCompile(`^` + ProviderName + `:///([^/]+)$`)
 
-	matches := providerIdRegexp.FindStringSubmatch(providerID)
+	matches := providerIDRegexp.FindStringSubmatch(providerID)
 	if len(matches) != 2 {
 		return "", fmt.Errorf("ProviderID \"%s\" didn't match expected format \"openstack:///InstanceID\"", providerID)
 	}

pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go

@@ -68,7 +68,7 @@ const (
 	activeStatus = "ACTIVE"
 	errorStatus  = "ERROR"
 
-	ServiceAnnotationLoadBalancerFloatingNetworkId = "loadbalancer.openstack.org/floating-network-id"
+	ServiceAnnotationLoadBalancerFloatingNetworkID = "loadbalancer.openstack.org/floating-network-id"
 
 	// ServiceAnnotationLoadBalancerInternal is the annotation used on the service
 	// to indicate that we want an internal loadbalancer service.
@@ -76,7 +76,7 @@ const (
 	ServiceAnnotationLoadBalancerInternal = "service.beta.kubernetes.io/openstack-internal-load-balancer"
 )
 
-// LoadBalancer implementation for LBaaS v2
+// LbaasV2 is a LoadBalancer implementation for Neutron LBaaS v2 API
 type LbaasV2 struct {
 	LoadBalancer
 }
@@ -368,12 +368,10 @@ func waitLoadbalancerDeleted(client *gophercloud.ServiceClient, loadbalancerID s
 		if err != nil {
 			if err == ErrNotFound {
 				return true, nil
-			} else {
-				return false, err
 			}
-		} else {
-			return false, nil
+			return false, err
 		}
+		return false, nil
 	})
 
 	if err == wait.ErrWaitTimeout {
@@ -442,7 +440,7 @@ func (lbaas *LbaasV2) createLoadBalancer(service *v1.Service, name string, inter
 	createOpts := loadbalancers.CreateOpts{
 		Name:        name,
 		Description: fmt.Sprintf("Kubernetes external service %s", name),
-		VipSubnetID: lbaas.opts.SubnetId,
+		VipSubnetID: lbaas.opts.SubnetID,
 		Provider:    lbaas.opts.LBProvider,
 	}
 
@@ -458,6 +456,7 @@ func (lbaas *LbaasV2) createLoadBalancer(service *v1.Service, name string, inter
 	return loadbalancer, nil
 }
 
+// GetLoadBalancer returns whether the specified load balancer exists and its status
 func (lbaas *LbaasV2) GetLoadBalancer(clusterName string, service *v1.Service) (*v1.LoadBalancerStatus, bool, error) {
 	loadBalancerName := cloudprovider.GetLoadBalancerName(service)
 	loadbalancer, err := getLoadbalancerByName(lbaas.lb, loadBalancerName)
@@ -485,7 +484,7 @@ func (lbaas *LbaasV2) GetLoadBalancer(clusterName string, service *v1.Service) (
 }
 
 // The LB needs to be configured with instance addresses on the same
-// subnet as the LB (aka opts.SubnetId).  Currently we're just
+// subnet as the LB (aka opts.SubnetID).  Currently we're just
 // guessing that the node's InternalIP is the right address - and that
 // should be sufficient for all "normal" cases.
 func nodeAddressForLB(node *v1.Node) (string, error) {
@@ -584,8 +583,8 @@ func isSecurityGroupNotFound(err error) bool {
 	return false
 }
 
-// getFloatingNetworkIdForLB returns a floating-network-id for cluster.
-func getFloatingNetworkIdForLB(client *gophercloud.ServiceClient) (string, error) {
+// getFloatingNetworkIDForLB returns a floating-network-id for cluster.
+func getFloatingNetworkIDForLB(client *gophercloud.ServiceClient) (string, error) {
 	var floatingNetworkIds []string
 
 	type NetworkWithExternalExt struct {
@@ -635,6 +634,7 @@ func getFloatingNetworkIdForLB(client *gophercloud.ServiceClient) (string, error
 // a list of regions (from config) and query/create loadbalancers in
 // each region.
 
+// EnsureLoadBalancer creates a new load balancer 'name', or updates the existing one.
 func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Service, nodes []*v1.Node) (*v1.LoadBalancerStatus, error) {
 	glog.V(4).Infof("EnsureLoadBalancer(%v, %v, %v, %v, %v, %v, %v)", clusterName, apiService.Namespace, apiService.Name, apiService.Spec.LoadBalancerIP, apiService.Spec.Ports, nodes, apiService.Annotations)
 
@@ -642,16 +642,16 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv
 		return nil, fmt.Errorf("there are no available nodes for LoadBalancer service %s/%s", apiService.Namespace, apiService.Name)
 	}
 
-	if len(lbaas.opts.SubnetId) == 0 {
-		// Get SubnetId automatically.
-		// The LB needs to be configured with instance addresses on the same subnet, so get SubnetId by one node.
+	if len(lbaas.opts.SubnetID) == 0 {
+		// Get SubnetID automatically.
+		// The LB needs to be configured with instance addresses on the same subnet, so get SubnetID by one node.
 		subnetID, err := getSubnetIDForLB(lbaas.compute, *nodes[0])
 		if err != nil {
 			glog.Warningf("Failed to find subnet-id for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err)
 			return nil, fmt.Errorf("no subnet-id for service %s/%s : subnet-id not set in cloud provider config, "+
 				"and failed to find subnet-id from OpenStack: %v", apiService.Namespace, apiService.Name, err)
 		}
-		lbaas.opts.SubnetId = subnetID
+		lbaas.opts.SubnetID = subnetID
 	}
 
 	ports := apiService.Spec.Ports
@@ -659,10 +659,10 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv
 		return nil, fmt.Errorf("no ports provided to openstack load balancer")
 	}
 
-	floatingPool := getStringFromServiceAnnotation(apiService, ServiceAnnotationLoadBalancerFloatingNetworkId, lbaas.opts.FloatingNetworkId)
+	floatingPool := getStringFromServiceAnnotation(apiService, ServiceAnnotationLoadBalancerFloatingNetworkID, lbaas.opts.FloatingNetworkID)
 	if len(floatingPool) == 0 {
 		var err error
-		floatingPool, err = getFloatingNetworkIdForLB(lbaas.network)
+		floatingPool, err = getFloatingNetworkIDForLB(lbaas.network)
 		if err != nil {
 			glog.Warningf("Failed to find floating-network-id for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err)
 		}
@@ -816,7 +816,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv
 				_, err := v2pools.CreateMember(lbaas.lb, pool.ID, v2pools.CreateMemberOpts{
 					ProtocolPort: int(port.NodePort),
 					Address:      addr,
-					SubnetID:     lbaas.opts.SubnetId,
+					SubnetID:     lbaas.opts.SubnetID,
 				}).Extract()
 				if err != nil {
 					return nil, fmt.Errorf("error creating LB pool member for node: %s, %v", node.Name, err)
@@ -1111,8 +1111,8 @@ func (lbaas *LbaasV2) ensureSecurityGroup(clusterName string, apiService *v1.Ser
 		// update loadbalancer vip port
 		if !found {
 			port.SecurityGroups = append(port.SecurityGroups, lbSecGroup.ID)
-			update_opts := neutronports.UpdateOpts{SecurityGroups: &port.SecurityGroups}
-			res := neutronports.Update(lbaas.network, portID, update_opts)
+			updateOpts := neutronports.UpdateOpts{SecurityGroups: &port.SecurityGroups}
+			res := neutronports.Update(lbaas.network, portID, updateOpts)
 			if res.Err != nil {
 				msg := fmt.Sprintf("Error occured updating port %s for loadbalancer service %s/%s: %v", portID, apiService.Namespace, apiService.Name, res.Err)
 				return fmt.Errorf(msg)
@@ -1152,20 +1152,21 @@ func (lbaas *LbaasV2) ensureSecurityGroup(clusterName string, apiService *v1.Ser
 	return nil
 }
 
+// UpdateLoadBalancer updates hosts under the specified load balancer.
 func (lbaas *LbaasV2) UpdateLoadBalancer(clusterName string, service *v1.Service, nodes []*v1.Node) error {
 	loadBalancerName := cloudprovider.GetLoadBalancerName(service)
 	glog.V(4).Infof("UpdateLoadBalancer(%v, %v, %v)", clusterName, loadBalancerName, nodes)
 
-	if len(lbaas.opts.SubnetId) == 0 && len(nodes) > 0 {
-		// Get SubnetId automatically.
-		// The LB needs to be configured with instance addresses on the same subnet, so get SubnetId by one node.
+	if len(lbaas.opts.SubnetID) == 0 && len(nodes) > 0 {
+		// Get SubnetID automatically.
+		// The LB needs to be configured with instance addresses on the same subnet, so get SubnetID by one node.
 		subnetID, err := getSubnetIDForLB(lbaas.compute, *nodes[0])
 		if err != nil {
 			glog.Warningf("Failed to find subnet-id for loadbalancer service %s/%s: %v", service.Namespace, service.Name, err)
 			return fmt.Errorf("no subnet-id for service %s/%s : subnet-id not set in cloud provider config, "+
 				"and failed to find subnet-id from OpenStack: %v", service.Namespace, service.Name, err)
 		}
-		lbaas.opts.SubnetId = subnetID
+		lbaas.opts.SubnetID = subnetID
 	}
 
 	ports := service.Spec.Ports
@@ -1254,7 +1255,7 @@ func (lbaas *LbaasV2) UpdateLoadBalancer(clusterName string, service *v1.Service
 			_, err := v2pools.CreateMember(lbaas.lb, pool.ID, v2pools.CreateMemberOpts{
 				Address:      addr,
 				ProtocolPort: int(port.NodePort),
-				SubnetID:     lbaas.opts.SubnetId,
+				SubnetID:     lbaas.opts.SubnetID,
 			}).Extract()
 			if err != nil {
 				return err
@@ -1372,6 +1373,7 @@ func (lbaas *LbaasV2) updateSecurityGroup(clusterName string, apiService *v1.Ser
 	return nil
 }
 
+// EnsureLoadBalancerDeleted deletes the specified load balancer
 func (lbaas *LbaasV2) EnsureLoadBalancerDeleted(clusterName string, service *v1.Service) error {
 	loadBalancerName := cloudprovider.GetLoadBalancerName(service)
 	glog.V(4).Infof("EnsureLoadBalancerDeleted(%v, %v)", clusterName, loadBalancerName)
@@ -1512,9 +1514,8 @@ func (lbaas *LbaasV2) EnsureSecurityGroupDeleted(clusterName string, service *v1
 		if isSecurityGroupNotFound(err) {
 			// It is OK when the security group has been deleted by others.
 			return nil
-		} else {
-			return fmt.Errorf("Error occurred finding security group: %s: %v", lbSecGroupName, err)
 		}
+		return fmt.Errorf("Error occurred finding security group: %s: %v", lbSecGroupName, err)
 	}
 
 	lbSecGroup := groups.Delete(lbaas.network, lbSecGroupID)

pkg/cloudprovider/providers/openstack/openstack_metrics.go

@@ -19,32 +19,32 @@ package openstack
 import "github.com/prometheus/client_golang/prometheus"
 
 const (
-	OpenstackSubsystem         = "openstack"
-	OpenstackOperationKey      = "cloudprovider_openstack_api_request_duration_seconds"
-	OpenstackOperationErrorKey = "cloudprovider_openstack_api_request_errors"
+	openstackSubsystem         = "openstack"
+	openstackOperationKey      = "cloudprovider_openstack_api_request_duration_seconds"
+	openstackOperationErrorKey = "cloudprovider_openstack_api_request_errors"
 )
 
 var (
-	OpenstackOperationsLatency = prometheus.NewHistogramVec(
+	openstackOperationsLatency = prometheus.NewHistogramVec(
 		prometheus.HistogramOpts{
-			Subsystem: OpenstackSubsystem,
-			Name:      OpenstackOperationKey,
+			Subsystem: openstackSubsystem,
+			Name:      openstackOperationKey,
 			Help:      "Latency of openstack api call",
 		},
 		[]string{"request"},
 	)
 
-	OpenstackApiRequestErrors = prometheus.NewCounterVec(
+	openstackAPIRequestErrors = prometheus.NewCounterVec(
 		prometheus.CounterOpts{
-			Subsystem: OpenstackSubsystem,
-			Name:      OpenstackOperationErrorKey,
+			Subsystem: openstackSubsystem,
+			Name:      openstackOperationErrorKey,
 			Help:      "Cumulative number of openstack Api call errors",
 		},
 		[]string{"request"},
 	)
 )
 
-func RegisterMetrics() {
-	prometheus.MustRegister(OpenstackOperationsLatency)
-	prometheus.MustRegister(OpenstackApiRequestErrors)
+func registerMetrics() {
+	prometheus.MustRegister(openstackOperationsLatency)
+	prometheus.MustRegister(openstackAPIRequestErrors)
 }

pkg/cloudprovider/providers/openstack/openstack_routes.go

@@ -29,17 +29,19 @@ import (
 	"k8s.io/kubernetes/pkg/cloudprovider"
 )
 
-var ErrNoRouterId = errors.New("router-id not set in cloud provider config")
+var errNoRouterID = errors.New("router-id not set in cloud provider config")
 
+// Routes implements the cloudprovider.Routes for OpenStack clouds
 type Routes struct {
 	compute *gophercloud.ServiceClient
 	network *gophercloud.ServiceClient
 	opts    RouterOpts
 }
 
+// NewRoutes creates a new instance of Routes
 func NewRoutes(compute *gophercloud.ServiceClient, network *gophercloud.ServiceClient, opts RouterOpts) (cloudprovider.Routes, error) {
-	if opts.RouterId == "" {
-		return nil, ErrNoRouterId
+	if opts.RouterID == "" {
+		return nil, errNoRouterID
 	}
 
 	return &Routes{
@@ -49,6 +51,7 @@ func NewRoutes(compute *gophercloud.ServiceClient, network *gophercloud.ServiceC
 	}, nil
 }
 
+// ListRoutes lists all managed routes that belong to the specified clusterName
 func (r *Routes) ListRoutes(clusterName string) ([]*cloudprovider.Route, error) {
 	glog.V(4).Infof("ListRoutes(%v)", clusterName)
 
@@ -70,7 +73,7 @@ func (r *Routes) ListRoutes(clusterName string) ([]*cloudprovider.Route, error)
 		return nil, err
 	}
 
-	router, err := routers.Get(r.network, r.opts.RouterId).Extract()
+	router, err := routers.Get(r.network, r.opts.RouterID).Extract()
 	if err != nil {
 		return nil, err
 	}
@@ -136,10 +139,11 @@ func updateAllowedAddressPairs(network *gophercloud.ServiceClient, port *neutron
 	return unwinder, nil
 }
 
+// CreateRoute creates the described managed route
 func (r *Routes) CreateRoute(clusterName string, nameHint string, route *cloudprovider.Route) error {
 	glog.V(4).Infof("CreateRoute(%v, %v, %v)", clusterName, nameHint, route)
 
-	onFailure := NewCaller()
+	onFailure := newCaller()
 
 	addr, err := getAddressByName(r.compute, route.TargetNode)
 	if err != nil {
@@ -148,7 +152,7 @@ func (r *Routes) CreateRoute(clusterName string, nameHint string, route *cloudpr
 
 	glog.V(4).Infof("Using nexthop %v for node %v", addr, route.TargetNode)
 
-	router, err := routers.Get(r.network, r.opts.RouterId).Extract()
+	router, err := routers.Get(r.network, r.opts.RouterID).Extract()
 	if err != nil {
 		return err
 	}
@@ -171,7 +175,7 @@ func (r *Routes) CreateRoute(clusterName string, nameHint string, route *cloudpr
 	if err != nil {
 		return err
 	}
-	defer onFailure.Call(unwind)
+	defer onFailure.call(unwind)
 
 	// get the port of addr on target node.
 	portID, err := getPortIDByIP(r.compute, route.TargetNode, addr)
@@ -200,25 +204,26 @@ func (r *Routes) CreateRoute(clusterName string, nameHint string, route *cloudpr
 		if err != nil {
 			return err
 		}
-		defer onFailure.Call(unwind)
+		defer onFailure.call(unwind)
 	}
 
 	glog.V(4).Infof("Route created: %v", route)
-	onFailure.Disarm()
+	onFailure.disarm()
 	return nil
 }
 
+// DeleteRoute deletes the specified managed route
 func (r *Routes) DeleteRoute(clusterName string, route *cloudprovider.Route) error {
 	glog.V(4).Infof("DeleteRoute(%v, %v)", clusterName, route)
 
-	onFailure := NewCaller()
+	onFailure := newCaller()
 
 	addr, err := getAddressByName(r.compute, route.TargetNode)
 	if err != nil {
 		return err
 	}
 
-	router, err := routers.Get(r.network, r.opts.RouterId).Extract()
+	router, err := routers.Get(r.network, r.opts.RouterID).Extract()
 	if err != nil {
 		return err
 	}
@@ -245,7 +250,7 @@ func (r *Routes) DeleteRoute(clusterName string, route *cloudprovider.Route) err
 	if err != nil {
 		return err
 	}
-	defer onFailure.Call(unwind)
+	defer onFailure.call(unwind)
 
 	// get the port of addr on target node.
 	portID, err := getPortIDByIP(r.compute, route.TargetNode, addr)
@@ -257,9 +262,9 @@ func (r *Routes) DeleteRoute(clusterName string, route *cloudprovider.Route) err
 		return err
 	}
 
-	addr_pairs := port.AllowedAddressPairs
+	addrPairs := port.AllowedAddressPairs
 	index = -1
-	for i, item := range addr_pairs {
+	for i, item := range addrPairs {
 		if item.IPAddress == route.DestinationCIDR {
 			index = i
 			break
@@ -268,18 +273,18 @@ func (r *Routes) DeleteRoute(clusterName string, route *cloudprovider.Route) err
 
 	if index != -1 {
 		// Delete element `index`
-		addr_pairs[index] = addr_pairs[len(addr_pairs)-1]
-		addr_pairs = addr_pairs[:len(addr_pairs)-1]
+		addrPairs[index] = addrPairs[len(addrPairs)-1]
+		addrPairs = addrPairs[:len(addrPairs)-1]
 
-		unwind, err := updateAllowedAddressPairs(r.network, port, addr_pairs)
+		unwind, err := updateAllowedAddressPairs(r.network, port, addrPairs)
 		if err != nil {
 			return err
 		}
-		defer onFailure.Call(unwind)
+		defer onFailure.call(unwind)
 	}
 
 	glog.V(4).Infof("Route deleted: %v", route)
-	onFailure.Disarm()
+	onFailure.disarm()
 	return nil
 }
 

pkg/cloudprovider/providers/openstack/openstack_routes_test.go

@@ -40,7 +40,7 @@ func TestRoutes(t *testing.T) {
 	}
 
 	// Pick the first router and server to try a test with
-	os.routeOpts.RouterId = getRouters(os)[0].ID
+	os.routeOpts.RouterID = getRouters(os)[0].ID
 	servername := getServers(os)[0].Name
 
 	r, ok := os.Routes()

pkg/cloudprovider/providers/openstack/openstack_test.go

@@ -37,9 +37,7 @@ import (
 )
 
 const (
-	volumeAvailableStatus = "available"
-	volumeInUseStatus     = "in-use"
-	testClusterName       = "testCluster"
+	testClusterName = "testCluster"
 
 	volumeStatusTimeoutSeconds = 30
 	// volumeStatus* is configuration of exponential backoff for
@@ -68,9 +66,8 @@ func WaitForVolumeStatus(t *testing.T, os *OpenStack, volumeName string, status
 				status,
 				volumeStatusTimeoutSeconds)
 			return true, nil
-		} else {
-			return false, nil
 		}
+		return false, nil
 	})
 	if err == wait.ErrWaitTimeout {
 		t.Logf("Volume (%s) status did not change to %s after %v seconds\n",
@@ -116,12 +113,12 @@ func TestReadConfig(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Should succeed when a valid config is provided: %s", err)
 	}
-	if cfg.Global.AuthUrl != "http://auth.url" {
-		t.Errorf("incorrect authurl: %s", cfg.Global.AuthUrl)
+	if cfg.Global.AuthURL != "http://auth.url" {
+		t.Errorf("incorrect authurl: %s", cfg.Global.AuthURL)
 	}
 
-	if cfg.Global.UserId != "user" {
-		t.Errorf("incorrect userid: %s", cfg.Global.UserId)
+	if cfg.Global.UserID != "user" {
+		t.Errorf("incorrect userid: %s", cfg.Global.UserID)
 	}
 
 	if cfg.Global.Password != "mypass" {
@@ -163,10 +160,10 @@ func TestToAuthOptions(t *testing.T) {
 	cfg := Config{}
 	cfg.Global.Username = "user"
 	cfg.Global.Password = "pass"
-	cfg.Global.DomainId = "2a73b8f597c04551a0fdc8e95544be8a"
+	cfg.Global.DomainID = "2a73b8f597c04551a0fdc8e95544be8a"
 	cfg.Global.DomainName = "local"
-	cfg.Global.AuthUrl = "http://auth.url"
-	cfg.Global.UserId = "user"
+	cfg.Global.AuthURL = "http://auth.url"
+	cfg.Global.UserID = "user"
 
 	ao := cfg.toAuthOptions()
 
@@ -179,20 +176,20 @@ func TestToAuthOptions(t *testing.T) {
 	if ao.Password != cfg.Global.Password {
 		t.Errorf("Password %s != %s", ao.Password, cfg.Global.Password)
 	}
-	if ao.DomainID != cfg.Global.DomainId {
-		t.Errorf("DomainID %s != %s", ao.DomainID, cfg.Global.DomainId)
+	if ao.DomainID != cfg.Global.DomainID {
+		t.Errorf("DomainID %s != %s", ao.DomainID, cfg.Global.DomainID)
 	}
-	if ao.IdentityEndpoint != cfg.Global.AuthUrl {
-		t.Errorf("IdentityEndpoint %s != %s", ao.IdentityEndpoint, cfg.Global.AuthUrl)
+	if ao.IdentityEndpoint != cfg.Global.AuthURL {
+		t.Errorf("IdentityEndpoint %s != %s", ao.IdentityEndpoint, cfg.Global.AuthURL)
 	}
-	if ao.UserID != cfg.Global.UserId {
-		t.Errorf("UserID %s != %s", ao.UserID, cfg.Global.UserId)
+	if ao.UserID != cfg.Global.UserID {
+		t.Errorf("UserID %s != %s", ao.UserID, cfg.Global.UserID)
 	}
 	if ao.DomainName != cfg.Global.DomainName {
 		t.Errorf("DomainName %s != %s", ao.DomainName, cfg.Global.DomainName)
 	}
-	if ao.TenantID != cfg.Global.TenantId {
-		t.Errorf("TenantID %s != %s", ao.TenantID, cfg.Global.TenantId)
+	if ao.TenantID != cfg.Global.TenantID {
+		t.Errorf("TenantID %s != %s", ao.TenantID, cfg.Global.TenantID)
 	}
 }
 
@@ -210,8 +207,8 @@ func TestCheckOpenStackOpts(t *testing.T) {
 				provider: nil,
 				lbOpts: LoadBalancerOpts{
 					LBVersion:            "v2",
-					SubnetId:             "6261548e-ffde-4bc7-bd22-59c83578c5ef",
-					FloatingNetworkId:    "38b8b5f9-64dc-4424-bf86-679595714786",
+					SubnetID:             "6261548e-ffde-4bc7-bd22-59c83578c5ef",
+					FloatingNetworkID:    "38b8b5f9-64dc-4424-bf86-679595714786",
 					LBMethod:             "ROUND_ROBIN",
 					LBProvider:           "haproxy",
 					CreateMonitor:        true,
@@ -232,7 +229,7 @@ func TestCheckOpenStackOpts(t *testing.T) {
 				provider: nil,
 				lbOpts: LoadBalancerOpts{
 					LBVersion:            "v2",
-					FloatingNetworkId:    "38b8b5f9-64dc-4424-bf86-679595714786",
+					FloatingNetworkID:    "38b8b5f9-64dc-4424-bf86-679595714786",
 					LBMethod:             "ROUND_ROBIN",
 					CreateMonitor:        true,
 					MonitorDelay:         delay,
@@ -252,8 +249,8 @@ func TestCheckOpenStackOpts(t *testing.T) {
 				provider: nil,
 				lbOpts: LoadBalancerOpts{
 					LBVersion:            "v2",
-					SubnetId:             "6261548e-ffde-4bc7-bd22-59c83578c5ef",
-					FloatingNetworkId:    "38b8b5f9-64dc-4424-bf86-679595714786",
+					SubnetID:             "6261548e-ffde-4bc7-bd22-59c83578c5ef",
+					FloatingNetworkID:    "38b8b5f9-64dc-4424-bf86-679595714786",
 					LBMethod:             "ROUND_ROBIN",
 					CreateMonitor:        true,
 					MonitorTimeout:       timeout,
@@ -303,8 +300,8 @@ func TestCheckOpenStackOpts(t *testing.T) {
 				provider: nil,
 				lbOpts: LoadBalancerOpts{
 					LBVersion:            "v2",
-					SubnetId:             "6261548e-ffde-4bc7-bd22-59c83578c5ef",
-					FloatingNetworkId:    "38b8b5f9-64dc-4424-bf86-679595714786",
+					SubnetID:             "6261548e-ffde-4bc7-bd22-59c83578c5ef",
+					FloatingNetworkID:    "38b8b5f9-64dc-4424-bf86-679595714786",
 					LBMethod:             "ROUND_ROBIN",
 					CreateMonitor:        true,
 					MonitorDelay:         delay,
@@ -323,8 +320,8 @@ func TestCheckOpenStackOpts(t *testing.T) {
 				provider: nil,
 				lbOpts: LoadBalancerOpts{
 					LBVersion:            "v2",
-					SubnetId:             "6261548e-ffde-4bc7-bd22-59c83578c5ef",
-					FloatingNetworkId:    "38b8b5f9-64dc-4424-bf86-679595714786",
+					SubnetID:             "6261548e-ffde-4bc7-bd22-59c83578c5ef",
+					FloatingNetworkID:    "38b8b5f9-64dc-4424-bf86-679595714786",
 					LBMethod:             "ROUND_ROBIN",
 					CreateMonitor:        true,
 					MonitorDelay:         delay,
@@ -356,39 +353,39 @@ func TestCaller(t *testing.T) {
 	called := false
 	myFunc := func() { called = true }
 
-	c := NewCaller()
-	c.Call(myFunc)
+	c := newCaller()
+	c.call(myFunc)
 
 	if !called {
-		t.Errorf("Caller failed to call function in default case")
+		t.Errorf("caller failed to call function in default case")
 	}
 
-	c.Disarm()
+	c.disarm()
 	called = false
-	c.Call(myFunc)
+	c.call(myFunc)
 
 	if called {
-		t.Error("Caller still called function when disarmed")
+		t.Error("caller still called function when disarmed")
 	}
 
-	// Confirm the "usual" deferred Caller pattern works as expected
+	// Confirm the "usual" deferred caller pattern works as expected
 
 	called = false
-	success_case := func() {
-		c := NewCaller()
-		defer c.Call(func() { called = true })
-		c.Disarm()
+	successCase := func() {
+		c := newCaller()
+		defer c.call(func() { called = true })
+		c.disarm()
 	}
-	if success_case(); called {
+	if successCase(); called {
 		t.Error("Deferred success case still invoked unwind")
 	}
 
 	called = false
-	failure_case := func() {
-		c := NewCaller()
-		defer c.Call(func() { called = true })
+	failureCase := func() {
+		c := newCaller()
+		defer c.call(func() { called = true })
 	}
-	if failure_case(); !called {
+	if failureCase(); !called {
 		t.Error("Deferred failure case failed to invoke unwind")
 	}
 }
@@ -563,15 +560,15 @@ func TestVolumes(t *testing.T) {
 	if err != nil {
 		t.Logf("Cannot find instance id: %v - perhaps you are running this test outside a VM launched by OpenStack", err)
 	} else {
-		diskId, err := os.AttachDisk(id, vol)
+		diskID, err := os.AttachDisk(id, vol)
 		if err != nil {
 			t.Fatalf("Cannot AttachDisk Cinder volume %s: %v", vol, err)
 		}
-		t.Logf("Volume (%s) attached, disk ID: %s\n", vol, diskId)
+		t.Logf("Volume (%s) attached, disk ID: %s\n", vol, diskID)
 
 		WaitForVolumeStatus(t, os, vol, volumeInUseStatus)
 
-		devicePath := os.GetDevicePath(diskId)
+		devicePath := os.GetDevicePath(diskID)
 		if diskPathRegexp.FindString(devicePath) == "" {
 			t.Fatalf("GetDevicePath returned and unexpected path for Cinder volume %s, returned %s", vol, devicePath)
 		}
@@ -654,10 +651,10 @@ func TestToAuth3Options(t *testing.T) {
 	cfg := Config{}
 	cfg.Global.Username = "user"
 	cfg.Global.Password = "pass"
-	cfg.Global.DomainId = "2a73b8f597c04551a0fdc8e95544be8a"
+	cfg.Global.DomainID = "2a73b8f597c04551a0fdc8e95544be8a"
 	cfg.Global.DomainName = "local"
-	cfg.Global.AuthUrl = "http://auth.url"
-	cfg.Global.UserId = "user"
+	cfg.Global.AuthURL = "http://auth.url"
+	cfg.Global.UserID = "user"
 
 	ao := cfg.toAuth3Options()
 
@@ -670,14 +667,14 @@ func TestToAuth3Options(t *testing.T) {
 	if ao.Password != cfg.Global.Password {
 		t.Errorf("Password %s != %s", ao.Password, cfg.Global.Password)
 	}
-	if ao.DomainID != cfg.Global.DomainId {
-		t.Errorf("DomainID %s != %s", ao.DomainID, cfg.Global.DomainId)
+	if ao.DomainID != cfg.Global.DomainID {
+		t.Errorf("DomainID %s != %s", ao.DomainID, cfg.Global.DomainID)
 	}
-	if ao.IdentityEndpoint != cfg.Global.AuthUrl {
-		t.Errorf("IdentityEndpoint %s != %s", ao.IdentityEndpoint, cfg.Global.AuthUrl)
+	if ao.IdentityEndpoint != cfg.Global.AuthURL {
+		t.Errorf("IdentityEndpoint %s != %s", ao.IdentityEndpoint, cfg.Global.AuthURL)
 	}
-	if ao.UserID != cfg.Global.UserId {
-		t.Errorf("UserID %s != %s", ao.UserID, cfg.Global.UserId)
+	if ao.UserID != cfg.Global.UserID {
+		t.Errorf("UserID %s != %s", ao.UserID, cfg.Global.UserID)
 	}
 	if ao.DomainName != cfg.Global.DomainName {
 		t.Errorf("DomainName %s != %s", ao.DomainName, cfg.Global.DomainName)

pkg/volume/cinder/attacher.go

@@ -35,7 +35,7 @@ import (
 
 type cinderDiskAttacher struct {
 	host           volume.VolumeHost
-	cinderProvider CinderProvider
+	cinderProvider BlockStorageProvider
 }
 
 var _ volume.Attacher = &cinderDiskAttacher{}
@@ -215,7 +215,7 @@ func (attacher *cinderDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath
 	volumeID := volumeSource.VolumeID
 
 	if devicePath == "" {
-		return "", fmt.Errorf("WaitForAttach failed for Cinder disk %q: devicePath is empty.", volumeID)
+		return "", fmt.Errorf("WaitForAttach failed for Cinder disk %q: devicePath is empty", volumeID)
 	}
 
 	ticker := time.NewTicker(probeVolumeInitDelay)
@@ -237,16 +237,15 @@ func (attacher *cinderDiskAttacher) WaitForAttach(spec *volume.Spec, devicePath
 			if exists && err == nil {
 				glog.Infof("Successfully found attached Cinder disk %q at %v.", volumeID, devicePath)
 				return devicePath, nil
-			} else {
-				// Log an error, and continue checking periodically
-				glog.Errorf("Error: could not find attached Cinder disk %q (path: %q): %v", volumeID, devicePath, err)
-				// Using exponential backoff instead of linear
-				ticker.Stop()
-				duration = time.Duration(float64(duration) * probeVolumeFactor)
-				ticker = time.NewTicker(duration)
 			}
+			// Log an error, and continue checking periodically
+			glog.Errorf("Error: could not find attached Cinder disk %q (path: %q): %v", volumeID, devicePath, err)
+			// Using exponential backoff instead of linear
+			ticker.Stop()
+			duration = time.Duration(float64(duration) * probeVolumeFactor)
+			ticker = time.NewTicker(duration)
 		case <-timer.C:
-			return "", fmt.Errorf("Could not find attached Cinder disk %q. Timeout waiting for mount paths to be created.", volumeID)
+			return "", fmt.Errorf("could not find attached Cinder disk %q. Timeout waiting for mount paths to be created", volumeID)
 		}
 	}
 }
@@ -299,7 +298,7 @@ func (attacher *cinderDiskAttacher) MountDevice(spec *volume.Spec, devicePath st
 
 type cinderDiskDetacher struct {
 	mounter        mount.Interface
-	cinderProvider CinderProvider
+	cinderProvider BlockStorageProvider
 }
 
 var _ volume.Detacher = &cinderDiskDetacher{}

pkg/volume/cinder/attacher_test.go

@@ -453,18 +453,18 @@ func (testcase *testcase) AttachDisk(instanceID, volumeID string) (string, error
 	if expected.volumeID == "" && expected.instanceID == "" {
 		// testcase.attach looks uninitialized, test did not expect to call
 		// AttachDisk
-		testcase.t.Errorf("Unexpected AttachDisk call!")
-		return "", errors.New("Unexpected AttachDisk call!")
+		testcase.t.Errorf("unexpected AttachDisk call")
+		return "", errors.New("unexpected AttachDisk call")
 	}
 
 	if expected.volumeID != volumeID {
-		testcase.t.Errorf("Unexpected AttachDisk call: expected volumeID %s, got %s", expected.volumeID, volumeID)
-		return "", errors.New("Unexpected AttachDisk call: wrong volumeID")
+		testcase.t.Errorf("unexpected AttachDisk call: expected volumeID %s, got %s", expected.volumeID, volumeID)
+		return "", errors.New("unexpected AttachDisk call: wrong volumeID")
 	}
 
 	if expected.instanceID != instanceID {
-		testcase.t.Errorf("Unexpected AttachDisk call: expected instanceID %s, got %s", expected.instanceID, instanceID)
-		return "", errors.New("Unexpected AttachDisk call: wrong instanceID")
+		testcase.t.Errorf("unexpected AttachDisk call: expected instanceID %s, got %s", expected.instanceID, instanceID)
+		return "", errors.New("unexpected AttachDisk call: wrong instanceID")
 	}
 
 	glog.V(4).Infof("AttachDisk call: %s, %s, returning %q, %v", volumeID, instanceID, expected.retDeviceName, expected.ret)
@@ -479,18 +479,18 @@ func (testcase *testcase) DetachDisk(instanceID, volumeID string) error {
 	if expected.devicePath == "" && expected.instanceID == "" {
 		// testcase.detach looks uninitialized, test did not expect to call
 		// DetachDisk
-		testcase.t.Errorf("Unexpected DetachDisk call!")
-		return errors.New("Unexpected DetachDisk call!")
+		testcase.t.Errorf("unexpected DetachDisk call")
+		return errors.New("unexpected DetachDisk call")
 	}
 
 	if expected.devicePath != volumeID {
-		testcase.t.Errorf("Unexpected DetachDisk call: expected volumeID %s, got %s", expected.devicePath, volumeID)
-		return errors.New("Unexpected DetachDisk call: wrong volumeID")
+		testcase.t.Errorf("unexpected DetachDisk call: expected volumeID %s, got %s", expected.devicePath, volumeID)
+		return errors.New("unexpected DetachDisk call: wrong volumeID")
 	}
 
 	if expected.instanceID != instanceID {
-		testcase.t.Errorf("Unexpected DetachDisk call: expected instanceID %s, got %s", expected.instanceID, instanceID)
-		return errors.New("Unexpected DetachDisk call: wrong instanceID")
+		testcase.t.Errorf("unexpected DetachDisk call: expected instanceID %s, got %s", expected.instanceID, instanceID)
+		return errors.New("unexpected DetachDisk call: wrong instanceID")
 	}
 
 	glog.V(4).Infof("DetachDisk call: %s, %s, returning %v", volumeID, instanceID, expected.ret)
@@ -527,18 +527,18 @@ func (testcase *testcase) DiskIsAttached(instanceID, volumeID string) (bool, err
 	if expected.volumeID == "" && expected.instanceID == "" {
 		// testcase.diskIsAttached looks uninitialized, test did not expect to
 		// call DiskIsAttached
-		testcase.t.Errorf("Unexpected DiskIsAttached call!")
-		return false, errors.New("Unexpected DiskIsAttached call!")
+		testcase.t.Errorf("unexpected DiskIsAttached call")
+		return false, errors.New("unexpected DiskIsAttached call")
 	}
 
 	if expected.volumeID != volumeID {
-		testcase.t.Errorf("Unexpected DiskIsAttached call: expected volumeID %s, got %s", expected.volumeID, volumeID)
-		return false, errors.New("Unexpected DiskIsAttached call: wrong volumeID")
+		testcase.t.Errorf("unexpected DiskIsAttached call: expected volumeID %s, got %s", expected.volumeID, volumeID)
+		return false, errors.New("unexpected DiskIsAttached call: wrong volumeID")
 	}
 
 	if expected.instanceID != instanceID {
-		testcase.t.Errorf("Unexpected DiskIsAttached call: expected instanceID %s, got %s", expected.instanceID, instanceID)
-		return false, errors.New("Unexpected DiskIsAttached call: wrong instanceID")
+		testcase.t.Errorf("unexpected DiskIsAttached call: expected instanceID %s, got %s", expected.instanceID, instanceID)
+		return false, errors.New("unexpected DiskIsAttached call: wrong instanceID")
 	}
 
 	glog.V(4).Infof("DiskIsAttached call: %s, %s, returning %v, %v", volumeID, instanceID, expected.isAttached, expected.ret)
@@ -551,18 +551,18 @@ func (testcase *testcase) GetAttachmentDiskPath(instanceID, volumeID string) (st
 	if expected.volumeID == "" && expected.instanceID == "" {
 		// testcase.diskPath looks uninitialized, test did not expect to
 		// call GetAttachmentDiskPath
-		testcase.t.Errorf("Unexpected GetAttachmentDiskPath call!")
-		return "", errors.New("Unexpected GetAttachmentDiskPath call!")
+		testcase.t.Errorf("unexpected GetAttachmentDiskPath call")
+		return "", errors.New("unexpected GetAttachmentDiskPath call")
 	}
 
 	if expected.volumeID != volumeID {
-		testcase.t.Errorf("Unexpected GetAttachmentDiskPath call: expected volumeID %s, got %s", expected.volumeID, volumeID)
-		return "", errors.New("Unexpected GetAttachmentDiskPath call: wrong volumeID")
+		testcase.t.Errorf("unexpected GetAttachmentDiskPath call: expected volumeID %s, got %s", expected.volumeID, volumeID)
+		return "", errors.New("unexpected GetAttachmentDiskPath call: wrong volumeID")
 	}
 
 	if expected.instanceID != instanceID {
-		testcase.t.Errorf("Unexpected GetAttachmentDiskPath call: expected instanceID %s, got %s", expected.instanceID, instanceID)
-		return "", errors.New("Unexpected GetAttachmentDiskPath call: wrong instanceID")
+		testcase.t.Errorf("unexpected GetAttachmentDiskPath call: expected instanceID %s, got %s", expected.instanceID, instanceID)
+		return "", errors.New("unexpected GetAttachmentDiskPath call: wrong instanceID")
 	}
 
 	glog.V(4).Infof("GetAttachmentDiskPath call: %s, %s, returning %v, %v", volumeID, instanceID, expected.retPath, expected.ret)
@@ -588,25 +588,25 @@ func (testcase *testcase) DiskIsAttachedByName(nodeName types.NodeName, volumeID
 	}
 
 	if expected.nodeName != nodeName {
-		testcase.t.Errorf("Unexpected DiskIsAttachedByName call: expected nodename %s, got %s", expected.nodeName, nodeName)
-		return false, instanceID, errors.New("Unexpected DiskIsAttachedByName call: wrong nodename")
+		testcase.t.Errorf("unexpected DiskIsAttachedByName call: expected nodename %s, got %s", expected.nodeName, nodeName)
+		return false, instanceID, errors.New("unexpected DiskIsAttachedByName call: wrong nodename")
 	}
 
 	if expected.volumeID == "" && expected.instanceID == "" {
 		// testcase.diskIsAttached looks uninitialized, test did not expect to
 		// call DiskIsAttached
-		testcase.t.Errorf("Unexpected DiskIsAttachedByName call!")
-		return false, instanceID, errors.New("Unexpected DiskIsAttachedByName call!")
+		testcase.t.Errorf("unexpected DiskIsAttachedByName call")
+		return false, instanceID, errors.New("unexpected DiskIsAttachedByName call")
 	}
 
 	if expected.volumeID != volumeID {
-		testcase.t.Errorf("Unexpected DiskIsAttachedByName call: expected volumeID %s, got %s", expected.volumeID, volumeID)
-		return false, instanceID, errors.New("Unexpected DiskIsAttachedByName call: wrong volumeID")
+		testcase.t.Errorf("unexpected DiskIsAttachedByName call: expected volumeID %s, got %s", expected.volumeID, volumeID)
+		return false, instanceID, errors.New("unexpected DiskIsAttachedByName call: wrong volumeID")
 	}
 
 	if expected.instanceID != instanceID {
-		testcase.t.Errorf("Unexpected DiskIsAttachedByName call: expected instanceID %s, got %s", expected.instanceID, instanceID)
-		return false, instanceID, errors.New("Unexpected DiskIsAttachedByName call: wrong instanceID")
+		testcase.t.Errorf("unexpected DiskIsAttachedByName call: expected instanceID %s, got %s", expected.instanceID, instanceID)
+		return false, instanceID, errors.New("unexpected DiskIsAttachedByName call: wrong instanceID")
 	}
 
 	glog.V(4).Infof("DiskIsAttachedByName call: %s, %s, returning %v, %v", volumeID, nodeName, expected.isAttached, expected.instanceID, expected.ret)

pkg/volume/cinder/cinder.go

@@ -38,15 +38,17 @@ import (
 )
 
 const (
+	// DefaultCloudConfigPath is the default path for cloud configuration
 	DefaultCloudConfigPath = "/etc/kubernetes/cloud-config"
 )
 
-// This is the primary entrypoint for volume plugins.
+// ProbeVolumePlugins is the primary entrypoint for volume plugins.
 func ProbeVolumePlugins() []volume.VolumePlugin {
 	return []volume.VolumePlugin{&cinderPlugin{}}
 }
 
-type CinderProvider interface {
+// BlockStorageProvider is the interface for accessing cinder functionality.
+type BlockStorageProvider interface {
 	AttachDisk(instanceID, volumeID string) (string, error)
 	DetachDisk(instanceID, volumeID string) error
 	DeleteVolume(volumeID string) error
@@ -120,7 +122,7 @@ func (plugin *cinderPlugin) GetAccessModes() []v1.PersistentVolumeAccessMode {
 }
 
 func (plugin *cinderPlugin) NewMounter(spec *volume.Spec, pod *v1.Pod, _ volume.VolumeOptions) (volume.Mounter, error) {
-	return plugin.newMounterInternal(spec, pod.UID, &CinderDiskUtil{}, plugin.host.GetMounter(plugin.GetPluginName()))
+	return plugin.newMounterInternal(spec, pod.UID, &DiskUtil{}, plugin.host.GetMounter(plugin.GetPluginName()))
 }
 
 func (plugin *cinderPlugin) newMounterInternal(spec *volume.Spec, podUID types.UID, manager cdManager, mounter mount.Interface) (volume.Mounter, error) {
@@ -147,7 +149,7 @@ func (plugin *cinderPlugin) newMounterInternal(spec *volume.Spec, podUID types.U
 }
 
 func (plugin *cinderPlugin) NewUnmounter(volName string, podUID types.UID) (volume.Unmounter, error) {
-	return plugin.newUnmounterInternal(volName, podUID, &CinderDiskUtil{}, plugin.host.GetMounter(plugin.GetPluginName()))
+	return plugin.newUnmounterInternal(volName, podUID, &DiskUtil{}, plugin.host.GetMounter(plugin.GetPluginName()))
 }
 
 func (plugin *cinderPlugin) newUnmounterInternal(volName string, podUID types.UID, manager cdManager, mounter mount.Interface) (volume.Unmounter, error) {
@@ -162,7 +164,7 @@ func (plugin *cinderPlugin) newUnmounterInternal(volName string, podUID types.UI
 }
 
 func (plugin *cinderPlugin) NewDeleter(spec *volume.Spec) (volume.Deleter, error) {
-	return plugin.newDeleterInternal(spec, &CinderDiskUtil{})
+	return plugin.newDeleterInternal(spec, &DiskUtil{})
 }
 
 func (plugin *cinderPlugin) newDeleterInternal(spec *volume.Spec, manager cdManager) (volume.Deleter, error) {
@@ -179,7 +181,7 @@ func (plugin *cinderPlugin) newDeleterInternal(spec *volume.Spec, manager cdMana
 }
 
 func (plugin *cinderPlugin) NewProvisioner(options volume.VolumeOptions) (volume.Provisioner, error) {
-	return plugin.newProvisionerInternal(options, &CinderDiskUtil{})
+	return plugin.newProvisionerInternal(options, &DiskUtil{})
 }
 
 func (plugin *cinderPlugin) newProvisionerInternal(options volume.VolumeOptions, manager cdManager) (volume.Provisioner, error) {
@@ -192,23 +194,22 @@ func (plugin *cinderPlugin) newProvisionerInternal(options volume.VolumeOptions,
 	}, nil
 }
 
-func (plugin *cinderPlugin) getCloudProvider() (CinderProvider, error) {
+func (plugin *cinderPlugin) getCloudProvider() (BlockStorageProvider, error) {
 	cloud := plugin.host.GetCloudProvider()
 	if cloud == nil {
 		if _, err := os.Stat(DefaultCloudConfigPath); err == nil {
 			var config *os.File
 			config, err = os.Open(DefaultCloudConfigPath)
 			if err != nil {
-				return nil, errors.New(fmt.Sprintf("unable to load OpenStack configuration from default path : %v", err))
-			} else {
-				defer config.Close()
-				cloud, err = cloudprovider.GetCloudProvider(openstack.ProviderName, config)
-				if err != nil {
-					return nil, errors.New(fmt.Sprintf("unable to create OpenStack cloud provider from default path : %v", err))
-				}
+				return nil, fmt.Errorf("unable to load OpenStack configuration from default path : %v", err)
+			}
+			defer config.Close()
+			cloud, err = cloudprovider.GetCloudProvider(openstack.ProviderName, config)
+			if err != nil {
+				return nil, fmt.Errorf("unable to create OpenStack cloud provider from default path : %v", err)
 			}
 		} else {
-			return nil, errors.New(fmt.Sprintf("OpenStack cloud provider was not initialized properly : %v", err))
+			return nil, fmt.Errorf("OpenStack cloud provider was not initialized properly : %v", err)
 		}
 	}
 

pkg/volume/cinder/cinder_util.go

@@ -35,11 +35,12 @@ import (
 	"k8s.io/utils/exec"
 )
 
-type CinderDiskUtil struct{}
+// DiskUtil has utility/helper methods
+type DiskUtil struct{}
 
-// Attaches a disk specified by a volume.CinderPersistenDisk to the current kubelet.
+// AttachDisk attaches a disk specified by a volume.CinderPersistenDisk to the current kubelet.
 // Mounts the disk to its global path.
-func (util *CinderDiskUtil) AttachDisk(b *cinderVolumeMounter, globalPDPath string) error {
+func (util *DiskUtil) AttachDisk(b *cinderVolumeMounter, globalPDPath string) error {
 	options := []string{}
 	if b.readOnly {
 		options = append(options, "ro")
@@ -98,8 +99,8 @@ func (util *CinderDiskUtil) AttachDisk(b *cinderVolumeMounter, globalPDPath stri
 	return nil
 }
 
-// Unmounts the device and detaches the disk from the kubelet's host machine.
-func (util *CinderDiskUtil) DetachDisk(cd *cinderVolumeUnmounter) error {
+// DetachDisk unmounts the device and detaches the disk from the kubelet's host machine.
+func (util *DiskUtil) DetachDisk(cd *cinderVolumeUnmounter) error {
 	globalPDPath := makeGlobalPDName(cd.plugin.host, cd.pdName)
 	if err := cd.mounter.Unmount(globalPDPath); err != nil {
 		return err
@@ -124,7 +125,8 @@ func (util *CinderDiskUtil) DetachDisk(cd *cinderVolumeUnmounter) error {
 	return nil
 }
 
-func (util *CinderDiskUtil) DeleteVolume(cd *cinderVolumeDeleter) error {
+// DeleteVolume uses the cloud entrypoint to delete specified volume
+func (util *DiskUtil) DeleteVolume(cd *cinderVolumeDeleter) error {
 	cloud, err := cd.plugin.getCloudProvider()
 	if err != nil {
 		return err
@@ -158,7 +160,8 @@ func getZonesFromNodes(kubeClient clientset.Interface) (sets.String, error) {
 	return zones, nil
 }
 
-func (util *CinderDiskUtil) CreateVolume(c *cinderVolumeProvisioner) (volumeID string, volumeSizeGB int, volumeLabels map[string]string, fstype string, err error) {
+// CreateVolume uses the cloud provider entrypoint for creating a volume
+func (util *DiskUtil) CreateVolume(c *cinderVolumeProvisioner) (volumeID string, volumeSizeGB int, volumeLabels map[string]string, fstype string, err error) {
 	cloud, err := c.plugin.getCloudProvider()
 	if err != nil {
 		return "", 0, nil, "", err
@@ -247,10 +250,10 @@ func probeAttachedVolume() error {
 }
 
 func scsiHostRescan() {
-	scsi_path := "/sys/class/scsi_host/"
-	if dirs, err := ioutil.ReadDir(scsi_path); err == nil {
+	scsiPath := "/sys/class/scsi_host/"
+	if dirs, err := ioutil.ReadDir(scsiPath); err == nil {
 		for _, f := range dirs {
-			name := scsi_path + f.Name() + "/scan"
+			name := scsiPath + f.Name() + "/scan"
 			data := []byte("- - -")
 			ioutil.WriteFile(name, data, 0666)
 		}