diff --git a/fwprovider/provider_test.go b/fwprovider/provider_test.go new file mode 100644 index 00000000..516cf0f2 --- /dev/null +++ b/fwprovider/provider_test.go @@ -0,0 +1,114 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +package fwprovider_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/bpg/terraform-provider-proxmox/fwprovider/test" + "github.com/bpg/terraform-provider-proxmox/proxmox/cluster" + "github.com/bpg/terraform-provider-proxmox/proxmox/helpers/ptr" + "github.com/bpg/terraform-provider-proxmox/proxmox/nodes/vms" + "github.com/bpg/terraform-provider-proxmox/utils" +) + +func TestIDGenerator_Sequence(t *testing.T) { + t.Parallel() + + const numIDs = 10 + + if utils.GetAnyStringEnv("TF_ACC") == "" { + t.Skip("Acceptance tests are disabled") + } + + te := test.InitEnvironment(t) + + ctx := context.Background() + + gen := cluster.NewIDGenerator(te.ClusterClient(), cluster.IDGeneratorConfig{RandomIDs: false}) + firstID, err := gen.NextID(ctx) + require.NoError(t, err) + + busyID := firstID + 5 + + _, err = te.ClusterClient().GetNextID(ctx, ptr.Ptr(busyID)) + require.NoError(t, err, "the VM ID %d should be available", busyID) + + err = te.NodeClient().VM(0).CreateVM(ctx, &vms.CreateRequestBody{VMID: busyID}) + require.NoError(t, err, "failed to create VM %d", busyID) + + t.Cleanup(func() { + err = te.NodeClient().VM(busyID).DeleteVM(ctx) + require.NoError(t, err, "failed to delete VM %d", busyID) + }) + + ids := make([]int, numIDs) + + t.Cleanup(func() { + for _, id := range ids { + if id > 100 { + _ = te.NodeClient().VM(id).DeleteVM(ctx) //nolint:errcheck + } + } + }) + + prevID := firstID + + for i := range numIDs { + id, err := gen.NextID(ctx) + require.NoError(t, err) + err = te.NodeClient().VM(0).CreateVM(ctx, &vms.CreateRequestBody{VMID: id}) + ids[i] = id + + require.NoError(t, err) + require.Greater(t, id, prevID, "the generated ID should be greater than the previous one") + + prevID = id + } +} + +func TestIDGenerator_Random(t *testing.T) { + t.Parallel() + + const ( + numIDs = 7 + randomIDStat = 1000 + randomIDEnd = 1010 + ) + + if utils.GetAnyStringEnv("TF_ACC") == "" { + t.Skip("Acceptance tests are disabled") + } + + te := test.InitEnvironment(t) + + ctx := context.Background() + + gen := cluster.NewIDGenerator(te.ClusterClient(), cluster.IDGeneratorConfig{RandomIDs: true, RandomIDStat: randomIDStat, RandomIDEnd: randomIDEnd}) + + ids := make([]int, numIDs) + + t.Cleanup(func() { + for _, id := range ids { + if id > 100 { + _ = te.NodeClient().VM(id).DeleteVM(ctx) //nolint:errcheck + } + } + }) + + for i := range numIDs { + id, err := gen.NextID(ctx) + require.NoError(t, err) + err = te.NodeClient().VM(0).CreateVM(ctx, &vms.CreateRequestBody{VMID: id}) + ids[i] = id + + require.NoError(t, err) + } +} diff --git a/fwprovider/test/test_environment.go b/fwprovider/test/test_environment.go index c8d325f5..72564044 100644 --- a/fwprovider/test/test_environment.go +++ b/fwprovider/test/test_environment.go @@ -25,6 +25,7 @@ import ( "github.com/bpg/terraform-provider-proxmox/fwprovider" "github.com/bpg/terraform-provider-proxmox/proxmox/access" + "github.com/bpg/terraform-provider-proxmox/proxmox/cluster" sdkV2provider "github.com/bpg/terraform-provider-proxmox/proxmoxtf/provider" "github.com/bpg/terraform-provider-proxmox/proxmox/api" @@ -182,6 +183,11 @@ func (e *Environment) NodeStorageClient() *storage.Client { return &storage.Client{Client: e.NodeClient(), StorageName: e.DatastoreID} } +// ClusterClient returns a new cluster client for the test environment. +func (e *Environment) ClusterClient() *cluster.Client { + return &cluster.Client{Client: e.Client()} +} + // testAccMuxProviders returns a map of mux servers for the acceptance tests. func muxProviders(t *testing.T) map[string]func() (tfprotov6.ProviderServer, error) { t.Helper() diff --git a/proxmox/cluster/id_generator.go b/proxmox/cluster/id_generator.go index 12d09400..3c8f24b0 100644 --- a/proxmox/cluster/id_generator.go +++ b/proxmox/cluster/id_generator.go @@ -15,6 +15,7 @@ import ( "os" "path/filepath" "strconv" + "strings" "time" "github.com/avast/retry-go/v4" @@ -24,8 +25,9 @@ import ( ) const ( - idGeneratorLockFile = "terraform-provider-proxmox-id-gen.lock" - idGeneratorSequenceFile = "terraform-provider-proxmox-id-gen.seq" + idGeneratorLockFile = "terraform-provider-proxmox-id-gen.lock" + idGeneratorSequenceFile = "terraform-provider-proxmox-id-gen.seq" + idGeneratorContentionWindow = 5 * time.Second ) // IDGenerator is responsible for generating unique identifiers for VMs and Containers. @@ -66,7 +68,7 @@ func NewIDGenerator(client *Client, config IDGeneratorConfig) IDGenerator { // while giving some protection against parallel runs of the provider // that might interfere with each other and reset the sequence at the same time stat, err := os.Stat(config.seqFName) - if err == nil && time.Since(stat.ModTime()) > 10*time.Second { + if err == nil && time.Since(stat.ModTime()) > idGeneratorContentionWindow { _ = os.Remove(config.seqFName) } } @@ -86,13 +88,16 @@ func (g IDGenerator) NextID(ctx context.Context) (int, error) { defer unlock() - id, err := retry.DoWithData(func() (*int, error) { - var newID *int + ctx, cancel := context.WithTimeout(ctx, idGeneratorContentionWindow+time.Second) + defer cancel() + var newID *int + + id, err := retry.DoWithData(func() (*int, error) { if g.config.RandomIDs { //nolint:gosec newID = ptr.Ptr(rand.Intn(g.config.RandomIDEnd-g.config.RandomIDStat) + g.config.RandomIDStat) - } else { + } else if newID == nil { newID, err = nextSequentialID(g.config.seqFName) if err != nil { return nil, err @@ -100,7 +105,17 @@ func (g IDGenerator) NextID(ctx context.Context) (int, error) { } return g.client.GetNextID(ctx, newID) - }) + }, + retry.OnRetry(func(_ uint, err error) { + if strings.Contains(err.Error(), "already exists") && newID != nil { + newID = ptr.Ptr(*newID + 1) + } + }), + retry.Context(ctx), + retry.UntilSucceeded(), + retry.DelayType(retry.FixedDelay), + retry.Delay(200*time.Millisecond), + ) if err != nil { return -1, fmt.Errorf("unable to retrieve the next available VM identifier: %w", err) }