From 06ad00463c8ec0426f72a559924e6a0adfe4e2a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oto=20Pet=C5=99=C3=ADk?= Date: Sun, 8 Oct 2023 03:00:34 +0200 Subject: [PATCH] feat(provider): configure temp directory (#607) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(provider): configure temp directory Resource 'proxmox_virtual_environment_file' often requires lot of disk space in /tmp, which can be space-limited. Instead of requiring to set TMPDIR environment variable before running terraform, make it a provider configuration option. Signed-off-by: Oto Petřík * fix: lint error, align names in the `client` struct Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> --------- Signed-off-by: Oto Petřík Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Co-authored-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> --- docs/index.md | 11 +++++++++ fwprovider/provider.go | 14 +++++++++++- proxmox/client.go | 41 +++++++++++++++++++++++----------- proxmox/nodes/storage.go | 3 ++- proxmoxtf/config.go | 23 ++++++++++++++----- proxmoxtf/provider/provider.go | 9 +++++++- proxmoxtf/provider/schema.go | 7 ++++++ proxmoxtf/resource/file.go | 8 +++---- 8 files changed, 91 insertions(+), 25 deletions(-) diff --git a/docs/index.md b/docs/index.md index ed06d841..89f51cb8 100644 --- a/docs/index.md +++ b/docs/index.md @@ -21,6 +21,7 @@ provider "proxmox" { username = "root@pam" password = "the-password-set-during-installation-of-proxmox-ve" insecure = true + tmp_dir = "/var/tmp" } ``` @@ -167,6 +168,15 @@ failed (changing feature flags for privileged container is only allowed for root > when using API Token authentication, even when `Administrator` role or > the `root@pam` user is used with the token. +### Temporary directory + +Using `proxmox_virtual_environment_file` with `.iso` files or disk images can require +large amount of space in the temporary directory of the computer running terraform. + +Consider pointing `tmp_dir` to a directory with enough space, especially if the default +temporary directory is limited by the system memory (e.g. `tmpfs` mounted +on `/tmp`). + ## Argument Reference In addition @@ -209,3 +219,4 @@ Proxmox `provider` block: - `name` - (Required) The name of the node. - `address` - (Required) The IP address of the node. - `port` - (Optional) SSH port of the node. Defaults to 22. +- `tmp_dir` - (Optional) Use custom temporary directory. (can also be sourced from `PROXMOX_VE_TMPDIR`) \ No newline at end of file diff --git a/fwprovider/provider.go b/fwprovider/provider.go index 6ad52a46..06e8a906 100644 --- a/fwprovider/provider.go +++ b/fwprovider/provider.go @@ -70,6 +70,7 @@ type proxmoxProviderModel struct { Port types.Int64 `tfsdk:"port"` } `tfsdk:"node"` } `tfsdk:"ssh"` + TmpDir types.String `tfsdk:"tmp_dir"` } func (p *proxmoxProvider) Metadata(_ context.Context, _ provider.MetadataRequest, resp *provider.MetadataResponse) { @@ -119,6 +120,10 @@ func (p *proxmoxProvider) Schema(_ context.Context, _ provider.SchemaRequest, re Description: "The username for the Proxmox VE API.", Optional: true, }, + "tmp_dir": schema.StringAttribute{ + Description: "The alternative temporary directory.", + Optional: true, + }, }, Blocks: map[string]schema.Block{ // have to define it as a list due to backwards compatibility @@ -355,7 +360,14 @@ func (p *proxmoxProvider) Configure( return } - client := proxmox.NewClient(apiClient, sshClient) + // Intentionally use 'PROXMOX_VE_TMPDIR' with 'TMP' instead of 'TEMP', to match os.TempDir's use of $TMPDIR + tmpDirOverride := utils.GetAnyStringEnv("PROXMOX_VE_TMPDIR", "PM_VE_TMPDIR") + + if !config.TmpDir.IsNull() { + tmpDirOverride = config.TmpDir.ValueString() + } + + client := proxmox.NewClient(apiClient, sshClient, tmpDirOverride) resp.ResourceData = client resp.DataSourceData = client diff --git a/proxmox/client.go b/proxmox/client.go index 637018b8..4766dd97 100644 --- a/proxmox/client.go +++ b/proxmox/client.go @@ -7,6 +7,8 @@ package proxmox import ( + "os" + "github.com/bpg/terraform-provider-proxmox/proxmox/access" "github.com/bpg/terraform-provider-proxmox/proxmox/api" "github.com/bpg/terraform-provider-proxmox/proxmox/cluster" @@ -42,54 +44,67 @@ type Client interface { // SSH returns a lower-level SSH client. SSH() ssh.Client + + // TempDir returns (possibly overridden) os.TempDir(). + TempDir() string } type client struct { - a api.Client - s ssh.Client + apiClient api.Client + sshClient ssh.Client + tmpDirOverride string } // NewClient creates a new API client. -func NewClient(a api.Client, s ssh.Client) Client { - return &client{a: a, s: s} +func NewClient(apiClient api.Client, sshClient ssh.Client, tmpDirOverride string) Client { + return &client{apiClient: apiClient, sshClient: sshClient, tmpDirOverride: tmpDirOverride} } // Access returns a client for managing access control. func (c *client) Access() *access.Client { - return &access.Client{Client: c.a} + return &access.Client{Client: c.apiClient} } // Cluster returns a client for managing the cluster. func (c *client) Cluster() *cluster.Client { - return &cluster.Client{Client: c.a} + return &cluster.Client{Client: c.apiClient} } // Node returns a client for managing resources on a specific node. func (c *client) Node(nodeName string) *nodes.Client { - return &nodes.Client{Client: c.a, NodeName: nodeName} + return &nodes.Client{Client: c.apiClient, NodeName: nodeName} } // Pool returns a client for managing resource pools. func (c *client) Pool() *pools.Client { - return &pools.Client{Client: c.a} + return &pools.Client{Client: c.apiClient} } // Storage returns a client for managing storage. func (c *client) Storage() *storage.Client { - return &storage.Client{Client: c.a} + return &storage.Client{Client: c.apiClient} } // Version returns a client for getting the version of the Proxmox Virtual Environment API. func (c *client) Version() *version.Client { - return &version.Client{Client: c.a} + return &version.Client{Client: c.apiClient} } // API returns a lower-lever REST API client. func (c *client) API() api.Client { - return c.a + return c.apiClient } -// SSH returns a lower-lever SSH client.s. +// SSH returns a lower-lever SSH client. func (c *client) SSH() ssh.Client { - return c.s + return c.sshClient +} + +// TempDir returns (possibly overridden) os.TempDir(). +func (c *client) TempDir() string { + if c.tmpDirOverride != "" { + return c.tmpDirOverride + } + + return os.TempDir() } diff --git a/proxmox/nodes/storage.go b/proxmox/nodes/storage.go index bf4e6a9d..b9834cdd 100644 --- a/proxmox/nodes/storage.go +++ b/proxmox/nodes/storage.go @@ -145,6 +145,7 @@ func (c *Client) APIUpload( datastoreID string, d *api.FileUploadRequest, uploadTimeout int, + tempDir string, ) (*DatastoreUploadResponseBody, error) { tflog.Debug(ctx, "uploading file to datastore using PVE API", map[string]interface{}{ "file_name": d.FileName, @@ -205,7 +206,7 @@ func (c *Client) APIUpload( // We need to store the multipart content in a temporary file to avoid using high amounts of memory. // This is necessary due to Proxmox VE not supporting chunked transfers in v6.1 and earlier versions. - tempMultipartFile, err := os.CreateTemp("", "multipart") + tempMultipartFile, err := os.CreateTemp(tempDir, "multipart") if err != nil { return nil, fmt.Errorf("failed to create temporary file: %w", err) } diff --git a/proxmoxtf/config.go b/proxmoxtf/config.go index 6dde876c..b7d3c575 100644 --- a/proxmoxtf/config.go +++ b/proxmoxtf/config.go @@ -8,6 +8,7 @@ package proxmoxtf import ( "errors" + "os" "github.com/bpg/terraform-provider-proxmox/proxmox" "github.com/bpg/terraform-provider-proxmox/proxmox/api" @@ -16,18 +17,21 @@ import ( // ProviderConfiguration is the configuration for the provider. type ProviderConfiguration struct { - apiClient api.Client - sshClient ssh.Client + apiClient api.Client + sshClient ssh.Client + tmpDirOverride string } // NewProviderConfiguration creates a new provider configuration. func NewProviderConfiguration( apiClient api.Client, sshClient ssh.Client, + tmpDirOverride string, ) ProviderConfiguration { return ProviderConfiguration{ - apiClient: apiClient, - sshClient: sshClient, + apiClient: apiClient, + sshClient: sshClient, + tmpDirOverride: tmpDirOverride, } } @@ -45,5 +49,14 @@ func (c *ProviderConfiguration) GetClient() (proxmox.Client, error) { ) } - return proxmox.NewClient(c.apiClient, c.sshClient), nil + return proxmox.NewClient(c.apiClient, c.sshClient, c.tmpDirOverride), nil +} + +// TempDir returns (possibly overridden) os.TempDir(). +func (c *ProviderConfiguration) TempDir() string { + if c.tmpDirOverride != "" { + return c.tmpDirOverride + } + + return os.TempDir() } diff --git a/proxmoxtf/provider/provider.go b/proxmoxtf/provider/provider.go index 9f0f5a5d..3fa5800d 100644 --- a/proxmoxtf/provider/provider.go +++ b/proxmoxtf/provider/provider.go @@ -155,7 +155,14 @@ func providerConfigure(_ context.Context, d *schema.ResourceData) (interface{}, return nil, diag.Errorf("error creating SSH client: %s", err) } - config := proxmoxtf.NewProviderConfiguration(apiClient, sshClient) + // Intentionally use 'PROXMOX_VE_TMPDIR' with 'TMP' instead of 'TEMP', to match os.TempDir's use of $TMPDIR + tmpDirOverride := utils.GetAnyStringEnv("PROXMOX_VE_TMPDIR", "PM_VE_TMPDIR") + + if v, ok := d.GetOk(mkProviderTmpDir); ok { + tmpDirOverride = v.(string) + } + + config := proxmoxtf.NewProviderConfiguration(apiClient, sshClient, tmpDirOverride) return config, nil } diff --git a/proxmoxtf/provider/schema.go b/proxmoxtf/provider/schema.go index 0d8da9bf..5dc0b790 100644 --- a/proxmoxtf/provider/schema.go +++ b/proxmoxtf/provider/schema.go @@ -22,6 +22,7 @@ const ( mkProviderPassword = "password" mkProviderUsername = "username" mkProviderAPIToken = "api_token" + mkProviderTmpDir = "tmp_dir" mkProviderSSH = "ssh" mkProviderSSHUsername = "username" mkProviderSSHPassword = "password" @@ -169,5 +170,11 @@ func createSchema() map[string]*schema.Schema { }, }, }, + mkProviderTmpDir: { + Type: schema.TypeString, + Optional: true, + Description: "The alternative temporary directory.", + ValidateFunc: validation.StringIsNotEmpty, + }, } } diff --git a/proxmoxtf/resource/file.go b/proxmoxtf/resource/file.go index 9c7d6530..582ca8b8 100644 --- a/proxmoxtf/resource/file.go +++ b/proxmoxtf/resource/file.go @@ -395,7 +395,7 @@ func fileCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag defer utils.CloseOrLogError(ctx)(res.Body) - tempDownloadedFile, err := os.CreateTemp("", "download") + tempDownloadedFile, err := os.CreateTemp(config.TempDir(), "download") if err != nil { return diag.FromErr(err) } @@ -471,8 +471,8 @@ func fileCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag } } - tempRawFile, err := os.CreateTemp("", "raw") - if err != nil { + tempRawFile, e := os.CreateTemp(config.TempDir(), "raw") + if e != nil { return diag.FromErr(err) } @@ -522,7 +522,7 @@ func fileCreate(ctx context.Context, d *schema.ResourceData, m interface{}) diag switch *contentType { case "iso", "vztmpl": uploadTimeout := d.Get(mkResourceVirtualEnvironmentFileTimeoutUpload).(int) - _, err = capi.Node(nodeName).APIUpload(ctx, datastoreID, request, uploadTimeout) + _, err = capi.Node(nodeName).APIUpload(ctx, datastoreID, request, uploadTimeout, config.TempDir()) default: // For all other content types, we need to upload the file to the node's // datastore using SFTP.