From edad0bec52a19d39ffd8f63a8d8061e75eb9323b Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Tue, 8 Mar 2016 12:09:30 +0100 Subject: [PATCH 01/17] fleetctl: add the "--replace" switch to submit command --- fleetctl/fleetctl.go | 1 + fleetctl/submit.go | 1 + 2 files changed, 2 insertions(+) diff --git a/fleetctl/fleetctl.go b/fleetctl/fleetctl.go index 69513e04f..1010ecd21 100644 --- a/fleetctl/fleetctl.go +++ b/fleetctl/fleetctl.go @@ -103,6 +103,7 @@ var ( Full bool NoLegend bool NoBlock bool + Replace bool BlockAttempts int Fields string SSHPort int diff --git a/fleetctl/submit.go b/fleetctl/submit.go index cbfe03f93..8a1cde01b 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 unit with the new one.") } func runSubmitUnits(args []string) (exit int) { From 9009260f9d2f9bcd936a1fb438c2d5eab4ab0339 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Tue, 8 Mar 2016 13:52:43 +0100 Subject: [PATCH 02/17] fleetctl: create checkUnitCreation() function that supports replace flag --- fleetctl/fleetctl.go | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/fleetctl/fleetctl.go b/fleetctl/fleetctl.go index 1010ecd21..a0417b9af 100644 --- a/fleetctl/fleetctl.go +++ b/fleetctl/fleetctl.go @@ -664,6 +664,36 @@ func createUnit(name string, uf *unit.UnitFile) (*schema.Unit, error) { return &u, nil } +// checkUnitCreation checks if the unit should be created +// It takes a unit file path as a prameter. +// 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) + } + return 0, nil + } + + if !sharedFlags.Replace { + log.Debugf("Found Unit(%s) in Registry, no need to recreate it", name) + warnOnDifferentLocalUnit(arg, unit) + return 1, nil + } + + 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: @@ -692,6 +722,13 @@ func lazyCreateUnits(args []string) error { continue } + ret, err := checkUnitCreation(arg) + if err != nil { + return err + } else if ret != 0 { + continue + } + // Assume that the name references a local unit file on // disk or if it is an instance unit and if so get its // corresponding unit From 243b24a43a0288b454f9e8764edb581944930332 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Tue, 8 Mar 2016 15:56:36 +0100 Subject: [PATCH 03/17] fleetctl: add isLocalUnitDifferent() and use it to check units Add isLocalUnitDifferent() and use it to check if the local unit differs from the one in the registry. This function handles both old scenario and the new one attended to replace units. --- fleetctl/fleetctl.go | 89 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 15 deletions(-) diff --git a/fleetctl/fleetctl.go b/fleetctl/fleetctl.go index a0417b9af..24694a35c 100644 --- a/fleetctl/fleetctl.go +++ b/fleetctl/fleetctl.go @@ -685,12 +685,20 @@ func checkUnitCreation(arg string) (int, error) { return 0, nil } - if !sharedFlags.Replace { - log.Debugf("Found Unit(%s) in Registry, no need to recreate it", name) - warnOnDifferentLocalUnit(arg, unit) - return 1, 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 0, nil + } } + log.Debugf("Found same Unit(%s) in Registry, no need to recreate it", name) + return 1, nil } @@ -711,17 +719,6 @@ 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) - 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) - continue - } - ret, err := checkUnitCreation(arg) if err != nil { return err @@ -764,6 +761,68 @@ 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 && os.IsNotExist(err) { + 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 fatal { + return false, err + } + } else if result == false { + if warnIfDifferent { + stderr("WARNING: Unit %s in registry differs from local unit file %s", su.Name, file) + } + return true, nil + } + + 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 name %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 { + return false, err + } else if result == false { + if warnIfDifferent { + stderr("WARNING: Unit %s in registry differs from local template unit file %s", su.Name, info.Template) + } + return true, nil + } + + return false, nil +} + func warnOnDifferentLocalUnit(loc string, su *schema.Unit) { suf := schema.MapSchemaUnitOptionsToUnitFile(su.Options) if _, err := os.Stat(loc); !os.IsNotExist(err) { From bb5977cdb14605f46c18035e6651ddc5816ebdf1 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Tue, 8 Mar 2016 16:37:23 +0100 Subject: [PATCH 04/17] fleetctl: improve the unit matching logic so we print messages to users --- fleetctl/fleetctl.go | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/fleetctl/fleetctl.go b/fleetctl/fleetctl.go index 24694a35c..de6818254 100644 --- a/fleetctl/fleetctl.go +++ b/fleetctl/fleetctl.go @@ -694,6 +694,8 @@ func checkUnitCreation(arg string) (int, error) { return 1, err } else if different { return 0, nil + } else { + stdout("Found same Unit(%s) in Registry, nothing to do", unit.Name) } } @@ -779,7 +781,7 @@ func matchLocalFileAndUnit(file string, su *schema.Unit) (bool, error) { a := schema.MapSchemaUnitOptionsToUnitFile(su.Options) _, err := os.Stat(file) - if err != nil && os.IsNotExist(err) { + if err == nil { b, err := getUnitFromFile(file) if err == nil { result = matchUnitFiles(a, b) @@ -791,15 +793,13 @@ func matchLocalFileAndUnit(file string, su *schema.Unit) (bool, error) { func isLocalUnitDifferent(file string, su *schema.Unit, warnIfDifferent bool, fatal bool) (bool, error) { result, err := matchLocalFileAndUnit(file, su) - if err != nil { - if fatal { - return false, err - } - } else if result == false { - if warnIfDifferent { + if err == nil { + if result == false && warnIfDifferent { stderr("WARNING: Unit %s in registry differs from local unit file %s", su.Name, file) } - return true, nil + return !result, nil + } else if fatal { + return false, err } info := unit.NewUnitNameInfo(path.Base(file)) @@ -811,16 +811,14 @@ func isLocalUnitDifferent(file string, su *schema.Unit, warnIfDifferent bool, fa templFile := path.Join(path.Dir(file), info.Template) result, err = matchLocalFileAndUnit(templFile, su) - if err != nil { - return false, err - } else if result == false { - if warnIfDifferent { + 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 true, nil + return !result, nil } - return false, nil + return false, err } func warnOnDifferentLocalUnit(loc string, su *schema.Unit) { From 0cf1dd6183c48e0e96a970866293e975cadcd2f1 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Tue, 8 Mar 2016 17:47:39 +0100 Subject: [PATCH 05/17] fleetctl: add replace support for load for the sake of testing --- fleetctl/load.go | 1 + 1 file changed, 1 insertion(+) diff --git a/fleetctl/load.go b/fleetctl/load.go index 0f7b0923c..1114f946f 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 unit and load the new one.") } func runLoadUnits(args []string) (exit int) { From 35463296f1beef91e4110afe29e472861d9f1c07 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Fri, 11 Mar 2016 13:36:26 +0100 Subject: [PATCH 06/17] fleetctl: save old unit if found and use it to pass the Desired state This is only for development purpose and debugging, all this will be later removed or optimised. Signed-off-by: Djalal Harouni --- fleetctl/fleetctl.go | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/fleetctl/fleetctl.go b/fleetctl/fleetctl.go index de6818254..f3a5efe61 100644 --- a/fleetctl/fleetctl.go +++ b/fleetctl/fleetctl.go @@ -633,13 +633,21 @@ func findUnits(args []string) (sus []schema.Unit, err error) { return filtered, nil } -func createUnit(name string, uf *unit.UnitFile) (*schema.Unit, error) { +func createUnit(name string, uf *unit.UnitFile, oldUnit *schema.Unit) (*schema.Unit, error) { + var oldState string + if uf == nil { return nil, fmt.Errorf("nil unit provided") } + + if oldUnit != nil { + oldState = oldUnit.DesiredState + } + u := schema.Unit{ Name: name, Options: schema.MapUnitFileToSchemaUnitOptions(uf), + DesiredState: oldState, } // TODO(jonboulle): this dependency on the API package is awkward, and // redundant with the check in api.unitsResource.set, but it is a @@ -668,7 +676,7 @@ func createUnit(name string, uf *unit.UnitFile) (*schema.Unit, error) { // It takes a unit file path as a prameter. // 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) { +func checkUnitCreation(arg string, old *schema.Unit) (int, error) { name := unitNameMangle(arg) // First, check if there already exists a Unit by the given name in the Registry @@ -693,6 +701,7 @@ func checkUnitCreation(arg string) (int, error) { if err != nil { return 1, err } else if different { + old = unit return 0, nil } else { stdout("Found same Unit(%s) in Registry, nothing to do", unit.Name) @@ -718,10 +727,11 @@ func lazyCreateUnits(args []string) error { errchan := make(chan error) var wg sync.WaitGroup for _, arg := range args { + var oldUnit schema.Unit arg = maybeAppendDefaultUnitType(arg) name := unitNameMangle(arg) - ret, err := checkUnitCreation(arg) + ret, err := checkUnitCreation(arg, &oldUnit) if err != nil { return err } else if ret != 0 { @@ -736,7 +746,7 @@ func lazyCreateUnits(args []string) error { return err } - _, err = createUnit(name, uf) + _, err = createUnit(name, uf, &oldUnit) if err != nil { return err } From f491d902e0c985a85f38c29d5a3ade7481e62518 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Fri, 11 Mar 2016 14:43:50 +0100 Subject: [PATCH 07/17] fleetctl: we want the content of unit and remove warnOnDifferentLocalUnit() --- fleetctl/fleetctl.go | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) diff --git a/fleetctl/fleetctl.go b/fleetctl/fleetctl.go index f3a5efe61..1c36677ae 100644 --- a/fleetctl/fleetctl.go +++ b/fleetctl/fleetctl.go @@ -556,7 +556,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 { @@ -701,7 +701,7 @@ func checkUnitCreation(arg string, old *schema.Unit) (int, error) { if err != nil { return 1, err } else if different { - old = unit + *old = *unit return 0, nil } else { stdout("Found same Unit(%s) in Registry, nothing to do", unit.Name) @@ -831,26 +831,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 5a7176f60ca7745518d24eee2476b3b242b29894 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Fri, 11 Mar 2016 17:16:29 +0100 Subject: [PATCH 08/17] unit: move MatchUnitFile() to unit package Move MatchUnitFile() to unit package we will use it inside fleetd to check for unit hash 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 1c36677ae..efed7aad2 100644 --- a/fleetctl/fleetctl.go +++ b/fleetctl/fleetctl.go @@ -773,16 +773,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 @@ -794,7 +784,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 b081532b98a240f61939bc03532d6934e70277b4 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Fri, 11 Mar 2016 17:24:57 +0100 Subject: [PATCH 09/17] units: check if the new unit is a new version of a previous unit Check if the new submited unit is a new version of an already submitted unit. --- api/units.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/api/units.go b/api/units.go index 67b4a9537..439e3ce7d 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 { @@ -108,9 +109,23 @@ func (ur *unitsResource) set(rw http.ResponseWriter, req *http.Request, item str } else if err := ValidateOptions(su.Options); err != nil { sendError(rw, http.StatusBadRequest, err) } else { - ur.create(rw, su.Name, &su) + // This is a new Unit + newUnit = true } return + } else if eu.Name == su.Name { + // 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 update to this new unit + a := schema.MapSchemaUnitOptionsToUnitFile(su.Options) + b := schema.MapSchemaUnitOptionsToUnitFile(eu.Options) + newUnit = !unit.MatchUnitFiles(a, b) + } + + if newUnit { + ur.create(rw, su.Name, &su) + return } if len(su.DesiredState) == 0 { From 28038f16642d5467c4f9d90072c6d5b4dfb78304 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Fri, 11 Mar 2016 17:27:13 +0100 Subject: [PATCH 10/17] registry: instruct the etcd driver to ignore previous keys This is a hack, we will have to improve it later. --- registry/job.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/registry/job.go b/registry/job.go index 8eeda2280..d406947ea 100644 --- a/registry/job.go +++ b/registry/job.go @@ -335,7 +335,7 @@ func (r *EtcdRegistry) CreateUnit(u *job.Unit) (err error) { } opts := &etcd.SetOptions{ - PrevExist: etcd.PrevNoExist, + PrevExist: etcd.PrevIgnore, } key := r.prefixed(jobPrefix, u.Name, "object") _, err = r.kAPI.Set(r.ctx(), key, val, opts) From ca8c901d125093e051ad0df9ada5a97bc544f2bc Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Fri, 11 Mar 2016 17:39:51 +0100 Subject: [PATCH 11/17] units: fix a bug where were returning without creating the actual unit --- api/units.go | 1 - 1 file changed, 1 deletion(-) diff --git a/api/units.go b/api/units.go index 439e3ce7d..0619ee02e 100644 --- a/api/units.go +++ b/api/units.go @@ -112,7 +112,6 @@ func (ur *unitsResource) set(rw http.ResponseWriter, req *http.Request, item str // This is a new Unit newUnit = true } - return } else if eu.Name == su.Name { // There is already a unit with the same name // check the hashes if they do not match then we will From 7fcf02e347d825cc03c335f2ef6aac1d02743789 Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Mon, 14 Mar 2016 13:41:55 +0100 Subject: [PATCH 12/17] fleetctl: currently we don't need to set the desiredstate This is just for debugging and dev purpose --- fleetctl/fleetctl.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/fleetctl/fleetctl.go b/fleetctl/fleetctl.go index efed7aad2..e102d616e 100644 --- a/fleetctl/fleetctl.go +++ b/fleetctl/fleetctl.go @@ -634,20 +634,13 @@ func findUnits(args []string) (sus []schema.Unit, err error) { } func createUnit(name string, uf *unit.UnitFile, oldUnit *schema.Unit) (*schema.Unit, error) { - var oldState string - if uf == nil { return nil, fmt.Errorf("nil unit provided") } - if oldUnit != nil { - oldState = oldUnit.DesiredState - } - u := schema.Unit{ Name: name, Options: schema.MapUnitFileToSchemaUnitOptions(uf), - DesiredState: oldState, } // TODO(jonboulle): this dependency on the API package is awkward, and // redundant with the check in api.unitsResource.set, but it is a From 9f9718fdd4c07659dd2418013ca094b2effcc68d Mon Sep 17 00:00:00 2001 From: Djalal Harouni Date: Mon, 14 Mar 2016 18:27:51 +0100 Subject: [PATCH 13/17] units: fix a bug where we are trying to create units The interface to create units and set the target state is the same, so to differenciate between those two we check if the units have the same name and the content of the passed unit is really there! if not we assume that the caller wants to set the target state and if it's not the case we also catch that by len(su.DesiredState) == 0 check --- api/units.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/api/units.go b/api/units.go index 0619ee02e..89e414fef 100644 --- a/api/units.go +++ b/api/units.go @@ -109,14 +109,16 @@ func (ur *unitsResource) set(rw http.ResponseWriter, req *http.Request, item str } else if err := ValidateOptions(su.Options); err != nil { sendError(rw, http.StatusBadRequest, err) } else { - // This is a new Unit - newUnit = true + ur.create(rw, su.Name, &su) } - } else if eu.Name == su.Name { + return + } 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 update to this new unit + // the job will be update to this new unit. + // if su.Options == 0 then probably we don't want to update + // the Unit options but only its target state. a := schema.MapSchemaUnitOptionsToUnitFile(su.Options) b := schema.MapSchemaUnitOptionsToUnitFile(eu.Options) newUnit = !unit.MatchUnitFiles(a, b) From 963d0019ab7af8eeb8d30c1cd64d878f83e8138a Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Mon, 14 Mar 2016 12:55:04 +0100 Subject: [PATCH 14/17] 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 2ded29539..a4236f116 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 unit and start the new one.") } func runStartUnit(args []string) (exit int) { From c7626e08067a1460fc1475b7b339b7238a487400 Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Tue, 15 Mar 2016 17:18:07 +0100 Subject: [PATCH 15/17] 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 0ce187ecc4c305f2b1b51b9814289a4136e69bf7 Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Wed, 16 Mar 2016 12:02:34 +0100 Subject: [PATCH 16/17] 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 905adf58c83b8903b28fe2b2606d54915619916e Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Wed, 16 Mar 2016 14:59:10 +0100 Subject: [PATCH 17/17] 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 | 118 +++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/functional/unit_action_test.go b/functional/unit_action_test.go index 6886fc3b3..e6019bcfb 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,110 @@ 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) + } + + var listUnitCmd string + if cmd == "submit" { + listUnitCmd = "list-unit-files" + } else { + listUnitCmd = "list-units" + } + + 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, listUnitCmd, "--no-legend") + if err != nil { + return fmt.Errorf("Failed to run %s: %v", listUnitCmd, 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, listUnitCmd, "--no-legend") + if err != nil { + return fmt.Errorf("Failed to run %s: %v", listUnitCmd, 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)