Merge pull request #54280 from NickrenREN/remove-dup-log

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Let the caller log error message

Talk to @anguslees offline and in resize pr, we should let the caller log the error message and in those functions, just return err.
Also error string should not be capitalized or end with punctuation

**What this PR does / why we need it**:

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:

**Release note**:
```release-note
NONE
```

/assign @anguslees

Author: Kubernetes Submit Queue

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,27 +117,24 @@ 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()
 
 	return parseMetadata(f)
 }
 
 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
 	}
 

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)
 		}
 	}
 

pkg/cloudprovider/providers/openstack/openstack_client.go

@@ -17,19 +17,18 @@ 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) {
 	network, err := openstack.NewNetworkV2(os.provider, gophercloud.EndpointOpts{
 		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
 }