Skip to content

Commit

Permalink
prevent equivalent disk_size values from causing cluster replacement (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
scott-the-programmer authored Dec 14, 2024
1 parent ac9ee40 commit 1e63fe9
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 77 deletions.
11 changes: 9 additions & 2 deletions minikube/generator/schema_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,15 @@ var schemaOverrides map[string]SchemaOverride = map[string]SchemaOverride{
Default: "4g",
Description: "Amount of RAM to allocate to Kubernetes (format: <number>[<unit>(case-insensitive)], where unit = b, k, kb, m, mb, g or gb)",
Type: String,
StateFunc: "state_utils.MemoryConverter()",
ValidateDiagFunc: "state_utils.MemoryValidator()",
StateFunc: "state_utils.ResourceSizeConverter()",
ValidateDiagFunc: "state_utils.ResourceSizeValidator()",
},
"disk_size": {
Default: "20000mb",
Description: "Disk size allocated to the minikube VM (format: <number>[<unit>(case-insensitive)], where unit = b, k, kb, m, mb, g or gb)",
Type: String,
StateFunc: "state_utils.ResourceSizeConverter()",
ValidateDiagFunc: "state_utils.ResourceSizeValidator()",
},
"cpus": {
Default: "2",
Expand Down
4 changes: 2 additions & 2 deletions minikube/generator/schema_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,8 @@ func TestOverride(t *testing.T) {
ForceNew: true,
Default: "4g",
StateFunc: state_utils.MemoryConverter(),
ValidateDiagFunc: state_utils.MemoryValidator(),
StateFunc: state_utils.ResourceSizeConverter(),
ValidateDiagFunc: state_utils.ResourceSizeValidator(),
},
}
Expand Down
45 changes: 35 additions & 10 deletions minikube/resource_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,13 @@ type mockClusterClientProperties struct {
name string
haNodes int
workerNodes int
diskSize int
}

func TestClusterCreation(t *testing.T) {
resource.Test(t, resource.TestCase{
IsUnitTest: true,
Providers: map[string]*schema.Provider{"minikube": NewProvider(mockSuccess(mockClusterClientProperties{t, "TestClusterCreation", 1, 0}))},
Providers: map[string]*schema.Provider{"minikube": NewProvider(mockSuccess(mockClusterClientProperties{t, "TestClusterCreation", 1, 0, 20000}))},
Steps: []resource.TestStep{
{
Config: testUnitClusterConfig("some_driver", "TestClusterCreation"),
Expand All @@ -54,13 +55,13 @@ func TestClusterCreation(t *testing.T) {
func TestClusterUpdate(t *testing.T) {
resource.Test(t, resource.TestCase{
IsUnitTest: true,
Providers: map[string]*schema.Provider{"minikube": NewProvider(mockUpdate(mockClusterClientProperties{t, "TestClusterCreation", 1, 0}))},
Providers: map[string]*schema.Provider{"minikube": NewProvider(mockUpdate(mockClusterClientProperties{t, "TestClusterUpdate", 1, 0, 20000}))},
Steps: []resource.TestStep{
{
Config: testUnitClusterConfig("some_driver", "TestClusterCreation"),
Config: testUnitClusterConfig("some_driver", "TestClusterUpdate"),
},
{
Config: testUnitClusterConfig_Update("some_driver", "TestClusterCreation"),
Config: testUnitClusterConfig_Update("some_driver", "TestClusterUpdate"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("minikube_cluster.new", "addons.2", "ingress"),
),
Expand All @@ -72,18 +73,31 @@ func TestClusterUpdate(t *testing.T) {
func TestClusterHA(t *testing.T) {
resource.Test(t, resource.TestCase{
IsUnitTest: true,
Providers: map[string]*schema.Provider{"minikube": NewProvider(mockSuccess(mockClusterClientProperties{t, "TestClusterCreationHA", 3, 5}))},
Providers: map[string]*schema.Provider{"minikube": NewProvider(mockSuccess(mockClusterClientProperties{t, "TestClusterCreationHA", 3, 5, 20000}))},
Steps: []resource.TestStep{
{
Config: testUnitClusterHAConfig("some_driver", "TestClusterCreationHA"),
},
},
})
}

func TestClusterDisk(t *testing.T) {
resource.Test(t, resource.TestCase{
IsUnitTest: true,
Providers: map[string]*schema.Provider{"minikube": NewProvider(mockSuccess(mockClusterClientProperties{t, "TestClusterCreationDisk", 1, 0, 20480}))},
Steps: []resource.TestStep{
{
Config: testUnitClusterDiskConfig("some_driver", "TestClusterCreationDisk"),
},
},
})
}

func TestClusterWait(t *testing.T) {
resource.Test(t, resource.TestCase{
IsUnitTest: true,
Providers: map[string]*schema.Provider{"minikube": NewProvider(mockSuccess(mockClusterClientProperties{t, "TestClusterCreationWait", 1, 0}))},
Providers: map[string]*schema.Provider{"minikube": NewProvider(mockSuccess(mockClusterClientProperties{t, "TestClusterCreationWait", 1, 0, 20000}))},
Steps: []resource.TestStep{
{
Config: testUnitClusterWaitConfig("some_driver", "TestClusterCreationWait"),
Expand Down Expand Up @@ -315,7 +329,7 @@ func TestClusterCreation_HyperV(t *testing.T) {
func mockUpdate(props mockClusterClientProperties) schema.ConfigureContextFunc {
ctrl := gomock.NewController(props.t)

mockClusterClient := getBaseMockClient(ctrl, props.name, props.haNodes, props.workerNodes)
mockClusterClient := getBaseMockClient(ctrl, props.name, props.haNodes, props.workerNodes, props.diskSize)

gomock.InOrder(
mockClusterClient.EXPECT().
Expand Down Expand Up @@ -352,7 +366,7 @@ func mockUpdate(props mockClusterClientProperties) schema.ConfigureContextFunc {
func mockSuccess(props mockClusterClientProperties) schema.ConfigureContextFunc {
ctrl := gomock.NewController(props.t)

mockClusterClient := getBaseMockClient(ctrl, props.name, props.haNodes, props.workerNodes)
mockClusterClient := getBaseMockClient(ctrl, props.name, props.haNodes, props.workerNodes, props.diskSize)

mockClusterClient.EXPECT().
GetAddons().
Expand All @@ -370,7 +384,7 @@ func mockSuccess(props mockClusterClientProperties) schema.ConfigureContextFunc
return configureContext
}

func getBaseMockClient(ctrl *gomock.Controller, clusterName string, haNodes int, workerNodes int) *lib.MockClusterClient {
func getBaseMockClient(ctrl *gomock.Controller, clusterName string, haNodes int, workerNodes int, diskSize int) *lib.MockClusterClient {
mockClusterClient := lib.NewMockClusterClient(ctrl)

os.Mkdir("test_output", 0755)
Expand Down Expand Up @@ -420,7 +434,7 @@ func getBaseMockClient(ctrl *gomock.Controller, clusterName string, haNodes int,
Network: clusterSchema["network"].Default.(string),
Memory: 4096,
CPUs: 2,
DiskSize: 20000,
DiskSize: diskSize,
Driver: "some_driver",
ListenAddress: clusterSchema["listen_address"].Default.(string),
HyperkitVpnKitSock: clusterSchema["hyperkit_vpnkit_sock"].Default.(string),
Expand Down Expand Up @@ -535,6 +549,17 @@ func testUnitClusterConfig(driver string, clusterName string) string {
`, driver, clusterName)
}

func testUnitClusterDiskConfig(driver string, clusterName string) string {
return fmt.Sprintf(`
resource "minikube_cluster" "new" {
driver = "%s"
cluster_name = "%s"
disk_size = "20g"
}
`, driver, clusterName)
}

func testUnitClusterHAConfig(driver string, clusterName string) string {
return fmt.Sprintf(`
resource "minikube_cluster" "new" {
Expand Down
8 changes: 5 additions & 3 deletions minikube/schema_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,12 +252,14 @@ var (

"disk_size": {
Type: schema.TypeString,
Description: "Disk size allocated to the minikube VM (format: <number>[<unit>], where unit = b, k, m or g).",
Description: "Disk size allocated to the minikube VM (format: <number>[<unit>(case-insensitive)], where unit = b, k, kb, m, mb, g or gb)",

Optional: true,
ForceNew: true,

Default: "20000mb",
StateFunc: state_utils.ResourceSizeConverter(),
ValidateDiagFunc: state_utils.ResourceSizeValidator(),
},

"dns_domain": {
Expand Down Expand Up @@ -672,8 +674,8 @@ var (
ForceNew: true,

Default: "4g",
StateFunc: state_utils.MemoryConverter(),
ValidateDiagFunc: state_utils.MemoryValidator(),
StateFunc: state_utils.ResourceSizeConverter(),
ValidateDiagFunc: state_utils.ResourceSizeValidator(),
},

"mount": {
Expand Down
45 changes: 0 additions & 45 deletions minikube/state_utils/memory.go

This file was deleted.

43 changes: 43 additions & 0 deletions minikube/state_utils/resource_size.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package state_utils

import (
"errors"
"strconv"

"github.com/hashicorp/go-cty/cty"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"

pkgutil "k8s.io/minikube/pkg/util"
)

func ResourceSizeConverter() schema.SchemaStateFunc {
return func(val interface{}) string {
size, ok := val.(string)
if !ok {
panic(errors.New("resource size is not a string"))
}
sizeMb, err := pkgutil.CalculateSizeInMB(size)
if err != nil {
panic(errors.New("invalid resource size value"))
}

return strconv.Itoa(sizeMb) + "mb"
}
}

func ResourceSizeValidator() schema.SchemaValidateDiagFunc {
return schema.SchemaValidateDiagFunc(func(val interface{}, path cty.Path) diag.Diagnostics {
size, ok := val.(string)
if !ok {
diag := diag.FromErr(errors.New("resource size is not a string"))
return diag
}
_, err := pkgutil.CalculateSizeInMB(size)
if err != nil {
diag := diag.FromErr(errors.New("invalid resource size value"))
return diag
}
return nil
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ import (
"github.com/stretchr/testify/require"
)

func TestMemoryConverter(t *testing.T) {
converter := MemoryConverter()
func TestResourceSizeConverter(t *testing.T) {
converter := ResourceSizeConverter()

t.Run("ValidMemoryFlagG", func(t *testing.T) {
t.Run("ValidSizeFlagG", func(t *testing.T) {
result := converter("2G")
assert.Equal(t, "2048mb", result)
})

t.Run("ValidMemoryFlagGb", func(t *testing.T) {
t.Run("ValidSizeFlagGb", func(t *testing.T) {
result := converter("2Gb")
assert.Equal(t, "2048mb", result)
})
Expand All @@ -26,37 +26,36 @@ func TestMemoryConverter(t *testing.T) {
assert.Equal(t, resultm, resultg)
})

t.Run("InvalidMemoryFlag", func(t *testing.T) {
assert.PanicsWithError(t, "memory flag is not a string", func() {
t.Run("InvalidSizeFlag", func(t *testing.T) {
assert.PanicsWithError(t, "resource size is not a string", func() {
converter(123)
})
})

t.Run("InvalidMemoryValue", func(t *testing.T) {
assert.PanicsWithError(t, "invalid memory value", func() {
t.Run("InvalidSizeValue", func(t *testing.T) {
assert.PanicsWithError(t, "invalid resource size value", func() {
converter("invalid")
})
})
}

func TestMemoryValidator(t *testing.T) {
validator := MemoryValidator()
func TestResourceSizeValidator(t *testing.T) {
validator := ResourceSizeValidator()

t.Run("ValidMemoryFlag", func(t *testing.T) {
t.Run("ValidSizeFlag", func(t *testing.T) {
diags := validator("2G", nil)
assert.Empty(t, diags)
})

t.Run("InvalidMemoryFlag", func(t *testing.T) {
t.Run("InvalidSizeFlag", func(t *testing.T) {
diags := validator(123, nil)
require.Len(t, diags, 1)
assert.Equal(t, diags[0].Summary, "memory flag is not a string")
assert.Equal(t, diags[0].Summary, "resource size is not a string")
})

t.Run("InvalidMemoryValue", func(t *testing.T) {
t.Run("InvalidSizeValue", func(t *testing.T) {
diags := validator("invalid", nil)
require.Len(t, diags, 1)
assert.True(t, diags.HasError())
})

}

0 comments on commit 1e63fe9

Please sign in to comment.