-
Notifications
You must be signed in to change notification settings - Fork 301
global units: do some agent-level validation #811
Conversation
Also related: #494 |
e0fa603
to
ebd69fa
Compare
@@ -126,6 +127,11 @@ func desiredAgentState(a *Agent, reg registry.Registry) (*AgentState, error) { | |||
|
|||
for _, u := range units { | |||
u := u | |||
md := u.RequiredTargetMetadata() | |||
if u.IsGlobal() && !machine.HasMetadata(&ms, md) { | |||
log.Infof("Agent unable to run global unit %s: missing required metadata %v", u.Name, md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
md
in the output is actually going to be a potentially cumbersome map. If you want to log this, we should first be able to identify what specific key caused the mismatch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... and this probably should not be logged at Infof
. How about V(1).Infof
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am just going to omit it because HasMetadata
already does its own verbose logging.
14ac320
to
ca6464e
Compare
Updated |
} | ||
|
||
func TestDesiredAgentState(t *testing.T) { | ||
testCases := []struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gorgeous test cases.
LGTM |
global units: do some agent-level validation
When launching global units, the agent should perform a minimal amount of validation to ensure it can actually run the unit. This should probably be nothing more than checking for metadata requirements. Specifically, any other kind of scheduling requirements (like Peers or Conflicts) are NOT supported.
Related: #659