Fix for systemd instantiated services when enabled via ignition

[sohankunkerkar]

Fixes #586

[coreosbot]

Can one of the admins verify this patch?

[dustymabe]

Since we need to combine all instantiated units of a given service into one line (i.e. enable [email protected] bar0 bar1 bar2) would it make sense to just collect what units we want to enable in createUnits() from units.go and then aggregate the information before writing out the file?

[arithx]

Also, overarching question: why use strings.Builder over a list of strings? It seems like you're treating them at as space-delimited lists anyways.

[jamescassell]

Seems like it would be a heck of a lot easier to update systemd to accept the [email protected] syntax, rather than the strange syntax it actually requires... but I guess if this lands fastest, I can't complain about "it's done"

Thanks for working on this!

[dustymabe]

A few comments inline in the code. In general I think we can collapse a lot of logic if we structure our data differently. We could build up a presets map using a new Preset type and hold all the information we need I think. Something like the following map/Type:

package main
import "fmt"

type Preset struct {
    unit string
    enabled bool
    instantiateable bool
    instances []string
}
func main() {
    presets := make(map[string]*Preset)
    presets["sshd.service"] = &Preset{"sshd.service", true, false, []string{}}
    presets["chronyd.service"] = &Preset{"chronyd.service", false, false, []string{}}
    presets["[email protected]"] = &Preset{"[email protected]", true, true, []string{"foo"}}
    presets["[email protected]"].instances = append(presets["[email protected]"].instances, "bar")
    for _, value := range presets {
        fmt.Println()
        fmt.Println("unit:", value.unit)
        fmt.Println("enabled:", value.enabled)
        fmt.Println("instantiateable:", value.instantiateable)
        if value.instantiateable {
            fmt.Println("instances:", value.instances)
        }
    }
}

[dustymabe]

Nice work.. Some new comments

[dustymabe]

new comments

[bgilbert]

Generally looks pretty good. Just some nits.

In your commit message:

Code to fix the systemd instatiated services when enabled/disabled via ignition

Code to is obvious, as is via ignition. What about Fix enabling systemd instantiated services, with a link to the original issue?

[dustymabe]

The Code LGTM - Nice work @sohankunkerkar