refactor(fleetctl): Break apart lazyCreateJobs

The submit action needs to fail if it cannot create a Job, while the other callers of lazyCreateJobs just want to make sure a job/payload exist. Add a test to solidify this behavior of fleetctl submit.
coreos · Apr 18, 2014 · 5f1340b · 5f1340b
1 parent f6ecbd6
commit 5f1340b
Show file tree

Hide file tree

Showing 4 changed files with 149 additions and 55 deletions.
diff --git a/fleetctl/cmd.go b/fleetctl/cmd.go
@@ -251,10 +251,11 @@ func getJobPayloadFromFile(file string) (*job.JobPayload, error) {
 		return nil, err
 	}
 
-	unitFile := unit.NewSystemdUnitFile(string(out))
+	payloadName := path.Base(file)
+	log.V(1).Infof("Payload(%s) found in local filesystem", payloadName)
 
-	name := path.Base(file)
-	payload := job.NewJobPayload(name, *unitFile)
+	unitFile := unit.NewSystemdUnitFile(string(out))
+	payload := job.NewJobPayload(payloadName, *unitFile)
 
 	return payload, nil
 }
@@ -312,66 +313,82 @@ func findJobs(args []string) (jobs []job.Job, err error) {
 	return jobs, nil
 }
 
-func lazyCreateJobs(args []string, signPayloads bool) error {
-	var err error
+func createJob(jobName string, jobPayload *job.JobPayload) (*job.Job, error) {
+	j := job.NewJob(jobName, *jobPayload)
 
-	var sc *sign.SignatureCreator
-	var sv *sign.SignatureVerifier
-	if signPayloads {
-		sc, err = sign.NewSignatureCreatorFromSSHAgent()
-		if err != nil {
-			return fmt.Errorf("Failed creating SignatureCreator: %v", err)
-		}
-		sv, err = sign.NewSignatureVerifierFromSSHAgent()
-		if err != nil {
-			return fmt.Errorf("Failed creating SignatureVerifier: %v", err)
-		}
+	if err := registryCtl.CreateJob(j); err != nil {
+		return nil, fmt.Errorf("Failed creating job %s: %v", j.Name, err)
 	}
 
-	for _, v := range args {
-		name := path.Base(v)
-		j := registryCtl.GetJob(name)
-		if j == nil {
-			log.V(1).Infof("Job(%s) not found in Registry", name)
-			payload, err := getJobPayloadFromFile(v)
-			if err != nil {
-				return fmt.Errorf("Failed getting Payload(%s) from file: %v", name, err)
-			}
+	log.V(1).Infof("Created Job(%s) in Registry", j.Name)
 
-			log.V(1).Infof("Payload(%s) found in local filesystem", name)
-			j = job.NewJob(name, *payload)
+	return j, nil
+}
 
-			err = registryCtl.CreateJob(j)
-			if err != nil {
-				return fmt.Errorf("Failed creating job %s: %v", j.Name, err)
-			}
+func signJob(j *job.Job) error {
+	sc, err := sign.NewSignatureCreatorFromSSHAgent()
+	if err != nil {
+		return fmt.Errorf("Failed creating SignatureCreator: %v", err)
+	}
 
-			log.V(1).Infof("Created Job(%s) in Registry", j.Name)
+	s, err := sc.SignPayload(&j.Payload)
+	if err != nil {
+		return fmt.Errorf("Failed signing Payload(%s): %v", j.Payload.Name, err)
+	}
 
-			if signPayloads {
-				s, err := sc.SignPayload(payload)
-				if err != nil {
-					return fmt.Errorf("Failed creating signature for Payload(%s): %v", payload.Name, err)
-				}
+	registryCtl.CreateSignatureSet(s)
+	log.V(1).Infof("Signed Payload(%s)", j.Payload.Name)
+
+	return nil
+}
+
+func verifyJob(j *job.Job) error {
+	sv, err := sign.NewSignatureVerifierFromSSHAgent()
+	if err != nil {
+		return fmt.Errorf("Failed creating SignatureVerifier: %v", err)
+	}
+
+	s := registryCtl.GetSignatureSetOfPayload(j.Payload.Name)
+	verified, err := sv.VerifyPayload(&j.Payload, s)
+	if err != nil {
+		return fmt.Errorf("Failed attempting to verify Payload(%s): %v", j.Payload.Name, err)
+	} else if !verified {
+		return fmt.Errorf("Unable to verify Payload(%s)", j.Payload.Name)
+	}
+
+	log.V(1).Infof("Verified signature of Payload(%s)", j.Payload.Name)
+	return nil
+}
 
-				registryCtl.CreateSignatureSet(s)
-				log.V(1).Infof("Created signature for Payload(%s)", name)
+func lazyCreateJobs(args []string, signAndVerify bool) error {
+	for _, arg := range args {
+		jobName := path.Base(arg)
+		if j := registryCtl.GetJob(jobName); j != nil {
+			log.V(1).Infof("Found Job(%s) in Registry, no need to recreate it", jobName)
+			if signAndVerify {
+				if err := verifyJob(j); err != nil {
+					return err
+				}
 			}
-		} else {
-			log.V(1).Infof("Found Job(%s) in Registry", name)
+			continue
 		}
 
-		if signPayloads {
-			s := registryCtl.GetSignatureSetOfPayload(name)
-			ok, err := sv.VerifyPayload(&(j.Payload), s)
-			if !ok || err != nil {
-				return fmt.Errorf("Failed checking signature for Payload(%s): %v", j.Payload.Name, err)
-			}
+		payload, err := getJobPayloadFromFile(arg)
+		if err != nil {
+			return fmt.Errorf("Failed getting Payload(%s) from file: %v", jobName, err)
+		}
 
-			log.V(1).Infof("Verified signature of Payload(%s)", j.Payload.Name)
+		j, err := createJob(jobName, payload)
+		if err != nil {
+			return err
 		}
-	}
 
+		if signAndVerify {
+			if err := signJob(j); err != nil {
+				return err
+			}
+		}
+	}
 	return nil
 }
 

diff --git a/fleetctl/start.go b/fleetctl/start.go
@@ -39,8 +39,7 @@ func init() {
 }
 
 func runStartUnit(args []string) (exit int) {
-	err := lazyCreateJobs(args, sharedFlags.Sign)
-	if err != nil {
+	if err := lazyCreateJobs(args, sharedFlags.Sign); err != nil {
 		fmt.Fprintf(os.Stderr, "%v\n", err)
 		return 1
 	}

diff --git a/fleetctl/submit.go b/fleetctl/submit.go
@@ -3,6 +3,7 @@ package main
 import (
 	"fmt"
 	"os"
+	"path"
 )
 
 var cmdSubmitUnit = &Command{
@@ -24,13 +25,31 @@ Submit a directory of units with glob matching:
 }
 
 func init() {
-	cmdSubmitUnit.Flags.BoolVar(&sharedFlags.Sign, "sign", false, "Sign unit file signatures and verify submitted units using local SSH identities")
+	cmdSubmitUnit.Flags.BoolVar(&sharedFlags.Sign, "sign", false, "Sign unit files units using local SSH identities")
 }
 
 func runSubmitUnits(args []string) (exit int) {
-	if err := lazyCreateJobs(args, sharedFlags.Sign); err != nil {
-		fmt.Fprintf(os.Stderr, "%v\n", err)
-		return 1
+	for _, arg := range args {
+		jobName := path.Base(arg)
+		payload, err := getJobPayloadFromFile(arg)
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "Failed getting Payload(%s) from file: %v\n", jobName, err)
+			return 1
+		}
+
+		j, err := createJob(jobName, payload)
+		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
+			}
+		}
 	}
 
 	return

diff --git a/functional/unit_action_test.go b/functional/unit_action_test.go
@@ -7,6 +7,65 @@ import (
 	"github.com/coreos/fleet/functional/platform"
 )
 
+func TestUnitSubmit(t *testing.T) {
+	cluster, err := platform.NewNspawnCluster("smoke")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cluster.Destroy()
+
+	if err := platform.CreateNClusterMembers(cluster, 1, platform.MachineConfig{}); err != nil {
+		t.Fatal(err)
+	}
+	_, err = waitForNMachines(1)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// submit a unit and assert it shows up
+	if _, _, err := fleetctl("submit", "fixtures/units/hello.service"); err != nil {
+		t.Fatalf("Unable to submit fleet unit: %v", err)
+	}
+	stdout, _, err := fleetctl("list-units", "--no-legend")
+	if err != nil {
+		t.Fatalf("Failed to run list-units: %v", err)
+	}
+	units := strings.Split(strings.TrimSpace(stdout), "\n")
+	if len(units) != 1 {
+		t.Fatalf("Did not find 1 unit in cluster: \n%s", stdout)
+	}
+
+	// submitting the same unit should fail
+	if _, _, err = fleetctl("submit", "fixtures/units/hello.service"); err == nil {
+		t.Fatalf("Expected failure when double-submitting unit, got success.")
+	}
+
+	// destroy the unit and ensure it disappears from the unit list
+	if _, _, err := fleetctl("destroy", "fixtures/units/hello.service"); err != nil {
+		t.Fatalf("Failed to destroy unit: %v", err)
+	}
+	stdout, _, err = fleetctl("list-units", "--no-legend")
+	if err != nil {
+		t.Fatalf("Failed to run list-units: %v", err)
+	}
+	if strings.TrimSpace(stdout) != "" {
+		t.Fatalf("Did not find 0 units in cluster: \n%s", stdout)
+	}
+
+	// submitting the unit after destruction should succeed
+	if _, _, err := fleetctl("submit", "fixtures/units/hello.service"); err != nil {
+		t.Fatalf("Unable to submit fleet unit: %v", err)
+	}
+	stdout, _, err = fleetctl("list-units", "--no-legend")
+	if err != nil {
+		t.Fatalf("Failed to run list-units: %v", err)
+	}
+	units = strings.Split(strings.TrimSpace(stdout), "\n")
+	if len(units) != 1 {
+		t.Fatalf("Did not find 1 unit in cluster: \n%s", stdout)
+	}
+}
+
 func TestUnitRestart(t *testing.T) {
 	cluster, err := platform.NewNspawnCluster("smoke")
 	if err != nil {