prevent equivalent disk_size values from causing cluster replacement

scott-the-programmer · scott-the-programmer · commit 7969d8630f1b · 2024-12-15T01:57:58.000+13:00
diff --git 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: <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",
diff --git 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
@@ -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,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", 3, 5, 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
@@ -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": {
@@ -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
diff --git a/minikube/state_utils/resource_size.go 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/resource_size_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())
 	})
-
 }

Original file line number	Diff line number	Diff line change
`@@ -366,8 +366,8 @@ func TestOverride(t *testing.T) {`
`366`	`366`	`ForceNew: true,`
`367`	`367`
`368`	`368`	`Default: "4g",`
`369`		`- StateFunc: state_utils.MemoryConverter(),`
`370`		`- ValidateDiagFunc: state_utils.MemoryValidator(),`
	`369`	`+ StateFunc: state_utils.ResourceSizeConverter(),`
	`370`	`+ ValidateDiagFunc: state_utils.ResourceSizeValidator(),`
`371`	`371`	`},`
`372`	`372`
`373`	`373`	`}`