andsens / bootstrap-vz

Bootstrap Debian images for virtualized environments
http://bootstrap-vz.readthedocs.io/
Other
263 stars 145 forks source link

Correct syntax for mount_opts #484

Closed kevin-olbrich closed 6 years ago

kevin-olbrich commented 6 years ago

Hi!

I would like to build images with discard mount option set.

Is this the correct syntax? Python code uses list but I am not familiar with yaml:

    root:
      filesystem: ext4
      size: 13GiB
      mount_opts:
        - defaults
        - discard
andsens commented 6 years ago

Yup. No need to worry though, your manifest is checked with a json schema before it is executed. Most errors like these are caught very early in the process.

p.s.: I actually think all datatypes in the manifest are validated, it's only when we get to how options can be used together that we get into uncertain territory, but even there we have the escape hatch of validating with code :-)

kevin-olbrich commented 6 years ago

{mount_opts}: Options to mount the partition with. This optional setting overwrites the default option list bootstrap-vz would normally use to mount the partiton (defaults). The List is specified as a string array where each option/argument is an item in that array.

volume:
  backing: qcow2
  partitions:
    type: gpt
    boot:
      filesystem: ext4
      size: 1GiB
      mount_opts:
        - defaults
        - discard
    swap:
      size: 2GiB
      mount_opts:
        - sw
        - discard
    root:
      filesystem: ext4
      size: 7GiB
      mount_opts:
        - defaults
        - discard
    var/log:
      filesystem: ext4
      size: 5GiB
      mount_opts:
        - defaults
        - discard

bootstrapvz.common.exceptions.ManifestError: {'backing': 'qcow2', 'partitions': {'type': 'gpt', 'boot': {'filesystem': 'ext4', 'mount_opts': ['defaults', 'discard'], 'size': '1GiB'}, 'swap': {'mount_opts': ['sw', 'discard'], 'size': '2GiB'}, 'root': {'filesystem': 'ext4', 'mount_opts': ['defaults', 'discard'], 'size': '7GiB'}, 'var/log': {'filesystem': 'ext4', 'mount_opts': ['defaults', 'discard'], 'size': '5GiB'}}} is not valid under any of the given schemas

kevin-olbrich commented 6 years ago

This might be the problem:

            if partition.mountopts is None:
                mount_opts = ['defaults']
            else:
                mount_opts = partition.mountopts

While "mount_opts" is an internal variable, the correct manifest entry would be "mountopts". As we also got "include_packages", "install_standard", "admin_user", etc. - "mount_opts" should be the one to go. Should I file a PR?

andsens commented 6 years ago

While "mount_opts" is an internal variable, the correct manifest entry would be "mountopts". As we also got "include_packages", "install_standard", "admin_user", etc. - "mount_opts" should be the one to go.

Agreed, especially because the documentation says so.

Should I file a PR?

That would be very much appreciated :-)