Fix issue #1221

command/agent/agent.go

@@ -24,26 +24,26 @@ import (
 )
 
 const (
-	// Path to save agent service definitions
+// Path to save agent service definitions
 	servicesDir = "services"
 
-	// Path to save local agent checks
+// Path to save local agent checks
 	checksDir     = "checks"
 	checkStateDir = "checks/state"
 
-	// The ID of the faux health checks for maintenance mode
+// The ID of the faux health checks for maintenance mode
 	serviceMaintCheckPrefix = "_service_maintenance"
 	nodeMaintCheckID        = "_node_maintenance"
 
-	// Default reasons for node/service maintenance mode
+// Default reasons for node/service maintenance mode
 	defaultNodeMaintReason = "Maintenance mode is enabled for this node, " +
-		"but no reason was provided. This is a default message."
+	"but no reason was provided. This is a default message."
 	defaultServiceMaintReason = "Maintenance mode is enabled for this " +
-		"service, but no reason was provided. This is a default message."
+	"service, but no reason was provided. This is a default message."
 )
 
 var (
-	// dnsNameRe checks if a name or tag is dns-compatible.
+// dnsNameRe checks if a name or tag is dns-compatible.
 	dnsNameRe = regexp.MustCompile(`^[a-zA-Z0-9\-]+$`)
 )
 
@@ -417,7 +417,7 @@ func (a *Agent) setupKeyrings(config *consul.Config) error {
 		}
 	}
 
-LOAD:
+	LOAD:
 	if _, err := os.Stat(fileLAN); err == nil {
 		config.SerfLANConfig.KeyringFile = fileLAN
 	}
@@ -628,9 +628,9 @@ func (a *Agent) sendCoordinate() {
 				continue
 			}
 
-			// TODO - Consider adding a distance check so we don't send
-			// an update if the position hasn't changed by more than a
-			// threshold.
+		// TODO - Consider adding a distance check so we don't send
+		// an update if the position hasn't changed by more than a
+		// threshold.
 			req := structs.CoordinateUpdateRequest{
 				Datacenter:   a.config.Datacenter,
 				Node:         a.config.NodeName,
@@ -739,16 +739,16 @@ func (a *Agent) AddService(service *structs.NodeService, chkTypes CheckTypes, pe
 	// Warn if the service name is incompatible with DNS
 	if !dnsNameRe.MatchString(service.Service) {
 		a.logger.Printf("[WARN] Service name %q will not be discoverable "+
-			"via DNS due to invalid characters. Valid characters include "+
-			"all alpha-numerics and dashes.", service.Service)
+		"via DNS due to invalid characters. Valid characters include "+
+		"all alpha-numerics and dashes.", service.Service)
 	}
 
 	// Warn if any tags are incompatible with DNS
 	for _, tag := range service.Tags {
 		if !dnsNameRe.MatchString(tag) {
 			a.logger.Printf("[WARN] Service tag %q will not be discoverable "+
-				"via DNS due to invalid characters. Valid characters include "+
-				"all alpha-numerics and dashes.", tag)
+			"via DNS due to invalid characters. Valid characters include "+
+			"all alpha-numerics and dashes.", tag)
 		}
 	}
 
@@ -1078,7 +1078,7 @@ func (a *Agent) persistCheckState(check *CheckTTL, status, output string) error
 
 	// Write the state to the file
 	file := filepath.Join(dir, stringHash(check.CheckID))
-	if err := ioutil.WriteFile(file, buf, 0600); err != nil {
+	if err := writeFileAtomic(file, buf, 0600); err != nil {
 		return fmt.Errorf("failed writing file %q: %s", file, err)
 	}
 

command/agent/util.go

@@ -4,10 +4,12 @@ import (
 	"bytes"
 	"crypto/md5"
 	"fmt"
+	"io/ioutil"
 	"math"
 	"os"
 	"os/exec"
 	"os/user"
+	"path"
 	"runtime"
 	"strconv"
 	"time"
@@ -16,12 +18,12 @@ import (
 )
 
 const (
-	// This scale factor means we will add a minute after we cross 128 nodes,
-	// another at 256, another at 512, etc. By 8192 nodes, we will scale up
-	// by a factor of 8.
-	//
-	// If you update this, you may need to adjust the tuning of
-	// CoordinateUpdatePeriod and CoordinateUpdateMaxBatchSize.
+// This scale factor means we will add a minute after we cross 128 nodes,
+// another at 256, another at 512, etc. By 8192 nodes, we will scale up
+// by a factor of 8.
+//
+// If you update this, you may need to adjust the tuning of
+// CoordinateUpdatePeriod and CoordinateUpdateMaxBatchSize.
 	aeScaleThreshold = 128
 )
 
@@ -108,7 +110,7 @@ func setFilePermissions(path string, p FilePermissions) error {
 		return fmt.Errorf("invalid user specified: %v", p.User())
 	}
 
-GROUP:
+	GROUP:
 	if p.Group() != "" {
 		if gid, err = strconv.Atoi(p.Group()); err != nil {
 			return fmt.Errorf("invalid group specified: %v", p.Group())
@@ -132,3 +134,34 @@ GROUP:
 
 	return nil
 }
+
+// writeFileAtomic writes to a temporary file and swaps temp with the real file path
+// as long as everything else succeeds
+func writeFileAtomic(filename string, data []byte, perm os.FileMode) error {
+	dir, name := path.Split(filename)
+	f, err := ioutil.TempFile(dir, name)
+	if err != nil {
+		return err
+	}
+	// Remove the file after swapping atomically. If we can create the file
+	// we should have the permissions to remove it
+	defer os.Remove(f.Name())
+	// write data
+	if _, err = f.Write(data); err != nil {
+		return err
+	}
+	// flush data to disk
+	if err = f.Sync(); err != nil {
+		return err
+	}
+	// close the file
+	if err = f.Close(); err != nil {
+		return err
+	}
+	// set permissions
+	if err = os.Chmod(f.Name(), perm); err != nil {
+		return err
+	}
+	// swap temp file with actual file location
+	return os.Rename(f.Name(), filename)
+}

command/agent/util_test.go

@@ -1,6 +1,7 @@
 package agent
 
 import (
+	"bytes"
 	"io/ioutil"
 	"os"
 	"runtime"
@@ -91,3 +92,24 @@ func TestSetFilePermissions(t *testing.T) {
 		t.Fatalf("bad: %s", fi.Mode())
 	}
 }
+
+func TestAtomicWrite(t *testing.T) {
+	data := []byte("Test data")
+	f, err := ioutil.TempFile("", "test_atomic_write")
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	defer os.Remove(f.Name())
+	err = writeFileAtomic(f.Name(), data, 0644)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	readData, err := ioutil.ReadFile(f.Name())
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	// verify that the read data is equal to the original data
+	if !bytes.Equal(data, readData) {
+		t.Fatalf("Size of original data: %d does not match size of read data: %d", len(data), len(readData))
+	}
+}