From cf7a0f4a9b312d6fb526dd32efa5fcf67ccac6bb Mon Sep 17 00:00:00 2001 From: Francisco Augusto Date: Thu, 13 Jul 2023 10:34:39 +0200 Subject: [PATCH] Backport storage class refactor fixes (#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 --- .../create/actions/createworker/aws.go | 26 ++++++++++++++----- .../create/actions/createworker/azure.go | 23 ++++++++++++---- .../actions/createworker/createworker.go | 22 ++++++++-------- .../files/azure/azure-storage-classes.yaml | 2 -- .../create/actions/createworker/gcp.go | 6 +++-- .../create/cluster/validation/gcpvalidator.go | 5 ++++ .../create/cluster/validation/validator.go | 8 +++--- pkg/commons/cluster.go | 6 ++--- 8 files changed, 64 insertions(+), 34 deletions(-) diff --git a/pkg/cluster/internal/create/actions/createworker/aws.go b/pkg/cluster/internal/create/actions/createworker/aws.go index ddffabed26..7466f6cfcf 100644 --- 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 index b01ed7aff6..2a0f00f11c 100644 --- 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 index 04c9e3d9ba..f65829066e 100644 --- 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 index 52cbb7d0e9..ebf2290ea7 100644 --- 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 index 9d38876d18..e457902bbe 100644 --- 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 index 9d64df966f..f7ad126527 100644 --- 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 index 0c9689d1d8..c6e1cb7a27 100644 --- 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 index a46958778f..717c3d21cd 100644 --- 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"`