From 1e63fe9257324731e91e302969d04eade2f41408 Mon Sep 17 00:00:00 2001 From: Scott Murray Date: Sun, 15 Dec 2024 02:08:06 +1300 Subject: [PATCH] prevent equivalent disk_size values from causing cluster replacement (#177) --- minikube/generator/schema_builder.go | 11 ++++- minikube/generator/schema_builder_test.go | 4 +- minikube/resource_cluster_test.go | 45 ++++++++++++++----- minikube/schema_cluster.go | 8 ++-- minikube/state_utils/memory.go | 45 ------------------- minikube/state_utils/resource_size.go | 43 ++++++++++++++++++ .../{memory_test.go => resource_size_test.go} | 29 ++++++------ 7 files changed, 108 insertions(+), 77 deletions(-) delete mode 100644 minikube/state_utils/memory.go create mode 100644 minikube/state_utils/resource_size.go rename minikube/state_utils/{memory_test.go => resource_size_test.go} (50%) diff --git a/minikube/generator/schema_builder.go b/minikube/generator/schema_builder.go index c8d21b1..366d799 100644 --- a/minikube/generator/schema_builder.go +++ b/minikube/generator/schema_builder.go @@ -60,8 +60,15 @@ var schemaOverrides map[string]SchemaOverride = map[string]SchemaOverride{ Default: "4g", Description: "Amount of RAM to allocate to Kubernetes (format: [(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: [(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", diff --git a/minikube/generator/schema_builder_test.go b/minikube/generator/schema_builder_test.go index 5d1a377..dda5ebe 100644 --- a/minikube/generator/schema_builder_test.go +++ b/minikube/generator/schema_builder_test.go @@ -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(), }, } diff --git a/minikube/resource_cluster_test.go b/minikube/resource_cluster_test.go index 4557b6e..505c356 100644 --- a/minikube/resource_cluster_test.go +++ b/minikube/resource_cluster_test.go @@ -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"), @@ -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"), ), @@ -72,7 +73,7 @@ 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"), @@ -80,10 +81,23 @@ func TestClusterHA(t *testing.T) { }, }) } + +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"), @@ -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(). @@ -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(). @@ -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) @@ -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), @@ -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" { diff --git a/minikube/schema_cluster.go b/minikube/schema_cluster.go index e4156fa..602714e 100644 --- a/minikube/schema_cluster.go +++ b/minikube/schema_cluster.go @@ -252,12 +252,14 @@ var ( "disk_size": { Type: schema.TypeString, - Description: "Disk size allocated to the minikube VM (format: [], where unit = b, k, m or g).", + Description: "Disk size allocated to the minikube VM (format: [(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": { @@ -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": { diff --git a/minikube/state_utils/memory.go b/minikube/state_utils/memory.go deleted file mode 100644 index 80495d7..0000000 --- a/minikube/state_utils/memory.go +++ /dev/null @@ -1,45 +0,0 @@ -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 MemoryConverter() schema.SchemaStateFunc { - return func(val interface{}) string { - memory, ok := val.(string) - if !ok { - panic(errors.New("memory flag is not a string")) - } - memoryMb, err := pkgutil.CalculateSizeInMB(memory) - if err != nil { - panic(errors.New("invalid memory value")) - } - - return strconv.Itoa(memoryMb) + "mb" - - } - -} - -func MemoryValidator() schema.SchemaValidateDiagFunc { - return schema.SchemaValidateDiagFunc(func(val interface{}, path cty.Path) diag.Diagnostics { - memory, ok := val.(string) - if !ok { - diag := diag.FromErr(errors.New("memory flag is not a string")) - return diag - } - _, err := pkgutil.CalculateSizeInMB(memory) - if err != nil { - diag := diag.FromErr(errors.New("invalid memory value")) - return diag - } - return nil - }) -} diff --git a/minikube/state_utils/resource_size.go b/minikube/state_utils/resource_size.go new file mode 100644 index 0000000..e211c06 --- /dev/null +++ b/minikube/state_utils/resource_size.go @@ -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 + }) +} diff --git a/minikube/state_utils/memory_test.go b/minikube/state_utils/resource_size_test.go similarity index 50% rename from minikube/state_utils/memory_test.go rename to minikube/state_utils/resource_size_test.go index 482fda1..15fc316 100644 --- a/minikube/state_utils/memory_test.go +++ b/minikube/state_utils/resource_size_test.go @@ -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) }) @@ -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()) }) - }