engine: resources: Add reversible API and file resource

This adds the first reversible resource (file) and the necessary engine
API hooks to make it all work. This allows a special "reversed" resource
to be added to the subsequent graph in the stream when an earlier
version "disappears". This disappearance can happen if it was previously
in an if statement that then becomes false.

It might be wise to combine the use of this meta parameter with the use
of the `realize` meta parameter to ensure that your reversed resource
actually runs at least once, if there's a chance that it might be gone
for a while.

This patch also adds a new test harness for testing resources. It
doesn't test the "live" aspect of resources, as it doesn't run Watch,
but it was designed to ensure CheckApply works as intended, and it runs
very quickly with a simplified timeline of happenings.

docs/documentation.md

@@ -270,6 +270,23 @@ and it can't guarantee it if the resource is blocked because of a failed
 pre-requisite resource.
 *XXX: This is currently not implemented!*
 
+#### Reverse
+
+Boolean. Reverse is a property that some resources can implement that specifies
+that some "reverse" operation should happen when that resource "disappears". A
+disappearance happens when a resource is defined in one instance of the graph,
+and is gone in the subsequent one. This disappearance can happen if it was
+previously in an if statement that then becomes false.
+
+This is helpful for building robust programs with the engine. The engine adds a
+"reversed" resource to that subsequent graph to accomplish the desired "reverse"
+mechanics. The specifics of what this entails is a property of the particular
+resource that is being "reversed".
+
+It might be wise to combine the use of this meta parameter with the use of the
+`realize` meta parameter to ensure that your reversed resource actually runs at
+least once, if there's a chance that it might be gone for a while.
+
 ### Lang metadata file
 
 Any module *must* have a metadata file in its root. It must be named

engine/cmp.go

@@ -152,6 +152,18 @@ func ResCmp(r1, r2 Res) error {
 		}
 	}
 
+	// compare meta params for resources with reversible traits
+	r1v, ok1 := r1.(ReversibleRes)
+	r2v, ok2 := r2.(ReversibleRes)
+	if ok1 != ok2 {
+		return fmt.Errorf("reversible differs") // they must be different (optional)
+	}
+	if ok1 && ok2 {
+		if r1v.ReversibleMeta().Cmp(r2v.ReversibleMeta()) != nil {
+			return fmt.Errorf("reversible differs")
+		}
+	}
+
 	return nil
 }
 
@@ -280,6 +292,18 @@ func AdaptCmp(r1, r2 CompatibleRes) error {
 		}
 	}
 
+	// compare meta params for resources with reversible traits
+	r1v, ok1 := r1.(ReversibleRes)
+	r2v, ok2 := r2.(ReversibleRes)
+	if ok1 != ok2 {
+		return fmt.Errorf("reversible differs") // they must be different (optional)
+	}
+	if ok1 && ok2 {
+		if r1v.ReversibleMeta().Cmp(r2v.ReversibleMeta()) != nil {
+			return fmt.Errorf("reversible differs")
+		}
+	}
+
 	return nil
 }
 

engine/copy.go

@@ -106,6 +106,16 @@ func ResCopy(r CopyableRes) (CopyableRes, error) {
 		}
 	}
 
+	// copy meta params for resources with reversible traits
+	if x, ok := r.(ReversibleRes); ok {
+		dst, ok := res.(ReversibleRes)
+		if !ok {
+			// programming error
+			panic("reversible interfaces are illogical")
+		}
+		dst.SetReversibleMeta(x.ReversibleMeta()) // no need to copy atm
+	}
+
 	return res, nil
 }
 

engine/graph/autoedge/autoedge.go

@@ -89,6 +89,9 @@ func AutoEdge(graph *pgraph.Graph, debug bool, logf func(format string, v ...int
 			}
 		}
 	}
+
+	// It would be great to ensure we didn't add any loops here, but instead
+	// of checking now, we'll move the check into the main loop.
 	return nil
 }
 

engine/graph/autogroup/autogroup.go

@@ -66,5 +66,8 @@ func AutoGroup(ag engine.AutoGrouper, g *pgraph.Graph, debug bool, logf func(for
 		}
 	}
 
+	// It would be great to ensure we didn't add any loops here, but instead
+	// of checking now, we'll move the check into the main loop.
+
 	return nil
 }

engine/graph/autogroup/util.go

@@ -18,6 +18,9 @@
 package autogroup
 
 import (
+	"fmt"
+
+	"github.com/purpleidea/mgmt/engine"
 	"github.com/purpleidea/mgmt/pgraph"
 	"github.com/purpleidea/mgmt/util/errwrap"
 )
@@ -112,8 +115,17 @@ func VertexMerge(g *pgraph.Graph, v1, v2 pgraph.Vertex, vertexMergeFn func(pgrap
 			// note: This branch isn't used if the vertexMergeFn
 			// decides to just merge logically on its own instead
 			// of actually returning something that we then merge.
-			v1 = v // TODO: ineffassign?
+			v1 = v // XXX: ineffassign?
 			//*v1 = *v
+
+			// Ensure that everything still validates. (For safety!)
+			r, ok := v1.(engine.Res) // TODO: v ?
+			if !ok {
+				return fmt.Errorf("not a Res")
+			}
+			if err := engine.Validate(r); err != nil {
+				return errwrap.Wrapf(err, "the Res did not Validate")
+			}
 		}
 	}
 	g.DeleteVertex(v2) // remove grouped vertex

engine/graph/reverse.go

@@ -0,0 +1,295 @@
+// Mgmt
+// Copyright (C) 2013-2019+ James Shubin and the project contributors
+// Written by James Shubin <james@shubin.ca> and the project contributors
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package graph
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path"
+	"sort"
+
+	"github.com/purpleidea/mgmt/engine"
+	engineUtil "github.com/purpleidea/mgmt/engine/util"
+	"github.com/purpleidea/mgmt/pgraph"
+	"github.com/purpleidea/mgmt/util/errwrap"
+)
+
+const (
+	// ReverseFile is the file name in the resource state dir where any
+	// reversal information is stored.
+	ReverseFile = "reverse"
+
+	// ReversePerm is the permissions mode used to create the ReverseFile.
+	ReversePerm = 0600
+)
+
+// Reversals adds the reversals onto the loaded graph. This should happen last,
+// and before Commit.
+func (obj *Engine) Reversals() error {
+	if obj.nextGraph == nil {
+		return fmt.Errorf("there is no active graph to add reversals to")
+	}
+
+	// Initially get all of the reversals to seek out all possible errors.
+	// XXX: The engine needs to know where data might have been stored if we
+	// XXX: want to potentially allow alternate read/write paths, like etcd.
+	// XXX: In this scenario, we'd have to store a token somewhere to let us
+	// XXX: know to look elsewhere for the special ReversalList read method.
+	data, err := obj.ReversalList() // (map[string]string, error)
+	if err != nil {
+		return errwrap.Wrapf(err, "the reversals had errors")
+	}
+
+	if len(data) == 0 {
+		return nil // end early
+	}
+
+	resMatch := func(r1, r2 engine.Res) bool { // simple match on UID only!
+		if r1.Kind() != r2.Kind() {
+			return false
+		}
+		if r1.Name() != r2.Name() {
+			return false
+		}
+		return true
+	}
+	resInList := func(needle engine.Res, haystack []engine.Res) bool {
+		for _, res := range haystack {
+			if resMatch(needle, res) {
+				return true
+			}
+		}
+		return false
+	}
+
+	if obj.Debug {
+		obj.Logf("decoding %d reversals...", len(data))
+	}
+	resources := []engine.Res{}
+
+	// do this in a sorted order so that it errors deterministically
+	sorted := []string{}
+	for key := range data {
+		sorted = append(sorted, key)
+	}
+	sort.Strings(sorted)
+	for _, key := range sorted {
+		val := data[key]
+		// XXX: replace this ResToB64 method with one that stores it in
+		// a human readable format, in case someone wants to hack and
+		// edit it manually.
+		// XXX: we probably want this to be YAML, it works with the diff
+		// too...
+		r, err := engineUtil.B64ToRes(val)
+		if err != nil {
+			return errwrap.Wrapf(err, "error decoding res with UID: `%s`", key)
+		}
+
+		res, ok := r.(engine.ReversibleRes)
+		if !ok {
+			// this requirement is here to keep things simpler...
+			return errwrap.Wrapf(err, "decoded res with UID: `%s` was not reversible", key)
+		}
+
+		matchFn := func(vertex pgraph.Vertex) (bool, error) {
+			r, ok := vertex.(engine.Res)
+			if !ok {
+				return false, fmt.Errorf("not a Res")
+			}
+			if !resMatch(r, res) {
+				return false, nil
+			}
+			return true, nil
+		}
+
+		// FIXME: not efficient, we could build a cache-map first
+		vertex, err := obj.nextGraph.VertexMatchFn(matchFn) // (Vertex, error)
+		if err != nil {
+			return errwrap.Wrapf(err, "error searching graph for match")
+		}
+		if vertex != nil { // found one!
+			continue // it doesn't need reversing yet
+		}
+
+		// TODO: check for (incompatible?) duplicates instead
+		if resInList(res, resources) { // we've already got this one...
+			continue
+		}
+
+		// We set this in two different places to be safe. It ensures
+		// that we erase the reversal state file after we've used it.
+		res.ReversibleMeta().Reversal = true // set this for later...
+
+		resources = append(resources, res)
+	}
+
+	if len(resources) == 0 {
+		return nil // end early
+	}
+
+	// Now that we've passed the chance of any errors, we modify the graph.
+	obj.Logf("adding %d reversals...", len(resources))
+	for _, res := range resources {
+		obj.nextGraph.AddVertex(res)
+	}
+	// TODO: Do we want a way for stored reversals to add edges too?
+
+	// It would be great to ensure we didn't add any loops here, but instead
+	// of checking now, we'll move the check into the main loop.
+	return nil
+}
+
+// ReversalList returns all the available pending reversal data on this host. It
+// can then be decoded by whatever method is appropriate for.
+func (obj *Engine) ReversalList() (map[string]string, error) {
+	result := make(map[string]string) // some key to contents
+
+	dir := obj.statePrefix() // loop through this dir...
+	files, err := ioutil.ReadDir(dir)
+	if err != nil && !os.IsNotExist(err) {
+		return nil, errwrap.Wrapf(err, "error reading list of state dirs")
+	} else if err != nil {
+		return result, nil // nothing found, no state dir exists yet
+	}
+
+	for _, x := range files {
+		key := x.Name() // some uid for the resource
+		file := path.Join(dir, key, ReverseFile)
+		content, err := ioutil.ReadFile(file)
+		if err != nil && !os.IsNotExist(err) {
+			return nil, errwrap.Wrapf(err, "could not read reverse file: %s", file)
+		} else if err != nil {
+			continue // file does not exist, skip
+		}
+
+		// file exists!
+		str := string(content)
+		result[key] = str // save
+	}
+
+	return result, nil
+}
+
+// ReversalInit performs the reversal initialization steps if necessary for this
+// resource.
+func (obj *State) ReversalInit() error {
+	res, ok := obj.Vertex.(engine.ReversibleRes)
+	if !ok {
+		return nil // nothing to do
+	}
+
+	if res.ReversibleMeta().Disabled {
+		return nil // nothing to do, reversal isn't enabled
+	}
+
+	// If the reversal is enabled, but we are the result of a previous
+	// reversal, then this will overwrite that older reversal request, and
+	// our resource should be designed to deal with that. This happens if we
+	// return a reversible resource as the reverse of a resource that was
+	// reversed. It's probably fairly rare.
+	if res.ReversibleMeta().Reversal {
+		obj.Logf("triangle reversal") // warn!
+	}
+
+	r, err := res.Reversed()
+	if err != nil {
+		return errwrap.Wrapf(err, "could not reverse: %s", res.String())
+	}
+	if r == nil {
+		return nil // this can't be reversed, or isn't implemented here
+	}
+
+	// We set this in two different places to be safe. It ensures that we
+	// erase the reversal state file after we've used it.
+	r.ReversibleMeta().Reversal = true // set this for later...
+
+	// XXX: replace this ResToB64 method with one that stores it in a human
+	// readable format, in case someone wants to hack and edit it manually.
+	// XXX: we probably want this to be YAML, it works with the diff too...
+	str, err := engineUtil.ResToB64(r)
+	if err != nil {
+		return errwrap.Wrapf(err, "could not encode: %s", res.String())
+	}
+
+	// TODO: put this method on traits.Reversible as part of the interface?
+	return obj.ReversalWrite(str, res.ReversibleMeta().Overwrite) // Store!
+}
+
+// ReversalClose performs the reversal shutdown steps if necessary for this
+// resource.
+func (obj *State) ReversalClose() error {
+	res, ok := obj.Vertex.(engine.ReversibleRes)
+	if !ok {
+		return nil // nothing to do
+	}
+
+	// Don't check res.ReversibleMeta().Disabled because we're removing the
+	// previous one. That value only applies if we're doing a new reversal.
+
+	if !res.ReversibleMeta().Reversal {
+		return nil // nothing to erase, we're not a reversal resource
+	}
+
+	if !obj.isStateOK { // did we successfully reverse?
+		obj.Logf("did not complete reversal") // warn
+		return nil
+	}
+
+	// TODO: put this method on traits.Reversible as part of the interface?
+	return obj.ReversalDelete() // Erase our reversal instructions.
+}
+
+// ReversalWrite stores the reversal state information for this resource.
+func (obj *State) ReversalWrite(str string, overwrite bool) error {
+	dir, err := obj.varDir("") // private version
+	if err != nil {
+		return errwrap.Wrapf(err, "could not get VarDir for reverse")
+	}
+	file := path.Join(dir, ReverseFile) // return a unique file
+
+	content, err := ioutil.ReadFile(file)
+	if err != nil && !os.IsNotExist(err) {
+		return errwrap.Wrapf(err, "could not read reverse file: %s", file)
+	}
+
+	// file exists and we shouldn't overwrite if different
+	if err == nil && !overwrite {
+		// compare to existing file
+		oldStr := string(content)
+		if str != oldStr {
+			obj.Logf("existing, pending, reversible resource exists")
+			//obj.Logf("diff:")
+			//obj.Logf("") // TODO: print the diff w/o and secret values
+			return fmt.Errorf("existing, pending, reversible resource exists")
+		}
+	}
+
+	return ioutil.WriteFile(file, []byte(str), ReversePerm)
+}
+
+// ReversalDelete removes the reversal state information for this resource.
+func (obj *State) ReversalDelete() error {
+	dir, err := obj.varDir("") // private version
+	if err != nil {
+		return errwrap.Wrapf(err, "could not get VarDir for reverse")
+	}
+	file := path.Join(dir, ReverseFile) // return a unique file
+
+	return errwrap.Wrapf(os.Remove(file), "could not remove reverse state file")
+}

engine/graph/state.go

@@ -203,6 +203,12 @@ func (obj *State) Init() error {
 	if obj.Debug {
 		obj.Logf("Init(%s)", res)
 	}
+
+	// write the reverse request to the disk...
+	if err := obj.ReversalInit(); err != nil {
+		return err // TODO: test this code path...
+	}
+
 	err := res.Init(obj.init)
 	if obj.Debug {
 		obj.Logf("Init(%s): Return(%+v)", res, err)
@@ -236,12 +242,23 @@ func (obj *State) Close() error {
 	if obj.Debug {
 		obj.Logf("Close(%s)", res)
 	}
-	err := res.Close()
+
-	if obj.Debug {
+	var reverr error
-		obj.Logf("Close(%s): Return(%+v)", res, err)
+	// clear the reverse request from the disk...
+	if err := obj.ReversalClose(); err != nil {
+		// TODO: test this code path...
+		// TODO: should this be an error or a warning?
+		reverr = err
 	}
 
-	return err
+	reterr := res.Close()
+	if obj.Debug {
+		obj.Logf("Close(%s): Return(%+v)", res, reterr)
+	}
+
+	reterr = errwrap.Append(reterr, reverr)
+
+	return reterr
 }
 
 // Poke sends a notification on the poke channel. This channel is used to notify

engine/resources/file.go

@@ -62,6 +62,7 @@ type FileRes struct {
 	traits.Edgeable
 	//traits.Groupable // TODO: implement this
 	traits.Recvable
+	traits.Reversible
 
 	init *engine.Init
 
@@ -1066,6 +1067,98 @@ func (obj *FileRes) Copy() engine.CopyableRes {
 	}
 }
 
+// Reversed returns the "reverse" or "reciprocal" resource. This is used to
+// "clean" up after a previously defined resource has been removed.
+func (obj *FileRes) Reversed() (engine.ReversibleRes, error) {
+	// NOTE: Previously, we did some more complicated management of reversed
+	// properties. For example, we could add mode and state even when they
+	// weren't originally specified. This code has now been simplified to
+	// avoid this complexity, because it's not really necessary, and it is
+	// somewhat illogical anyways.
+
+	// TODO: reversing this could be tricky, since we'd store it all
+	if obj.isDir() { // XXX: limit this error to a defined state or content?
+		return nil, fmt.Errorf("can't reverse a dir yet")
+	}
+
+	cp, err := engine.ResCopy(obj)
+	if err != nil {
+		return nil, errwrap.Wrapf(err, "could not copy")
+	}
+	rev, ok := cp.(engine.ReversibleRes)
+	if !ok {
+		return nil, fmt.Errorf("not reversible")
+	}
+	rev.ReversibleMeta().Disabled = true // the reverse shouldn't run again
+
+	res, ok := cp.(*FileRes)
+	if !ok {
+		return nil, fmt.Errorf("copied res was not our kind")
+	}
+
+	// these are already copied in, and we don't need to change them...
+	//res.Path = obj.Path
+	//res.Dirname = obj.Dirname
+	//res.Basename = obj.Basename
+
+	//res.Source = "" // XXX: what should we do with this?
+
+	// these are already copied in, and we don't need to change them...
+	//res.Recurse = obj.Recurse
+	//res.Force = obj.Force
+
+	if obj.State == FileStateExists {
+		res.State = FileStateAbsent
+	}
+	if obj.State == FileStateAbsent {
+		res.State = FileStateExists
+	}
+
+	// If we've specified content, we might need to restore the original, OR
+	// if we're removing the file with a `state => "absent"`, save it too...
+	// The `res.State != FileStateAbsent` check is an optional optimization.
+	if (obj.Content != nil || obj.State == FileStateAbsent) && res.State != FileStateAbsent {
+		content, err := ioutil.ReadFile(obj.getPath())
+		if err != nil && !os.IsNotExist(err) {
+			return nil, errwrap.Wrapf(err, "could not read file for reversal storage")
+		}
+		res.Content = nil
+		if err == nil {
+			str := string(content)
+			res.Content = &str // set contents
+		}
+	}
+	if res.State == FileStateAbsent { // can't specify content when absent!
+		res.Content = nil
+	}
+
+	// There is a race if the operating system is adding/changing/removing
+	// the file between the ioutil.Readfile at the top and here. If there is
+	// a discrepancy between the two, then you might get an unexpected
+	// reverse, but in reality, your perspective is pretty absurd. This is a
+	// user error, and not an issue we actually care about, afaict.
+	fileInfo, err := os.Stat(obj.getPath())
+	if err != nil && !os.IsNotExist(err) {
+		return nil, errwrap.Wrapf(err, "could not stat file for reversal information")
+	}
+	res.Owner = ""
+	res.Group = ""
+	res.Mode = ""
+	if err == nil {
+		stUnix, ok := fileInfo.Sys().(*syscall.Stat_t)
+		// XXX: add a !ok error scenario or some alternative?
+		if ok { // if not, this isn't unix
+			res.Owner = strconv.FormatInt(int64(stUnix.Uid), 10) // Uid is a uint32
+			res.Group = strconv.FormatInt(int64(stUnix.Gid), 10) // Gid is a uint32
+		}
+
+		// TODO: use Mode().String() when we support full rwx style mode specs!
+		res.Mode = fmt.Sprintf("%#o", fileInfo.Mode().Perm()) // 0400, 0777, etc.
+	}
+
+	return res, nil
+}
+
 // smartPath adds a trailing slash to the path if it is a directory.
 func smartPath(fileInfo os.FileInfo) string {
 	smartPath := fileInfo.Name() // absolute path

engine/resources/resources_test.go

@@ -30,6 +30,7 @@ import (
 
 	"github.com/purpleidea/mgmt/engine"
 	"github.com/purpleidea/mgmt/util"
+	"github.com/purpleidea/mgmt/util/errwrap"
 )
 
 // TODO: consider providing this as a lib so that we can add tests into the
@@ -554,3 +555,461 @@ func TestResources1(t *testing.T) {
 		})
 	}
 }
+
+// TestResources2 just tests a partial execution of the resource by running
+// CheckApply and Reverse and basics without the mainloop. It's a less accurate
+// representation of a running resource, but is still useful for many
+// circumstances. This also uses a simpler timeline, because it was not possible
+// to get the reference passing of the reversed resource working with the fancy
+// version.
+func TestResources2(t *testing.T) {
+	type test struct { // an individual test
+		name     string
+		timeline []func() error // TODO: this could be a generator that keeps pushing out steps until it's done!
+		expect   func() error   // function to check for expected state
+		startup  func() error   // function to run as startup (unused?)
+		cleanup  func() error   // function to run as cleanup
+	}
+
+	// resValidate runs Validate on the res.
+	resValidate := func(res engine.Res) func() error {
+		// run Close
+		return func() error {
+			return res.Validate()
+		}
+	}
+	// resInit runs Init on the res.
+	resInit := func(res engine.Res) func() error {
+		logf := func(format string, v ...interface{}) {
+			// noop for now
+		}
+		init := &engine.Init{
+			//Debug: debug,
+			Logf: logf,
+
+			// unused
+			Send: func(st interface{}) error {
+				return nil
+			},
+			Recv: func() map[string]*engine.Send {
+				return map[string]*engine.Send{}
+			},
+		}
+		// run Init
+		return func() error {
+			return res.Init(init)
+
+		}
+	}
+	// resCheckApply runs CheckApply with noop = false for the res. It
+	// errors if the returned values aren't what we were expecting.
+	resCheckApply := func(res engine.Res, expCheckOK bool, expErr error) func() error {
+		return func() error {
+			checkOK, err := res.CheckApply(true) // no noop!
+			if err != expErr {
+				return fmt.Errorf("error from CheckApply did not match expected: `%+v` != `%+v`", err, expErr)
+			}
+			if checkOK != expCheckOK {
+				return fmt.Errorf("result from CheckApply did not match expected: `%t` != `%t`", checkOK, expCheckOK)
+			}
+			return nil
+		}
+	}
+	// resClose runs Close on the res.
+	resClose := func(res engine.Res) func() error {
+		// run Close
+		return func() error {
+			return res.Close()
+		}
+	}
+	// resReversal runs Reverse on the resource and stores the result in the
+	// rev variable. This should be called before the res CheckApply, and
+	// usually before Init, but after Validate.
+	resReversal := func(res engine.Res, rev *engine.Res) func() error {
+		return func() error {
+			r, ok := res.(engine.ReversibleRes)
+			if !ok {
+				return fmt.Errorf("res is not a ReversibleRes")
+			}
+
+			// We don't really need this to be checked here.
+			//if r.ReversibleMeta().Disabled {
+			//	return fmt.Errorf("res did not specify Meta:reverse")
+			//}
+
+			if r.ReversibleMeta().Reversal {
+				//logf("triangle reversal") // warn!
+			}
+
+			reversed, err := r.Reversed()
+			if err != nil {
+				return errwrap.Wrapf(err, "could not reverse: %s", r.String())
+			}
+			if reversed == nil {
+				return nil // this can't be reversed, or isn't implemented here
+			}
+
+			reversed.ReversibleMeta().Reversal = true // set this for later...
+
+			retRes, ok := reversed.(engine.Res)
+			if !ok {
+				return fmt.Errorf("not a Res")
+			}
+
+			*rev = retRes // store!
+			return nil
+		}
+	}
+	fileWrite := func(p, s string) func() error {
+		// write the file to path
+		return func() error {
+			return ioutil.WriteFile(p, []byte(s), 0666)
+		}
+	}
+	fileExpect := func(p, s string) func() error {
+		// check the contents at the path match the string we expect
+		return func() error {
+			content, err := ioutil.ReadFile(p)
+			if err != nil {
+				return err
+			}
+			if string(content) != s {
+				return fmt.Errorf("contents did not match in %s", p)
+			}
+			return nil
+		}
+	}
+	fileAbsent := func(p string) func() error {
+		// does the file exist?
+		return func() error {
+			_, err := os.Stat(p)
+			if !os.IsNotExist(err) {
+				return fmt.Errorf("file was supposed to be absent, got: %+v", err)
+			}
+			return nil
+		}
+	}
+	fileRemove := func(p string) func() error {
+		// remove the file at path
+		return func() error {
+			err := os.Remove(p)
+			// if the file isn't there, don't error
+			if err != nil && !os.IsNotExist(err) {
+				return err
+			}
+			return nil
+		}
+	}
+
+	testCases := []test{}
+	{
+		//file "/tmp/somefile" {
+		//	content => "some new text\n",
+		//	state => "exists",
+		//
+		//	Meta:reverse => true,
+		//}
+		r1 := makeRes("file", "r1")
+		res := r1.(*FileRes) // if this panics, the test will panic
+		p := "/tmp/somefile"
+		res.Path = p
+		res.State = "exists"
+		content := "some new text\n"
+		res.Content = &content
+		original := "this is the original state\n" // original state
+		var r2 engine.Res                          // future reversed resource
+
+		timeline := []func() error{
+			fileWrite(p, original),
+			fileExpect(p, original),
+			resValidate(r1),
+			resReversal(r1, &r2), // runs in Init to snapshot
+			func() error { // random test
+				if st := r2.(*FileRes).State; st != "absent" {
+					return fmt.Errorf("unexpected state: %s", st)
+				}
+				return nil
+			},
+			resInit(r1),
+			resCheckApply(r1, false, nil), // changed
+			fileExpect(p, content),
+			resCheckApply(r1, true, nil), // it's already good
+			resClose(r1),
+			//resValidate(r2), // no!!!
+			func() error {
+				// wrap it b/c it is currently nil
+				return r2.Validate()
+			},
+			func() error {
+				return resInit(r2)()
+			},
+			func() error {
+				return resCheckApply(r2, false, nil)()
+			},
+			func() error {
+				return resCheckApply(r2, true, nil)()
+			},
+			func() error {
+				return resClose(r2)()
+			},
+			fileAbsent(p), // ensure it's absent
+		}
+
+		testCases = append(testCases, test{
+			name:     "some file",
+			timeline: timeline,
+			expect:   func() error { return nil },
+			startup:  func() error { return nil },
+			cleanup:  func() error { return nil },
+		})
+	}
+	{
+		//file "/tmp/somefile" {
+		//	content => "some new text\n",
+		//
+		//	Meta:reverse => true,
+		//}
+		//# and there's an existing file at this path...
+		r1 := makeRes("file", "r1")
+		res := r1.(*FileRes) // if this panics, the test will panic
+		p := "/tmp/somefile"
+		res.Path = p
+		//res.State = "exists" // unspecified
+		content := "some new text\n"
+		res.Content = &content
+		original := "this is the original state\n" // original state
+		var r2 engine.Res                          // future reversed resource
+
+		timeline := []func() error{
+			fileWrite(p, original),
+			fileExpect(p, original),
+			resValidate(r1),
+			resReversal(r1, &r2), // runs in Init to snapshot
+			func() error { // random test
+				// state should be unspecified
+				if st := r2.(*FileRes).State; st == "absent" || st == "exists" {
+					return fmt.Errorf("unexpected state: %s", st)
+				}
+				return nil
+			},
+			resInit(r1),
+			resCheckApply(r1, false, nil), // changed
+			fileExpect(p, content),
+			resCheckApply(r1, true, nil), // it's already good
+			resClose(r1),
+			//resValidate(r2),
+			func() error {
+				// wrap it b/c it is currently nil
+				return r2.Validate()
+			},
+			func() error {
+				return resInit(r2)()
+			},
+			func() error {
+				return resCheckApply(r2, false, nil)()
+			},
+			func() error {
+				return resCheckApply(r2, true, nil)()
+			},
+			func() error {
+				return resClose(r2)()
+			},
+			fileExpect(p, original), // we restored the contents!
+			fileRemove(p),           // cleanup
+		}
+
+		testCases = append(testCases, test{
+			name:     "some file restore",
+			timeline: timeline,
+			expect:   func() error { return nil },
+			startup:  func() error { return nil },
+			cleanup:  func() error { return nil },
+		})
+	}
+	{
+		//file "/tmp/somefile" {
+		//	content => "some new text\n",
+		//
+		//	Meta:reverse => true,
+		//}
+		//# and there's NO existing file at this path...
+		//# NOTE: This is a corner case subtlety... if you don't want
+		// this particular behaviour, then specify `state` and you won't
+		// get something potentially unexpected. This is probably the
+		// better choice, because it's otherwise hard to emulate this
+		// behaviour, where as specifying state gets you the other
+		// possibility.
+		r1 := makeRes("file", "r1")
+		res := r1.(*FileRes) // if this panics, the test will panic
+		p := "/tmp/somefile"
+		res.Path = p
+		//res.State = "exists" // unspecified
+		content := "some new text\n"
+		res.Content = &content
+		var r2 engine.Res // future reversed resource
+
+		timeline := []func() error{
+			fileRemove(p), // ensure no file exists
+			resValidate(r1),
+			resReversal(r1, &r2), // runs in Init to snapshot
+			func() error { // random test
+				// state should be unspecified i think
+				// TODO: or should it be absent?
+				if st := r2.(*FileRes).State; st == "absent" || st == "exists" {
+					return fmt.Errorf("unexpected state: %s", st)
+				}
+				return nil
+			},
+			resInit(r1),
+			resCheckApply(r1, false, nil), // changed
+			fileExpect(p, content),
+			resCheckApply(r1, true, nil), // it's already good
+			resClose(r1),
+			//resValidate(r2),
+			func() error {
+				// wrap it b/c it is currently nil
+				return r2.Validate()
+			},
+			func() error {
+				return resInit(r2)()
+			},
+			//func() error {
+			//	return resCheckApply(r2, false, nil)()
+			//},
+			func() error { // it's already in the correct state
+				return resCheckApply(r2, true, nil)()
+			},
+			func() error {
+				return resClose(r2)()
+			},
+			// TODO: instead should r2 have removed the file?
+			// TODO: the subtlety is that state wasn't specified :/
+			fileExpect(p, content), // we never changed it back...
+			fileRemove(p),          // cleanup
+		}
+
+		testCases = append(testCases, test{
+			name:     "ambiguous file restore",
+			timeline: timeline,
+			expect:   func() error { return nil },
+			startup:  func() error { return nil },
+			cleanup:  func() error { return nil },
+		})
+	}
+	{
+		//file "/tmp/somefile" {
+		//	state => "absent",
+		//
+		//	Meta:reverse => true,
+		//}
+		r1 := makeRes("file", "r1")
+		res := r1.(*FileRes) // if this panics, the test will panic
+		p := "/tmp/somefile"
+		res.Path = p
+		res.State = "absent"
+		original := "this is the original state\n" // original state
+		var r2 engine.Res                          // future reversed resource
+
+		timeline := []func() error{
+			fileWrite(p, original),
+			fileExpect(p, original),
+			resValidate(r1),
+			resReversal(r1, &r2), // runs in Init to snapshot
+			func() error { // random test
+				if st := r2.(*FileRes).State; st != "exists" {
+					return fmt.Errorf("unexpected state: %s", st)
+				}
+				return nil
+			},
+			resInit(r1),
+			resCheckApply(r1, false, nil), // changed
+			fileAbsent(p),                 // ensure it got removed
+			resCheckApply(r1, true, nil),  // it's already good
+			resClose(r1),
+			//resValidate(r2), // no!!!
+			func() error {
+				// wrap it b/c it is currently nil
+				return r2.Validate()
+			},
+			func() error {
+				return resInit(r2)()
+			},
+			func() error {
+				return resCheckApply(r2, false, nil)()
+			},
+			func() error {
+				return resCheckApply(r2, true, nil)()
+			},
+			func() error {
+				return resClose(r2)()
+			},
+			fileExpect(p, original), // ensure it's back to original
+		}
+
+		testCases = append(testCases, test{
+			name:     "some removal",
+			timeline: timeline,
+			expect:   func() error { return nil },
+			startup:  func() error { return nil },
+			cleanup:  func() error { return nil },
+		})
+	}
+
+	names := []string{}
+	for index, tc := range testCases { // run all the tests
+		if tc.name == "" {
+			t.Errorf("test #%d: not named", index)
+			continue
+		}
+		if util.StrInList(tc.name, names) {
+			t.Errorf("test #%d: duplicate sub test name of: %s", index, tc.name)
+			continue
+		}
+		names = append(names, tc.name)
+		t.Run(fmt.Sprintf("test #%d (%s)", index, tc.name), func(t *testing.T) {
+			timeline, expect, startup, cleanup := tc.timeline, tc.expect, tc.startup, tc.cleanup
+
+			t.Logf("test #%d: starting...\n", index)
+			defer t.Logf("test #%d: done!", index)
+
+			//debug := testing.Verbose() // set via the -test.v flag to `go test`
+			//logf := func(format string, v ...interface{}) {
+			//	t.Logf(fmt.Sprintf("test #%d: ", index)+format, v...)
+			//}
+
+			t.Logf("test #%d: running startup()", index)
+			if err := startup(); err != nil {
+				t.Errorf("test #%d: FAIL", index)
+				t.Errorf("test #%d: could not startup: %+v", index, err)
+			}
+			defer func() {
+				t.Logf("test #%d: running cleanup()", index)
+				if err := cleanup(); err != nil {
+					t.Errorf("test #%d: FAIL", index)
+					t.Errorf("test #%d: could not cleanup: %+v", index, err)
+				}
+			}()
+
+			// run timeline
+			t.Logf("test #%d: executing timeline", index)
+			for ix, step := range timeline {
+				t.Logf("test #%d: step(%d)...", index, ix)
+				if err := step(); err != nil {
+					t.Errorf("test #%d: FAIL", index)
+					t.Errorf("test #%d: step(%d) action failed: %s", index, ix, err.Error())
+					break
+				}
+			}
+
+			t.Logf("test #%d: shutting down...", index)
+
+			if err := expect(); err != nil {
+				t.Errorf("test #%d: FAIL", index)
+				t.Errorf("test #%d: expect failed: %s", index, err.Error())
+				return
+			}
+
+			// all done!
+		})
+	}
+}

engine/reverse.go

@@ -41,10 +41,14 @@ type ReversibleRes interface {
 
 	// Reversed returns the "reverse" or "reciprocal" resource. This is used
 	// to "clean" up after a previously defined resource has been removed.
-	// Interestingly, this returns the core Res interface instead of a
+	// Interestingly, this could return the core Res interface instead of a
 	// ReversibleRes, because there is no requirement that the reverse of a
 	// Res be the same kind of Res, and the reverse might not be reversible!
-	Reversed() (Res, error)
+	// However, in practice, it's nice to use some of the Reversible meta
+	// params in the built value, so keep things simple and have this be a
+	// reversible res. The Res itself doesn't have to implement Reversed()
+	// in a meaningful way, it can just return nil and it will get ignored.
+	Reversed() (ReversibleRes, error)
 }
 
 // ReversibleMeta provides some parameters specific to reversible resources.
@@ -53,6 +57,16 @@ type ReversibleMeta struct {
 	// resource.
 	Disabled bool
 
+	// Reversal specifies that the resource was built from a reversal. This
+	// must be set if the resource was built by a reversal.
+	Reversal bool
+
+	// Overwrite specifies that we should overwrite any existing stored
+	// reversible resource if one that is pending already exists. If this is
+	// false, and a resource with the same name and kind exists, then this
+	// will cause an error.
+	Overwrite bool
+
 	// TODO: add options here, including whether to reverse edges, etc...
 }
 
@@ -61,5 +75,11 @@ func (obj *ReversibleMeta) Cmp(rm *ReversibleMeta) error {
 	if obj.Disabled != rm.Disabled {
 		return fmt.Errorf("values for Disabled are different")
 	}
+	if obj.Reversal != rm.Reversal { // TODO: do we want to compare these?
+		return fmt.Errorf("values for Reversal are different")
+	}
+	if obj.Overwrite != rm.Overwrite {
+		return fmt.Errorf("values for Overwrite are different")
+	}
 	return nil
 }

examples/lang/reverse1.mcl

@@ -0,0 +1,25 @@
+import "datetime"
+import "math"
+
+$now = datetime.now()
+
+# alternate every four seconds
+$mod0 = math.mod($now, 8) == 0
+$mod1 = math.mod($now, 8) == 1
+$mod2 = math.mod($now, 8) == 2
+$mod3 = math.mod($now, 8) == 3
+$mod = $mod0 || $mod1 || $mod2 || $mod3
+
+file "/tmp/mgmt/" {
+	state => "exists",
+}
+
+# file should disappear and re-appear every four seconds
+if $mod {
+	file "/tmp/mgmt/hello" {
+		content => "please say abracadabra...\n",
+		state => "exists",
+
+		Meta:reverse => true,
+	}
+}

examples/lang/reverse2.mcl

@@ -0,0 +1,25 @@
+import "datetime"
+import "math"
+
+$now = datetime.now()
+
+# alternate every four seconds
+$mod0 = math.mod($now, 8) == 0
+$mod1 = math.mod($now, 8) == 1
+$mod2 = math.mod($now, 8) == 2
+$mod3 = math.mod($now, 8) == 3
+$mod = $mod0 || $mod1 || $mod2 || $mod3
+
+file "/tmp/mgmt/" {
+	state => "exists",
+}
+
+# file should re-appear and disappear every four seconds
+# it will even preserve and then restore the pre-existing content!
+if $mod {
+	file "/tmp/mgmt/hello" {
+		state => "absent", # delete the file
+
+		Meta:reverse => true,
+	}
+}

examples/lang/reverse3.mcl

@@ -0,0 +1,25 @@
+import "datetime"
+import "math"
+
+$now = datetime.now()
+
+# alternate every four seconds
+$mod0 = math.mod($now, 8) == 0
+$mod1 = math.mod($now, 8) == 1
+$mod2 = math.mod($now, 8) == 2
+$mod3 = math.mod($now, 8) == 3
+$mod = $mod0 || $mod1 || $mod2 || $mod3
+
+file "/tmp/mgmt/" {
+	state => "exists",
+}
+
+# file should change the mode every four seconds
+# editing the file contents at anytime is allowed
+if $mod {
+	file "/tmp/mgmt/hello" {
+		mode => "0777",
+
+		Meta:reverse => true,
+	}
+}

lib/main.go

@@ -706,6 +706,24 @@ func (obj *Main) Run() error {
 				continue
 			}
 
+			// XXX: can we change this into a ge.Apply operation?
+			// run reversals; modifies the graph
+			if err := obj.ge.Reversals(); err != nil {
+				obj.ge.Abort() // delete graph
+				Logf("error running the reversals: %+v", err)
+				continue
+			}
+
+			// Double check before we commit.
+			if err := obj.ge.Apply(func(graph *pgraph.Graph) error {
+				_, e := graph.TopologicalSort() // am i a dag or not?
+				return e
+			}); err != nil { // apply an operation to the new graph
+				obj.ge.Abort() // delete graph
+				Logf("error running the TopologicalSort: %+v", err)
+				continue
+			}
+
 			// TODO: do we want to do a transitive reduction?
 			// FIXME: run a type checker that verifies all the send->recv relationships
 

test/test-govet.sh

@@ -16,6 +16,13 @@ function run-test()
 
 GO_VERSION=($(go version))
 
+function typos() {
+	if grep -i 'reversable' "$1"; then	# the word is "reversible"
+		return 1
+	fi
+	return 0
+}
+
 function simplify-gocase() {
 	if grep 'case _ = <-' "$1"; then
 		return 1	# 'case _ = <- can be simplified to: case <-'
@@ -106,6 +113,7 @@ for file in `find . -maxdepth 9 -type f -name '*.go' -not -path './old/*' -not -
 	#	continue
 	#fi
 	run-test grep 'log.Print' "$file" | grep '\\n"' && fail_test 'no newline needed in log.Print*()'	# no \n needed in log.Printf or log.Println
+	run-test typos "$file"
 	run-test simplify-gocase "$file"
 	run-test token-coloncheck "$file"
 	run-test naked-error "$file"