From b2c50120ea552d078e9634228f8b90b356a163b9 Mon Sep 17 00:00:00 2001 From: maidlover <117573165+maidl0ver@users.noreply.github.com> Date: Tue, 12 Aug 2025 22:46:34 +0000 Subject: [PATCH] feat(lxc): Add missing configuration options for container rootfs (#2067) * Add mount_options for container rootfs Signed-off-by: maidl0ver Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Rename mount key to mountoptions Signed-off-by: maidl0ver Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Add a line to the docs Signed-off-by: maidl0ver Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Add mount_options to a test Signed-off-by: maidl0ver Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * lint Signed-off-by: maidl0ver Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * feat(firewall): adds forward_policy to cluster firewall (#2064) Signed-off-by: Marshall Ford Co-authored-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * fix: update container image URL in acc test Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Add validation and diff suppression Co-authored-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Add a default value check for mount options in containerRead Signed-off-by: maidl0ver Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Check for changes to mount options Signed-off-by: maidl0ver Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Add disk change detection Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Update schema Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Add disk size to container update Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Remove redundant datastore ID Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Change type assertion Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Change volume name for containers Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Add fields to containerRead Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Change default disk mount options value to nil Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * Set volume format for container creation Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> * fix(lxc): root fs creation for storage-backed mp Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> * fix rootfs unmarshalling from API response Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> * fixes for edge cases Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> * fix linter Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> --------- Signed-off-by: maidl0ver Signed-off-by: maidlover <117573165+maidl0ver@users.noreply.github.com> Signed-off-by: Marshall Ford Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Co-authored-by: Marshall Ford Co-authored-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> --- .../virtual_environment_container.md | 1 + fwprovider/test/resource_container_test.go | 60 ++++++- proxmox/nodes/containers/containers_types.go | 6 +- proxmoxtf/resource/container/container.go | 148 +++++++++++++++--- 4 files changed, 189 insertions(+), 26 deletions(-) diff --git a/docs/resources/virtual_environment_container.md b/docs/resources/virtual_environment_container.md index 70c7c879..4b1f4118 100644 --- a/docs/resources/virtual_environment_container.md +++ b/docs/resources/virtual_environment_container.md @@ -134,6 +134,7 @@ output "ubuntu_container_public_key" { - `size` - (Optional) The size of the root filesystem in gigabytes (defaults to `4`). When set to 0 a directory or zfs/btrfs subvolume will be created. Requires `datastore_id` to be set. + - `mount_options` (Optional) List of extra mount options. - `initialization` - (Optional) The initialization configuration. - `dns` - (Optional) The DNS configuration. - `domain` - (Optional) The DNS search domain. diff --git a/fwprovider/test/resource_container_test.go b/fwprovider/test/resource_container_test.go index 3b4dc02e..93510379 100644 --- a/fwprovider/test/resource_container_test.go +++ b/fwprovider/test/resource_container_test.go @@ -48,7 +48,7 @@ func TestAccResourceContainer(t *testing.T) { FileName: ptr.Ptr(imageFileName), Node: ptr.Ptr(te.NodeName), Storage: ptr.Ptr(te.DatastoreID), - URL: ptr.Ptr(fmt.Sprintf("%s/images/system/ubuntu-23.04-standard_23.04-1_amd64.tar.zst", te.ContainerImagesServer)), + URL: ptr.Ptr(fmt.Sprintf("%s/images/system/ubuntu-24.10-standard_24.10-1_amd64.tar.zst", te.ContainerImagesServer)), }) require.NoError(t, err) @@ -110,9 +110,10 @@ func TestAccResourceContainer(t *testing.T) { "device_passthrough.0.mode": "0660", "initialization.0.dns.#": "0", }), - ResourceAttributesSet(accTestContainerName, []string{ - "ipv4.vmbr0", - }), + // TODO: depends on DHCP, which may not work in some environments + // ResourceAttributesSet(accTestContainerName, []string{ + // "ipv4.vmbr0", + // }), func(*terraform.State) error { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() @@ -142,6 +143,7 @@ func TestAccResourceContainer(t *testing.T) { disk { datastore_id = "local-lvm" size = 4 + mount_options = ["discard"] } mount_point { volume = "local-lvm" @@ -178,6 +180,56 @@ func TestAccResourceContainer(t *testing.T) { "description": "my\ndescription\nvalue\n", "device_passthrough.#": "1", "initialization.0.dns.#": "0", + "disk.0.mount_options.#": "1", + }), + ), + }, + { + // remove disk options + Config: te.RenderConfig(` + resource "proxmox_virtual_environment_container" "test_container" { + node_name = "{{.NodeName}}" + vm_id = {{.TestContainerID}} + timeout_delete = 10 + unprivileged = true + disk { + datastore_id = "local-lvm" + size = 4 + mount_options = [] + } + mount_point { + volume = "local-lvm" + size = "4G" + path = "mnt/local" + } + device_passthrough { + path = "/dev/zero" + } + description = <<-EOT + my + description + value + EOT + initialization { + hostname = "test" + ip_config { + ipv4 { + address = "172.16.10.10/15" + gateway = "172.16.0.1" + } + } + } + network_interface { + name = "vmbr0" + } + operating_system { + template_file_id = "local:vztmpl/{{.ImageFileName}}" + type = "ubuntu" + } + }`, WithRootUser()), + Check: resource.ComposeTestCheckFunc( + ResourceAttributes(accTestContainerName, map[string]string{ + "disk.0.mount_options.#": "0", }), ), }, diff --git a/proxmox/nodes/containers/containers_types.go b/proxmox/nodes/containers/containers_types.go index 1b1d8825..3663e2d1 100644 --- a/proxmox/nodes/containers/containers_types.go +++ b/proxmox/nodes/containers/containers_types.go @@ -546,7 +546,7 @@ func (r *CustomRootFS) EncodeValues(key string, v *url.Values) error { if r.MountOptions != nil { if len(*r.MountOptions) > 0 { - values = append(values, fmt.Sprintf("mount=%s", strings.Join(*r.MountOptions, ";"))) + values = append(values, fmt.Sprintf("mountoptions=%s", strings.Join(*r.MountOptions, ";"))) } } @@ -875,6 +875,8 @@ func (r *CustomRootFS) UnmarshalJSON(b []byte) error { r.Volume = v[0] } else if len(v) == 2 { switch v[0] { + case "volume": + r.Volume = v[1] case "acl": bv := types.CustomBool(v[1] == "1") r.ACL = &bv @@ -902,7 +904,7 @@ func (r *CustomRootFS) UnmarshalJSON(b []byte) error { case "size": r.Size = new(types.DiskSize) - err := r.Size.UnmarshalJSON([]byte(v[1])) + err = r.Size.UnmarshalJSON([]byte(v[1])) if err != nil { return fmt.Errorf("failed to unmarshal disk size: %w", err) } diff --git a/proxmoxtf/resource/container/container.go b/proxmoxtf/resource/container/container.go index 3b675d21..f64445d5 100644 --- a/proxmoxtf/resource/container/container.go +++ b/proxmoxtf/resource/container/container.go @@ -52,7 +52,10 @@ const ( dvCPUUnits = 1024 dvDescription = "" dvDevicePassthroughMode = "0660" + dvDiskACL = false dvDiskDatastoreID = "local" + dvDiskQuota = false + dvDiskReplicate = false dvDiskSize = 4 dvFeaturesNesting = false dvFeaturesKeyControl = false @@ -107,7 +110,11 @@ const ( mkCPUUnits = "units" mkDescription = "description" mkDisk = "disk" + mkDiskACL = "acl" mkDiskDatastoreID = "datastore_id" + mkDiskMountOptions = "mount_options" + mkDiskQuota = "quota" + mkDiskReplicate = "replicate" mkDiskSize = "size" mkFeatures = "features" mkFeaturesNesting = "nesting" @@ -329,13 +336,20 @@ func Container() *schema.Resource { DefaultFunc: func() (interface{}, error) { return []interface{}{ map[string]interface{}{ - mkDiskDatastoreID: dvDiskDatastoreID, - mkDiskSize: dvDiskSize, + mkDiskDatastoreID: dvDiskDatastoreID, + mkDiskSize: dvDiskSize, + mkDiskMountOptions: nil, }, }, nil }, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ + mkDiskACL: { + Type: schema.TypeBool, + Description: "Explicitly enable or disable ACL support", + Optional: true, + Default: dvDiskACL, + }, mkDiskDatastoreID: { Type: schema.TypeString, Description: "The datastore id", @@ -343,6 +357,18 @@ func Container() *schema.Resource { ForceNew: true, Default: dvDiskDatastoreID, }, + mkDiskQuota: { + Type: schema.TypeBool, + Description: "Enable user quotas for the container rootfs", + Optional: true, + Default: dvDiskQuota, + }, + mkDiskReplicate: { + Type: schema.TypeBool, + Description: "Will include this volume to a storage replica job", + Optional: true, + Default: dvDiskReplicate, + }, mkDiskSize: { Type: schema.TypeInt, Description: "The rootfs size in gigabytes", @@ -351,6 +377,17 @@ func Container() *schema.Resource { Default: dvDiskSize, ValidateDiagFunc: validation.ToDiagFunc(validation.IntAtLeast(0)), }, + mkDiskMountOptions: { + Type: schema.TypeList, + Description: "Extra mount options", + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateFunc: validation.StringIsNotEmpty, + }, + DiffSuppressFunc: structure.SuppressIfListsAreEqualIgnoringOrder, + DiffSuppressOnRefresh: true, + }, }, }, MaxItems: 1, @@ -1458,6 +1495,23 @@ func containerCreateCustom(ctx context.Context, d *schema.ResourceData, m interf return diag.FromErr(err) } + vmIDUntyped, hasVMID := d.GetOk(mkVMID) + vmID := vmIDUntyped.(int) + + if !hasVMID { + vmIDNew, err := config.GetIDGenerator().NextID(ctx) + if err != nil { + return diag.FromErr(err) + } + + vmID = vmIDNew + + err = d.Set(mkVMID, vmID) + if err != nil { + return diag.FromErr(err) + } + } + nodeName := d.Get(mkNodeName).(string) container := Container() @@ -1709,12 +1763,21 @@ func containerCreateCustom(ctx context.Context, d *schema.ResourceData, m interf var rootFS *containers.CustomRootFS + diskMountOptions := []string{} + + if diskBlock[mkDiskMountOptions] != nil { + for _, opt := range diskBlock[mkDiskMountOptions].([]any) { + diskMountOptions = append(diskMountOptions, opt.(string)) + } + } + diskSize := diskBlock[mkDiskSize].(int) if diskDatastoreID != "" && (diskSize != dvDiskSize || len(mountPoints) > 0) { // This is a special case where the rootfs size is set to a non-default value at creation time. // see https://pve.proxmox.com/pve-docs/chapter-pct.html#_storage_backed_mount_points rootFS = &containers.CustomRootFS{ - Volume: fmt.Sprintf("%s:%d", diskDatastoreID, diskSize), + Volume: fmt.Sprintf("%s:%d", diskDatastoreID, diskSize), + MountOptions: &diskMountOptions, } } @@ -1831,22 +1894,6 @@ func containerCreateCustom(ctx context.Context, d *schema.ResourceData, m interf tags := d.Get(mkTags).([]interface{}) template := types.CustomBool(d.Get(mkTemplate).(bool)) unprivileged := types.CustomBool(d.Get(mkUnprivileged).(bool)) - vmIDUntyped, hasVMID := d.GetOk(mkVMID) - vmID := vmIDUntyped.(int) - - if !hasVMID { - vmIDNew, err := config.GetIDGenerator().NextID(ctx) - if err != nil { - return diag.FromErr(err) - } - - vmID = vmIDNew - - err = d.Set(mkVMID, vmID) - if err != nil { - return diag.FromErr(err) - } - } // Attempt to create the container using the retrieved values. createBody := containers.CreateRequestBody{ @@ -2253,12 +2300,22 @@ func containerRead(ctx context.Context, d *schema.ResourceData, m interface{}) d if containerConfig.RootFS != nil { volumeParts := strings.Split(containerConfig.RootFS.Volume, ":") + disk[mkDiskACL] = containerConfig.RootFS.ACL + disk[mkDiskReplicate] = containerConfig.RootFS.Replicate + disk[mkDiskQuota] = containerConfig.RootFS.Quota disk[mkDiskDatastoreID] = volumeParts[0] + disk[mkDiskSize] = containerConfig.RootFS.Size.InGigabytes() + if containerConfig.RootFS.MountOptions != nil { + disk[mkDiskMountOptions] = *containerConfig.RootFS.MountOptions + } else { + disk[mkDiskMountOptions] = []string{} + } } else { // Default value of "storage" is "local" according to the API documentation. disk[mkDiskDatastoreID] = "local" disk[mkDiskSize] = dvDiskSize + disk[mkDiskMountOptions] = []string{} } currentDisk := d.Get(mkDisk).([]interface{}) @@ -2275,7 +2332,10 @@ func containerRead(ctx context.Context, d *schema.ResourceData, m interface{}) d } } else if len(currentDisk) > 0 || disk[mkDiskDatastoreID] != dvDiskDatastoreID || - disk[mkDiskSize] != dvDiskSize { + disk[mkDiskACL] != dvDiskACL || + disk[mkDiskReplicate] != dvDiskReplicate || + disk[mkDiskQuota] != dvDiskQuota || + len(disk[mkDiskMountOptions].([]string)) > 0 { err := d.Set(mkDisk, []interface{}{disk}) diags = append(diags, diag.FromErr(err)...) } @@ -2917,6 +2977,50 @@ func containerUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) updateBody.CPUUnits = &cpuUnits } + if d.HasChange(mkDisk) { + diskBlock, err := structure.GetSchemaBlock( + container, + d, + []string{mkDisk}, + 0, + true, + ) + if err != nil { + return diag.FromErr(err) + } + + rootFS := &containers.CustomRootFS{} + // Disk ID for the rootfs is always 0 + diskID := 0 + vmID := d.Get(mkVMID).(int) + rootFS.Volume = diskBlock[mkDiskDatastoreID].(string) + rootFS.Volume = getContainerDiskVolume(rootFS.Volume, vmID, diskID) + + acl := types.CustomBool(diskBlock[mkDiskACL].(bool)) + mountOptions := diskBlock[mkDiskMountOptions].([]interface{}) + quota := types.CustomBool(diskBlock[mkDiskQuota].(bool)) + replicate := types.CustomBool(diskBlock[mkDiskReplicate].(bool)) + size := types.DiskSizeFromGigabytes(int64(diskBlock[mkDiskSize].(int))) + + rootFS.ACL = &acl + rootFS.Quota = "a + rootFS.Replicate = &replicate + rootFS.Size = size + + mountOptionsStrings := make([]string, 0, len(mountOptions)) + + for _, option := range mountOptions { + mountOptionsStrings = append(mountOptionsStrings, option.(string)) + } + + // Always set, including empty, to allow clearing mount options + rootFS.MountOptions = &mountOptionsStrings + + updateBody.RootFS = rootFS + + rebootRequired = true + } + if d.HasChange(mkFeatures) { features, err := containerGetFeatures(container, d) if err != nil { @@ -3424,3 +3528,7 @@ func parseImportIDWithNodeName(id string) (string, string, error) { return nodeName, id, nil } + +func getContainerDiskVolume(rawVolume string, vmID int, diskID int) string { + return fmt.Sprintf("%s:vm-%d-disk-%d", rawVolume, vmID, diskID) +}