Multiple default storageClasses when specifying storageClass in EKS cluster

[Brianbrifri]

When creating an EKS cluster and specifying a custom storage class (e.g. st1), the default gp2 storageClass is created along side the custom st1 storageClass which is also marked as default.

Where storageClass = "st1":

const vpc = new awsx.ec2.Vpc("Kafka-VPC", {numberOfAvailabilityZones: 3});

// Create an EKS cluster with the given configuration.
const cluster = new eks.Cluster("kafka-cluster", {
    vpcId: vpc.id,
    subnetIds: vpc.privateSubnetIds,
    instanceType: instanceType,
    desiredCapacity: desiredCapacity,
    minSize: minSize,
    maxSize: maxSize,
    storageClasses: storageClass,
    deployDashboard: deployDashboard,
});

This results in:

NAME                                   PROVISIONER             AGE
gp2 (default)                          kubernetes.io/aws-ebs   23m
kafka-cluster-st1-evr2qvmh (default)   kubernetes.io/aws-ebs   21m

This can cause issues when helm charts rely on one default storage class since the storageClass name cannot be passed directly to the charts.

Expected behavior: One storageClass is marked as default after EKS creation (preferably the user specified class)
Actual behavior: Both the EKS created gp2 AND the user specified st1 storage class are marked as default.

Other thoughts:
Ability to add/remove default status to storageClasses

[metral]

By default, the gp2 storage class is created for us automatically by EKS for clusters with version >= 1.11. This is not created by Pulumi.

To get the desired result, you'll have to either patch the gp2 storage class to not be default after the cluster is up using kubectl, but preferably your helm values.yaml should take a storage class param to explicitly use the intended class as to get around EKS' defaults - for example, here is how the AWS EFS helm chart specifies the desired class it needs.

[Brianbrifri]

Is there a way of querying the storage class names from the pulumi/kubernetes api? That's the only way I know of to get the actual name of the user provided storage class to be able to pass it to the helm charts.

[Brianbrifri]

I know #139 has just been opened but until then, I'm hoping there is a workaround.

[Brianbrifri]

@metral As a follow up to this, I commented out the storageClasses parameter above, ran pulumi update, then pulumi deleted the st1 storageClass but then created another default gp2 storageClass named kafka-cluster-gp2-dd7h3rgp along side the default gp2 storageClass created by AWS EKS.

Is this intended behavior?

[metral]

I know #139 has just been opened but until then, I'm hoping there is a workaround.

As of now there isn't, but #139 is slated to be worked on in this current milestone (M24). The dupe gp2 class was removed in #136 but has not made its way into a new version of the EKS package. I'll cut a new release in the mean time to get around this.

[metral]

Update: v0.18.6 has been released to get around the dupe storageclass.

[lukehoban]

@metral Is this fixed?

[metral]

#136 / v0.18.6 addressed the duplicate gp2 storage class issue, but not this one.

I'm currently working on a PR downstream for #139 which ultimately should help mediate the original goal here of deploying Helm charts to a known SC - but in the tests of validating the exported SC's by checking the SC names returned during pulumi preview, I seem to be hitting pulumi/pulumi#2433 or possibly something to do with the recent changes to how metadata is handled via pulumi/pulumi-kubernetes#572.

pulumi/pulumi-azure#192 (comment) precisely explains what i'm seeing: during a preview the metadata.name on the SC returned is undefined, but after disabling the name check test, running the first update, and re-enabling the name check test, the metadata.name is populated as expected.

[metral]

Expected behavior: One storageClass is marked as default after EKS creation (preferably the user specified class)

Actual behavior: Both the EKS created gp2 AND the user specified st1 storage class are marked as default.

w.r.t this ^ unfortunately you can mark many SC's as default in k8s, as its just an annotation:

Please note that at most one StorageClass can be marked as default. If two or more of them are marked as default, a PersistentVolumeClaim without storageClassName explicitly specified cannot be created.

e.g. from an existing cluster w/ many SC's marked default

$ kubectl get sc
NAME                                                     PROVISIONER             AGE
gp2 (default)                                            kubernetes.io/aws-ebs   8m54s
test-expose-storage-classes-2-mygp2-8jgjj4j0 (default)   kubernetes.io/aws-ebs   103s
test-expose-storage-classes-2-mysc1-bmmw423s             kubernetes.io/aws-ebs   103s

Hence the recommendation for the chart to specify its storageclass, rather than defer to the default SC set. But to do so, we need to export the SC's names, and that is what i'm working on downstream, what is tracked in #139, and what I refer to in my prev comment above.

[Brianbrifri]

Ah of course. Is there a way through the Pulumi kubernetes api to set/remove those default annotations? Such as through transformations?

[metral]

Not sure about transformations - @lblackstone any thoughts on the comment above?

I've opened up #172 to track the export of all user created StorageClasses, so that you may pass the desired StorageClass name to the Helm chart for your use case, and get around the issue of multiple default StorageClasses being set in a cluster.

[Brianbrifri]

Being able to have the storage class name should be good enough in 99% of scenarios. Only in the case of using a community helm chart where the storage class name is unable to be specified would you have to rely on the default storage class. But I'm guessing that's a very unlikely scenario.

[metral]

Only in the case of using a community helm chart where the storage class name is unable to be specified would you have to rely on the default storage class.

Generally in these cases with Helm, your options are 1) either open a PR to the chart upstream to enable the configuration of the storage class, or 2) fork the chart in question, make the storage class edits you need downstream, and maintain it with the updates from upstream / stable - the easiest path being # 1.

Transformations may be possible here (@lblackstone?), but specifying the storage class seems like a valid config setting that belongs in the chart.

#172 should help you either way you choose.

[lukehoban]

@metral What work is being tracked in this issue now?

[metral]

#172 will close out this issue and #139, by supplying the exported names of all storage classes. This can then be used to pass into the Helm charts per the original use-case need.