Merge pull request #142 from colabsaumoh/fix-govet-3

Fix linter issues and ignore notExist error for remove.

Makefile

@@ -142,7 +142,8 @@ static-checks: vendor
 		-e LOCAL_USER_ID=$(LOCAL_USER_ID) \
 		-v $(CURDIR):/go/src/$(PACKAGE_NAME) \
 		-w /go/src/$(PACKAGE_NAME) \
-		$(CALICO_BUILD) gometalinter --deadline=300s --disable-all --enable=goimports --vendor ./...
+		$(CALICO_BUILD) \
+		gometalinter --deadline=300s --disable-all --enable=vet --enable=errcheck  --enable=goimports --vendor ./...
 
 .PHONY: fix
 ## Fix static checks

pkg/backends/calico/client.go

@@ -332,7 +332,7 @@ func (c *client) WatchPrefix(prefix string, keys []string, lastRevision uint64,
 		for _, key := range keys {
 			rev, ok := c.revisionsByPrefix[key]
 			if !ok {
-				log.Fatalf("Watch prefix check for unknown prefix: ", key)
+				log.Fatalf("Watch prefix check for unknown prefix: %s", key)
 			}
 			log.Debugf("Found key prefix %s at rev %d", key, rev)
 			if rev > lastRevision {

pkg/resource/template/fileStat_posix.go

@@ -9,6 +9,8 @@ import (
 	"io"
 	"os"
 	"syscall"
+
+	log "github.com/sirupsen/logrus"
 )
 
 // fileStat return a fileInfo describing the named file.
@@ -18,13 +20,22 @@ func fileStat(name string) (fi fileInfo, err error) {
 		if err != nil {
 			return fi, err
 		}
-		defer f.Close()
-		stats, _ := f.Stat()
+		defer func() {
+			if e := f.Close(); e != nil {
+				log.WithError(e).WithField("filename", name).Error("error closing file")
+			}
+		}()
+		stats, err := f.Stat()
+		if err != nil {
+			return fi, err
+		}
 		fi.Uid = stats.Sys().(*syscall.Stat_t).Uid
 		fi.Gid = stats.Sys().(*syscall.Stat_t).Gid
 		fi.Mode = stats.Mode()
 		h := md5.New()
-		io.Copy(h, f)
+		if _, err := io.Copy(h, f); err != nil {
+			return fileInfo{}, err
+		}
 		fi.Md5 = fmt.Sprintf("%x", h.Sum(nil))
 		return fi, nil
 	}

pkg/resource/template/processor.go

@@ -68,8 +68,7 @@ type watchProcessor struct {
 }
 
 func WatchProcessor(config Config, stopChan, doneChan chan bool, errChan chan error) Processor {
-	var wg sync.WaitGroup
-	return &watchProcessor{config, stopChan, doneChan, errChan, wg}
+	return &watchProcessor{config, stopChan, doneChan, errChan, sync.WaitGroup{}}
 }
 
 func (p *watchProcessor) Process() {

pkg/resource/template/resource.go

@@ -75,7 +75,7 @@ func NewTemplateResource(path string, config Config) (*TemplateResource, error)
 		return nil, fmt.Errorf("Cannot process template resource %s - %s", path, err.Error())
 	}
 
-	tr := tc.TemplateResource
+	tr := &tc.TemplateResource
 	tr.keepStageFile = config.KeepStageFile
 	tr.noop = config.Noop
 	tr.storeClient = config.StoreClient
@@ -108,7 +108,7 @@ func NewTemplateResource(path string, config Config) (*TemplateResource, error)
 	tr.PrefixedKeys = appendPrefix(tr.Prefix, tr.Keys)
 
 	tr.Src = filepath.Join(config.TemplateDir, tr.Src)
-	return &tr, nil
+	return tr, nil
 }
 
 // setVars sets the Vars for template resource.
@@ -155,16 +155,29 @@ func (t *TemplateResource) createStageFile() error {
 	}
 
 	if err = tmpl.Execute(temp, nil); err != nil {
-		temp.Close()
-		os.Remove(temp.Name())
+		// The key error to return is the failure to execute.
+		// to preserve that error ignore the errors in close and clean
+		temp.Close()           // nolint:errcheck
+		os.Remove(temp.Name()) // nolint:errcheck
 		return err
 	}
-	defer temp.Close()
+	defer func() {
+		if e := temp.Close(); e != nil {
+			// Just log the error but don't carry it up the calling stack.
+			log.WithError(e).WithField("filename", temp.Name()).Error("error closing file")
+		}
+	}()
 
 	// Set the owner, group, and mode on the stage file now to make it easier to
 	// compare against the destination configuration file later.
-	os.Chmod(temp.Name(), t.FileMode)
-	os.Chown(temp.Name(), t.Uid, t.Gid)
+	if err := os.Chmod(temp.Name(), t.FileMode); err != nil {
+		log.WithError(err).WithField("filename", temp.Name()).Error("error changing mode")
+		return err
+	}
+	if err := os.Chown(temp.Name(), t.Uid, t.Gid); err != nil {
+		log.WithError(err).WithField("filename", temp.Name()).Error("error changing ownership")
+		return err
+	}
 	t.StageFile = temp
 	return nil
 }
@@ -179,7 +192,16 @@ func (t *TemplateResource) sync() error {
 	if t.keepStageFile {
 		log.Info("Keeping staged file: " + staged)
 	} else {
-		defer os.Remove(staged)
+		defer func() {
+			if e := os.Remove(staged); e != nil {
+				if !os.IsNotExist(e) {
+					// Just log the error but don't carry it up the calling stack.
+					log.WithError(e).WithField("filename", staged).Error("error removing file")
+				} else {
+					log.Debugf("Ignore not exists err. %s", e.Error())
+				}
+			}
+		}()
 	}
 
 	log.Debug("Comparing candidate config to " + t.Dest)
@@ -213,12 +235,16 @@ func (t *TemplateResource) sync() error {
 				if rerr != nil {
 					return rerr
 				}
-				err := ioutil.WriteFile(t.Dest, contents, t.FileMode)
+				if err := ioutil.WriteFile(t.Dest, contents, t.FileMode); err != nil {
+					log.WithError(err).WithField("filename", t.Dest).Error("failed to write to file")
+					return err
+				}
 				// make sure owner and group match the temp file, in case the file was created with WriteFile
-				os.Chown(t.Dest, t.Uid, t.Gid)
-				if err != nil {
+				if err := os.Chown(t.Dest, t.Uid, t.Gid); err != nil {
+					log.WithError(err).WithField("filename", t.Dest).Error("failed to change owner")
 					return err
 				}
+
 			} else {
 				return err
 			}