From b757099568917790983bf40d0cc18a2bdfe480c8 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Wed, 16 Mar 2016 15:55:10 +0100 Subject: [PATCH 01/12] fleetctl: add replace variables that will be used later This patch adds some variables that will be used in the next patch to implement replace units feature. The replace flag and the current command that's being executed. --- fleetctl/fleetctl.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fleetctl/fleetctl.go b/fleetctl/fleetctl.go index 69513e04f..31ec4d3de 100644 --- a/fleetctl/fleetctl.go +++ b/fleetctl/fleetctl.go @@ -78,6 +78,7 @@ var ( Debug bool Version bool Help bool + currentCommand string ClientDriver string ExperimentalAPI bool @@ -103,6 +104,7 @@ var ( Full bool NoLegend bool NoBlock bool + Replace bool BlockAttempts int Fields string SSHPort int @@ -287,6 +289,10 @@ func main() { } } + // We use this to know in which context are we: + // submit, load or start + globalFlags.currentCommand = cmd.Name + os.Exit(cmd.Run(cmd.Flags.Args())) } From 2c50f549f82c98019d7e0b2e021e374b2e00416d Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Thu, 17 Mar 2016 09:54:01 +0100 Subject: [PATCH 02/12] fleetctl: add checkUnitCreation() and isLocalUnitDifferent() Add checkUnitCreation() to check if the unit should be created or not. This function handles the new replace logic. Add isLocalUnitDifferent() since we don't really want to warn if the unit do really differ in case "--replace" switch was set. At the same time factor out unit matching logic. --- fleetctl/fleetctl.go | 151 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 140 insertions(+), 11 deletions(-) diff --git a/fleetctl/fleetctl.go b/fleetctl/fleetctl.go index 31ec4d3de..806d3cd00 100644 --- a/fleetctl/fleetctl.go +++ b/fleetctl/fleetctl.go @@ -75,10 +75,10 @@ var ( // flags used by all commands globalFlags = struct { - Debug bool - Version bool - Help bool - currentCommand string + Debug bool + Version bool + Help bool + currentCommand string ClientDriver string ExperimentalAPI bool @@ -669,6 +669,81 @@ func createUnit(name string, uf *unit.UnitFile) (*schema.Unit, error) { return &u, nil } +// checkReplaceUnitState checks if the unit should be replaced +// It takes a Unit object as a parameter +// It returns 0 on success and if the unit should be replaced, 1 if the +// unit should not be replaced; and any error acountered +func checkReplaceUnitState(unit *schema.Unit) (int, error) { + allowedReplace := map[string][]job.JobState{ + "submit": []job.JobState{ + job.JobStateInactive, + }, + "load": []job.JobState{ + job.JobStateInactive, + job.JobStateLoaded, + }, + "start": []job.JobState{ + job.JobStateInactive, + job.JobStateLoaded, + job.JobStateLaunched, + }, + } + + if allowedJobs, ok := allowedReplace[globalFlags.currentCommand]; ok { + for _, j := range allowedJobs { + if job.JobState(unit.DesiredState) == j { + return 0, nil + } + } + stderr("Warning: can not replace Unit(%s) in state '%s', use the appropriate command", unit.Name, unit.DesiredState) + } else { + return 1, fmt.Errorf("error 'replace' is not supported for this command") + } + + return 1, nil +} + +// checkUnitCreation checks if the unit should be created +// It takes a unit file path as a parameter. +// It returns 0 on success and if the unit should be created, 1 if the +// unit should not be created; and any error acountered +func checkUnitCreation(arg string) (int, error) { + name := unitNameMangle(arg) + + // First, check if there already exists a Unit by the given name in the Registry + unit, err := cAPI.Unit(name) + if err != nil { + return 1, fmt.Errorf("error retrieving Unit(%s) from Registry: %v", name, err) + } + + // check if the unit is running + if unit == nil { + if sharedFlags.Replace { + log.Debugf("Unit(%s) was not found in Registry", name) + } + // Create a new unit + return 0, nil + } + + // if sharedFlags.Replace is not set then we warn + different, err := isLocalUnitDifferent(arg, unit, !sharedFlags.Replace, false) + + // if sharedFlags.Replace is set then we fail for errors + if sharedFlags.Replace { + if err != nil { + return 1, err + } else if different { + return checkReplaceUnitState(unit) + } else { + stdout("Found same Unit(%s) in Registry, nothing to do", unit.Name) + } + } else if different == false { + log.Debugf("Found same Unit(%s) in Registry, no need to recreate it", name) + } + + return 1, nil +} + // lazyCreateUnits iterates over a set of unit names and, for each, attempts to // ensure that a unit by that name exists in the Registry, by checking a number // of conditions and acting on the first one that succeeds, in order of: @@ -686,14 +761,10 @@ func lazyCreateUnits(args []string) error { arg = maybeAppendDefaultUnitType(arg) name := unitNameMangle(arg) - // First, check if there already exists a Unit by the given name in the Registry - u, err := cAPI.Unit(name) + ret, err := checkUnitCreation(arg) if err != nil { - return fmt.Errorf("error retrieving Unit(%s) from Registry: %v", name, err) - } - if u != nil { - log.Debugf("Found Unit(%s) in Registry, no need to recreate it", name) - warnOnDifferentLocalUnit(arg, u) + return err + } else if ret != 0 { continue } @@ -732,6 +803,64 @@ func lazyCreateUnits(args []string) error { return nil } +// matchUnitFiles compares two unitFiles +// Returns true if the units match, false otherwise. +func matchUnitFiles(a *unit.UnitFile, b *unit.UnitFile) bool { + if a.Hash() == b.Hash() { + return true + } + + return false +} + +// matchLocalFileAndUnit compares a file with a Unit +// Returns true if the contents of the file matches the unit one, false +// otherwise; and any error ocountered +func matchLocalFileAndUnit(file string, su *schema.Unit) (bool, error) { + result := false + a := schema.MapSchemaUnitOptionsToUnitFile(su.Options) + + _, err := os.Stat(file) + if err == nil { + b, err := getUnitFromFile(file) + if err == nil { + result = matchUnitFiles(a, b) + } + } + + return result, err +} + +func isLocalUnitDifferent(file string, su *schema.Unit, warnIfDifferent bool, fatal bool) (bool, error) { + result, err := matchLocalFileAndUnit(file, su) + if err == nil { + if result == false && warnIfDifferent { + stderr("WARNING: Unit %s in registry differs from local unit file %s", su.Name, file) + } + return !result, nil + } else if fatal { + return false, err + } + + info := unit.NewUnitNameInfo(path.Base(file)) + if info == nil { + return false, fmt.Errorf("error extracting information from unit name %s", file) + } else if !info.IsInstance() { + return false, fmt.Errorf("error Unit %s does not seem to be a template unit", file) + } + + templFile := path.Join(path.Dir(file), info.Template) + result, err = matchLocalFileAndUnit(templFile, su) + if err == nil { + if result == false && warnIfDifferent { + stderr("WARNING: Unit %s in registry differs from local template unit file %s", su.Name, info.Template) + } + return !result, nil + } + + return false, err +} + func warnOnDifferentLocalUnit(loc string, su *schema.Unit) { suf := schema.MapSchemaUnitOptionsToUnitFile(su.Options) if _, err := os.Stat(loc); !os.IsNotExist(err) { From 55a23ee07f94effe1597801c11b25e733320b371 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Wed, 16 Mar 2016 16:55:42 +0100 Subject: [PATCH 03/12] fleetctl: remove warnOnDifferentLocalUnit() Just use isLocalUnitDifferent() instead of old warnOnDifferentLocalUnit() --- fleetctl/fleetctl.go | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/fleetctl/fleetctl.go b/fleetctl/fleetctl.go index 806d3cd00..05f4dadbc 100644 --- a/fleetctl/fleetctl.go +++ b/fleetctl/fleetctl.go @@ -561,7 +561,7 @@ func getUnitFileFromTemplate(uni *unit.UnitNameInfo, fileName string) (*unit.Uni } if tmpl != nil { - warnOnDifferentLocalUnit(fileName, tmpl) + isLocalUnitDifferent(fileName, tmpl, true, false) uf = schema.MapSchemaUnitOptionsToUnitFile(tmpl.Options) log.Debugf("Template Unit(%s) found in registry", uni.Template) } else { @@ -861,26 +861,6 @@ func isLocalUnitDifferent(file string, su *schema.Unit, warnIfDifferent bool, fa return false, err } -func warnOnDifferentLocalUnit(loc string, su *schema.Unit) { - suf := schema.MapSchemaUnitOptionsToUnitFile(su.Options) - if _, err := os.Stat(loc); !os.IsNotExist(err) { - luf, err := getUnitFromFile(loc) - if err == nil && luf.Hash() != suf.Hash() { - stderr("WARNING: Unit %s in registry differs from local unit file %s", su.Name, loc) - return - } - } - if uni := unit.NewUnitNameInfo(path.Base(loc)); uni != nil && uni.IsInstance() { - file := path.Join(path.Dir(loc), uni.Template) - if _, err := os.Stat(file); !os.IsNotExist(err) { - tmpl, err := getUnitFromFile(file) - if err == nil && tmpl.Hash() != suf.Hash() { - stderr("WARNING: Unit %s in registry differs from local template unit file %s", su.Name, uni.Template) - } - } - } -} - func lazyLoadUnits(args []string) ([]*schema.Unit, error) { units := make([]string, 0, len(args)) for _, j := range args { From 831c63ab382f6ab7b1c83aa513d3b47d35b10e6f Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Wed, 16 Mar 2016 16:58:01 +0100 Subject: [PATCH 04/12] unit: move MatchUnitFile() to unit package Move MatchUnitFile() to unit package we will use it inside fleetd to check for unit matching. --- fleetctl/fleetctl.go | 12 +----------- unit/unit.go | 10 ++++++++++ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/fleetctl/fleetctl.go b/fleetctl/fleetctl.go index 05f4dadbc..7cca74c6a 100644 --- a/fleetctl/fleetctl.go +++ b/fleetctl/fleetctl.go @@ -803,16 +803,6 @@ func lazyCreateUnits(args []string) error { return nil } -// matchUnitFiles compares two unitFiles -// Returns true if the units match, false otherwise. -func matchUnitFiles(a *unit.UnitFile, b *unit.UnitFile) bool { - if a.Hash() == b.Hash() { - return true - } - - return false -} - // matchLocalFileAndUnit compares a file with a Unit // Returns true if the contents of the file matches the unit one, false // otherwise; and any error ocountered @@ -824,7 +814,7 @@ func matchLocalFileAndUnit(file string, su *schema.Unit) (bool, error) { if err == nil { b, err := getUnitFromFile(file) if err == nil { - result = matchUnitFiles(a, b) + result = unit.MatchUnitFiles(a, b) } } diff --git a/unit/unit.go b/unit/unit.go index 2da58a6bb..6c54c0b91 100644 --- a/unit/unit.go +++ b/unit/unit.go @@ -136,6 +136,16 @@ func (u *UnitFile) Hash() Hash { return Hash(sha1.Sum(u.Bytes())) } +// MatchUnitFiles compares two unitFiles +// Returns true if the units match, false otherwise. +func MatchUnitFiles(a *UnitFile, b *UnitFile) bool { + if a.Hash() == b.Hash() { + return true + } + + return false +} + // RecognizedUnitType determines whether or not the given unit name represents // a recognized unit type. func RecognizedUnitType(name string) bool { From fb6fcebd9dc213a1da7f90b38a45f847ba4ca450 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Wed, 16 Mar 2016 17:19:34 +0100 Subject: [PATCH 05/12] units: when creating units check if this is a new version If there is a unit with the same name, check if the content of both differ if so then we create a new one in the registry. --- api/units.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/api/units.go b/api/units.go index 67b4a9537..47ebfa8c4 100644 --- a/api/units.go +++ b/api/units.go @@ -76,6 +76,7 @@ func (ur *unitsResource) set(rw http.ResponseWriter, req *http.Request, item str } var su schema.Unit + newUnit := false dec := json.NewDecoder(req.Body) err := dec.Decode(&su) if err != nil { @@ -105,11 +106,28 @@ func (ur *unitsResource) set(rw http.ResponseWriter, req *http.Request, item str if len(su.Options) == 0 { err := errors.New("unit does not exist and options field empty") sendError(rw, http.StatusConflict, err) + return } else if err := ValidateOptions(su.Options); err != nil { sendError(rw, http.StatusBadRequest, err) + return } else { - ur.create(rw, su.Name, &su) + newUnit = true } + } else if eu.Name == su.Name && len(su.Options) > 0 { + // There is already a unit with the same name + // check the hashes if they do not match then we will + // create a new unit with the same name and later + // the job will be updated to this new unit. + // if su.Options == 0 then probably we don't want to update + // the Unit Options but only its target job state, in + // this case just ignore. + a := schema.MapSchemaUnitOptionsToUnitFile(su.Options) + b := schema.MapSchemaUnitOptionsToUnitFile(eu.Options) + newUnit = !unit.MatchUnitFiles(a, b) + } + + if newUnit { + ur.create(rw, su.Name, &su) return } From 2a4ff2f5cc18400737c5c55391c5ce4065f9ee64 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Wed, 16 Mar 2016 17:30:49 +0100 Subject: [PATCH 06/12] registry: instruct etcd driver to ignore errors in case job keys already exists Since we started to support replacing units and updating unit states, we instruct etcd driver to ignore job keys that already exists and just replace them. --- registry/job.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/registry/job.go b/registry/job.go index 8eeda2280..152962df6 100644 --- a/registry/job.go +++ b/registry/job.go @@ -335,14 +335,13 @@ func (r *EtcdRegistry) CreateUnit(u *job.Unit) (err error) { } opts := &etcd.SetOptions{ - PrevExist: etcd.PrevNoExist, + // Since we support replacing Unit state just ignore + // previous keys. + PrevExist: etcd.PrevIgnore, } key := r.prefixed(jobPrefix, u.Name, "object") _, err = r.kAPI.Set(r.ctx(), key, val, opts) if err != nil { - if isEtcdError(err, etcd.ErrorCodeNodeExist) { - err = errors.New("job already exists") - } return } From 9e02fd7d8d49a18024cddc2ec71ef32cb04bd519 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Thu, 17 Mar 2016 10:02:31 +0100 Subject: [PATCH 07/12] fleetctl: add -replace switch to load and submit commands --- fleetctl/load.go | 1 + fleetctl/submit.go | 1 + 2 files changed, 2 insertions(+) diff --git a/fleetctl/load.go b/fleetctl/load.go index 9b1127dab..a0f65edda 100644 --- a/fleetctl/load.go +++ b/fleetctl/load.go @@ -43,6 +43,7 @@ func init() { cmdLoadUnits.Flags.BoolVar(&sharedFlags.Sign, "sign", false, "DEPRECATED - this option cannot be used") cmdLoadUnits.Flags.IntVar(&sharedFlags.BlockAttempts, "block-attempts", 0, "Wait until the jobs are loaded, performing up to N attempts before giving up. A value of 0 indicates no limit. Does not apply to global units.") cmdLoadUnits.Flags.BoolVar(&sharedFlags.NoBlock, "no-block", false, "Do not wait until the jobs have been loaded before exiting. Always the case for global units.") + cmdLoadUnits.Flags.BoolVar(&sharedFlags.Replace, "replace", false, "Replace the old loaded unit with a new one.") } func runLoadUnits(args []string) (exit int) { diff --git a/fleetctl/submit.go b/fleetctl/submit.go index 1297cff50..6c38a1352 100644 --- a/fleetctl/submit.go +++ b/fleetctl/submit.go @@ -33,6 +33,7 @@ Submit a directory of units with glob matching: func init() { cmdSubmitUnit.Flags.BoolVar(&sharedFlags.Sign, "sign", false, "DEPRECATED - this option cannot be used") + cmdSubmitUnit.Flags.BoolVar(&sharedFlags.Replace, "replace", false, "Replace the old submitted unit with a new one.") } func runSubmitUnits(args []string) (exit int) { From 799b12156b2147d3fe9609b46f3684c29ea6a43d Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Mon, 14 Mar 2016 12:55:04 +0100 Subject: [PATCH 08/12] fleetctl: add an option -replace to the start command --- fleetctl/start.go | 1 + 1 file changed, 1 insertion(+) diff --git a/fleetctl/start.go b/fleetctl/start.go index 99e03c2d1..9af3318ba 100644 --- a/fleetctl/start.go +++ b/fleetctl/start.go @@ -51,6 +51,7 @@ func init() { cmdStartUnit.Flags.BoolVar(&sharedFlags.Sign, "sign", false, "DEPRECATED - this option cannot be used") cmdStartUnit.Flags.IntVar(&sharedFlags.BlockAttempts, "block-attempts", 0, "Wait until the units are launched, performing up to N attempts before giving up. A value of 0 indicates no limit. Does not apply to global units.") cmdStartUnit.Flags.BoolVar(&sharedFlags.NoBlock, "no-block", false, "Do not wait until the units have launched before exiting. Always the case for global units.") + cmdStartUnit.Flags.BoolVar(&sharedFlags.Replace, "replace", false, "Replace the old started unit with a new one.") } func runStartUnit(args []string) (exit int) { From b6ccdcb9428e5da1345d29face15f0124b1877c6 Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Tue, 15 Mar 2016 17:18:07 +0100 Subject: [PATCH 09/12] functional: introduce new helpers for testing the replace option Add new helpers copyFile(), genNewFleetService() & destroyUnitRetrying() to prepare the new functional tests for the replace options. copyFile() is a helper to copy one file to another. genNewFleetService() is a helper to replace a string with a new one. It's necessary for the next functional tests. destroyUnitRetrying() runs "fleetctl --replace" repeatedly to ensure the unit is actually removed. --- functional/unit_action_test.go | 67 ++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/functional/unit_action_test.go b/functional/unit_action_test.go index 78a150fd7..0cd202bfc 100644 --- a/functional/unit_action_test.go +++ b/functional/unit_action_test.go @@ -15,6 +15,8 @@ package functional import ( + "fmt" + "io/ioutil" "strings" "testing" @@ -224,3 +226,68 @@ func TestUnitSSHActions(t *testing.T) { t.Errorf("Could not find expected string in journal output:\n%s", stdout) } } + +// copyFile() +func copyFile(newFile, oldFile string) error { + input, err := ioutil.ReadFile(oldFile) + if err != nil { + return err + } + err = ioutil.WriteFile(newFile, []byte(input), 0644) + if err != nil { + return err + } + return nil +} + +// genNewFleetService() is a helper for generating a temporary fleet service +// that reads from oldFile, replaces oldVal with newVal, and stores the result +// to newFile. +func genNewFleetService(newFile, oldFile, newVal, oldVal string) error { + input, err := ioutil.ReadFile(oldFile) + if err != nil { + return err + } + lines := strings.Split(string(input), "\n") + + for i, line := range lines { + if strings.Contains(line, oldVal) { + lines[i] = strings.Replace(line, oldVal, newVal, len(oldVal)) + } + } + output := strings.Join(lines, "\n") + err = ioutil.WriteFile(newFile, []byte(output), 0644) + if err != nil { + return err + } + return nil +} + +// destroyUnitRetrying() destroys the unit and ensure it disappears from the +// unit list. It could take a little time until the unit gets destroyed. +func destroyUnitRetrying(cluster platform.Cluster, m platform.Member, serviceFile string) error { + maxAttempts := 3 + found := false + var stdout string + var err error + for { + if _, _, err := cluster.Fleetctl(m, "destroy", serviceFile); err != nil { + return fmt.Errorf("Failed to destroy unit: %v", err) + } + stdout, _, err = cluster.Fleetctl(m, "list-units", "--no-legend") + if err != nil { + return fmt.Errorf("Failed to run list-units: %v", err) + } + if strings.TrimSpace(stdout) == "" || maxAttempts == 0 { + found = true + break + } + maxAttempts-- + } + + if !found { + return fmt.Errorf("Did not find 0 units in cluster: \n%s", stdout) + } + + return nil +} From 6338b3e436be329ab1a8163c42b5cad0a8decb12 Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Wed, 16 Mar 2016 12:02:34 +0100 Subject: [PATCH 10/12] functional: add new tests for the replace option TestUnit{Submit,Load,Start}Replace() tests whether a command "fleetctl {submit,load,start} --replace hello.service" works respectively. As most of the test sequences are identical, the common part is split into replaceUnitCommon(). --- functional/unit_action_test.go | 98 ++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/functional/unit_action_test.go b/functional/unit_action_test.go index 0cd202bfc..6886fc3b3 100644 --- a/functional/unit_action_test.go +++ b/functional/unit_action_test.go @@ -17,12 +17,18 @@ package functional import ( "fmt" "io/ioutil" + "os" "strings" "testing" "github.com/coreos/fleet/functional/platform" ) +const ( + tmpHelloService = "/tmp/hello.service" + fxtHelloService = "fixtures/units/hello.service" +) + // TestUnitRunnable is the simplest test possible, deplying a single-node // cluster and ensuring a unit can enter an 'active' state func TestUnitRunnable(t *testing.T) { @@ -169,6 +175,30 @@ func TestUnitRestart(t *testing.T) { } +// TestUnitSubmitReplace() tests whether a command "fleetctl submit --replace +// hello.service" works or not. +func TestUnitSubmitReplace(t *testing.T) { + if err := replaceUnitCommon("submit"); err != nil { + t.Fatal(err) + } +} + +// TestUnitLoadReplace() tests whether a command "fleetctl load --replace +// hello.service" works or not. +func TestUnitLoadReplace(t *testing.T) { + if err := replaceUnitCommon("load"); err != nil { + t.Fatal(err) + } +} + +// TestUnitStartReplace() tests whether a command "fleetctl start --replace +// hello.service" works or not. +func TestUnitStartReplace(t *testing.T) { + if err := replaceUnitCommon("start"); err != nil { + t.Fatal(err) + } +} + func TestUnitSSHActions(t *testing.T) { cluster, err := platform.NewNspawnCluster("smoke") if err != nil { @@ -227,6 +257,74 @@ func TestUnitSSHActions(t *testing.T) { } } +// replaceUnitCommon() tests whether a command "fleetctl {submit,load,start} +// --replace hello.service" works or not. +func replaceUnitCommon(cmd string) error { + // check if cmd is one of the supported commands. + listCmds := []string{"submit", "load", "start"} + found := false + for _, ccmd := range listCmds { + if ccmd == cmd { + found = true + } + } + if !found { + return fmt.Errorf("invalid command %s", cmd) + } + + cluster, err := platform.NewNspawnCluster("smoke") + if err != nil { + return fmt.Errorf("%v", err) + } + defer cluster.Destroy() + + m, err := cluster.CreateMember() + if err != nil { + return fmt.Errorf("%v", err) + } + _, err = cluster.WaitForNMachines(m, 1) + if err != nil { + return fmt.Errorf("%v", err) + } + + // run a command for a unit and assert it shows up + if _, _, err := cluster.Fleetctl(m, cmd, fxtHelloService); err != nil { + return fmt.Errorf("Unable to %s fleet unit: %v", cmd, err) + } + stdout, _, err := cluster.Fleetctl(m, "list-units", "--no-legend") + if err != nil { + return fmt.Errorf("Failed to run list-units: %v", err) + } + units := strings.Split(strings.TrimSpace(stdout), "\n") + if len(units) != 1 { + return fmt.Errorf("Did not find 1 unit in cluster: \n%s", stdout) + } + + // replace the unit and assert it shows up + err = genNewFleetService(tmpHelloService, fxtHelloService, "sleep 2", "sleep 1") + if err != nil { + return fmt.Errorf("Failed to generate a temp fleet service: %v", err) + } + if _, _, err := cluster.Fleetctl(m, cmd, "--replace", tmpHelloService); err != nil { + return fmt.Errorf("Unable to replace fleet unit: %v", err) + } + stdout, _, err = cluster.Fleetctl(m, "list-units", "--no-legend") + if err != nil { + return fmt.Errorf("Failed to run list-units: %v", err) + } + units = strings.Split(strings.TrimSpace(stdout), "\n") + if len(units) != 1 { + return fmt.Errorf("Did not find 1 unit in cluster: \n%s", stdout) + } + os.Remove(tmpHelloService) + + if err := destroyUnitRetrying(cluster, m, fxtHelloService); err != nil { + return fmt.Errorf("Cannot destroy unit %v", fxtHelloService) + } + + return nil +} + // copyFile() func copyFile(newFile, oldFile string) error { input, err := ioutil.ReadFile(oldFile) From 6d387167e07eff24d3a241a1db27e3f8d306004f Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Thu, 17 Mar 2016 10:17:55 +0100 Subject: [PATCH 11/12] functional: make {submit,load,start} run for multiple units For commands fleetctl {submit,load,start}, also test loading multiple units at the same time, and replacing each of them one after another. --- functional/unit_action_test.go | 111 +++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/functional/unit_action_test.go b/functional/unit_action_test.go index 6886fc3b3..4e9da0afe 100644 --- a/functional/unit_action_test.go +++ b/functional/unit_action_test.go @@ -27,6 +27,8 @@ import ( const ( tmpHelloService = "/tmp/hello.service" fxtHelloService = "fixtures/units/hello.service" + tmpFixtures = "/tmp/fixtures" + numUnitsReplace = 9 ) // TestUnitRunnable is the simplest test possible, deplying a single-node @@ -181,6 +183,10 @@ func TestUnitSubmitReplace(t *testing.T) { if err := replaceUnitCommon("submit"); err != nil { t.Fatal(err) } + + if err := replaceUnitMultiple("submit", numUnitsReplace); err != nil { + t.Fatal(err) + } } // TestUnitLoadReplace() tests whether a command "fleetctl load --replace @@ -189,6 +195,10 @@ func TestUnitLoadReplace(t *testing.T) { if err := replaceUnitCommon("load"); err != nil { t.Fatal(err) } + + if err := replaceUnitMultiple("load", numUnitsReplace); err != nil { + t.Fatal(err) + } } // TestUnitStartReplace() tests whether a command "fleetctl start --replace @@ -197,6 +207,10 @@ func TestUnitStartReplace(t *testing.T) { if err := replaceUnitCommon("start"); err != nil { t.Fatal(err) } + + if err := replaceUnitMultiple("start", numUnitsReplace); err != nil { + t.Fatal(err) + } } func TestUnitSSHActions(t *testing.T) { @@ -325,6 +339,103 @@ func replaceUnitCommon(cmd string) error { return nil } +// replaceUnitMultiple() tests whether a command "fleetctl {submit,load,start} +// --replace hello.service" works or not. +func replaceUnitMultiple(cmd string, n int) error { + // check if cmd is one of the supported commands. + listCmds := []string{"submit", "load", "start"} + found := false + for _, ccmd := range listCmds { + if ccmd == cmd { + found = true + } + } + if !found { + return fmt.Errorf("invalid command %s", cmd) + } + + cluster, err := platform.NewNspawnCluster("smoke") + if err != nil { + return fmt.Errorf("%v", err) + } + defer cluster.Destroy() + + m, err := cluster.CreateMember() + if err != nil { + return fmt.Errorf("%v", err) + } + _, err = cluster.WaitForNMachines(m, 1) + if err != nil { + return fmt.Errorf("%v", err) + } + + if _, err := os.Stat(tmpFixtures); os.IsNotExist(err) { + os.Mkdir(tmpFixtures, 0755) + } + + var stdout string + for i := 1; i <= n; i++ { + curHelloService := fmt.Sprintf("/tmp/hello%d.service", i) + tmpHelloFixture := fmt.Sprintf("/tmp/fixtures/hello%d.service", i) + + // generate a new service derived by fixtures, and store it under /tmp + err = copyFile(tmpHelloFixture, fxtHelloService) + if err != nil { + return fmt.Errorf("Failed to copy a temp fleet service: %v", err) + } + + // run a command for a unit and assert it shows up + if _, _, err := cluster.Fleetctl(m, cmd, tmpHelloFixture); err != nil { + return fmt.Errorf("Unable to %s fleet unit: %v", cmd, err) + } + + stdout, _, err = cluster.Fleetctl(m, "list-unit-files", "--no-legend") + if err != nil { + return fmt.Errorf("Failed to run %s: %v", "list-unit-files", err) + } + units := strings.Split(strings.TrimSpace(stdout), "\n") + if len(units) != i { + return fmt.Errorf("Did not find %d units in cluster: \n%s", i, stdout) + } + + // generate a new service derived by fixtures, and store it under /tmp + err = genNewFleetService(curHelloService, fxtHelloService, "sleep 2", "sleep 1") + if err != nil { + return fmt.Errorf("Failed to generate a temp fleet service: %v", err) + } + } + + for i := 1; i <= n; i++ { + curHelloService := fmt.Sprintf("/tmp/hello%d.service", i) + + // replace the unit and assert it shows up + if _, _, err = cluster.Fleetctl(m, cmd, "--replace", curHelloService); err != nil { + return fmt.Errorf("Unable to replace fleet unit: %v", err) + } + stdout, _, err = cluster.Fleetctl(m, "list-unit-files", "--no-legend") + if err != nil { + return fmt.Errorf("Failed to run %s: %v", "list-unit-files", err) + } + units := strings.Split(strings.TrimSpace(stdout), "\n") + if len(units) != n { + return fmt.Errorf("Did not find %d units in cluster: \n%s", n, stdout) + } + } + + // clean up temp services under /tmp + for i := 1; i <= n; i++ { + curHelloService := fmt.Sprintf("/tmp/hello%d.service", i) + os.Remove(curHelloService) + + if err := destroyUnitRetrying(cluster, m, fxtHelloService); err != nil { + return fmt.Errorf("Cannot destroy unit %v", fxtHelloService) + } + } + os.Remove(tmpFixtures) + + return nil +} + // copyFile() func copyFile(newFile, oldFile string) error { input, err := ioutil.ReadFile(oldFile) From 4e4cc9410215458d8df4b2c092680ad3afc67f6a Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Thu, 17 Mar 2016 11:36:49 +0100 Subject: [PATCH 12/12] functional: make replace tests wait for N active units After "fleetctl start --replace", it should wait until expected number of active units are available. So use waitForNActiveUnits() after running list-units. However, for "fleetctl {load,submit} --replace", it should expect 0 active units. To distinguish that, create a new wrapper waitForActiveUnitsReplaceCmds(). --- functional/unit_action_test.go | 41 ++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/functional/unit_action_test.go b/functional/unit_action_test.go index 4e9da0afe..2e2f7806b 100644 --- a/functional/unit_action_test.go +++ b/functional/unit_action_test.go @@ -313,6 +313,9 @@ func replaceUnitCommon(cmd string) error { if len(units) != 1 { return fmt.Errorf("Did not find 1 unit in cluster: \n%s", stdout) } + if err := waitForActiveUnitsReplaceCmds(cluster, m, cmd, 1); err != nil { + return err + } // replace the unit and assert it shows up err = genNewFleetService(tmpHelloService, fxtHelloService, "sleep 2", "sleep 1") @@ -330,6 +333,9 @@ func replaceUnitCommon(cmd string) error { if len(units) != 1 { return fmt.Errorf("Did not find 1 unit in cluster: \n%s", stdout) } + if err := waitForActiveUnitsReplaceCmds(cluster, m, cmd, 1); err != nil { + return err + } os.Remove(tmpHelloService) if err := destroyUnitRetrying(cluster, m, fxtHelloService); err != nil { @@ -397,6 +403,9 @@ func replaceUnitMultiple(cmd string, n int) error { if len(units) != i { return fmt.Errorf("Did not find %d units in cluster: \n%s", i, stdout) } + if err := waitForActiveUnitsReplaceCmds(cluster, m, cmd, i); err != nil { + return err + } // generate a new service derived by fixtures, and store it under /tmp err = genNewFleetService(curHelloService, fxtHelloService, "sleep 2", "sleep 1") @@ -420,6 +429,9 @@ func replaceUnitMultiple(cmd string, n int) error { if len(units) != n { return fmt.Errorf("Did not find %d units in cluster: \n%s", n, stdout) } + if err := waitForActiveUnitsReplaceCmds(cluster, m, cmd, i); err != nil { + return err + } } // clean up temp services under /tmp @@ -472,6 +484,35 @@ func genNewFleetService(newFile, oldFile, newVal, oldVal string) error { return nil } +// waitForActiveUnitsReplaceCmds() is a wrapper for waiting for N active units. +// The expected number of active units are given as a parameter "count". +// If cmd is "start", it expects that "count" active units are active. +// Otherwise, for "load" or "submit", it expects no active unit. +func waitForActiveUnitsReplaceCmds(cluster platform.Cluster, m platform.Member, cmd string, count int) error { + if cmd == "start" { + units, err := cluster.WaitForNActiveUnits(m, count) + if err != nil { + fmt.Errorf("%v", err) + } + _, found := units["hello.service"] + if len(units) != count || !found { + fmt.Errorf("Expected hello.service to be sole active unit, got %v", units) + } + } else { + // cmd is "load" or "submit", then there's no active unit + units, err := cluster.WaitForNActiveUnits(m, 0) + if err != nil { + fmt.Errorf("%v", err) + } + _, found := units["hello.service"] + if len(units) != 0 || !found { + fmt.Errorf("Expected hello.service to be sole active unit, got %v", units) + } + } + + return nil +} + // destroyUnitRetrying() destroys the unit and ensure it disappears from the // unit list. It could take a little time until the unit gets destroyed. func destroyUnitRetrying(cluster platform.Cluster, m platform.Member, serviceFile string) error {