fix: validate whole plan, rather than topmost two layers (#363)

This PR moves layer/plan validation into dedicated functions, with `Layer.Validate` covering validation that is specific to a layer (such as an invalid value) and `Plan.Validate` covering validation that covers the plan as a whole (such as cycles or missing values).

Plan validation includes layer validation, and is done in the service manager's `updatePlanLayers` and in `plan.CombineLayers`.

We also use the check period as the upper bound for the timeout in checks, which was happening before, but less explicitly.

No additional validation is done - all of the validation code is moved rather than changed.

Fixes #349

internals/overlord/servstate/manager.go

@@ -174,6 +174,10 @@ func (m *ServiceManager) updatePlanLayers(layers []*plan.Layer) error {
 		Checks:     combined.Checks,
 		LogTargets: combined.LogTargets,
 	}
+	err = p.Validate()
+	if err != nil {
+		return err
+	}
 	m.updatePlan(p)
 	return nil
 }
@@ -541,6 +545,11 @@ func (m *ServiceManager) SetServiceArgs(serviceArgs map[string][]string) error {
 		}
 	}
 
+	err = newLayer.Validate()
+	if err != nil {
+		return err
+	}
+
 	return m.appendLayer(newLayer)
 }
 

internals/overlord/servstate/manager_test.go

@@ -752,6 +752,17 @@ services:
         command: /bin/b
 `[1:])
 	s.planLayersHasLen(c, s.manager, 3)
+
+	// Make sure that layer validation is happening.
+	layer, err = plan.ParseLayer(0, "label4", []byte(`
+checks:
+    bad-check:
+        override: replace
+        level: invalid
+        tcp:
+            port: 8080
+`))
+	c.Check(err, ErrorMatches, `(?s).*plan check.*must be "alive" or "ready".*`)
 }
 
 func (s *S) TestSetServiceArgs(c *C) {