Backport storage class refactor fixes (kubernetes-sigs#205)

* Move storage class installation below * Fix gcp storage class parameters * Support encrypted in aws storage class parameters * Fix default storage class * Add fstype storage parameter to gcp * Added fsType validation to gcp * Added fsType validation to gcp --------- Co-authored-by: lreciomelero <[email protected]>
stg-0 · Jul 13, 2023 · cf7a0f4 · cf7a0f4
1 parent 5641992
commit cf7a0f4
Show file tree

Hide file tree

Showing 8 changed files with 64 additions and 34 deletions.
diff --git a/pkg/cluster/internal/create/actions/createworker/aws.go b/pkg/cluster/internal/create/actions/createworker/aws.go
@@ -36,8 +36,6 @@ import (
 	"sigs.k8s.io/kind/pkg/exec"
 )
 
-var defaultAWSSc = "gp2"
-
 var storageClassAWSTemplate = StorageClassDef{
 	APIVersion: "storage.k8s.io/v1",
 	Kind:       "StorageClass",
@@ -338,11 +336,22 @@ func getEcrToken(p commons.ProviderParams) (string, error) {
 }
 
 func (b *AWSBuilder) configureStorageClass(n nodes.Node, k string, sc commons.StorageClass) error {
+	var c string
+	var err error
 	var cmd exec.Cmd
 
-	cmd = n.Command("kubectl", "--kubeconfig", k, "delete", "storageclass", defaultAWSSc)
-	if err := cmd.Run(); err != nil {
-		return errors.Wrap(err, "failed to delete default StorageClass")
+	// Remove annotation from default storage class
+	c = "kubectl --kubeconfig " + k + " get sc | grep '(default)' | awk '{print $1}'"
+	output, err := commons.ExecuteCommand(n, c)
+	if err != nil {
+		return errors.Wrap(err, "failed to get default storage class")
+	}
+	if strings.TrimSpace(output) != "" && strings.TrimSpace(output) != "No resources found" {
+		c = "kubectl --kubeconfig " + k + " annotate sc " + strings.TrimSpace(output) + " " + defaultScAnnotation + "-"
+		_, err = commons.ExecuteCommand(n, c)
+		if err != nil {
+			return errors.Wrap(err, "failed to remove annotation from default storage class")
+		}
 	}
 
 	params := b.getParameters(sc)
@@ -356,10 +365,10 @@ func (b *AWSBuilder) configureStorageClass(n nodes.Node, k string, sc commons.St
 
 	cmd = n.Command("kubectl", "--kubeconfig", k, "apply", "-f", "-")
 	if err = cmd.SetStdin(strings.NewReader(storageClass)).Run(); err != nil {
-		return errors.Wrap(err, "failed to create StorageClass")
+		return errors.Wrap(err, "failed to create default storage class")
 	}
-	return nil
 
+	return nil
 }
 
 func (b *AWSBuilder) getParameters(sc commons.StorageClass) commons.SCParameters {
@@ -397,6 +406,9 @@ func (b *AWSBuilder) getPvcSizeOverrideVars(sc commons.StorageClass) (string, []
 	if (sc.Class == "premium" && sc.Parameters.Type == "") || sc.Parameters.Type == "io2" || sc.Parameters.Type == "io1" {
 		return "storage-class.yaml", []byte("storage_class_pvc_size: 4Gi"), nil
 	}
+	if sc.Parameters.Type == "st1" || sc.Parameters.Type == "sc1" {
+		return "storage-class.yaml", []byte("storage_class_pvc_size: 125Gi"), nil
+	}
 	return "", []byte(""), nil
 }
 

diff --git a/pkg/cluster/internal/create/actions/createworker/azure.go b/pkg/cluster/internal/create/actions/createworker/azure.go
@@ -245,11 +245,22 @@ func getAcrToken(p commons.ProviderParams, acrService string) (string, error) {
 }
 
 func (b *AzureBuilder) configureStorageClass(n nodes.Node, k string, sc commons.StorageClass) error {
+	var c string
+	var err error
 	var cmd exec.Cmd
 
-	cmd = n.Command("kubectl", "--kubeconfig", k, "annotate", "sc", "default", defaultScAnnotation+"-")
-	if err := cmd.SetStdin(strings.NewReader(azureStorageClasses)).Run(); err != nil {
-		return errors.Wrap(err, "failed to unannotate default Azure Storage Classes")
+	// Remove annotation from default storage class
+	c = "kubectl --kubeconfig " + k + " get sc | grep '(default)' | awk '{print $1}'"
+	output, err := commons.ExecuteCommand(n, c)
+	if err != nil {
+		return errors.Wrap(err, "failed to get default storage class")
+	}
+	if strings.TrimSpace(output) != "" && strings.TrimSpace(output) != "No resources found" {
+		c = "kubectl --kubeconfig " + k + " annotate sc " + strings.TrimSpace(output) + " " + defaultScAnnotation + "-"
+		_, err = commons.ExecuteCommand(n, c)
+		if err != nil {
+			return errors.Wrap(err, "failed to remove annotation from default storage class")
+		}
 	}
 
 	params := b.getParameters(sc)
@@ -258,12 +269,14 @@ func (b *AzureBuilder) configureStorageClass(n nodes.Node, k string, sc commons.
 		return err
 	}
 
+	storageClass = strings.ReplaceAll(storageClass, "fsType", "csi.storage.k8s.io/fstype")
+
 	cmd = n.Command("kubectl", "--kubeconfig", k, "apply", "-f", "-")
 	if err = cmd.SetStdin(strings.NewReader(storageClass)).Run(); err != nil {
-		return errors.Wrap(err, "failed to create StorageClass")
+		return errors.Wrap(err, "failed to create default storage class")
 	}
-	return nil
 
+	return nil
 }
 
 func (b *AzureBuilder) getParameters(sc commons.StorageClass) commons.SCParameters {

diff --git a/pkg/cluster/internal/create/actions/createworker/createworker.go b/pkg/cluster/internal/create/actions/createworker/createworker.go
@@ -335,15 +335,6 @@ func (a *action) Execute(ctx *actions.ActionContext) error {
 			ctx.Status.End(true)
 		}
 
-		ctx.Status.Start("Installing StorageClass in workload cluster 💾")
-		defer ctx.Status.End(false)
-
-		err = infra.configureStorageClass(n, kubeconfigPath, descriptorFile.StorageClass)
-		if err != nil {
-			return errors.Wrap(err, "failed to configuring StorageClass in workload cluster")
-		}
-		ctx.Status.End(true) // End Installing StorageClass in workload cluster
-
 		if provider.capxProvider == "gcp" {
 			// XXX Ref kubernetes/kubernetes#86793 Starting from v1.18, gcp cloud-controller-manager requires RBAC to patch,update service/status (in-tree)
 			ctx.Status.Start("Creating Kubernetes RBAC for internal loadbalancing 🔐")
@@ -413,6 +404,15 @@ func (a *action) Execute(ctx *actions.ActionContext) error {
 
 		ctx.Status.End(true) // End Preparing nodes in workload cluster
 
+		ctx.Status.Start("Installing StorageClass in workload cluster 💾")
+		defer ctx.Status.End(false)
+
+		err = infra.configureStorageClass(n, kubeconfigPath, descriptorFile.StorageClass)
+		if err != nil {
+			return errors.Wrap(err, "failed to configuring StorageClass in workload cluster")
+		}
+		ctx.Status.End(true) // End Installing StorageClass in workload cluster
+
 		ctx.Status.Start("Enabling workload cluster's self-healing 🏥")
 		defer ctx.Status.End(false)
 
@@ -511,9 +511,9 @@ func (a *action) Execute(ctx *actions.ActionContext) error {
 					return errors.Wrap(err, "failed to apply allow CAPA as egress GlobalNetworkPolicy")
 				}
 			}
-		}
 
-		ctx.Status.End(true) // End Installing Network Policy Engine in workload cluster
+			ctx.Status.End(true) // End Installing Network Policy Engine in workload cluster
+		}
 
 		if descriptorFile.DeployAutoscaler && !(descriptorFile.InfraProvider == "azure" && descriptorFile.ControlPlane.Managed) {
 			ctx.Status.Start("Adding Cluster-Autoescaler 🗚")

diff --git a/pkg/cluster/internal/create/actions/createworker/files/azure/azure-storage-classes.yaml b/pkg/cluster/internal/create/actions/createworker/files/azure/azure-storage-classes.yaml
@@ -62,8 +62,6 @@ volumeBindingMode: Immediate
 apiVersion: storage.k8s.io/v1
 kind: StorageClass
 metadata:
-  annotations:
-    storageclass.kubernetes.io/is-default-class: "true"
   name: default
 allowVolumeExpansion: true
 provisioner: disk.csi.azure.com

diff --git a/pkg/cluster/internal/create/actions/createworker/gcp.go b/pkg/cluster/internal/create/actions/createworker/gcp.go
@@ -207,12 +207,14 @@ func (b *GCPBuilder) configureStorageClass(n nodes.Node, k string, sc commons.St
 		return err
 	}
 
+	storageClass = strings.ReplaceAll(storageClass, "fsType", "csi.storage.k8s.io/fstype")
+
 	cmd = n.Command("kubectl", "--kubeconfig", k, "apply", "-f", "-")
 	if err = cmd.SetStdin(strings.NewReader(storageClass)).Run(); err != nil {
-		return errors.Wrap(err, "failed to create StorageClass")
+		return errors.Wrap(err, "failed to create storage class")
 	}
-	return nil
 
+	return nil
 }
 
 func (b *GCPBuilder) getParameters(sc commons.StorageClass) commons.SCParameters {

diff --git a/pkg/cmd/kind/create/cluster/validation/gcpvalidator.go b/pkg/cmd/kind/create/cluster/validation/gcpvalidator.go
@@ -2,6 +2,7 @@ package validation
 
 import (
 	"errors"
+	"fmt"
 	"regexp"
 	"strconv"
 	"strings"
@@ -109,6 +110,7 @@ func (v *GCPValidator) storageClassParametersValidation(descriptorFile commons.D
 	sc := descriptorFile.StorageClass
 	k8s_version := descriptorFile.K8SVersion
 	minor, _ := strconv.Atoi(strings.Split(k8s_version, ".")[1])
+	fstypes := []string{"ext4", "ext3", "ext2", "xfs", "ntfs"}
 	err := verifyFields(descriptorFile)
 	if err != nil {
 		return err
@@ -126,6 +128,9 @@ func (v *GCPValidator) storageClassParametersValidation(descriptorFile commons.D
 	if sc.Parameters.Type != "pd-extreme" && sc.Parameters.ProvisionedIopsOnCreate != "" {
 		return errors.New("Parameter provisioned_iops_on_create only can be supported for type pd-extreme")
 	}
+	if sc.Parameters.FsType != "" && !slices.Contains(fstypes, sc.Parameters.FsType) {
+		return errors.New("Unsupported fsType: " + sc.Parameters.FsType + ". Supported types: " + fmt.Sprint(strings.Join(fstypes, ", ")))
+	}
 
 	if sc.Parameters.ProvisionedIopsOnCreate != "" {
 		_, err = strconv.Atoi(sc.Parameters.ProvisionedIopsOnCreate)

diff --git a/pkg/cmd/kind/create/cluster/validation/validator.go b/pkg/cmd/kind/create/cluster/validation/validator.go
@@ -99,14 +99,14 @@ func verifyFields(descriptor commons.DescriptorFile) error {
 	supportedFields := []string{}
 	switch descriptor.InfraProvider {
 	case "gcp":
-		supportedFields = []string{"type", "provisioned_iops_on_create", "replication_type", "labels"}
-		err := verifyAdditionalFields(params, []string{"Type", "ProvisionedIopsOnCreate", "ReplicationType", "Labels"})
+		supportedFields = []string{"type", "fsType", "provisioned-iops-on-create", "replication-type", "labels"}
+		err := verifyAdditionalFields(params, []string{"Type", "FsType", "ProvisionedIopsOnCreate", "ReplicationType", "Labels"})
 		if err != nil {
 			return errors.New(err.Error() + "Supported fields for " + descriptor.InfraProvider + ": " + strings.Join(supportedFields, ", "))
 		}
 	case "aws":
-		supportedFields = []string{"type", "iopsPerGB", "fsType", "allowAutoIOPSPerGBIncrease", "iops", "throughput", "blockExpress", "blockSize", "labels"}
-		err := verifyAdditionalFields(params, []string{"Type", "IopsPerGB", "FsType", "AllowAutoIOPSPerGBIncrease", "Iops", "Throughput", "BlockExpress", "BlockSize", "Labels"})
+		supportedFields = []string{"type", "iopsPerGB", "fsType", "allowAutoIOPSPerGBIncrease", "iops", "encrypted", "throughput", "blockExpress", "blockSize", "labels"}
+		err := verifyAdditionalFields(params, []string{"Type", "IopsPerGB", "FsType", "AllowAutoIOPSPerGBIncrease", "Iops", "Encrypted", "Throughput", "BlockExpress", "BlockSize", "Labels"})
 		if err != nil {
 			return errors.New(err.Error() + "Supported fields for " + descriptor.InfraProvider + ": " + strings.Join(supportedFields, ", "))
 		}

diff --git a/pkg/commons/cluster.go b/pkg/commons/cluster.go
@@ -269,9 +269,9 @@ type StorageClass struct {
 type SCParameters struct {
 	Type string `yaml:"type,omitempty" validate:"omitempty"`
 
-	ProvisionedIopsOnCreate string `yaml:"provisioned_iops_on_create,omitempty"  validate:"omitempty"`
-	ReplicationType         string `yaml:"replication_type,omitempty" validate:"omitempty,oneof='none' 'regional-pd'"`
-	DiskEncryptionKmsKey    string `yaml:"disk_encryption_kms_key,omitempty"  validate:"omitempty"`
+	ProvisionedIopsOnCreate string `yaml:"provisioned-iops-on-create,omitempty"  validate:"omitempty"`
+	ReplicationType         string `yaml:"replication-type,omitempty" validate:"omitempty,oneof='none' 'regional-pd'"`
+	DiskEncryptionKmsKey    string `yaml:"disk-encryption-kms-key,omitempty"  validate:"omitempty"`
 	Labels                  string `yaml:"labels,omitempty"  validate:"omitempty"`
 
 	IopsPerGB                  string `yaml:"iopsPerGB,omitempty" validate:"omitempty,excluded_with=Iops"`