config/types/systemd: add templating to systemd

[arithx]

Fixes coreos/bugs#2085

[dghubble]

Commented on the upstream issue. I'm for "fixing" this issue by removing the feature rather than expanding it further. coreos/bugs#2085 (comment)

[arithx]

Closing

[dghubble]

Oh, I mean, this is the fix technically. So awesome that you've added it, thanks.

I'd like to see what @dgonyeo thinks about the idea of dropping the feature since he's the maintainer.

[cgonyeo]

One issue with dropping this feature and just making sure the docs are good is that in the etcd and flannel sections the user lacks enough control over the generated unit to actually use coreos-metadata without this feature.

Unless we're also talking about dropping the etcd and flannel sections, I'd prefer to keep this feature in, or at least come up with some alternative way for users to use coreos-metadata results in those sections.

[dghubble]

In what way is writing the unit directly not plausible? I haven't needed the dynamic data feature in my experience running clusters across clouds and bare-metal, but maybe there is a case I haven't hit.

[crawford]

the etcd and flannel sections the user lacks enough control

We should fix this instead.

[cgonyeo]

@crawford think it would be reasonable for ct to allow users to add arbitrary things to the produced units for etcd and flannel, and to remove the dynamic data feature then?

One thing to note is that this would result in users needing separate configs for each provider a user wishes to use, since coreos-metadata produces provider-specific environment variables.

[dghubble]

Ok, admittedly I don't use the special etcd and flannel sections so perhaps they're useful to some cases / people?

[crawford]

One thing to note is that this would result in users needing separate configs for each provider a user wishes to use, since coreos-metadata produces provider-specific environment variables.

This is one of the main reasons we created CT. I don't think it makes sense to remove this feature and ask users to write separate configs for each platform.

[cgonyeo]

If templating is used, the units must also have:

Requires=coreos-metadata.service
After=coreos-metadata.service
EnvironmentFile=/run/metadata/coreos

Here's where these lines are added for the generated units:

container-linux-config-transpiler/config/types/common.go

Line 79 in 2458180

out.Unit.Add("Requires=coreos-metadata.service")

Unfortunately users may have added these lines themselves to their units, so you should probably check for those. Maybe modify types/util/UnitSection.Add to check for preexisting matching keys, and to blow them away before appending the new value.

[dghubble]

@crawford If it's a goal for CT to abstract away cases where a user would reference cloud-provider specific variables, dynamic data should support systemd units (per initial issue) and also dropins. This expands the scope and I tend to think its not worthwhile, but that's my opinion. If we continue with it, we should think about aliasing the other cloud-provider specific variables - as soon as I try to use the 2nd NIC, the current abstraction doesn't work and I have to write the systemd unit directly.