SUSE-Enceladus / keg

Kiwi Entwicklungs-Gerät (tool to produce kiwi image descriptions)
GNU General Public License v3.0
8 stars 6 forks source link

Discussion: drop jinja2 template #95

Closed jgleissner closed 2 years ago

jgleissner commented 2 years ago

The current template is not flexible enough to accommodate SLE Micro image definitions, as there is no support for the systemdisk parameter. I've pushed a branch to keg-recipes to make the preferences section fully configurable which would also enable support for SLE Micro images. The diff can be inspected here: https://github.com/SUSE-Enceladus/keg-recipes/compare/develop...template-improvements?expand=1

I haven't opened a PR for this yet because I wanted to get a few opinions wrt whether we should even continue the route via jinja. For the initial design I envisioned the image definition on keg side to be a somewhat more abstract representation of an image description, but in reality the image definition in keg is structurally very much tied to the kiwi description. I guess this isn't really a problem because kiwi is most probably going to remain our image builder for the foreseeable future. The real value of keg is that we can compose kiwi descriptions from building blocks, thus eliminating cut&paste descriptions and data duplication vastly, not so much abstraction.

In the above mentioned branch in keg-recipes, the jinja template basically just becomes an XML generator, and a rather awkward one at that. Working with jinja programmatically is very tedious. It still defines the basic structure of the kiwi file, but if we fully align the input data structure with kiwi's layout, we shouldn't even need that. keg could produce the kiwi dictionary directly and serialize it into XML, perhaps by using dicttoxml or more probably an implementation based on it, as dicttoxml is unlikely to support comments, which we need, and also doesn't seem to be actively maintained.

Dropping the template would mean that users couldn't use their own templates if they wanted to do things rather differently, but if keg was a universal kiwi generator, I'm not sure why you would need that.

Some investigation in the feasibility would be necessary, of course.

@rjschwei @aosthof @jesusbv Any opinions?

rjschwei commented 2 years ago

Well,

Generally I am not a big fan of Jinja. My initial thought for keg was that we compose XML snippets and compose the description via a modified depth first search through the tree. Such a modified depth first search would have allowed us to also inherit from SLE 12 for example in a SLE 15 description eliminating even more duplication. So I am open to looking at his in a different way. A general template is still nice as it will provide structure to the kiwi file making it easier to diff and more human palatable. We can probably come up with some trick to have markers that put comments in the right places in the xml file. kiwi also understand yaml files so we could also decide to switch from generating the XML file to generating a yaml file. But that will make it extremely hard for us to not break things.

In summary, yes I am open to dropping jinja and looking at a different approach that gives us more flexibility. It'll come at the expense of keg getting more complex but I don't see that as an issue.

jesusbv commented 2 years ago

No issue from my side in dropping Jinja due to the limitations, even better if that brings an opportunity to make keg better. I agree that a general template and flexibity are worthy the possible/probable cost in keg or using another tool.

My 2 cents

jgleissner commented 2 years ago

Well, the XML snippets as input approach turned out to be quite limiting and I think that keg composes the dictionary the way it does now makes it far more flexible. Writing the kiwi file in yaml instead of xml would probably make it a bit easier from keg side, but I'm not sure its use is wide-spread and whether they're may be issues with it. If keg produces the kiwi file directly, it should be fairly simple to support both formats though.

For the flexibility we need to produce the variety of image builds, the template approach is IMO not terribly useful because, as said, the jinja template is basically an (ugly) XML generator. But there may be use cases where the template approach is useful, as it allows the user to produce basically any kind of output, although I don't really see any. It would be easy enough though to simply keep the template output as an option.

jgleissner commented 2 years ago

I have the first draft of the internal XML generator implementation ready. It can create a SLE Micro image description that kiwi successfully validates. It will require some changes to how the recipes data is structured, and will require the user to specific which keys should be translated to XML attributes in the kiwi file. So far, the template "knew" what was supposed to be an attribute and what a child member, but that has the obvious limitation that you need to implement special knowledge in the template (and also the reason why it can't deal with SLE Micro descriptions at the minute). That means, the new image definition would look like this, using SLE Micro as example:

  image:
    description:
      _attributes:
        type: system
      author: Public Cloud Team
      contact: public-cloud-dev@susecloud.net
    preferences:
      version: 0.9.0
      packagemanager: zypper
      rpm-check-signatures: "false"
      locale: en_US
      keytable: us.map.gz
      timezone: UTC
    users:
      user:
        - _attributes:
            name: root
            groups: root
            home: /root
            password: $1$wYJUgpM5$RXMMeASDc035eX.NbYWFl0
    include:
      - base/common
      - products/sle-micro/byos
    profiles:
      Azure:
        description: Azure configuration
        include:
          - csp/azure/settings/micro-os
      EC2:
        description: EC2 configuration
        include:
          - csp/ec2/settings/micro-os
      GCE:
        description: GCE configuration
        include:
          - csp/gce/settings/micro-os

This is aligned now with the structure kiwi uses. The new special keys may make it a little less pretty, but I think it's not too bad. The profile.yaml files in the data tree would also change in the same way and hence look like this:

preferences:
  type:
    _attributes:
      bootloader: grub2
      bootpartition: "false"
      boottimeout: 1
      firmware: uefi
      devicepersistency: by-label
      filesystem: xfs
      image: vmx
      kernelcmdline:
        console: ttyS0
        net.ifnames: 0
    size:
      _attributes:
        unit: M
      _text: 10240

The new generator also has support for user comments, adding a key starting with _comment will be translated to an XML comment. The XML generator has built-in magic to convert a dict attribute (like kernelcmdline in this case) to a key=value ... string just like the jinja template did, so kernelcmdline (and now any attribute actually) can still be composed from different sources.

For the namespaced data, nothing changes source side. keg iterates through the package list namespaces and produces a dict suitable for the XML generator, and that opportunity filters out any package or archive that is configured only of architectures that are not enabled.

This is a bit of a shift in data structure, but I think it's worth it because aligning the structure closely with kiwi's makes sense and may allow kiwi users to feel a bit more at home, and increases flexibility from my PoV. I haven't removed template support so this could still be used as an option, although existing templates would need to be adjusted.

What do you think?

rjschwei commented 2 years ago

I like it, nice work and just in time as we need

_attribute:
  arch: x86_64
....

for preferences to accommodate support for aarch64 in Azure. Examples check into IBS. This is much easier in the new setup, thus +100

Of course that comes with the "this is now urgent" requirement since we need to have aarch64 support for 15 SP4 and preferable have no manual fiddling of the descriptions generated with keg.

jgleissner commented 2 years ago

I agree it's good timing, and the inline XML generator will help with that. I see to ways two implement arch specific preferences.

One would be to introduce an archs key to profile section the so the image definition would look something like this:

profiles:
  Azure:
    description: Azure config
    include:
      - csp/azure/general
    archs:
      - arch: aarch64
        include: csp/azure/aarch64
      - arch: x86_64
        include: csp/azure/x86_64

When expanding the profiles, keg would create once for each arch with their specific includes. keg will get a command line switch for architecture selection so profiles for architecture that are not enabled are skipped.

The other way would be to handle this on XML generator level. We could add some special keys to configure that, so for instance the preferences section of a multi-arch enabled config would look something like this:

preferences:
  _archs:
    - aarch64
    - x86_64
  type:
    _attributes:
      firmware: efi
      format: vhd-fixed
      kernelcmdline:
        dis_ucode_ldr: []
        earlyprintk: ttyS0
        licensetag:
          - _arch: aarch64
            _data: aarch64_license
          - _arch: x86_64
            _data: x86_license

The XML generator would output a preferences element for each (enabled) arch, and selects the specific attribute keys accordingly. It may be a bit nicer as it keeps arch specific stuff out of the image definition and we wouldn't need to add arch specific data directories, but would make the XML generator a bit more complicated. One drawback would be that because the arch specific sections come from the same source profile, we couldn't generate architecture specific change logs (if we want to do that).

I did a POC implementation for approach 2 as it was my first idea, but I'm not sure we really need the XML generator to be architecture aware (of course _arch(s) could be renamed to something more generic).

Any opinions?

rjschwei commented 2 years ago

Or something like this where inherit is from the directory structure:

packages:
  _attributes:
    type: image
    profiles: Azure-Basic,Azure-Standard
  - content:
    - cloud-init
    - hyper-v

preferences:
  _inherit: Azure
  _attributes:
    profiles: Azure-Standard
    arch: aarch64
  type:
    _attributes:
      kernelcmdline:
        - console=tty1
        - console=ttyAMA0

preferences:
  _inherit: Azure
  _attributes:
    profiles: Azure-Standard
    arch: x86_64
  type:
    _attributes:
      kernelcmdline:
        - console=ttyS0
        - dis_ucode_ldr

preferences:
  _name: Azure
  type:
    _attributes:
      bootpartition: true
jgleissner commented 2 years ago

Following up the discussion about making keg "dumber", i.e. make it only merge stuff together and run it through the XML generator, I've put together an example image definition. I've stripped the image definition populate step in keg to basically only go through the definition and expand all include statements, whereas before it would construct the preferences and packages sections from the profile definitions. Here is what this approach would look like source side:

image.yaml:

include-paths:
  - sle15/sp1
  - sle15/sp2
  - sle15/sp3
  - sle15/sp4
schema: Null
image:
  _attributes:
    name: SLES15-SP4
    displayname: SLES15-SP4
    schemaversion: 6.2
  description:
    specification: "SUSE Linux Enterprise Server 15 SP4 guest image"
image:
  profiles:
    profile:
      - _attributes:
          name: Azure-Basic
          description: Azure Configuration
      - _attributes:
          name: Azure-Standard
          description: Azure Configuration
      - _attributes:
          name: EC2
          description: EC2 Configuration
      - _attributes:
          name: GCE
          description: GCE Configuration
  preferences:
    - _include: preferences/common
    - _attributes:
        profiles: Azure-Basic
      _include: preferences/csp/azure/basic
    - _attributes:
        profiles: Azure-Standard
        arch: aarch64
      _include: preferences/csp/azure/standard/aarch64
    - _attributes:
        profiles: Azure-Standard
        arch: x86_64
      _include: preferences/csp/azure/standard/x86_64
    - _attributes:
        profiles: EC2
      _include: preferences/csp/ec2
    - _attributes:
        profiles: GCE
      _include: preferences/csp/gce
  packages:
    - _attributes:
        type: bootstrap
      _include:
        - packages/base/bootstrap
        - packages/products/sles/bootstrap
    - _attributes:
        type: image
      _include:
        - packages/base/common
        - packages/base/efiboot
        - packages/base/sle-modules
        - packages/base/sle
        - packages/products/sles
    - _attributes:
        type: image
        profiles: Azure-Basic,Azure-Basic
        _include:
          - packages/csp/azure/azure-kernel
          - packages/csp/azure/azure-tools
          - packages/csp/azure/common
          - packages/csp/azure/cloudreg-switch
    - _attributes:
        type: image
        profiles: EC2
        _include:
          - packages/csp/ec2/base
          - packages/csp/ec2/aws-tools
    - _attributes:
       type: image
       profiles: GCE
       _include:
         - packages/csp/gce/base
         - packages/csp/gce/gce-tools
config:
  _include:
    - config/base/common
    - config/common/cloud-netconfig
    - config/base/cloud-register
    - config/products/sles-ondemand
  profiles:
    - profile: Azure-Basic
      _include:
        - config/csp/azure/basic
    - profile: Azure-Basic,Azure-Standard
      _include:
        - config/csp/azure/common
    - profile: EC2
      _include:
        - config/csp/ec2/common
    - profile: GCE
      _include:
        - config/csp/gce/common
overlayfiles:
  _include:
    - overlay/base/common
    - overlay/products/sles
  profiles:
    - profile: Azure-Basic,Azure-Standard
      _include:
        - overlay/csp/azure/common
    - profile: EC2
      _include:
        - overlay/csp/ec2/common
    - profile: GCE
      _include:
        - overlay/csp/gce/common

The config and overlayfiles keys are not part of the image description but for keg to be processed further.

And here are some selected data files: preferences/common/preferences.yaml:

version: 0.9.0
packagemanager: zypper
rpm-check-signatures: "false"
locale: en_US
keytable: us.map.gz
timezone: UTC

preferences/csp/defaults.yaml:

type:
  _attributes:
    bootloader: grub2
    bootpartition: "false"
    boottimeout: 1
    firmware: uefi
    devicepersistency: by-label
    filesystem: xfs
    image: vmx
    kernelcmdline:
      console: ttyS0
      net.ifnames: 0
  size:
    _attributes:
      unit: M
    _text: 10240

preferences/csp/azure/standard/aarch64/preferences.yaml:

type:
  _attributes:
    kernelcmdline:
      console:
        - tty1console
        - ttyAMA0
      earlycon: pl011,0xeffeb000

packages/common/packages.yaml:

_namespace_common:
  package:
    - _attributes:
        name: audit
    - _attributes:
        name: blog
    - _attributes:
        name: chrony
    - _attributes:
        name: cifs-utils
    - _attributes:
        name: dosfstools
    - _attributes:
        name: dracut
    - _attributes:
        name: grub
    ...

I still used namespacing in the packages file, otherwise keg would just keep overwriting the package key of previous includes. The XML generator does not emit an element for keys that start with _namespace (but it can print a comment instead).

I've generated a valid config.kiwi with this, so it would work, but I can't say a wholeheartedly like it, due to several reasons:

The image definition gets significantly more complicated, and one of my design goals was to keep the image definition simple so they would hopefully not need to be touched often (since we have a few of them), and reduce the margin of error.

Data modules are now separated by type, so you have modules for preferences, package, modules, config, and overlayfiles. So far you can group together bits that logically belong together (e.g. a package that contains a service, an overlay file providing config for that service, and a config snippet that enables it). (Current) Keg merges it all into its profiles dictionary, and constructs the profiles, preferences, and packages section from it, but with the dumb keg approach, the user would be responsible for creating those.

The package configs would essentially double in size, as keg wouldn't auto-expand the short format anymore (well it could, it seems a bit counter-intuitive to have that while trying to make keg less content-aware).

Lastly, change log generation would become more tricky. So far, because keg knows exactly what belongs to each profile as it merges the sources into a separate dict outside the image dict first, it can produce accurate change logs for each profile. With this approach there is just the image dictionary. Profile specific sections would still have tags (now set by the user instead of being automatically handled by tags), so keg could parse the dictionary to figure out which bits belong to which profile. Should be feasible of course, but also goes a bit against the idea of removing content knowledge from keg.

In conclusion, it seems feasible to go that route, but it seems a step back in terms of user friendliness and features, and would be a significant departure from keg's current design. The image definition will get more complex and hence may need to be touched more frequently. Data modules would need to get broken into smaller bits since different types can't be grouped together.

jgleissner commented 2 years ago

XML generator implementation was merged with https://github.com/SUSE-Enceladus/keg/pull/105.