-
Notifications
You must be signed in to change notification settings - Fork 302
feat(job): substitute Requirements for instance unit jobs #479
Conversation
@@ -103,7 +105,13 @@ func (j *Job) Requirements() map[string][]string { | |||
requirements[key] = make([]string, 0) | |||
} | |||
|
|||
requirements[key] = value | |||
if inst != nil { |
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.
Actually, don't we need to replace certain fields even if it isn't an instance unit?
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.
Yes, probably. I'll have to think about this further as it means the current untiNameIsInstance approach doesn't make the most sense.
Not super happy about this. Got any suggestions? |
I'm worried about the distinction between a Unit and a NamedUnit. How would you feel about putting the NamedUnit functionality in Unit, while allowing the Job model to continue to own the name in the datastore? |
How about this? (tiny change, but feels cleaner to me) |
Yeah, that feels better. I think we can move forward with it for now. |
@jonboulle We need docs with this PR. |
How's this? |
|
||
##### Dynamic requirements | ||
|
||
fleet supports several systemd specifiers to allow requirements to be dynamically determined based on a Job's name. This means that the same unit can be used for multiple Jobs and the requirements are dynamically substituted at evaluation time. |
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.
How about "dynamically substituted when the unit is scheduled"? The words "substituted" and "evaluation" feel like they overlap too much.
@jonboulle Thanks, it looks good. LGTM after we sort out that comment. |
Updated. |
@jonboulle squash and merge |
This reworks the template/instance logic and adds specifier substitution when parsing the Requirements for a Job. Instead of an InstanceUnit, we switch to a UnitNameInfo model which encapsulates information that can be derived from a unit's Name (namely, the %n %N %i %p specifiers). Also includes documentation for end users.
feat(job): substitute Requirements for instance unit jobs
onward |
Builds on #475, works towards #303