Repository: coreos/fleet
Pull request #1273 (2015-06-27 03:35:43)

check for loading/launching templates in fleet http api
this pr additionally makes fleetctl check the error when attempting to change desired unit state.


Patch:
diff --git a/api/units.go b/api/units.go
index 24b3598e..10070d93 100644
--- a/api/units.go
+++ b/api/units.go
@@ -27,6 +27,7 @@ import (
 	"github.com/coreos/fleet/log"
 	"github.com/coreos/fleet/pkg"
 	"github.com/coreos/fleet/schema"
+	"github.com/coreos/fleet/unit"
 )
 
 func wireUpUnitsResource(mux *http.ServeMux, prefix string, cAPI client.API) {
@@ -115,6 +116,13 @@ func (ur *unitsResource) set(rw http.ResponseWriter, req *http.Request, item str
 		return
 	}
 
+	un := unit.NewUnitNameInfo(su.Name)
+	if un.IsTemplate() && job.JobState(su.DesiredState) != job.JobStateInactive {
+		err := fmt.Errorf("cannot activate template %q", su.Name)
+		sendError(rw, http.StatusBadRequest, err)
+		return
+	}
+
 	ur.update(rw, su.Name, su.DesiredState)
 }
 
diff --git a/fleetctl/fleetctl.go b/fleetctl/fleetctl.go
index 97057cd9..b71a9baa 100644
--- a/fleetctl/fleetctl.go
+++ b/fleetctl/fleetctl.go
@@ -718,7 +718,9 @@ func setTargetStateOfUnits(units []string, state job.JobState) ([]*schema.Unit,
 		}
 
 		log.Debugf("Setting Unit(%s) target state to %s", u.Name, state)
-		cAPI.SetUnitTargetState(u.Name, string(state))
+		if err := cAPI.SetUnitTargetState(u.Name, string(state)); err != nil {
+			return nil, err
+		}
 		triggered = append(triggered, u)
 	}
 
diff --git a/unit/unit.go b/unit/unit.go
index d269433c..4f9be39b 100644
--- a/unit/unit.go
+++ b/unit/unit.go
@@ -208,6 +208,12 @@ func (nu UnitNameInfo) IsInstance() bool {
 	return len(nu.Instance) > 0
 }
 
+// IsTemplate returns a boolean indicating whether the UnitNameInfo appears to be
+// a Template unit
+func (nu UnitNameInfo) IsTemplate() bool {
+	return len(nu.Template) > 0 && !nu.IsInstance()
+}
+
 // NewUnitNameInfo generates a UnitNameInfo from the given name. If the given string
 // is not a correct unit name, nil is returned.
 func NewUnitNameInfo(un string) *UnitNameInfo {


Test patch:
diff --git a/api/units_test.go b/api/units_test.go
index 6b56c626..6a9d37fe 100644
--- a/api/units_test.go
+++ b/api/units_test.go
@@ -371,6 +371,23 @@ func TestUnitsSetDesiredState(t *testing.T) {
 				"YYY.service": "inactive",
 			},
 		},
+		// Attempts to load/launch a template unit should fail
+		{
+			initJobs: []job.Job{
+				job.Job{Name: "XXX@.service", Unit: newUnit(t, "[Service]\nFoo=Bar")},
+			},
+			initStates: map[string]job.JobState{
+				"XXX@.service": "inactive",
+			},
+			item: "XXX@.service",
+			arg: schema.Unit{
+				DesiredState: "launched",
+			},
+			code: http.StatusBadRequest,
+			finalStates: map[string]job.JobState{
+				"XXX@.service": "inactive",
+			},
+		},
 	}
 
 	for i, tt := range tests {
diff --git a/unit/unit_test.go b/unit/unit_test.go
index b025ad5c..d3844676 100644
--- a/unit/unit_test.go
+++ b/unit/unit_test.go
@@ -114,15 +114,16 @@ func TestNamedUnit(t *testing.T) {
 		tmpl   string
 		inst   string
 		isinst bool
+		istmpl bool
 	}{
-		{"foo.service", "foo", "foo", "", "", false},
-		{"foo@.service", "foo@", "foo", "foo@.service", "", false},
-		{"foo@bar.service", "foo@bar", "foo", "foo@.service", "bar", true},
-		{"foo@bar@baz.service", "foo@bar@baz", "foo", "foo@.service", "bar@baz", true},
-		{"foo.1@.service", "foo.1@", "foo.1", "foo.1@.service", "", false},
-		{"foo.1@2.service", "foo.1@2", "foo.1", "foo.1@.service", "2", true},
-		{"ssh@.socket", "ssh@", "ssh", "ssh@.socket", "", false},
-		{"ssh@1.socket", "ssh@1", "ssh", "ssh@.socket", "1", true},
+		{"foo.service", "foo", "foo", "", "", false, false},
+		{"foo@.service", "foo@", "foo", "foo@.service", "", false, true},
+		{"foo@bar.service", "foo@bar", "foo", "foo@.service", "bar", true, false},
+		{"foo@bar@baz.service", "foo@bar@baz", "foo", "foo@.service", "bar@baz", true, false},
+		{"foo.1@.service", "foo.1@", "foo.1", "foo.1@.service", "", false, true},
+		{"foo.1@2.service", "foo.1@2", "foo.1", "foo.1@.service", "2", true, false},
+		{"ssh@.socket", "ssh@", "ssh", "ssh@.socket", "", false, true},
+		{"ssh@1.socket", "ssh@1", "ssh", "ssh@.socket", "1", true, false},
 	}
 	for _, tt := range tts {
 		u := NewUnitNameInfo(tt.fname)
@@ -149,6 +150,10 @@ func TestNamedUnit(t *testing.T) {
 		if i != tt.isinst {
 			t.Errorf("NewUnitNameInfo(%s).IsInstance returned %t, want %t", tt.name, i, tt.isinst)
 		}
+		i = u.IsTemplate()
+		if i != tt.istmpl {
+			t.Errorf("NewUnitNameInfo(%s).IsTemplate returned %t, want %t", tt.name, i, tt.istmpl)
+		}
 	}
 
 	bad := []string{"foo", "bar@baz"}