Issue open-horizon#3667 - Bug: check error on deferred file close

Signed-off-by: Max McAdam <max@fredcom.com>

api/api_publickey.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/golang/glog"
 	"github.com/gorilla/mux"
+	"github.com/open-horizon/anax/cutil"
 )
 
 func (a *API) publickey(w http.ResponseWriter, r *http.Request) {
@@ -90,7 +91,7 @@ func returnFileBytes(filename string, w http.ResponseWriter) error {
 	// Open the file so that we can read any header info that might be there.
 	file, err := os.Open(filename)
 	if file != nil {
-		defer file.Close()
+		defer cutil.CloseFileLogError(file)
 	}
 
 	if err != nil {

cli/cliconfig/config.go

@@ -13,6 +13,7 @@ import (
 	"strings"
 
 	"github.com/open-horizon/anax/cli/cliutils"
+	"github.com/open-horizon/anax/cutil"
 	"github.com/open-horizon/anax/i18n"
 )
 
@@ -209,7 +210,7 @@ func GetConfigFromNonJsonFile(configFile string) (map[string]string, error) {
 	if err != nil {
 		return hzn_vars, err
 	}
-	defer fileHandle.Close()
+	defer cutil.CloseFileLogError(fileHandle)
 
 	// regex for comment line
 	r, _ := regexp.Compile(`^(\s)*#(.*)*`)

cli/cliutils/cliutils.go

@@ -1030,7 +1030,7 @@ func GetEnvVarFromFile(filename string, key string) (string, error) {
 			return "", err
 		}
 	}
-	defer fHandle.Close()
+	defer cutil.CloseFileLogError(fHandle)
 
 	scanner := bufio.NewScanner(fHandle)
 	for scanner.Scan() {

cli/cliutils/stdout_logging.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/open-horizon/anax/cutil"
 	"github.com/open-horizon/anax/i18n"
 )
 
@@ -64,7 +65,7 @@ func LogLinux(instanceId string, tailing bool) {
 	if err != nil {
 		Fatal(NOT_FOUND, msgPrinter.Sprintf("%v could not be opened or does not exist: %v", sysLogPath, err))
 	}
-	defer file.Close()
+	defer cutil.CloseFileLogError(file)
 	// Check file stats and capture the current size of the file if we will be tailing it.
 	var file_size int64
 	if tailing {
@@ -114,12 +115,11 @@ func LogLinux(instanceId string, tailing bool) {
 				file_size = new_file_size
 			} else {
 				// Log rotation has occurred. Re-open the new syslog file and capture the current size.
-				file.Close()
+				cutil.CloseFileLogError(file)
 				file, err = os.Open(sysLogPath)
 				if err != nil {
 					Fatal(NOT_FOUND, msgPrinter.Sprintf("%v could not be opened or does not exist: %v", sysLogPath, err))
 				}
-				defer file.Close()
 				// Setup a new reader on the new file.
 				reader = bufio.NewReader(file)
 				fi, err := file.Stat()

cli/exchange/service.go

@@ -659,7 +659,7 @@ func SaveOpYamlToFile(sId string, clusterDeployment string, filePath string, for
 	if err != nil {
 		cliutils.Fatal(cliutils.INTERNAL_ERROR, msgPrinter.Sprintf("Failed to create file %v. %v", fileName, err))
 	}
-	defer file.Close()
+	defer cutil.CloseFileLogError(file)
 
 	if _, err := file.Write(archiveData); err != nil {
 		cliutils.Fatal(cliutils.INTERNAL_ERROR, msgPrinter.Sprintf("Failed to save the clusterDeployment operator yaml to file %v. %v", fileName, err))

cli/sdo/voucher.go

@@ -15,6 +15,7 @@ import (
 	"github.com/open-horizon/anax/cli/exchange"
 	"github.com/open-horizon/anax/cli/register"
 	"github.com/open-horizon/anax/config"
+	"github.com/open-horizon/anax/cutil"
 	anaxExchange "github.com/open-horizon/anax/exchange"
 	"github.com/open-horizon/anax/exchangecommon"
 	"github.com/open-horizon/anax/i18n"
@@ -54,7 +55,7 @@ type InspectOutput struct {
 
 // hzn sdo voucher inspect <voucher-file>
 func VoucherInspect(voucherFile *os.File) {
-	defer voucherFile.Close()
+	defer cutil.CloseFileLogError(voucherFile)
 	msgPrinter := i18n.GetMessagePrinter()
 	cliutils.Verbose(msgPrinter.Sprintf("Inspecting voucher file name: %s", voucherFile.Name()))
 
@@ -317,7 +318,7 @@ func VoucherDownload(org, userCreds, device, outputFile string, overwrite bool)
 
 // hzn sdo voucher import <voucher-file>
 func VoucherImport(org, userCreds string, voucherFile *os.File, example, policyFilePath, patternName, userInputFileName, haGroupName string) {
-	defer voucherFile.Close()
+	defer cutil.CloseFileLogError(voucherFile)
 	msgPrinter := i18n.GetMessagePrinter()
 	cliutils.Verbose(msgPrinter.Sprintf("Importing voucher file name: %s", voucherFile.Name()))
 

cli/sync_service/object.go

@@ -16,6 +16,7 @@ import (
 	"github.com/open-horizon/anax/cli/cliconfig"
 	"github.com/open-horizon/anax/cli/cliutils"
 	"github.com/open-horizon/anax/config"
+	"github.com/open-horizon/anax/cutil"
 	"github.com/open-horizon/anax/exchange"
 	"github.com/open-horizon/anax/i18n"
 	"github.com/open-horizon/edge-sync-service/common"
@@ -354,7 +355,7 @@ func ObjectPublish(org string, userPw string, objType string, objId string, objP
 
 		// Create public key. Sign data. Set "hashAlgorithm", "publicKey" and "signature" field
 		file, err := os.Open(objFile)
-		defer file.Close()
+		defer cutil.CloseFileLogError(file)
 		if err != nil {
 			cliutils.Fatal(cliutils.CLI_INPUT_ERROR, msgPrinter.Sprintf("unable to open object file %v: %v", objFile, err))
 		}
@@ -408,7 +409,7 @@ func ObjectPublish(org string, userPw string, objType string, objId string, objP
 		if err != nil {
 			cliutils.Fatal(cliutils.CLI_INPUT_ERROR, msgPrinter.Sprintf("unable to open object file %v: %v", objFile, err))
 		}
-		defer file.Close()
+		defer cutil.CloseFileLogError(file)
 
 		if noChunkUpload {
 			cliutils.ExchangePutPost("Model Management Service", http.MethodPut, cliutils.GetMMSUrl(), urlPath, cliutils.OrgAndCreds(org, userPw), []int{204}, file, nil)

cli/sync_service/sync_service.go

@@ -413,45 +413,54 @@ func loadCSS(org string, fileType string, fileObjects []string) error {
 	msgPrinter := i18n.GetMessagePrinter()
 
 	for _, fileName := range fileObjects {
-		cliutils.Verbose(msgPrinter.Sprintf("Loading %v into CSS", fileName))
+		if err := loadCSSFile(org, fileType, fileName); err != nil {
+			return err
+		}
+	}
 
-		if fileObject, err := os.Open(fileName); err != nil {
-			return errors.New(msgPrinter.Sprintf("unable to open file object %v, error %v", fileName, err))
-		} else if fileBytes, err := ioutil.ReadAll(fileObject); err != nil {
-			return errors.New(msgPrinter.Sprintf("unable to read file object %v, error %v", fileName, err))
-		} else {
+	if len(fileObjects) > 0 {
+		msgPrinter.Printf("Configuration files %v loaded into the File sync service.", fileObjects)
+		msgPrinter.Println()
+	}
 
-			defer fileObject.Close()
+	return nil
 
-			fileObjectName := path.Base(fileName)
+}
 
-			metadata := &cssFileMeta{
-				ObjectID:   fileObjectName,
-				ObjectType: fileType,
-			}
+func loadCSSFile(org string, fileType string, fileName string) error {
+	// get message printer
+	msgPrinter := i18n.GetMessagePrinter()
 
-			// Get this host's IP address because that's where the CSS is listening.
-			hostIP := os.Getenv("HZN_DEV_HOST_IP")
-			if hostIP == "" {
-				hostIP = "localhost"
-			}
+	cliutils.Verbose(msgPrinter.Sprintf("Loading %v into CSS", fileName))
 
-			// Form the CSS URL and then PUT the file into the CSS.
-			url := fmt.Sprintf("http://%v:%v/api/v1/objects/%v/%v/%v", hostIP, getCSSPort(), org, fileType, fileObjectName)
+	if fileObject, err := os.Open(fileName); err != nil {
+		return errors.New(msgPrinter.Sprintf("unable to open file object %v, error %v", fileName, err))
+	} else if fileBytes, err := ioutil.ReadAll(fileObject); err != nil {
+		return errors.New(msgPrinter.Sprintf("unable to read file object %v, error %v", fileName, err))
+	} else {
+		defer cutil.CloseFileLogError(fileObject)
 
-			if err := putFile(url, org, metadata, fileBytes); err != nil {
-				return errors.New(msgPrinter.Sprintf("unable to add file %v to the CSS, error %v", fileName, err))
-			}
+		fileObjectName := path.Base(fileName)
+
+		metadata := &cssFileMeta{
+			ObjectID:   fileObjectName,
+			ObjectType: fileType,
 		}
-	}
 
-	if len(fileObjects) > 0 {
-		msgPrinter.Printf("Configuration files %v loaded into the File sync service.", fileObjects)
-		msgPrinter.Println()
-	}
+		// Get this host's IP address because that's where the CSS is listening.
+		hostIP := os.Getenv("HZN_DEV_HOST_IP")
+		if hostIP == "" {
+			hostIP = "localhost"
+		}
 
-	return nil
+		// Form the CSS URL and then PUT the file into the CSS.
+		url := fmt.Sprintf("http://%v:%v/api/v1/objects/%v/%v/%v", hostIP, getCSSPort(), org, fileType, fileObjectName)
 
+		if err := putFile(url, org, metadata, fileBytes); err != nil {
+			return errors.New(msgPrinter.Sprintf("unable to add file %v to the CSS, error %v", fileName, err))
+		}
+	}
+	return nil
 }
 
 type cssFileMeta struct {

cli/unregister/unregister.go

@@ -373,7 +373,7 @@ func backupEventLogs() error {
 	if err != nil {
 		return errors.New(msgPrinter.Sprintf("failed to backup eventlogs file %v. %v", fileName, err))
 	}
-	defer file.Close()
+	defer cutil.CloseFileLogError(file)
 
 	if _, err := file.Write(elogsJson); err != nil {
 		return errors.New(msgPrinter.Sprintf("failed to save the eventlogs to file %v. %v", fileName, err))

cutil/cutil.go

@@ -614,7 +614,7 @@ func GetCPUCount(cpuinfo_file string) (int, error) {
 	if fh, err := os.Open(cpuinfo_file); err != nil {
 		return 0, err
 	} else {
-		defer fh.Close()
+		defer CloseFileLogError(fh)
 
 		cpu_count := 0
 		scanner := bufio.NewScanner(fh)
@@ -653,7 +653,7 @@ func GetMachineSerial(cpuInfoFile string) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	defer fh.Close()
+	defer CloseFileLogError(fh)
 
 	scanner := bufio.NewScanner(fh)
 	r, _ := regexp.Compile("Serial([ \t]*):")
@@ -694,7 +694,7 @@ func GetMemInfo(meminfo_file string) (uint64, uint64, error) {
 	if fh, err := os.Open(meminfo_file); err != nil {
 		return 0, 0, err
 	} else {
-		defer fh.Close()
+		defer CloseFileLogError(fh)
 
 		total_mem := uint64(0)
 		avail_mem := uint64(0)
@@ -826,3 +826,13 @@ func GetCertificateVersion(certFilePath string) string {
 	}
 	return version
 }
+
+// use this when deferring a file close so that any error message returned by the close is logged
+func CloseFileLogError(f *os.File) {
+	if f == nil {
+		return
+	}
+	if err := f.Close(); err != nil {
+		glog.Errorf("Error closing file: %v", err)
+	}
+}

helm/client.go

@@ -4,6 +4,7 @@ import (
 	"encoding/base64"
 	"fmt"
 	"github.com/golang/glog"
+	"github.com/open-horizon/anax/cutil"
 	"io/ioutil"
 	"os"
 )
@@ -43,7 +44,7 @@ func ConvertB64StringToFile(b64Package string) (string, error) {
 	} else if f, err := ioutil.TempFile("", TEMP_PACKAGE_PREFIX); err != nil {
 		return "", err
 	} else {
-		defer f.Close()
+		defer cutil.CloseFileLogError(f)
 		num, err := f.Write(sDec)
 		if err != nil {
 			return "", err

imagefetch/imagepull.go

@@ -24,7 +24,7 @@ func dockerCredsFromConfigFile(configFilePath string) (*docker.AuthConfiguration
 
 	f, err := os.Open(configFilePath)
 	if f != nil {
-		defer f.Close()
+		defer cutil.CloseFileLogError(f)
 	}
 	if err != nil {
 		return nil, err