chore(coreos-cloudinit): bump to 0.7.6

fix(update): Fix restart of update-engine

Documentation/cloud-config.md

@@ -99,7 +99,8 @@ For more information on fleet configuration, see the [fleet documentation][fleet
 
 The `coreos.update.*` parameters manipulate settings related to how CoreOS instances are updated.
 
-These fields will be written out to and replace `/etc/coreos/update.conf`. If only one of the parameters is given it will only overwrite the given field.
+These fields will be written out to and replace `/etc/coreos/update.conf`. If only one of the parameters is given it will only overwrite the given field. 
+The `reboot-strategy` parameter also affects the behaviour of [locksmith](https://github.com/coreos/locksmith). 
 
 - **reboot-strategy**: One of "reboot", "etcd-lock", "best-effort" or "off" for controlling when reboots are issued after an update is performed.
   - _reboot_: Reboot immediately after an update is applied.
@@ -109,6 +110,10 @@ These fields will be written out to and replace `/etc/coreos/update.conf`. If on
 - **server**: is the omaha endpoint URL which will be queried for updates.
 - **group**:  signifies the channel which should be used for automatic updates.  This value defaults to the version of the image initially downloaded. (one of "master", "alpha", "beta", "stable")
 
+*Note: cloudinit will only manipulate the locksmith unit file in the systemd runtime directory (`/run/systemd/system/locksmithd.service`). If any manual modifications are made to an overriding unit configuration file (e.g. `/etc/systemd/system/locksmithd.service`), cloudinit will no longer be able to control the locksmith service unit.*
+
+##### Example
+
 ```
 #cloud-config
 coreos:
@@ -150,9 +155,6 @@ coreos:
           Restart=always
           ExecStart=/usr/bin/docker start -a redis_server
           ExecStop=/usr/bin/docker stop -t 2 redis_server
-          
-          [Install]
-          WantedBy=local.target
 ```
 
 Start the built-in `etcd` and `fleet` services:
@@ -277,7 +279,7 @@ For example, if you have an installation of GitHub Enterprise, you can provide a
 
 users:
   - name: elroy
-    coreos-ssh-import-url: https://token:<OAUTH-TOKEN>@github-enterprise.example.com/users/elroy/keys
+    coreos-ssh-import-url: https://github-enterprise.example.com/api/v3/users/elroy/keys?access_token=<TOKEN>
 ```
 
 You can also specify any URL whose response matches the JSON format for public keys:

coreos-cloudinit.go

@@ -14,7 +14,7 @@ import (
 	"github.com/coreos/coreos-cloudinit/system"
 )
 
-const version = "0.7.4"
+const version = "0.7.6"
 
 func main() {
 	var printVersion bool

initialize/config.go

@@ -4,7 +4,6 @@ import (
 	"errors"
 	"fmt"
 	"log"
-	"path"
 
 	"github.com/coreos/coreos-cloudinit/third_party/launchpad.net/goyaml"
 
@@ -20,11 +19,9 @@ type CloudConfigFile interface {
 }
 
 // CloudConfigUnit represents a CoreOS specific configuration option that can generate
-// an associated system.Unit to be created/enabled appropriately
+// associated system.Units to be created/enabled appropriately
 type CloudConfigUnit interface {
-	// Unit should either return (*system.Unit, error), or (nil, nil) if nothing
-	// needs to be done for this configuration option.
-	Unit(root string) (*system.Unit, error)
+	Units(root string) ([]system.Unit, error)
 }
 
 // CloudConfig encapsulates the entire cloud-config configuration file and maps directly to YAML
@@ -215,27 +212,25 @@ func Apply(cfg CloudConfig, env *Environment) error {
 	}
 
 	for _, ccu := range []CloudConfigUnit{cfg.Coreos.Etcd, cfg.Coreos.Fleet, cfg.Coreos.Update} {
-		u, err := ccu.Unit(env.Root())
+		u, err := ccu.Units(env.Root())
 		if err != nil {
 			return err
 		}
-		if u != nil {
-			cfg.Coreos.Units = append(cfg.Coreos.Units, *u)
-		}
+		cfg.Coreos.Units = append(cfg.Coreos.Units, u...)
 	}
 
 	for _, file := range cfg.WriteFiles {
-		file.Path = path.Join(env.Root(), file.Path)
-		if err := system.WriteFile(&file); err != nil {
+		path, err := system.WriteFile(&file, env.Root())
+		if err != nil {
 			return err
 		}
-		log.Printf("Wrote file %s to filesystem", file.Path)
+		log.Printf("Wrote file %s to filesystem", path)
 	}
 
 	commands := make(map[string]string, 0)
 	reload := false
 	for _, unit := range cfg.Coreos.Units {
-		dst := system.UnitDestination(&unit, env.Root())
+		dst := unit.Destination(env.Root())
 		if unit.Content != "" {
 			log.Printf("Writing unit %s to filesystem at path %s", unit.Name, dst)
 			if err := system.PlaceUnit(&unit, dst); err != nil {
@@ -247,7 +242,12 @@ func Apply(cfg CloudConfig, env *Environment) error {
 
 		if unit.Mask {
 			log.Printf("Masking unit file %s", unit.Name)
-			if err := system.MaskUnit(unit.Name, env.Root()); err != nil {
+			if err := system.MaskUnit(&unit, env.Root()); err != nil {
+				return err
+			}
+		} else if unit.Runtime {
+			log.Printf("Ensuring runtime unit file %s is unmasked", unit.Name)
+			if err := system.UnmaskUnit(&unit, env.Root()); err != nil {
 				return err
 			}
 		}

initialize/etcd.go

@@ -28,9 +28,9 @@ func (ee EtcdEnvironment) String() (out string) {
 	return
 }
 
-// Unit creates a Unit file drop-in for etcd, using any configured
+// Units creates a Unit file drop-in for etcd, using any configured
 // options and adding a default MachineID if unset.
-func (ee EtcdEnvironment) Unit(root string) (*system.Unit, error) {
+func (ee EtcdEnvironment) Units(root string) ([]system.Unit, error) {
 	if ee == nil {
 		return nil, nil
 	}
@@ -45,10 +45,11 @@ func (ee EtcdEnvironment) Unit(root string) (*system.Unit, error) {
 		}
 	}
 
-	return &system.Unit{
+	etcd := system.Unit{
 		Name:    "etcd.service",
 		Runtime: true,
 		DropIn:  true,
 		Content: ee.String(),
-	}, nil
+	}
+	return []system.Unit{etcd}, nil
 }

initialize/etcd_test.go

@@ -59,7 +59,7 @@ Environment="ETCD_PEER_BIND_ADDR=127.0.0.1:7002"
 }
 
 func TestEtcdEnvironmentWrittenToDisk(t *testing.T) {
-	ec := EtcdEnvironment{
+	ee := EtcdEnvironment{
 		"name":           "node001",
 		"discovery":      "http://disco.example.com/foobar",
 		"peer-bind-addr": "127.0.0.1:7002",
@@ -70,17 +70,18 @@ func TestEtcdEnvironmentWrittenToDisk(t *testing.T) {
 	}
 	defer os.RemoveAll(dir)
 
-	u, err := ec.Unit(dir)
+	uu, err := ee.Units(dir)
 	if err != nil {
 		t.Fatalf("Generating etcd unit failed: %v", err)
 	}
-	if u == nil {
-		t.Fatalf("Returned nil etcd unit unexpectedly")
+	if len(uu) != 1 {
+		t.Fatalf("Expected 1 unit to be returned, got %d", len(uu))
 	}
+	u := uu[0]
 
-	dst := system.UnitDestination(u, dir)
+	dst := u.Destination(dir)
 	os.Stderr.WriteString("writing to " + dir + "\n")
-	if err := system.PlaceUnit(u, dst); err != nil {
+	if err := system.PlaceUnit(&u, dst); err != nil {
 		t.Fatalf("Writing of EtcdEnvironment failed: %v", err)
 	}
 
@@ -124,17 +125,18 @@ func TestEtcdEnvironmentWrittenToDiskDefaultToMachineID(t *testing.T) {
 		t.Fatalf("Failed writing out /etc/machine-id: %v", err)
 	}
 
-	u, err := ee.Unit(dir)
+	uu, err := ee.Units(dir)
 	if err != nil {
 		t.Fatalf("Generating etcd unit failed: %v", err)
 	}
-	if u == nil {
-		t.Fatalf("Returned nil etcd unit unexpectedly")
+	if len(uu) == 0 {
+		t.Fatalf("Returned empty etcd units unexpectedly")
 	}
+	u := uu[0]
 
-	dst := system.UnitDestination(u, dir)
+	dst := u.Destination(dir)
 	os.Stderr.WriteString("writing to " + dir + "\n")
-	if err := system.PlaceUnit(u, dst); err != nil {
+	if err := system.PlaceUnit(&u, dst); err != nil {
 		t.Fatalf("Writing of EtcdEnvironment failed: %v", err)
 	}
 
@@ -159,8 +161,8 @@ func TestEtcdEnvironmentWhenNil(t *testing.T) {
 	if ee != nil {
 		t.Fatalf("EtcdEnvironment is not nil")
 	}
-	u, err := ee.Unit("")
-	if u != nil || err != nil {
-		t.Fatalf("Unit returned a non-nil value for nil input")
+	uu, err := ee.Units("")
+	if len(uu) != 0 || err != nil {
+		t.Fatalf("Units returned value for nil input")
 	}
 }

initialize/fleet.go

@@ -19,16 +19,17 @@ func (fe FleetEnvironment) String() (out string) {
 	return
 }
 
-// Unit generates a Unit file drop-in for fleet, if any fleet options were
+// Units generates a Unit file drop-in for fleet, if any fleet options were
 // configured in cloud-config
-func (fe FleetEnvironment) Unit(root string) (*system.Unit, error) {
+func (fe FleetEnvironment) Units(root string) ([]system.Unit, error) {
 	if len(fe) < 1 {
 		return nil, nil
 	}
-	return &system.Unit{
+	fleet := system.Unit{
 		Name:    "fleet.service",
 		Runtime: true,
 		DropIn:  true,
 		Content: fe.String(),
-	}, nil
+	}
+	return []system.Unit{fleet}, nil
 }

initialize/fleet_test.go

@@ -19,20 +19,21 @@ Environment="FLEET_PUBLIC_IP=12.34.56.78"
 
 func TestFleetUnit(t *testing.T) {
 	cfg := make(FleetEnvironment, 0)
-	u, err := cfg.Unit("/")
-	if u != nil {
+	uu, err := cfg.Units("/")
+	if len(uu) != 0 {
 		t.Errorf("unexpectedly generated unit with empty FleetEnvironment")
 	}
 
 	cfg["public-ip"] = "12.34.56.78"
 
-	u, err = cfg.Unit("/")
+	uu, err = cfg.Units("/")
 	if err != nil {
 		t.Errorf("error generating fleet unit: %v", err)
 	}
-	if u == nil {
-		t.Fatalf("unexpectedly got nil unit generating fleet unit!")
+	if len(uu) != 1 {
+		t.Fatalf("expected 1 unit generated, got %d", len(uu))
 	}
+	u := uu[0]
 	if !u.Runtime {
 		t.Errorf("bad Runtime for generated fleet unit!")
 	}

initialize/manage_etc_hosts_test.go

@@ -50,9 +50,7 @@ func TestEtcHostsWrittenToDisk(t *testing.T) {
 		t.Fatalf("manageEtcHosts returned nil file unexpectedly")
 	}
 
-	f.Path = path.Join(dir, f.Path)
-
-	if err := system.WriteFile(f); err != nil {
+	if _, err := system.WriteFile(f, dir); err != nil {
 		t.Fatalf("Error writing EtcHosts: %v", err)
 	}
 

initialize/oem_test.go

@@ -31,8 +31,7 @@ func TestOEMReleaseWrittenToDisk(t *testing.T) {
 		t.Fatalf("OEMRelease returned nil file unexpectedly")
 	}
 
-	f.Path = path.Join(dir, f.Path)
-	if err := system.WriteFile(f); err != nil {
+	if _, err := system.WriteFile(f, dir); err != nil {
 		t.Fatalf("Writing of OEMRelease failed: %v", err)
 	}
 

initialize/update.go

@@ -12,7 +12,8 @@ import (
 )
 
 const (
-	locksmithUnit = "locksmithd.service"
+	locksmithUnit    = "locksmithd.service"
+	updateEngineUnit = "update-engine.service"
 )
 
 // updateOption represents a configurable update option, which, if set, will be
@@ -36,7 +37,6 @@ var updateOptions = []*updateOption{
 	&updateOption{
 		key:    "group",
 		prefix: "GROUP=",
-		valid:  []string{"master", "beta", "alpha", "stable"},
 	},
 	&updateOption{
 		key:    "server",
@@ -126,24 +126,40 @@ func (uc UpdateConfig) File(root string) (*system.File, error) {
 	}, nil
 }
 
-// GetUnit generates a locksmith system.Unit, if reboot-strategy was set in
-// cloud-config, for the cloud-init initializer to act on appropriately
-func (uc UpdateConfig) Unit(root string) (*system.Unit, error) {
-	strategy, ok := uc["reboot-strategy"]
-	if !ok {
-		return nil, nil
+// Units generates units for the cloud-init initializer to act on:
+// - a locksmith system.Unit, if "reboot-strategy" was set in cloud-config
+// - an update_engine system.Unit, if "group" was set in cloud-config
+func (uc UpdateConfig) Units(root string) ([]system.Unit, error) {
+	var units []system.Unit
+	if strategy, ok := uc["reboot-strategy"]; ok {
+		ls := &system.Unit{
+			Name:    locksmithUnit,
+			Command: "restart",
+			Mask:    false,
+			Runtime: true,
+		}
+
+		if strategy == "off" {
+			ls.Command = "stop"
+			ls.Mask = true
+		}
+		units = append(units, *ls)
 	}
 
-	u := &system.Unit{
-		Name:    locksmithUnit,
-		Command: "restart",
-		Mask:    false,
+	rue := false
+	if _, ok := uc["group"]; ok {
+		rue = true
+	}
+	if _, ok := uc["server"]; ok {
+		rue = true
+	}
+	if rue {
+		ue := system.Unit{
+			Name:    updateEngineUnit,
+			Command: "restart",
+		}
+		units = append(units, ue)
 	}
 
-	if strategy == "off" {
-		u.Command = "stop"
-		u.Mask = true
-	}
-
-	return u, nil
+	return units, nil
 }

initialize/update_test.go

@@ -38,12 +38,12 @@ func TestEmptyUpdateConfig(t *testing.T) {
 	if f != nil {
 		t.Errorf("getting file from empty UpdateConfig should have returned nil, got %v", f)
 	}
-	u, err := uc.Unit("")
+	uu, err := uc.Units("")
 	if err != nil {
 		t.Error("unexpected error getting unit from empty UpdateConfig")
 	}
-	if u != nil {
-		t.Errorf("getting unit from empty UpdateConfig should have returned nil, got %v", u)
+	if len(uu) != 0 {
+		t.Errorf("getting unit from empty UpdateConfig should have returned zero units, got %d", len(uu))
 	}
 }
 
@@ -106,6 +106,21 @@ SERVER=http://foo.com`
 			t.Errorf("File has incorrect contents, got %v, want %v", got, want)
 		}
 	}
+
+	uu, err := u.Units(dir)
+	if err != nil {
+		t.Errorf("unexpected error getting units from UpdateConfig: %v", err)
+	} else if len(uu) != 1 {
+		t.Errorf("unexpected number of files returned from UpdateConfig: want 1, got %d", len(uu))
+	} else {
+		unit := uu[0]
+		if unit.Name != "update-engine.service" {
+			t.Errorf("bad name for generated unit: want update-engine.service, got %s", unit.Name)
+		}
+		if unit.Command != "restart" {
+			t.Errorf("bad command for generated unit: want restart, got %s", unit.Command)
+		}
+	}
 }
 
 func TestRebootStrategies(t *testing.T) {
@@ -145,12 +160,13 @@ func TestRebootStrategies(t *testing.T) {
 				t.Errorf("couldn't find expected line %v for reboot-strategy=%v", s.line)
 			}
 		}
-		u, err := uc.Unit(dir)
+		uu, err := uc.Units(dir)
 		if err != nil {
 			t.Errorf("failed to generate unit for reboot-strategy=%v!", s.name)
-		} else if u == nil {
-			t.Errorf("generated empty unit for reboot-strategy=%v", s.name)
+		} else if len(uu) != 1 {
+			t.Errorf("unexpected number of units for reboot-strategy=%v: %d", s.name, len(uu))
 		} else {
+			u := uu[0]
 			if u.Name != locksmithUnit {
 				t.Errorf("unit generated for reboot strategy=%v had bad name: %v", s.name, u.Name)
 			}
@@ -189,8 +205,7 @@ func TestUpdateConfWrittenToDisk(t *testing.T) {
 			t.Fatal("Unexpectedly got nil updateconfig file")
 		}
 
-		f.Path = path.Join(dir, f.Path)
-		if err := system.WriteFile(f); err != nil {
+		if _, err := system.WriteFile(f, dir); err != nil {
 			t.Fatalf("Error writing update config: %v", err)
 		}
 

initialize/workspace.go

@@ -3,6 +3,7 @@ package initialize
 import (
 	"io/ioutil"
 	"path"
+	"strings"
 
 	"github.com/coreos/coreos-cloudinit/system"
 )
@@ -28,21 +29,23 @@ func PersistScriptInWorkspace(script system.Script, workspace string) (string, e
 	}
 	tmp.Close()
 
+	relpath := strings.TrimPrefix(tmp.Name(), workspace)
+
 	file := system.File{
-		Path: tmp.Name(),
+		Path:               relpath,
 		RawFilePermissions: "0744",
-		Content: string(script),
+		Content:            string(script),
 	}
 
-	err = system.WriteFile(&file)
-	return file.Path, err
+	return system.WriteFile(&file, workspace)
 }
 
 func PersistUnitNameInWorkspace(name string, workspace string) error {
 	file := system.File{
-		Path: path.Join(workspace, "scripts", "unit-name"),
+		Path:               path.Join("scripts", "unit-name"),
 		RawFilePermissions: "0644",
-		Content: name,
+		Content:            name,
 	}
-	return system.WriteFile(&file)
+	_, err := system.WriteFile(&file, workspace)
+	return err
 }

system/file.go

@@ -31,33 +31,55 @@ func (f *File) Permissions() (os.FileMode, error) {
 	return os.FileMode(perm), nil
 }
 
-func WriteFile(f *File) error {
+func WriteFile(f *File, root string) (string, error) {
 	if f.Encoding != "" {
-		return fmt.Errorf("Unable to write file with encoding %s", f.Encoding)
+		return "", fmt.Errorf("Unable to write file with encoding %s", f.Encoding)
 	}
 
-	if err := os.MkdirAll(path.Dir(f.Path), os.FileMode(0755)); err != nil {
-		return err
+	fullpath := path.Join(root, f.Path)
+	dir := path.Dir(fullpath)
+
+	if err := EnsureDirectoryExists(dir); err != nil {
+		return "", err
 	}
 
 	perm, err := f.Permissions()
 	if err != nil {
-		return err
+		return "", err
 	}
 
-	if err := ioutil.WriteFile(f.Path, []byte(f.Content), perm); err != nil {
-		return err
+	var tmp *os.File
+	// Create a temporary file in the same directory to ensure it's on the same filesystem
+	if tmp, err = ioutil.TempFile(dir, "cloudinit-temp"); err != nil {
+		return "", err
+	}
+
+	if err := ioutil.WriteFile(tmp.Name(), []byte(f.Content), perm); err != nil {
+		return "", err
+	}
+
+	if err := tmp.Close(); err != nil {
+		return "", err
+	}
+
+	// Ensure the permissions are as requested (since WriteFile can be affected by sticky bit)
+	if err := os.Chmod(tmp.Name(), perm); err != nil {
+		return "", err
 	}
 
 	if f.Owner != "" {
 		// We shell out since we don't have a way to look up unix groups natively
-		cmd := exec.Command("chown", f.Owner, f.Path)
+		cmd := exec.Command("chown", f.Owner, tmp.Name())
 		if err := cmd.Run(); err != nil {
-			return err
+			return "", err
 		}
 	}
 
-	return nil
+	if err := os.Rename(tmp.Name(), fullpath); err != nil {
+		return "", err
+	}
+
+	return fullpath, nil
 }
 
 func EnsureDirectoryExists(dir string) error {

system/file_test.go

@@ -4,7 +4,6 @@ import (
 	"io/ioutil"
 	"os"
 	"path"
-	"syscall"
 	"testing"
 )
 
@@ -13,18 +12,22 @@ func TestWriteFileUnencodedContent(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Unable to create tempdir: %v", err)
 	}
-	defer syscall.Rmdir(dir)
+	defer os.RemoveAll(dir)
 
-	fullPath := path.Join(dir, "tmp", "foo")
+	fn := "foo"
+	fullPath := path.Join(dir, fn)
 
 	wf := File{
-		Path:        fullPath,
-		Content:     "bar",
+		Path:               fn,
+		Content:            "bar",
 		RawFilePermissions: "0644",
 	}
 
-	if err := WriteFile(&wf); err != nil {
+	path, err := WriteFile(&wf, dir)
+	if err != nil {
 		t.Fatalf("Processing of WriteFile failed: %v", err)
+	} else if path != fullPath {
+		t.Fatalf("WriteFile returned bad path: want %s, got %s", fullPath, path)
 	}
 
 	fi, err := os.Stat(fullPath)
@@ -51,15 +54,15 @@ func TestWriteFileInvalidPermission(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Unable to create tempdir: %v", err)
 	}
-	defer syscall.Rmdir(dir)
+	defer os.RemoveAll(dir)
 
 	wf := File{
-		Path:        path.Join(dir, "tmp", "foo"),
-		Content:     "bar",
+		Path:               path.Join(dir, "tmp", "foo"),
+		Content:            "bar",
 		RawFilePermissions: "pants",
 	}
 
-	if err := WriteFile(&wf); err == nil {
+	if _, err := WriteFile(&wf, dir); err == nil {
 		t.Fatalf("Expected error to be raised when writing file with invalid permission")
 	}
 }
@@ -69,17 +72,21 @@ func TestWriteFilePermissions(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Unable to create tempdir: %v", err)
 	}
-	defer syscall.Rmdir(dir)
+	defer os.RemoveAll(dir)
 
-	fullPath := path.Join(dir, "tmp", "foo")
+	fn := "foo"
+	fullPath := path.Join(dir, fn)
 
 	wf := File{
-		Path:               fullPath,
+		Path:               fn,
 		RawFilePermissions: "0755",
 	}
 
-	if err := WriteFile(&wf); err != nil {
+	path, err := WriteFile(&wf, dir)
+	if err != nil {
 		t.Fatalf("Processing of WriteFile failed: %v", err)
+	} else if path != fullPath {
+		t.Fatalf("WriteFile returned bad path: want %s, got %s", fullPath, path)
 	}
 
 	fi, err := os.Stat(fullPath)
@@ -97,15 +104,15 @@ func TestWriteFileEncodedContent(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Unable to create tempdir: %v", err)
 	}
-	defer syscall.Rmdir(dir)
+	defer os.RemoveAll(dir)
 
 	wf := File{
-		Path: path.Join(dir, "tmp", "foo"),
-		Content: "",
+		Path:     path.Join(dir, "tmp", "foo"),
+		Content:  "",
 		Encoding: "base64",
 	}
 
-	if err := WriteFile(&wf); err == nil {
+	if _, err := WriteFile(&wf, dir); err == nil {
 		t.Fatalf("Expected error to be raised when writing file with encoding")
 	}
 }

system/systemd.go

@@ -51,10 +51,10 @@ func (u *Unit) Group() (group string) {
 
 type Script []byte
 
-// UnitDestination builds the appropriate absolute file path for
-// the given Unit. The root argument indicates the effective base
+// Destination builds the appropriate absolute file path for
+// the Unit. The root argument indicates the effective base
 // directory of the system (similar to a chroot).
-func UnitDestination(u *Unit, root string) string {
+func (u *Unit) Destination(root string) string {
 	dir := "etc"
 	if u.Runtime {
 		dir = "run"
@@ -78,12 +78,12 @@ func PlaceUnit(u *Unit, dst string) error {
 	}
 
 	file := File{
-		Path:               dst,
+		Path:               filepath.Base(dst),
 		Content:            u.Content,
 		RawFilePermissions: "0644",
 	}
 
-	err := WriteFile(&file)
+	_, err := WriteFile(&file, dir)
 	if err != nil {
 		return err
 	}
@@ -179,12 +179,12 @@ func MachineID(root string) string {
 	return id
 }
 
-// MaskUnit masks a Unit by the given name by symlinking its unit file (in
-// /etc/systemd/system) to /dev/null, analogous to `systemctl mask`
+// MaskUnit masks the given Unit by symlinking its unit file to
+// /dev/null, analogous to `systemctl mask`.
 // N.B.: Unlike `systemctl mask`, this function will *remove any existing unit
-// file* in /etc/systemd/system, to ensure that the mask will succeed.
-func MaskUnit(unit string, root string) error {
-	masked := path.Join(root, "etc", "systemd", "system", unit)
+// file at the location*, to ensure that the mask will succeed.
+func MaskUnit(unit *Unit, root string) error {
+	masked := unit.Destination(root)
 	if _, err := os.Stat(masked); os.IsNotExist(err) {
 		if err := os.MkdirAll(path.Dir(masked), os.FileMode(0755)); err != nil {
 			return err
@@ -194,3 +194,38 @@ func MaskUnit(unit string, root string) error {
 	}
 	return os.Symlink("/dev/null", masked)
 }
+
+// UnmaskUnit is analogous to systemd's unit_file_unmask. If the file
+// associated with the given Unit is empty or appears to be a symlink to
+// /dev/null, it is removed.
+func UnmaskUnit(unit *Unit, root string) error {
+	masked := unit.Destination(root)
+	ne, err := nullOrEmpty(masked)
+	if os.IsNotExist(err) {
+		return nil
+	} else if err != nil {
+		return err
+	}
+	if !ne {
+		log.Printf("%s is not null or empty, refusing to unmask", masked)
+		return nil
+	}
+	return os.Remove(masked)
+}
+
+// nullOrEmpty checks whether a given path appears to be an empty regular file
+// or a symlink to /dev/null
+func nullOrEmpty(path string) (bool, error) {
+	fi, err := os.Stat(path)
+	if err != nil {
+		return false, err
+	}
+	m := fi.Mode()
+	if m.IsRegular() && fi.Size() <= 0 {
+		return true, nil
+	}
+	if m&os.ModeCharDevice > 0 {
+		return true, nil
+	}
+	return false, nil
+}

system/systemd_test.go

@@ -25,10 +25,10 @@ Address=10.209.171.177/19
 	}
 	defer os.RemoveAll(dir)
 
-	dst := UnitDestination(&u, dir)
+	dst := u.Destination(dir)
 	expectDst := path.Join(dir, "run", "systemd", "network", "50-eth0.network")
 	if dst != expectDst {
-		t.Fatalf("UnitDestination returned %s, expected %s", dst, expectDst)
+		t.Fatalf("unit.Destination returned %s, expected %s", dst, expectDst)
 	}
 
 	if err := PlaceUnit(&u, dst); err != nil {
@@ -69,18 +69,18 @@ func TestUnitDestination(t *testing.T) {
 		DropIn: false,
 	}
 
-	dst := UnitDestination(&u, dir)
+	dst := u.Destination(dir)
 	expectDst := path.Join(dir, "etc", "systemd", "system", "foobar.service")
 	if dst != expectDst {
-		t.Errorf("UnitDestination returned %s, expected %s", dst, expectDst)
+		t.Errorf("unit.Destination returned %s, expected %s", dst, expectDst)
 	}
 
 	u.DropIn = true
 
-	dst = UnitDestination(&u, dir)
+	dst = u.Destination(dir)
 	expectDst = path.Join(dir, "etc", "systemd", "system", "foobar.service.d", cloudConfigDropIn)
 	if dst != expectDst {
-		t.Errorf("UnitDestination returned %s, expected %s", dst, expectDst)
+		t.Errorf("unit.Destination returned %s, expected %s", dst, expectDst)
 	}
 }
 
@@ -100,10 +100,10 @@ Where=/media/state
 	}
 	defer os.RemoveAll(dir)
 
-	dst := UnitDestination(&u, dir)
+	dst := u.Destination(dir)
 	expectDst := path.Join(dir, "etc", "systemd", "system", "media-state.mount")
 	if dst != expectDst {
-		t.Fatalf("UnitDestination returned %s, expected %s", dst, expectDst)
+		t.Fatalf("unit.Destination returned %s, expected %s", dst, expectDst)
 	}
 
 	if err := PlaceUnit(&u, dst); err != nil {
@@ -156,7 +156,8 @@ func TestMaskUnit(t *testing.T) {
 	defer os.RemoveAll(dir)
 
 	// Ensure mask works with units that do not currently exist
-	if err := MaskUnit("foo.service", dir); err != nil {
+	uf := &Unit{Name: "foo.service"}
+	if err := MaskUnit(uf, dir); err != nil {
 		t.Fatalf("Unable to mask new unit: %v", err)
 	}
 	fooPath := path.Join(dir, "etc", "systemd", "system", "foo.service")
@@ -169,11 +170,12 @@ func TestMaskUnit(t *testing.T) {
 	}
 
 	// Ensure mask works with unit files that already exist
+	ub := &Unit{Name: "bar.service"}
 	barPath := path.Join(dir, "etc", "systemd", "system", "bar.service")
 	if _, err := os.Create(barPath); err != nil {
 		t.Fatalf("Error creating new unit file: %v", err)
 	}
-	if err := MaskUnit("bar.service", dir); err != nil {
+	if err := MaskUnit(ub, dir); err != nil {
 		t.Fatalf("Unable to mask existing unit: %v", err)
 	}
 	barTgt, err := os.Readlink(barPath)
@@ -184,3 +186,94 @@ func TestMaskUnit(t *testing.T) {
 		t.Fatalf("unit not masked, got unit target", barTgt)
 	}
 }
+
+func TestUnmaskUnit(t *testing.T) {
+	dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-")
+	if err != nil {
+		t.Fatalf("Unable to create tempdir: %v", err)
+	}
+	defer os.RemoveAll(dir)
+
+	nilUnit := &Unit{Name: "null.service"}
+	if err := UnmaskUnit(nilUnit, dir); err != nil {
+		t.Errorf("unexpected error from unmasking nonexistent unit: %v", err)
+	}
+
+	uf := &Unit{Name: "foo.service", Content: "[Service]\nExecStart=/bin/true"}
+	dst := uf.Destination(dir)
+	if err := os.MkdirAll(path.Dir(dst), os.FileMode(0755)); err != nil {
+		t.Fatalf("Unable to create unit directory: %v", err)
+	}
+	if _, err := os.Create(dst); err != nil {
+		t.Fatalf("Unable to write unit file: %v", err)
+	}
+
+	if err := ioutil.WriteFile(dst, []byte(uf.Content), 700); err != nil {
+		t.Fatalf("Unable to write unit file: %v", err)
+	}
+	if err := UnmaskUnit(uf, dir); err != nil {
+		t.Errorf("unmask of non-empty unit returned unexpected error: %v", err)
+	}
+	got, _ := ioutil.ReadFile(dst)
+	if string(got) != uf.Content {
+		t.Errorf("unmask of non-empty unit mutated unit contents unexpectedly")
+	}
+
+	ub := &Unit{Name: "bar.service"}
+	dst = ub.Destination(dir)
+	if err := os.Symlink("/dev/null", dst); err != nil {
+		t.Fatalf("Unable to create masked unit: %v", err)
+	}
+	if err := UnmaskUnit(ub, dir); err != nil {
+		t.Errorf("unmask of unit returned unexpected error: %v", err)
+	}
+	if _, err := os.Stat(dst); !os.IsNotExist(err) {
+		t.Errorf("expected %s to not exist after unmask, but got err: %s", err)
+	}
+}
+
+func TestNullOrEmpty(t *testing.T) {
+	dir, err := ioutil.TempDir(os.TempDir(), "coreos-cloudinit-")
+	if err != nil {
+		t.Fatalf("Unable to create tempdir: %v", err)
+	}
+	defer os.RemoveAll(dir)
+
+	non := path.Join(dir, "does_not_exist")
+	ne, err := nullOrEmpty(non)
+	if !os.IsNotExist(err) {
+		t.Errorf("nullOrEmpty on nonexistent file returned bad error: %v", err)
+	}
+	if ne {
+		t.Errorf("nullOrEmpty returned true unxpectedly")
+	}
+
+	regEmpty := path.Join(dir, "regular_empty_file")
+	_, err = os.Create(regEmpty)
+	if err != nil {
+		t.Fatalf("Unable to create tempfile: %v", err)
+	}
+	gotNe, gotErr := nullOrEmpty(regEmpty)
+	if !gotNe || gotErr != nil {
+		t.Errorf("nullOrEmpty of regular empty file returned %t, %v - want true, nil", gotNe, gotErr)
+	}
+
+	reg := path.Join(dir, "regular_file")
+	if err := ioutil.WriteFile(reg, []byte("asdf"), 700); err != nil {
+		t.Fatalf("Unable to create tempfile: %v", err)
+	}
+	gotNe, gotErr = nullOrEmpty(reg)
+	if gotNe || gotErr != nil {
+		t.Errorf("nullOrEmpty of regular file returned %t, %v - want false, nil", gotNe, gotErr)
+	}
+
+	null := path.Join(dir, "null")
+	if err := os.Symlink(os.DevNull, null); err != nil {
+		t.Fatalf("Unable to create /dev/null link: %s", err)
+	}
+	gotNe, gotErr = nullOrEmpty(null)
+	if !gotNe || gotErr != nil {
+		t.Errorf("nullOrEmpty of null symlink returned %t, %v - want true, nil", gotNe, gotErr)
+	}
+
+}