diff --git a/docs/documentation.md b/docs/documentation.md index c69c238b..41953d25 100644 --- a/docs/documentation.md +++ b/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 diff --git a/engine/cmp.go b/engine/cmp.go index 4f1f0076..5880aedc 100644 --- a/engine/cmp.go +++ b/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 } diff --git a/engine/copy.go b/engine/copy.go index 195b23c9..5c7b8997 100644 --- a/engine/copy.go +++ b/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 } diff --git a/engine/graph/autoedge/autoedge.go b/engine/graph/autoedge/autoedge.go index 7d14f75a..d912b70a 100644 --- a/engine/graph/autoedge/autoedge.go +++ b/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 } diff --git a/engine/graph/autogroup/autogroup.go b/engine/graph/autogroup/autogroup.go index 54387368..0e5bd6d1 100644 --- a/engine/graph/autogroup/autogroup.go +++ b/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 } diff --git a/engine/graph/autogroup/util.go b/engine/graph/autogroup/util.go index 7ba788ce..38dc414f 100644 --- a/engine/graph/autogroup/util.go +++ b/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 diff --git a/engine/graph/reverse.go b/engine/graph/reverse.go new file mode 100644 index 00000000..a436f933 --- /dev/null +++ b/engine/graph/reverse.go @@ -0,0 +1,295 @@ +// Mgmt +// Copyright (C) 2013-2019+ James Shubin and the project contributors +// Written by James Shubin 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 . + +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") +} diff --git a/engine/graph/state.go b/engine/graph/state.go index ae3c7fba..251f48f3 100644 --- a/engine/graph/state.go +++ b/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 { - obj.Logf("Close(%s): Return(%+v)", res, err) + + var reverr error + // 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 diff --git a/engine/resources/file.go b/engine/resources/file.go index f704517c..c06d6a29 100644 --- a/engine/resources/file.go +++ b/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 diff --git a/engine/resources/resources_test.go b/engine/resources/resources_test.go index 9f1cb097..d0feb308 100644 --- a/engine/resources/resources_test.go +++ b/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! + }) + } +} diff --git a/engine/reverse.go b/engine/reverse.go index 21f62cdb..799cb5e2 100644 --- a/engine/reverse.go +++ b/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 } diff --git a/examples/lang/reverse1.mcl b/examples/lang/reverse1.mcl new file mode 100644 index 00000000..d1bb9364 --- /dev/null +++ b/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, + } +} diff --git a/examples/lang/reverse2.mcl b/examples/lang/reverse2.mcl new file mode 100644 index 00000000..b362435d --- /dev/null +++ b/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, + } +} diff --git a/examples/lang/reverse3.mcl b/examples/lang/reverse3.mcl new file mode 100644 index 00000000..cc1176ee --- /dev/null +++ b/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, + } +} diff --git a/lib/main.go b/lib/main.go index 667ed9f4..8695712a 100644 --- a/lib/main.go +++ b/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 diff --git a/test/test-govet.sh b/test/test-govet.sh index bbea891d..ea4dc58e 100755 --- a/test/test-govet.sh +++ b/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"