Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

feat(fleetctl): add template unit support to fleetctl start/submit #480

Merged
merged 1 commit into from
Jun 4, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 59 additions & 4 deletions fleetctl/fleetctl.go
Original file line number Diff line number Diff line change
Expand Up @@ -385,9 +385,25 @@ func verifyJob(j *job.Job) error {
return nil
}

// lazyCreateJobs iterates over a set of Job names and, for each, attempts to
// ensure that a Job by that name exists in the Registry, by checking a number
// of conditions and acting on the first one that succeeds, in order of:
// 1. a Job by that name already existing in the Registry
// 2. a unit file by that name existing on disk
// 3. a corresponding unit template (if applicable) existing in the Registry
// 4. a corresponding unit template (if applicable) existing on disk
// Any error encountered during these steps is returned immediately (i.e.
// subsequent Jobs are not acted on). An error is also returned if none of the
// above conditions match a given Job.
// If signAndVerify is true, the Job will be signed (if it is created), or have
// its signature verified if it already exists in the Registry.
func lazyCreateJobs(args []string, signAndVerify bool) error {
for _, arg := range args {
// TODO(jonboulle): this loop is getting too unwieldy; factor it out

jobName := unitNameMangle(arg)

// First, check if there already exists a Job by the given name in the Registry
j, err := registryCtl.GetJob(jobName)
if err != nil {
return fmt.Errorf("error retrieving Job(%s) from Registry: %v", jobName, err)
Expand All @@ -402,16 +418,55 @@ func lazyCreateJobs(args []string, signAndVerify bool) error {
continue
}

unit, err := getUnitFromFile(arg)
// Failing that, assume the name references a local unit file on disk, and attempt to load that, if it exists
if _, err := os.Stat(arg); !os.IsNotExist(err) {
unit, err := getUnitFromFile(arg)
if err != nil {
return fmt.Errorf("failed getting Unit(%s) from file: %v", jobName, err)
}
j, err = createJob(jobName, unit)
if err != nil {
return err
}
if signAndVerify {
if err := signJob(j); err != nil {
return err
}
}
continue
}

// Otherwise (if the unit file does not exist), check if the name appears to be an instance unit,
// and if so, check for a corresponding template unit in the registry or on disk
uni := unit.NewUnitNameInfo(jobName)
if uni == nil {
return fmt.Errorf("error extracting information from unit name %s", jobName)
} else if !uni.IsInstance() {
return fmt.Errorf("unable to find Unit(%s) in Registry or on filesystem", jobName)
}
tmpl, err := registryCtl.GetJob(uni.Template)
if err != nil {
return fmt.Errorf("failed getting Unit(%s) from file: %v", jobName, err)
return fmt.Errorf("error retrieving template Job(%s) from Registry: %v", uni.Template, err)
}
var u *unit.Unit
if tmpl == nil {
if _, err := os.Stat(uni.Template); os.IsNotExist(err) {
return fmt.Errorf("unable to find template for Unit(%s) in Registry or on disk", jobName)
}
u, err = getUnitFromFile(uni.Template)
if err != nil {
return fmt.Errorf("failed getting template Unit(%s) from file: %v", uni.Template, err)
}
} else {
u = &tmpl.Unit
}

j, err = createJob(jobName, unit)
// If we found a template Unit or Job, create a near-identical Job in
// the Registry - same Unit as the template, but different name
j, err = createJob(jobName, u)
if err != nil {
return err
}

if signAndVerify {
if err := signJob(j); err != nil {
return err
Expand Down
26 changes: 3 additions & 23 deletions fleetctl/submit.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package main
import (
"fmt"
"os"
"path"
)

var cmdSubmitUnit = &Command{
Expand All @@ -29,28 +28,9 @@ func init() {
}

func runSubmitUnits(args []string) (exit int) {
for _, arg := range args {
jobName := path.Base(arg)
unit, err := getUnitFromFile(arg)
if err != nil {
fmt.Fprintf(os.Stderr, "Failed getting Unit(%s) from file: %v\n", jobName, err)
return 1
}

j, err := createJob(jobName, unit)
if err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
return 1
}

if sharedFlags.Sign {
err := signJob(j)
if err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
return 1
}
}
if err := lazyCreateJobs(args, sharedFlags.Sign); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually an important change. The code you're replacing fails if it cannot create a job. Now if someone makes a change to a local unit and calls fleetctl submit X, the command will succeed, and could be misleading.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, but a) we do warn about this explicitly:

  This operation is idempotent; if a named unit already exists in the cluster, it will not be resubmitted.

and b), this is the behaviour today for fleetctl start foo.service && vi foo.service && fleetctl start foo.service; I don't think it's any less misleading there.

So we should take this opportunity to solve for both.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously I'd thought about adding a check - if a unit already exists in the registry and locally, compare hashes and print a warning to the user. Thoughts? (Would do that as a subsequent change, not here)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that comparison/warning would be great.

fmt.Fprintf(os.Stderr, "%v\n", err)
exit = 1
}

return
}