Skip to content

Commit

Permalink
Merge pull request #659 from Juniper/bug/654-resource-pool-race-condi…
Browse files Browse the repository at this point in the history
…tion

Include tests to exercise resource pool race condition
  • Loading branch information
chrismarget-j committed May 5, 2024
2 parents 98b37dd + 75c5c0c commit bac49b0
Show file tree
Hide file tree
Showing 6 changed files with 165 additions and 70 deletions.
1 change: 1 addition & 0 deletions apstra/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ var (
ResourceAgentProfile = resourceAgentProfile{}
ResourceDatacenterGenericSystem = resourceDatacenterGenericSystem{}
ResourceDatacenterRoutingZone = resourceDatacenterRoutingZone{}
ResourceIpv4Pool = resourceIpv4Pool{}
ResourceTemplatePodBased = resourceTemplatePodBased{}
ResourceTemplateCollapsed = resourceTemplateCollapsed{}
)
Expand Down
216 changes: 151 additions & 65 deletions apstra/resource_ipv4_pool_test.go
Original file line number Diff line number Diff line change
@@ -1,87 +1,173 @@
package tfapstra
//go:build integration

package tfapstra_test

import (
"context"
"fmt"
tfapstra "github.com/Juniper/terraform-provider-apstra/apstra"
"github.com/apparentlymart/go-cidr/cidr"
"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
"github.com/stretchr/testify/require"
"math"
"net"
"strconv"
"strings"
"testing"

testutils "github.com/Juniper/terraform-provider-apstra/apstra/test_utils"
"github.com/hashicorp/terraform-plugin-testing/helper/resource"
)

const (
resourceIpv4PoolTemplateHCL = `
// resource config
resource "apstra_ipv4_pool" "test" {
resource %q %q {
name = "%s"
subnets = [%s]
subnets = [%s
]
}
`
resourceIpv4PoolSubnetTemplateHCL = "{network = %q}"
resourceIpv4PoolSubnetTemplateHCL = "\n {network = %q},"
)

func TestAccResourceIpv4Pool_basic(t *testing.T) {
var (
testAccResourceIpv4PoolCfg1Name = acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
testAccResourceIpv4PoolCfg1Subnets = strings.Join([]string{
fmt.Sprintf(resourceIpv4PoolSubnetTemplateHCL, "192.168.0.0/16"),
}, ",")
testAccResourceIpv4PoolCfg1 = fmt.Sprintf(resourceIpv4PoolTemplateHCL, testAccResourceIpv4PoolCfg1Name, testAccResourceIpv4PoolCfg1Subnets)

testAccResourceIpv4PoolCfg2Name = acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
testAccResourceIpv4PoolCfg2Subnets = strings.Join([]string{
fmt.Sprintf(resourceIpv4PoolSubnetTemplateHCL, "192.168.1.0/24"),
fmt.Sprintf(resourceIpv4PoolSubnetTemplateHCL, "192.168.0.0/24"),
fmt.Sprintf(resourceIpv4PoolSubnetTemplateHCL, "192.168.2.0/23"),
}, ",")
testAccResourceIpv4PoolCfg2 = fmt.Sprintf(resourceIpv4PoolTemplateHCL, testAccResourceIpv4PoolCfg2Name, testAccResourceIpv4PoolCfg2Subnets)
func ipv4Subnets(t testing.TB, block string, size, count int) []string {
base, n, err := net.ParseCIDR(block)
require.NoError(t, err)
require.Equalf(t, base.String(), n.IP.String(), "%s is not a base network address", block)

ones, bits := n.Mask.Size()
a := math.Pow(2, float64(bits-ones))
b := math.Pow(2, float64(bits-size)) * float64(count)
require.Greaterf(t, a, b, "%d %d-bit subnets won't fit in %s", count, size, block)

result := make([]string, count)
for i := 0; i < count; i++ {
subnet, err := cidr.Subnet(n, size-ones, i)
require.NoError(t, err)
result[i] = subnet.String()
}

return result
}

type ipv4PoolConfig struct {
name string
subnets []string
}

func (o ipv4PoolConfig) render(rType, rName string) string {
sb := new(strings.Builder)
for _, subnet := range o.subnets {
sb.WriteString(fmt.Sprintf(resourceIpv4PoolSubnetTemplateHCL, subnet))
}

return fmt.Sprintf(resourceIpv4PoolTemplateHCL, rType, rName,
o.name,
sb.String(),
)
}

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
Steps: []resource.TestStep{
// Create and Read testing
{
Config: insecureProviderConfigHCL + testAccResourceIpv4PoolCfg1,
Check: resource.ComposeAggregateTestCheckFunc(
// Verify ID has any value set
resource.TestCheckResourceAttrSet("apstra_ipv4_pool.test", "id"),
// Verify name and overall usage statistics
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "name", testAccResourceIpv4PoolCfg1Name),
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "status", "not_in_use"),
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "total", "65536"),
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "used", "0"),
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "used_percentage", "0"),
// Verify number of subnets
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "subnets.#", "1"),
// Verify first subnet
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "subnets.0.network", "192.168.0.0/16"),
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "subnets.0.status", "pool_element_available"),
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "subnets.0.total", "65536"),
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "subnets.0.used", "0"),
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "subnets.0.used_percentage", "0"),
),
},
// Update and Read testing
{
Config: insecureProviderConfigHCL + testAccResourceIpv4PoolCfg2,
Check: resource.ComposeAggregateTestCheckFunc(
// Verify ID has any value set
resource.TestCheckResourceAttrSet("apstra_ipv4_pool.test", "id"),
// Verify name and overall usage statistics
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "name", testAccResourceIpv4PoolCfg2Name),
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "status", "not_in_use"),
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "total", "1024"),
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "used", "0"),
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "used_percentage", "0"),
// Verify number of subnets
resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "subnets.#", "3"),
//// cannot verify subnets here because they're not sorted
//resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "subnets.0.network", "192.168.0.0/24"),
//resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "subnets.1.network", "192.168.1.0/24"),
//resource.TestCheckResourceAttr("apstra_ipv4_pool.test", "subnets.2.network", "192.168.2.0/23"),
),
func (o ipv4PoolConfig) testChecks(t testing.TB, rType, rName string) testChecks {
t.Helper()
result := newTestChecks(rType + "." + rName)

var totalIPs int
prefixes := make([]net.IPNet, len(o.subnets))
for i, s := range o.subnets {
ip, prefix, err := net.ParseCIDR(s)
require.NoError(t, err)
require.EqualValuesf(t, ip.String(), prefix.IP.String(), "%s is not a base address", s)

prefixes[i] = *prefix

ones, bits := prefix.Mask.Size()
totalIPs += int(math.Pow(2, float64(bits-ones)))
}

// required and computed attributes can always be checked
result.append(t, "TestCheckResourceAttrSet", "id")
result.append(t, "TestCheckResourceAttr", "name", o.name)
result.append(t, "TestCheckResourceAttr", "status", "not_in_use")
result.append(t, "TestCheckResourceAttr", "total", strconv.Itoa(totalIPs))
result.append(t, "TestCheckResourceAttr", "used", "0")
result.append(t, "TestCheckResourceAttr", "used_percentage", "0")
result.append(t, "TestCheckResourceAttr", "subnets.#", strconv.Itoa(len(o.subnets)))

for _, p := range prefixes {
ones, bits := p.Mask.Size()
v := map[string]string{
"network": p.String(),
"status": "pool_element_available",
"total": strconv.Itoa(int(math.Pow(2, float64(bits-ones)))),
"used": "0",
"used_percentage": "0",
}
result.appendSetNestedCheck(t, "subnets.*", v)
}

return result
}

func TestAccResourceIpv4Pool(t *testing.T) {
ctx := context.Background()
require.NoError(t, testutils.TestCfgFileToEnv())

type testCase struct {
stepConfigs []ipv4PoolConfig
}

testCases := map[string]testCase{
"simple": {
stepConfigs: []ipv4PoolConfig{
{
name: acctest.RandString(6),
subnets: []string{"10.0.0.0/24", "10.0.1.0/24"},
},
{
name: acctest.RandString(6),
subnets: []string{"10.1.0.0/24", "10.1.1.0/24", "10.1.2.0/24"},
},
},
},
})
// AOS-46273
//"lots": {
// stepConfigs: []ipv4PoolConfig{
// {
// name: acctest.RandString(6),
// subnets: ipv4Subnets(t, "10.0.0.0/8", 28, 50),
// },
// },
//},
}

resourceType := tfapstra.ResourceName(ctx, &tfapstra.ResourceIpv4Pool)

for tName, tCase := range testCases {
tName, tCase := tName, tCase
t.Run(tName, func(t *testing.T) {
t.Parallel()

steps := make([]resource.TestStep, len(tCase.stepConfigs))
for i, stepConfig := range tCase.stepConfigs {
config := stepConfig.render(resourceType, tName)
checks := stepConfig.testChecks(t, resourceType, tName)

chkLog := checks.string()
stepName := fmt.Sprintf("test case %q step %d", tName, i+1)

t.Logf("\n// ------ begin config for %s ------\n%s// -------- end config for %s ------\n\n", stepName, config, stepName)
t.Logf("\n// ------ begin checks for %s ------\n%s// -------- end checks for %s ------\n\n", stepName, chkLog, stepName)

steps[i] = resource.TestStep{
Config: insecureProviderConfigHCL + config,
Check: resource.ComposeAggregateTestCheckFunc(checks.checks...),
}
}

resource.Test(t, resource.TestCase{
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
Steps: steps,
})
})
}
}
5 changes: 5 additions & 0 deletions apstra/test_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,11 @@ func (o *testChecks) append(t testing.TB, testCheckFuncName string, testCheckFun
}
}

func (o *testChecks) appendSetNestedCheck(t testing.TB, attrName string, m map[string]string) {
o.checks = append(o.checks, resource.TestCheckTypeSetElemNestedAttrs(o.path, attrName, m))
o.logLines.appendf("TestCheckTypeSetElemNestedAttrs(%s, %s)", attrName, m)
}

func (o *testChecks) extractFromState(t testing.TB, id string, targetMap map[string]string) {
o.checks = append(o.checks, extractValueFromTerraformState(t, o.path, id, targetMap))
o.logLines.appendf("extractValueFromTerraformState(%s, %q)", o.path, id)
Expand Down
4 changes: 2 additions & 2 deletions apstra/test_utils/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func GetTestClient(t testing.TB, ctx context.Context) *apstra.Client {
defer testClientMutex.Unlock()

if sharedClient == nil {
err := testCfgFileToEnv()
err := TestCfgFileToEnv()
if err != nil {
t.Fatal(err)
}
Expand All @@ -59,7 +59,7 @@ func GetTestClient(t testing.TB, ctx context.Context) *apstra.Client {
return sharedClient
}

func testCfgFileToEnv() error {
func TestCfgFileToEnv() error {
absPath, err := filepath.Abs(testConfigFile)
if err != nil {
return fmt.Errorf("error expanding config file path %s - %w", testConfigFile, err)
Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ toolchain go1.21.1

require (
github.com/IBM/netaddr v1.5.0
github.com/Juniper/apstra-go-sdk v0.0.0-20240422203629-497c16d7c591
github.com/Juniper/apstra-go-sdk v0.0.0-20240430192907-3b8e9f7648cf
github.com/apparentlymart/go-cidr v1.1.0
github.com/chrismarget-j/go-licenses v0.0.0-20240224210557-f22f3e06d3d4
github.com/google/go-cmp v0.6.0
github.com/goreleaser/goreleaser v1.23.0
Expand Down
6 changes: 4 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ github.com/DataDog/zstd v1.4.5 h1:EndNeuB0l9syBZhut0wns3gV1hL8zX8LIu6ZiVHWLIQ=
github.com/DataDog/zstd v1.4.5/go.mod h1:1jcaCB/ufaK+sKp1NBhlGmpz41jOoPQ35bpF36t7BBo=
github.com/IBM/netaddr v1.5.0 h1:IJlFZe1+nFs09TeMB/HOP4+xBnX2iM/xgiDOgZgTJq0=
github.com/IBM/netaddr v1.5.0/go.mod h1:DDBPeYgbFzoXHjSz9Jwk7K8wmWV4+a/Kv0LqRnb8we4=
github.com/Juniper/apstra-go-sdk v0.0.0-20240422203629-497c16d7c591 h1:lsYejC4zGdzv+c4dWihuVCNKVtHrFU3DshAmjGWAuPk=
github.com/Juniper/apstra-go-sdk v0.0.0-20240422203629-497c16d7c591/go.mod h1:Xwj3X8v/jRZWv28o6vQAqD4lz2JmzaSYLZ2ch1SS89w=
github.com/Juniper/apstra-go-sdk v0.0.0-20240430192907-3b8e9f7648cf h1:4PVRSN7ChmQDy1oTjpGcVXqL3hohO+1N9NBCuMxDN5M=
github.com/Juniper/apstra-go-sdk v0.0.0-20240430192907-3b8e9f7648cf/go.mod h1:Xwj3X8v/jRZWv28o6vQAqD4lz2JmzaSYLZ2ch1SS89w=
github.com/Kunde21/markdownfmt/v3 v3.1.0 h1:KiZu9LKs+wFFBQKhrZJrFZwtLnCCWJahL+S+E/3VnM0=
github.com/Kunde21/markdownfmt/v3 v3.1.0/go.mod h1:tPXN1RTyOzJwhfHoon9wUr4HGYmWgVxSQN6VBJDkrVc=
github.com/Masterminds/goutils v1.1.1 h1:5nUrii3FMTL5diU80unEVvNevw1nH4+ZV4DSLVJLSYI=
Expand Down Expand Up @@ -138,6 +138,8 @@ github.com/alessio/shellescape v1.4.1 h1:V7yhSDDn8LP4lc4jS8pFkt0zCnzVJlG5JXy9BVK
github.com/alessio/shellescape v1.4.1/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPpyFjDCtHLS30=
github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be h1:9AeTilPcZAjCFIImctFaOjnTIavg87rW78vTPkQqLI8=
github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be/go.mod h1:ySMOLuWl6zY27l47sB3qLNK6tF2fkHG55UZxx8oIVo4=
github.com/apparentlymart/go-cidr v1.1.0 h1:2mAhrMoF+nhXqxTzSZMUzDHkLjmIHC+Zzn4tdgBZjnU=
github.com/apparentlymart/go-cidr v1.1.0/go.mod h1:EBcsNrHc3zQeuaeCeCtQruQm+n9/YjEn/vI25Lg7Gwc=
github.com/apparentlymart/go-textseg/v12 v12.0.0/go.mod h1:S/4uRK2UtaQttw1GenVJEynmyUenKwP++x/+DdGV/Ec=
github.com/apparentlymart/go-textseg/v15 v15.0.0 h1:uYvfpb3DyLSCGWnctWKGj857c6ew1u1fNQOlOtuGxQY=
github.com/apparentlymart/go-textseg/v15 v15.0.0/go.mod h1:K8XmNZdhEBkdlyDdvbmmsvpAG721bKi0joRfFdHIWJ4=
Expand Down

0 comments on commit bac49b0

Please sign in to comment.