From 8af03d0faedf93f8765dcbc3bddd33f1026b7186 Mon Sep 17 00:00:00 2001 From: NickrenREN Date: Fri, 20 Oct 2017 15:02:37 +0800 Subject: [PATCH] let the caller log error message --- .../providers/openstack/metadata.go | 19 +-- .../providers/openstack/openstack.go | 26 ++-- .../providers/openstack/openstack_client.go | 16 +-- .../openstack/openstack_loadbalancer.go | 136 +++++++++--------- .../providers/openstack/openstack_test.go | 8 +- .../providers/openstack/openstack_volumes.go | 65 +++------ 6 files changed, 110 insertions(+), 160 deletions(-) diff --git a/pkg/cloudprovider/providers/openstack/metadata.go b/pkg/cloudprovider/providers/openstack/metadata.go index 18fb4a5b0fb..959d91b92e5 100644 --- a/pkg/cloudprovider/providers/openstack/metadata.go +++ b/pkg/cloudprovider/providers/openstack/metadata.go @@ -52,7 +52,7 @@ const ( configDriveID = "configDrive" ) -var ErrBadMetadata = errors.New("Invalid OpenStack metadata, got empty uuid") +var ErrBadMetadata = errors.New("invalid OpenStack metadata, got empty uuid") // Assumes the "2012-08-10" meta_data.json format. // See http://docs.openstack.org/user-guide/cli_config_drive.html @@ -89,8 +89,7 @@ func getMetadataFromConfigDrive() (*Metadata, error) { "-o", "device", ).CombinedOutput() if err != nil { - glog.V(2).Infof("Unable to run blkid: %v", err) - return nil, err + return nil, fmt.Errorf("unable to run blkid: %v", err) } dev = strings.TrimSpace(string(out)) } @@ -109,8 +108,7 @@ func getMetadataFromConfigDrive() (*Metadata, error) { err = mounter.Mount(dev, mntdir, "vfat", []string{"ro"}) } if err != nil { - glog.Errorf("Error mounting configdrive %s: %v", dev, err) - return nil, err + return nil, fmt.Errorf("error mounting configdrive %s: %v", dev, err) } defer mounter.Unmount(mntdir) @@ -119,8 +117,7 @@ func getMetadataFromConfigDrive() (*Metadata, error) { f, err := os.Open( filepath.Join(mntdir, configDrivePath)) if err != nil { - glog.Errorf("Error reading %s on config drive: %v", configDrivePath, err) - return nil, err + return nil, fmt.Errorf("error reading %s on config drive: %v", configDrivePath, err) } defer f.Close() @@ -128,18 +125,16 @@ func getMetadataFromConfigDrive() (*Metadata, error) { } func getMetadataFromMetadataService() (*Metadata, error) { - // Try to get JSON from metdata server. + // Try to get JSON from metadata server. glog.V(4).Infof("Attempting to fetch metadata from %s", metadataUrl) resp, err := http.Get(metadataUrl) if err != nil { - glog.V(3).Infof("Cannot read %s: %v", metadataUrl, err) - return nil, 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) - glog.V(3).Infof("%v", err) + err = fmt.Errorf("unexpected status code when reading metadata from %s: %s", metadataUrl, resp.Status) return nil, err } diff --git a/pkg/cloudprovider/providers/openstack/openstack.go b/pkg/cloudprovider/providers/openstack/openstack.go index e8dbd36336d..d21bd0a14a1 100644 --- a/pkg/cloudprovider/providers/openstack/openstack.go +++ b/pkg/cloudprovider/providers/openstack/openstack.go @@ -53,9 +53,9 @@ const ( defaultTimeOut = 60 * time.Second ) -var ErrNotFound = errors.New("Failed to find object") -var ErrMultipleResults = errors.New("Multiple results where only one expected") -var ErrNoAddressFound = errors.New("No address found for host") +var ErrNotFound = errors.New("failed to find object") +var ErrMultipleResults = errors.New("multiple results where only one expected") +var ErrNoAddressFound = errors.New("no address found for host") // encoding.TextUnmarshaler interface for time.Duration type MyDuration struct { @@ -179,8 +179,7 @@ func (cfg Config) toAuth3Options() tokens3.AuthOptions { func readConfig(config io.Reader) (Config, error) { if config == nil { - err := fmt.Errorf("no OpenStack cloud provider config file given") - return Config{}, err + return Config{}, fmt.Errorf("no OpenStack cloud provider config file given") } var cfg Config @@ -549,7 +548,6 @@ func isNotFound(err error) bool { func (os *OpenStack) Zones() (cloudprovider.Zones, bool) { glog.V(1).Info("Claiming to support Zones") - return os, true } @@ -563,8 +561,7 @@ func (os *OpenStack) GetZone() (cloudprovider.Zone, error) { FailureDomain: md.AvailabilityZone, Region: os.region, } - glog.V(1).Infof("Current zone is %v", zone) - + glog.V(4).Infof("Current zone is %v", zone) return zone, nil } @@ -592,7 +589,6 @@ func (os *OpenStack) GetZoneByProviderID(providerID string) (cloudprovider.Zone, Region: os.region, } glog.V(4).Infof("The instance %s in zone %v", srv.Name, zone) - return zone, nil } @@ -618,7 +614,6 @@ func (os *OpenStack) GetZoneByNodeName(nodeName types.NodeName) (cloudprovider.Z Region: os.region, } glog.V(4).Infof("The instance %s in zone %v", srv.Name, zone) - return zone, nil } @@ -653,7 +648,6 @@ func (os *OpenStack) Routes() (cloudprovider.Routes, bool) { } glog.V(1).Info("Claiming to support Routes") - return r, true } @@ -703,19 +697,18 @@ func (os *OpenStack) volumeService(forceVersion string) (volumeService, error) { } default: err_txt := fmt.Sprintf("Config error: unrecognised bs-version \"%v\"", os.bsOpts.BSVersion) - glog.Warningf(err_txt) return nil, errors.New(err_txt) } } func checkMetadataSearchOrder(order string) error { if order == "" { - return errors.New("Invalid value in section [Metadata] with key `search-order`. Value cannot be empty") + return errors.New("invalid value in section [Metadata] with key `search-order`. Value cannot be empty") } elements := strings.Split(order, ",") if len(elements) > 2 { - return errors.New("Invalid value in section [Metadata] with key `search-order`. Value cannot contain more than 2 elements") + return errors.New("invalid value in section [Metadata] with key `search-order`. Value cannot contain more than 2 elements") } for _, id := range elements { @@ -724,9 +717,8 @@ func checkMetadataSearchOrder(order string) error { case configDriveID: case metadataID: default: - errTxt := "Invalid element '%s' found in section [Metadata] with key `search-order`." + - "Supported elements include '%s' and '%s'" - return fmt.Errorf(errTxt, id, configDriveID, metadataID) + return fmt.Errorf("invalid element %q found in section [Metadata] with key `search-order`."+ + "Supported elements include %q and %q", id, configDriveID, metadataID) } } diff --git a/pkg/cloudprovider/providers/openstack/openstack_client.go b/pkg/cloudprovider/providers/openstack/openstack_client.go index 916c4a09c56..7d18ce42b08 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_client.go +++ b/pkg/cloudprovider/providers/openstack/openstack_client.go @@ -17,10 +17,10 @@ limitations under the License. package openstack import ( + "fmt" + "github.com/gophercloud/gophercloud" "github.com/gophercloud/gophercloud/openstack" - - "github.com/golang/glog" ) func (os *OpenStack) NewNetworkV2() (*gophercloud.ServiceClient, error) { @@ -28,8 +28,7 @@ func (os *OpenStack) NewNetworkV2() (*gophercloud.ServiceClient, error) { Region: os.region, }) if err != nil { - glog.Warningf("Failed to find network v2 endpoint for region %s: %v", os.region, err) - return nil, err + return nil, fmt.Errorf("failed to find network v2 endpoint for region %s: %v", os.region, err) } return network, nil } @@ -39,8 +38,7 @@ func (os *OpenStack) NewComputeV2() (*gophercloud.ServiceClient, error) { Region: os.region, }) if err != nil { - glog.Warningf("Failed to find compute v2 endpoint for region %s: %v", os.region, err) - return nil, err + return nil, fmt.Errorf("failed to find compute v2 endpoint for region %s: %v", os.region, err) } return compute, nil } @@ -50,8 +48,7 @@ func (os *OpenStack) NewBlockStorageV1() (*gophercloud.ServiceClient, error) { Region: os.region, }) if err != nil { - glog.Errorf("Unable to initialize cinder v1 client for region %s: %v", os.region, err) - return nil, err + return nil, fmt.Errorf("unable to initialize cinder v1 client for region %s: %v", os.region, err) } return storage, nil } @@ -61,8 +58,7 @@ func (os *OpenStack) NewBlockStorageV2() (*gophercloud.ServiceClient, error) { Region: os.region, }) if err != nil { - glog.Errorf("Unable to initialize cinder v2 client for region %s: %v", os.region, err) - return nil, err + return nil, fmt.Errorf("unable to initialize cinder v2 client for region %s: %v", os.region, err) } return storage, nil } diff --git a/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go b/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go index c55f0e22a48..cf2713b444f 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go +++ b/pkg/cloudprovider/providers/openstack/openstack_loadbalancer.go @@ -649,7 +649,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv 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) if len(nodes) == 0 { - return nil, fmt.Errorf("There are no available nodes for LoadBalancer service %s/%s", apiService.Namespace, apiService.Name) + return nil, fmt.Errorf("there are no available nodes for LoadBalancer service %s/%s", apiService.Namespace, apiService.Name) } if len(lbaas.opts.SubnetId) == 0 { @@ -658,7 +658,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv 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, "+ + 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 @@ -683,10 +683,10 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv glog.V(4).Infof("Ensure an external loadbalancer service.") internalAnnotation = false } else { - return nil, fmt.Errorf("floating-network-id or loadbalancer.openstack.org/floating-network-id should be specified when ensuring an external loadbalancer service.") + return nil, fmt.Errorf("floating-network-id or loadbalancer.openstack.org/floating-network-id should be specified when ensuring an external loadbalancer service") } default: - return nil, fmt.Errorf("unknow service.beta.kubernetes.io/openstack-internal-load-balancer annotation: %v, specify \"true\" or \"false\".", + return nil, fmt.Errorf("unknown service.beta.kubernetes.io/openstack-internal-load-balancer annotation: %v, specify \"true\" or \"false\" ", internal) } @@ -694,17 +694,17 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv // TODO: Convert all error messages to use an event recorder for _, port := range ports { if port.Protocol != v1.ProtocolTCP { - return nil, fmt.Errorf("Only TCP LoadBalancer is supported for openstack load balancers") + return nil, fmt.Errorf("only TCP LoadBalancer is supported for openstack load balancers") } } sourceRanges, err := service.GetLoadBalancerSourceRanges(apiService) if err != nil { - return nil, fmt.Errorf("Failed to get source ranges for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) + return nil, fmt.Errorf("failed to get source ranges for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) } if !service.IsAllowAll(sourceRanges) && !lbaas.opts.ManageSecurityGroups { - return nil, fmt.Errorf("Source range restrictions are not supported for openstack load balancers without managing security groups") + return nil, fmt.Errorf("source range restrictions are not supported for openstack load balancers without managing security groups") } affinity := apiService.Spec.SessionAffinity @@ -722,13 +722,13 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv loadbalancer, err := getLoadbalancerByName(lbaas.network, name) if err != nil { if err != ErrNotFound { - return nil, fmt.Errorf("Error getting loadbalancer %s: %v", name, err) + return nil, fmt.Errorf("error getting loadbalancer %s: %v", name, err) } glog.V(2).Infof("Creating loadbalancer %s", name) loadbalancer, err = lbaas.createLoadBalancer(apiService, name, internalAnnotation) if err != nil { // Unknown error, retry later - return nil, fmt.Errorf("Error creating loadbalancer %s: %v", name, err) + return nil, fmt.Errorf("error creating loadbalancer %s: %v", name, err) } } else { glog.V(2).Infof("LoadBalancer %s already exists", name) @@ -743,7 +743,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv oldListeners, err := getListenersByLoadBalancerID(lbaas.network, loadbalancer.ID) if err != nil { - return nil, fmt.Errorf("Error getting LB %s listeners: %v", name, err) + return nil, fmt.Errorf("error getting LB %s listeners: %v", name, err) } for portIndex, port := range ports { listener := getListenerForPort(oldListeners, port) @@ -757,7 +757,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv }).Extract() if err != nil { // Unknown error, retry later - return nil, fmt.Errorf("Error creating LB listener: %v", err) + return nil, fmt.Errorf("error creating LB listener: %v", err) } waitLoadbalancerActiveProvisioningStatus(lbaas.network, loadbalancer.ID) } @@ -770,7 +770,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv pool, err := getPoolByListenerID(lbaas.network, loadbalancer.ID, listener.ID) if err != nil && err != ErrNotFound { // Unknown error, retry later - return nil, fmt.Errorf("Error getting pool for listener %s: %v", listener.ID, err) + return nil, fmt.Errorf("error getting pool for listener %s: %v", listener.ID, err) } if pool == nil { glog.V(4).Infof("Creating pool for listener %s", listener.ID) @@ -783,7 +783,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv }).Extract() if err != nil { // Unknown error, retry later - return nil, fmt.Errorf("Error creating pool for listener %s: %v", listener.ID, err) + return nil, fmt.Errorf("error creating pool for listener %s: %v", listener.ID, err) } waitLoadbalancerActiveProvisioningStatus(lbaas.network, loadbalancer.ID) } @@ -791,7 +791,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv glog.V(4).Infof("Pool for listener %s: %s", listener.ID, pool.ID) members, err := getMembersByPoolID(lbaas.network, pool.ID) if err != nil && !isNotFound(err) { - return nil, fmt.Errorf("Error getting pool members %s: %v", pool.ID, err) + return nil, fmt.Errorf("error getting pool members %s: %v", pool.ID, err) } for _, node := range nodes { addr, err := nodeAddressForLB(node) @@ -801,7 +801,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv glog.Warningf("Failed to create LB pool member for node %s: %v", node.Name, err) continue } else { - return nil, fmt.Errorf("Error getting address for node %s: %v", node.Name, err) + return nil, fmt.Errorf("error getting address for node %s: %v", node.Name, err) } } @@ -813,7 +813,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv SubnetID: lbaas.opts.SubnetId, }).Extract() if err != nil { - return nil, fmt.Errorf("Error creating LB pool member for node: %s, %v", node.Name, err) + return nil, fmt.Errorf("error creating LB pool member for node: %s, %v", node.Name, err) } waitLoadbalancerActiveProvisioningStatus(lbaas.network, loadbalancer.ID) @@ -830,7 +830,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv glog.V(4).Infof("Deleting obsolete member %s for pool %s address %s", member.ID, pool.ID, member.Address) err := v2pools.DeleteMember(lbaas.network, pool.ID, member.ID).ExtractErr() if err != nil && !isNotFound(err) { - return nil, fmt.Errorf("Error deleting obsolete member %s for pool %s address %s: %v", member.ID, pool.ID, member.Address, err) + return nil, fmt.Errorf("error deleting obsolete member %s for pool %s address %s: %v", member.ID, pool.ID, member.Address, err) } waitLoadbalancerActiveProvisioningStatus(lbaas.network, loadbalancer.ID) } @@ -846,7 +846,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv MaxRetries: int(lbaas.opts.MonitorMaxRetries), }).Extract() if err != nil { - return nil, fmt.Errorf("Error creating LB pool healthmonitor: %v", err) + return nil, fmt.Errorf("error creating LB pool healthmonitor: %v", err) } waitLoadbalancerActiveProvisioningStatus(lbaas.network, loadbalancer.ID) monitorID = monitor.ID @@ -865,7 +865,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv // get pool for listener pool, err := getPoolByListenerID(lbaas.network, loadbalancer.ID, listener.ID) if err != nil && err != ErrNotFound { - return nil, fmt.Errorf("Error getting pool for obsolete listener %s: %v", listener.ID, err) + return nil, fmt.Errorf("error getting pool for obsolete listener %s: %v", listener.ID, err) } if pool != nil { // get and delete monitor @@ -874,21 +874,21 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv glog.V(4).Infof("Deleting obsolete monitor %s for pool %s", monitorID, pool.ID) err = v2monitors.Delete(lbaas.network, monitorID).ExtractErr() if err != nil && !isNotFound(err) { - return nil, fmt.Errorf("Error deleting obsolete monitor %s for pool %s: %v", monitorID, pool.ID, err) + return nil, fmt.Errorf("error deleting obsolete monitor %s for pool %s: %v", monitorID, pool.ID, err) } waitLoadbalancerActiveProvisioningStatus(lbaas.network, loadbalancer.ID) } // get and delete pool members members, err := getMembersByPoolID(lbaas.network, pool.ID) if err != nil && !isNotFound(err) { - return nil, fmt.Errorf("Error getting members for pool %s: %v", pool.ID, err) + return nil, fmt.Errorf("error getting members for pool %s: %v", pool.ID, err) } if members != nil { for _, member := range members { glog.V(4).Infof("Deleting obsolete member %s for pool %s address %s", member.ID, pool.ID, member.Address) err := v2pools.DeleteMember(lbaas.network, pool.ID, member.ID).ExtractErr() if err != nil && !isNotFound(err) { - return nil, fmt.Errorf("Error deleting obsolete member %s for pool %s address %s: %v", member.ID, pool.ID, member.Address, err) + return nil, fmt.Errorf("error deleting obsolete member %s for pool %s address %s: %v", member.ID, pool.ID, member.Address, err) } waitLoadbalancerActiveProvisioningStatus(lbaas.network, loadbalancer.ID) } @@ -897,14 +897,14 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv // delete pool err = v2pools.Delete(lbaas.network, pool.ID).ExtractErr() if err != nil && !isNotFound(err) { - return nil, fmt.Errorf("Error deleting obsolete pool %s for listener %s: %v", pool.ID, listener.ID, err) + return nil, fmt.Errorf("error deleting obsolete pool %s for listener %s: %v", pool.ID, listener.ID, err) } waitLoadbalancerActiveProvisioningStatus(lbaas.network, loadbalancer.ID) } // delete listener err = listeners.Delete(lbaas.network, listener.ID).ExtractErr() if err != nil && !isNotFound(err) { - return nil, fmt.Errorf("Error deleteting obsolete listener: %v", err) + return nil, fmt.Errorf("error deleteting obsolete listener: %v", err) } waitLoadbalancerActiveProvisioningStatus(lbaas.network, loadbalancer.ID) glog.V(2).Infof("Deleted obsolete listener: %s", listener.ID) @@ -913,7 +913,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv portID := loadbalancer.VipPortID floatIP, err := getFloatingIPByPortID(lbaas.network, portID) if err != nil && err != ErrNotFound { - return nil, fmt.Errorf("Error getting floating ip for port %s: %v", portID, err) + return nil, fmt.Errorf("error getting floating ip for port %s: %v", portID, err) } if floatIP == nil && floatingPool != "" && !internalAnnotation { glog.V(4).Infof("Creating floating ip for loadbalancer %s port %s", loadbalancer.ID, portID) @@ -929,7 +929,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancer(clusterName string, apiService *v1.Serv floatIP, err = floatingips.Create(lbaas.network, floatIPOpts).Extract() if err != nil { - return nil, fmt.Errorf("Error creating LB floatingip %+v: %v", floatIPOpts, err) + return nil, fmt.Errorf("error creating LB floatingip %+v: %v", floatIPOpts, err) } } @@ -961,7 +961,7 @@ func (lbaas *LbaasV2) ensureSecurityGroup(clusterName string, apiService *v1.Ser if len(lbaas.opts.NodeSecurityGroupIDs) == 0 { lbaas.opts.NodeSecurityGroupIDs, err = getNodeSecurityGroupIDForLB(lbaas.compute, nodes) if err != nil { - return fmt.Errorf("Failed to find node-security-group for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) + return fmt.Errorf("failed to find node-security-group for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) } } glog.V(4).Infof("find node-security-group %v for loadbalancer service %s/%s", lbaas.opts.NodeSecurityGroupIDs, apiService.Namespace, apiService.Name) @@ -975,7 +975,7 @@ func (lbaas *LbaasV2) ensureSecurityGroup(clusterName string, apiService *v1.Ser // get service source ranges sourceRanges, err := service.GetLoadBalancerSourceRanges(apiService) if err != nil { - return fmt.Errorf("Failed to get source ranges for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) + return fmt.Errorf("failed to get source ranges for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) } // ensure security group for LB @@ -988,7 +988,7 @@ func (lbaas *LbaasV2) ensureSecurityGroup(clusterName string, apiService *v1.Ser // create it later lbSecGroupID = "" } 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) } } if len(lbSecGroupID) == 0 { @@ -1000,7 +1000,7 @@ func (lbaas *LbaasV2) ensureSecurityGroup(clusterName string, apiService *v1.Ser lbSecGroup, err := groups.Create(lbaas.network, lbSecGroupCreateOpts).Extract() if err != nil { - return fmt.Errorf("Failed to create Security Group for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) + return fmt.Errorf("failed to create Security Group for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) } lbSecGroupID = lbSecGroup.ID @@ -1011,7 +1011,7 @@ func (lbaas *LbaasV2) ensureSecurityGroup(clusterName string, apiService *v1.Ser network, _, err := net.ParseCIDR(sourceRange) if err != nil { - return fmt.Errorf("Error parsing source range %s as a CIDR: %v", sourceRange, err) + return fmt.Errorf("error parsing source range %s as a CIDR: %v", sourceRange, err) } if network.To4() == nil { @@ -1031,7 +1031,7 @@ func (lbaas *LbaasV2) ensureSecurityGroup(clusterName string, apiService *v1.Ser _, err = rules.Create(lbaas.network, lbSecGroupRuleCreateOpts).Extract() if err != nil { - return fmt.Errorf("Error occured creating rule for SecGroup %s: %v", lbSecGroup.ID, err) + return fmt.Errorf("error occured creating rule for SecGroup %s: %v", lbSecGroup.ID, err) } } } @@ -1049,7 +1049,7 @@ func (lbaas *LbaasV2) ensureSecurityGroup(clusterName string, apiService *v1.Ser _, err = rules.Create(lbaas.network, lbSecGroupRuleCreateOpts).Extract() if err != nil { - return fmt.Errorf("Error occured creating rule for SecGroup %s: %v", lbSecGroup.ID, err) + return fmt.Errorf("error occured creating rule for SecGroup %s: %v", lbSecGroup.ID, err) } lbSecGroupRuleCreateOpts = rules.CreateOpts{ @@ -1064,7 +1064,7 @@ func (lbaas *LbaasV2) ensureSecurityGroup(clusterName string, apiService *v1.Ser _, err = rules.Create(lbaas.network, lbSecGroupRuleCreateOpts).Extract() if err != nil { - return fmt.Errorf("Error occured creating rule for SecGroup %s: %v", lbSecGroup.ID, err) + return fmt.Errorf("error occured creating rule for SecGroup %s: %v", lbSecGroup.ID, err) } // get security groups of port @@ -1119,7 +1119,7 @@ func (lbaas *LbaasV2) ensureSecurityGroup(clusterName string, apiService *v1.Ser // Add the rules in the Node Security Group err = createNodeSecurityGroup(lbaas.network, nodeSecurityGroupID, int(port.NodePort), port.Protocol, lbSecGroupID) if err != nil { - return fmt.Errorf("Error occured creating security group for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) + return fmt.Errorf("error occured creating security group for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) } } } @@ -1137,7 +1137,7 @@ func (lbaas *LbaasV2) UpdateLoadBalancer(clusterName string, service *v1.Service 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, "+ + 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 @@ -1153,7 +1153,7 @@ func (lbaas *LbaasV2) UpdateLoadBalancer(clusterName string, service *v1.Service return err } if loadbalancer == nil { - return fmt.Errorf("Loadbalancer %s does not exist", loadBalancerName) + return fmt.Errorf("loadbalancer %s does not exist", loadBalancerName) } // Get all listeners for this loadbalancer, by "port key". @@ -1165,7 +1165,7 @@ func (lbaas *LbaasV2) UpdateLoadBalancer(clusterName string, service *v1.Service lbListeners := make(map[portKey]listeners.Listener) allListeners, err := getListenersByLoadBalancerID(lbaas.network, loadbalancer.ID) if err != nil { - return fmt.Errorf("Error getting listeners for LB %s: %v", loadBalancerName, err) + return fmt.Errorf("error getting listeners for LB %s: %v", loadBalancerName, err) } for _, l := range allListeners { key := portKey{Protocol: listeners.Protocol(l.Protocol), Port: l.ProtocolPort} @@ -1178,7 +1178,7 @@ func (lbaas *LbaasV2) UpdateLoadBalancer(clusterName string, service *v1.Service for _, listenerID := range listenerIDs { pool, err := getPoolByListenerID(lbaas.network, loadbalancer.ID, listenerID) if err != nil { - return fmt.Errorf("Error getting pool for listener %s: %v", listenerID, err) + return fmt.Errorf("error getting pool for listener %s: %v", listenerID, err) } lbPools[listenerID] = *pool } @@ -1201,19 +1201,19 @@ func (lbaas *LbaasV2) UpdateLoadBalancer(clusterName string, service *v1.Service Port: int(port.Port), }] if !ok { - return fmt.Errorf("Loadbalancer %s does not contain required listener for port %d and protocol %s", loadBalancerName, port.Port, port.Protocol) + return fmt.Errorf("loadbalancer %s does not contain required listener for port %d and protocol %s", loadBalancerName, port.Port, port.Protocol) } // Get pool associated with this listener pool, ok := lbPools[listener.ID] if !ok { - return fmt.Errorf("Loadbalancer %s does not contain required pool for listener %s", loadBalancerName, listener.ID) + return fmt.Errorf("loadbalancer %s does not contain required pool for listener %s", loadBalancerName, listener.ID) } // Find existing pool members (by address) for this port getMembers, err := getMembersByPoolID(lbaas.network, pool.ID) if err != nil { - return fmt.Errorf("Error getting pool members %s: %v", pool.ID, err) + return fmt.Errorf("error getting pool members %s: %v", pool.ID, err) } members := make(map[string]v2pools.Member) for _, member := range getMembers { @@ -1254,7 +1254,7 @@ func (lbaas *LbaasV2) UpdateLoadBalancer(clusterName string, service *v1.Service if lbaas.opts.ManageSecurityGroups { err := lbaas.updateSecurityGroup(clusterName, service, nodes, loadbalancer) if err != nil { - return fmt.Errorf("Failed to update Securty Group for loadbalancer service %s/%s: %v", service.Namespace, service.Name, err) + return fmt.Errorf("failed to update Securty Group for loadbalancer service %s/%s: %v", service.Namespace, service.Name, err) } } @@ -1268,7 +1268,7 @@ func (lbaas *LbaasV2) updateSecurityGroup(clusterName string, apiService *v1.Ser var err error lbaas.opts.NodeSecurityGroupIDs, err = getNodeSecurityGroupIDForLB(lbaas.compute, nodes) if err != nil { - return fmt.Errorf("Failed to find node-security-group for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) + return fmt.Errorf("failed to find node-security-group for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) } glog.V(4).Infof("find node-security-group %v for loadbalancer service %s/%s", lbaas.opts.NodeSecurityGroupIDs, apiService.Namespace, apiService.Name) @@ -1280,7 +1280,7 @@ func (lbaas *LbaasV2) updateSecurityGroup(clusterName string, apiService *v1.Ser lbSecGroupName := getSecurityGroupName(clusterName, apiService) lbSecGroupID, err := groups.IDFromName(lbaas.network, lbSecGroupName) if err != nil { - return fmt.Errorf("Error occurred finding security group: %s: %v", lbSecGroupName, err) + return fmt.Errorf("error occurred finding security group: %s: %v", lbSecGroupName, err) } ports := apiService.Spec.Ports @@ -1301,14 +1301,13 @@ func (lbaas *LbaasV2) updateSecurityGroup(clusterName string, apiService *v1.Ser } secGroupRules, err := getSecurityGroupRules(lbaas.network, opts) if err != nil && !isNotFound(err) { - msg := fmt.Sprintf("Error finding rules for remote group id %s in security group id %s: %v", lbSecGroupID, removal, err) - return fmt.Errorf(msg) + return fmt.Errorf("error finding rules for remote group id %s in security group id %s: %v", lbSecGroupID, removal, err) } for _, rule := range secGroupRules { res := rules.Delete(lbaas.network, rule.ID) if res.Err != nil && !isNotFound(res.Err) { - return fmt.Errorf("Error occurred deleting security group rule: %s: %v", rule.ID, res.Err) + return fmt.Errorf("error occurred deleting security group rule: %s: %v", rule.ID, res.Err) } } } @@ -1324,8 +1323,7 @@ func (lbaas *LbaasV2) updateSecurityGroup(clusterName string, apiService *v1.Ser } secGroupRules, err := getSecurityGroupRules(lbaas.network, opts) if err != nil && !isNotFound(err) { - msg := fmt.Sprintf("Error finding rules for remote group id %s in security group id %s: %v", lbSecGroupID, nodeSecurityGroupID, err) - return fmt.Errorf(msg) + return fmt.Errorf("error finding rules for remote group id %s in security group id %s: %v", lbSecGroupID, nodeSecurityGroupID, err) } if len(secGroupRules) != 0 { // Do not add rule when find rules for remote group in the Node Security Group @@ -1335,7 +1333,7 @@ func (lbaas *LbaasV2) updateSecurityGroup(clusterName string, apiService *v1.Ser // Add the rules in the Node Security Group err = createNodeSecurityGroup(lbaas.network, nodeSecurityGroupID, int(port.NodePort), port.Protocol, lbSecGroupID) if err != nil { - return fmt.Errorf("Error occured creating security group for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) + return fmt.Errorf("error occured creating security group for loadbalancer service %s/%s: %v", apiService.Namespace, apiService.Name, err) } } } @@ -1372,7 +1370,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancerDeleted(clusterName string, service *v1. // get all listeners associated with this loadbalancer listenerList, err := getListenersByLoadBalancerID(lbaas.network, loadbalancer.ID) if err != nil { - return fmt.Errorf("Error getting LB %s listeners: %v", loadbalancer.ID, err) + return fmt.Errorf("error getting LB %s listeners: %v", loadbalancer.ID, err) } // get all pools (and health monitors) associated with this loadbalancer @@ -1381,7 +1379,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancerDeleted(clusterName string, service *v1. for _, listener := range listenerList { pool, err := getPoolByListenerID(lbaas.network, loadbalancer.ID, listener.ID) if err != nil && err != ErrNotFound { - return fmt.Errorf("Error getting pool for listener %s: %v", listener.ID, err) + return fmt.Errorf("error getting pool for listener %s: %v", listener.ID, err) } if pool != nil { poolIDs = append(poolIDs, pool.ID) @@ -1397,7 +1395,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancerDeleted(clusterName string, service *v1. for _, pool := range poolIDs { membersList, err := getMembersByPoolID(lbaas.network, pool) if err != nil && !isNotFound(err) { - return fmt.Errorf("Error getting pool members %s: %v", pool, err) + return fmt.Errorf("error getting pool members %s: %v", pool, err) } for _, member := range membersList { memberIDs = append(memberIDs, member.ID) @@ -1460,7 +1458,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancerDeleted(clusterName string, service *v1. // 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) } } @@ -1492,7 +1490,7 @@ func (lbaas *LbaasV2) EnsureLoadBalancerDeleted(clusterName string, service *v1. for _, rule := range secGroupRules { res := rules.Delete(lbaas.network, rule.ID) if res.Err != nil && !isNotFound(res.Err) { - return fmt.Errorf("Error occurred deleting security group rule: %s: %v", rule.ID, res.Err) + return fmt.Errorf("error occurred deleting security group rule: %s: %v", rule.ID, res.Err) } } } @@ -1517,7 +1515,7 @@ func (lb *LbaasV1) GetLoadBalancer(clusterName string, service *v1.Service) (*v1 if vip.PortID != "" { floatingIP, err := getFloatingIPByPortID(lb.network, vip.PortID) if err != nil { - return nil, false, fmt.Errorf("Error getting floating ip for port %s: %v", vip.PortID, err) + return nil, false, fmt.Errorf("error getting floating ip for port %s: %v", vip.PortID, err) } status.Ingress = []v1.LoadBalancerIngress{{IP: floatingIP.FloatingIP}} } else { @@ -1536,7 +1534,7 @@ func (lb *LbaasV1) EnsureLoadBalancer(clusterName string, apiService *v1.Service 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) if len(nodes) == 0 { - return nil, fmt.Errorf("There are no available nodes for LoadBalancer service %s/%s", apiService.Namespace, apiService.Name) + return nil, fmt.Errorf("there are no available nodes for LoadBalancer service %s/%s", apiService.Namespace, apiService.Name) } if len(lb.opts.SubnetId) == 0 { @@ -1545,7 +1543,7 @@ func (lb *LbaasV1) EnsureLoadBalancer(clusterName string, apiService *v1.Service subnetID, err := getSubnetIDForLB(lb.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, "+ + 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) } lb.opts.SubnetId = subnetID @@ -1565,10 +1563,10 @@ func (lb *LbaasV1) EnsureLoadBalancer(clusterName string, apiService *v1.Service glog.V(4).Infof("Ensure an external loadbalancer service.") internalAnnotation = false } else { - return nil, fmt.Errorf("floating-network-id or loadbalancer.openstack.org/floating-network-id should be specified when ensuring an external loadbalancer service.") + return nil, fmt.Errorf("floating-network-id or loadbalancer.openstack.org/floating-network-id should be specified when ensuring an external loadbalancer service") } default: - return nil, fmt.Errorf("unknow service.beta.kubernetes.io/openstack-internal-load-balancer annotation: %v, specify \"true\" or \"false\".", + return nil, fmt.Errorf("unknown service.beta.kubernetes.io/openstack-internal-load-balancer annotation: %v, specify \"true\" or \"false\" ", internal) } @@ -1582,7 +1580,7 @@ func (lb *LbaasV1) EnsureLoadBalancer(clusterName string, apiService *v1.Service // The service controller verified all the protocols match on the ports, just check and use the first one // TODO: Convert all error messages to use an event recorder if ports[0].Protocol != v1.ProtocolTCP { - return nil, fmt.Errorf("Only TCP LoadBalancer is supported for openstack load balancers") + return nil, fmt.Errorf("only TCP LoadBalancer is supported for openstack load balancers") } affinity := apiService.Spec.SessionAffinity @@ -1602,7 +1600,7 @@ func (lb *LbaasV1) EnsureLoadBalancer(clusterName string, apiService *v1.Service } if !service.IsAllowAll(sourceRanges) { - return nil, fmt.Errorf("Source range restrictions are not supported for openstack load balancers") + return nil, fmt.Errorf("source range restrictions are not supported for openstack load balancers") } glog.V(2).Infof("Checking if openstack load balancer already exists: %s", cloudprovider.GetLoadBalancerName(apiService)) @@ -1632,7 +1630,7 @@ func (lb *LbaasV1) EnsureLoadBalancer(clusterName string, apiService *v1.Service LBMethod: lbmethod, }).Extract() if err != nil { - return nil, fmt.Errorf("Error creating pool for openstack load balancer %s: %v", name, err) + return nil, fmt.Errorf("error creating pool for openstack load balancer %s: %v", name, err) } for _, node := range nodes { @@ -1647,7 +1645,7 @@ func (lb *LbaasV1) EnsureLoadBalancer(clusterName string, apiService *v1.Service Address: addr, }).Extract() if err != nil { - return nil, fmt.Errorf("Error creating member for the pool(%s) of openstack load balancer %s: %v", + return nil, fmt.Errorf("error creating member for the pool(%s) of openstack load balancer %s: %v", pool.ID, name, err) } } @@ -1661,12 +1659,12 @@ func (lb *LbaasV1) EnsureLoadBalancer(clusterName string, apiService *v1.Service MaxRetries: int(lb.opts.MonitorMaxRetries), }).Extract() if err != nil { - return nil, fmt.Errorf("Error creating monitor for openstack load balancer %s: %v", name, err) + return nil, fmt.Errorf("error creating monitor for openstack load balancer %s: %v", name, err) } _, err = pools.AssociateMonitor(lb.network, pool.ID, mon.ID).Extract() if err != nil { - return nil, fmt.Errorf("Error associating monitor(%s) with pool(%s) for"+ + return nil, fmt.Errorf("error associating monitor(%s) with pool(%s) for"+ "openstack load balancer %s: %v", mon.ID, pool.ID, name, err) } } @@ -1688,7 +1686,7 @@ func (lb *LbaasV1) EnsureLoadBalancer(clusterName string, apiService *v1.Service vip, err := vips.Create(lb.network, createOpts).Extract() if err != nil { - return nil, fmt.Errorf("Error creating vip for openstack load balancer %s: %v", name, err) + return nil, fmt.Errorf("error creating vip for openstack load balancer %s: %v", name, err) } status := &v1.LoadBalancerStatus{} @@ -1705,7 +1703,7 @@ func (lb *LbaasV1) EnsureLoadBalancer(clusterName string, apiService *v1.Service floatIP, err := floatingips.Create(lb.network, floatIPOpts).Extract() if err != nil { - return nil, fmt.Errorf("Error creating floatingip for openstack load balancer %s: %v", name, err) + return nil, fmt.Errorf("error creating floatingip for openstack load balancer %s: %v", name, err) } status.Ingress = []v1.LoadBalancerIngress{{IP: floatIP.FloatingIP}} diff --git a/pkg/cloudprovider/providers/openstack/openstack_test.go b/pkg/cloudprovider/providers/openstack/openstack_test.go index 2c5f0886da5..3acdcdd1d49 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_test.go +++ b/pkg/cloudprovider/providers/openstack/openstack_test.go @@ -227,7 +227,7 @@ func TestCheckOpenStackOpts(t *testing.T) { SearchOrder: "", }, }, - expectedError: fmt.Errorf("Invalid value in section [Metadata] with key `search-order`. Value cannot be empty"), + expectedError: fmt.Errorf("invalid value in section [Metadata] with key `search-order`. Value cannot be empty"), }, { name: "test5", @@ -237,7 +237,7 @@ func TestCheckOpenStackOpts(t *testing.T) { SearchOrder: "value1,value2,value3", }, }, - expectedError: fmt.Errorf("Invalid value in section [Metadata] with key `search-order`. Value cannot contain more than 2 elements"), + expectedError: fmt.Errorf("invalid value in section [Metadata] with key `search-order`. Value cannot contain more than 2 elements"), }, { name: "test6", @@ -247,8 +247,8 @@ func TestCheckOpenStackOpts(t *testing.T) { SearchOrder: "value1", }, }, - expectedError: fmt.Errorf("Invalid element '%s' found in section [Metadata] with key `search-order`."+ - "Supported elements include '%s' and '%s'", "value1", configDriveID, metadataID), + expectedError: fmt.Errorf("invalid element %q found in section [Metadata] with key `search-order`."+ + "Supported elements include %q and %q", "value1", configDriveID, metadataID), }, } diff --git a/pkg/cloudprovider/providers/openstack/openstack_volumes.go b/pkg/cloudprovider/providers/openstack/openstack_volumes.go index f670bce6d06..ebe7970f797 100644 --- a/pkg/cloudprovider/providers/openstack/openstack_volumes.go +++ b/pkg/cloudprovider/providers/openstack/openstack_volumes.go @@ -17,7 +17,6 @@ limitations under the License. package openstack import ( - "errors" "fmt" "io/ioutil" "path" @@ -127,8 +126,7 @@ func (volumes *VolumesV1) getVolume(volumeID string) (Volume, error) { timeTaken := time.Since(startTime).Seconds() recordOpenstackOperationMetric("get_v1_volume", timeTaken, err) if err != nil { - glog.Errorf("Error occurred getting volume by ID: %s", volumeID) - return Volume{}, err + return Volume{}, fmt.Errorf("error occurred getting volume by ID: %s, err: %v", volumeID, err) } volume := Volume{ @@ -151,8 +149,7 @@ func (volumes *VolumesV2) getVolume(volumeID string) (Volume, error) { timeTaken := time.Since(startTime).Seconds() recordOpenstackOperationMetric("get_v2_volume", timeTaken, err) if err != nil { - glog.Errorf("Error occurred getting volume by ID: %s", volumeID) - return Volume{}, err + return Volume{}, fmt.Errorf("error occurred getting volume by ID: %s, err: %v", volumeID, err) } volume := Volume{ @@ -174,10 +171,6 @@ func (volumes *VolumesV1) deleteVolume(volumeID string) error { err := volumes_v1.Delete(volumes.blockstorage, volumeID).ExtractErr() timeTaken := time.Since(startTime).Seconds() recordOpenstackOperationMetric("delete_v1_volume", timeTaken, err) - if err != nil { - glog.Errorf("Cannot delete volume %s: %v", volumeID, err) - } - return err } @@ -186,10 +179,6 @@ func (volumes *VolumesV2) deleteVolume(volumeID string) error { err := volumes_v2.Delete(volumes.blockstorage, volumeID).ExtractErr() timeTaken := time.Since(startTime).Seconds() recordOpenstackOperationMetric("delete_v2_volume", timeTaken, err) - if err != nil { - glog.Errorf("Cannot delete volume %s: %v", volumeID, err) - } - return err } @@ -200,7 +189,6 @@ func (os *OpenStack) OperationPending(diskName string) (bool, string, error) { } volumeStatus := volume.Status if volumeStatus == VolumeErrorStatus { - glog.Errorf("status of volume %s is %s", diskName, volumeStatus) return false, volumeStatus, nil } if volumeStatus == VolumeAvailableStatus || volumeStatus == VolumeInUseStatus || volumeStatus == VolumeDeletedStatus { @@ -226,9 +214,7 @@ func (os *OpenStack) AttachDisk(instanceID, volumeID string) (string, error) { glog.V(4).Infof("Disk %s is already attached to instance %s", volumeID, instanceID) return volume.ID, nil } - errmsg := fmt.Sprintf("Disk %s is attached to a different instance (%s)", volumeID, volume.AttachedServerId) - glog.V(2).Infof(errmsg) - return "", errors.New(errmsg) + return "", fmt.Errorf("disk %s is attached to a different instance (%s)", volumeID, volume.AttachedServerId) } startTime := time.Now() @@ -239,8 +225,7 @@ func (os *OpenStack) AttachDisk(instanceID, volumeID string) (string, error) { timeTaken := time.Since(startTime).Seconds() recordOpenstackOperationMetric("attach_disk", timeTaken, err) if err != nil { - glog.Errorf("Failed to attach %s volume to %s compute: %v", volumeID, instanceID, err) - return "", err + return "", fmt.Errorf("failed to attach %s volume to %s compute: %v", volumeID, instanceID, err) } glog.V(2).Infof("Successfully attached %s volume to %s compute", volumeID, instanceID) return volume.ID, nil @@ -259,18 +244,14 @@ func (os *OpenStack) DetachDisk(instanceID, volumeID string) error { } if volume.Status != VolumeInUseStatus { - errmsg := fmt.Sprintf("can not detach volume %s, its status is %s.", volume.Name, volume.Status) - glog.Errorf(errmsg) - return errors.New(errmsg) + return fmt.Errorf("can not detach volume %s, its status is %s", volume.Name, volume.Status) } cClient, err := os.NewComputeV2() if err != nil { return err } if volume.AttachedServerId != instanceID { - errMsg := fmt.Sprintf("Disk: %s has no attachments or is not attached to compute: %s", volume.Name, instanceID) - glog.Errorf(errMsg) - return errors.New(errMsg) + return fmt.Errorf("disk: %s has no attachments or is not attached to compute: %s", volume.Name, instanceID) } else { startTime := time.Now() // This is a blocking call and effects kubelet's performance directly. @@ -279,8 +260,7 @@ func (os *OpenStack) DetachDisk(instanceID, volumeID string) error { timeTaken := time.Since(startTime).Seconds() recordOpenstackOperationMetric("detach_disk", timeTaken, err) if err != nil { - glog.Errorf("Failed to delete volume %s from compute %s attached %v", volume.ID, instanceID, err) - return err + return fmt.Errorf("failed to delete volume %s from compute %s attached %v", volume.ID, instanceID, err) } glog.V(2).Infof("Successfully detached volume: %s from compute: %s", volume.ID, instanceID) } @@ -291,9 +271,8 @@ func (os *OpenStack) DetachDisk(instanceID, volumeID string) error { // getVolume retrieves Volume by its ID. func (os *OpenStack) getVolume(volumeID string) (Volume, error) { volumes, err := os.volumeService("") - if err != nil || volumes == nil { - glog.Errorf("Unable to initialize cinder client for region: %s", os.region) - return Volume{}, err + if err != nil { + return Volume{}, fmt.Errorf("unable to initialize cinder client for region: %s, err: %v", os.region, err) } return volumes.getVolume(volumeID) } @@ -301,9 +280,8 @@ func (os *OpenStack) getVolume(volumeID string) (Volume, error) { // CreateVolume creates a volume of given size (in GiB) func (os *OpenStack) CreateVolume(name string, size int, vtype, availability string, tags *map[string]string) (string, string, bool, error) { volumes, err := os.volumeService("") - if err != nil || volumes == nil { - glog.Errorf("Unable to initialize cinder client for region: %s", os.region) - return "", "", os.bsOpts.IgnoreVolumeAZ, err + if err != nil { + return "", "", os.bsOpts.IgnoreVolumeAZ, fmt.Errorf("unable to initialize cinder client for region: %s, err: %v", os.region, err) } opts := VolumeCreateOpts{ @@ -319,8 +297,7 @@ func (os *OpenStack) CreateVolume(name string, size int, vtype, availability str volumeID, volumeAZ, err := volumes.createVolume(opts) if err != nil { - glog.Errorf("Failed to create a %d GB volume: %v", size, err) - return "", "", os.bsOpts.IgnoreVolumeAZ, err + return "", "", os.bsOpts.IgnoreVolumeAZ, fmt.Errorf("failed to create a %d GB volume: %v", size, err) } glog.Infof("Created volume %v in Availability Zone: %v Ignore volume AZ: %v", volumeID, volumeAZ, os.bsOpts.IgnoreVolumeAZ) @@ -365,15 +342,11 @@ func (os *OpenStack) DeleteVolume(volumeID string) error { } volumes, err := os.volumeService("") - if err != nil || volumes == nil { - glog.Errorf("Unable to initialize cinder client for region: %s", os.region) - return err + if err != nil { + return fmt.Errorf("unable to initialize cinder client for region: %s, err: %v", os.region, err) } err = volumes.deleteVolume(volumeID) - if err != nil { - glog.Errorf("Cannot delete volume %s: %v", volumeID, err) - } return err } @@ -387,9 +360,7 @@ func (os *OpenStack) GetAttachmentDiskPath(instanceID, volumeID string) (string, return "", err } if volume.Status != VolumeInUseStatus { - errmsg := fmt.Sprintf("can not get device path of volume %s, its status is %s.", volume.Name, volume.Status) - glog.Errorf(errmsg) - return "", errors.New(errmsg) + return "", fmt.Errorf("can not get device path of volume %s, its status is %s ", volume.Name, volume.Status) } if volume.AttachedServerId != "" { if instanceID == volume.AttachedServerId { @@ -397,12 +368,10 @@ func (os *OpenStack) GetAttachmentDiskPath(instanceID, volumeID string) (string, // see http://developer.openstack.org/api-ref-blockstorage-v1.html return volume.AttachedDevice, nil } else { - errMsg := fmt.Sprintf("Disk %q is attached to a different compute: %q, should be detached before proceeding", volumeID, volume.AttachedServerId) - glog.Errorf(errMsg) - return "", errors.New(errMsg) + return "", fmt.Errorf("disk %q is attached to a different compute: %q, should be detached before proceeding", volumeID, volume.AttachedServerId) } } - return "", fmt.Errorf("volume %s has no ServerId.", volumeID) + return "", fmt.Errorf("volume %s has no ServerId", volumeID) } // DiskIsAttached queries if a volume is attached to a compute instance