Consider PVC with a WaitForFirstConsumer SC ready

As discussed in https://github.com/helm/helm/issues/10733.

Charts which have PVC associated with resources not starting
immediately after install (like a CronJob) will wait forever if
the associated StorageClass has a volume binding mode of
`WaitForFirstConsumer`.

This PR attempts to solve this by querying the associated
StorageClass (or the default one) and check its `volumeBindingMode`
field.

Signed-off-by: Adrien Fillon <adrien.fillon@manomano.com>

pkg/kube/ready.go

@@ -18,6 +18,7 @@ package kube // import "helm.sh/helm/v3/pkg/kube"
 
 import (
 	"context"
+	"fmt"
 
 	appsv1 "k8s.io/api/apps/v1"
 	appsv1beta1 "k8s.io/api/apps/v1beta1"
@@ -25,6 +26,7 @@ import (
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
 	extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
+	storagev1 "k8s.io/api/storage/v1"
 	apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -37,6 +39,8 @@ import (
 	deploymentutil "helm.sh/helm/v3/internal/third_party/k8s.io/kubernetes/deployment/util"
 )
 
+const defaultStorageClassAnnotation = "storageclass.kubernetes.io/is-default-class"
+
 // ReadyCheckerOption is a function that configures a ReadyChecker.
 type ReadyCheckerOption func(*ReadyChecker)
 
@@ -131,7 +135,13 @@ func (c *ReadyChecker) IsReady(ctx context.Context, v *resource.Info) (bool, err
 		if err != nil {
 			return false, err
 		}
-		if !c.volumeReady(claim) {
+
+		storageClass, err := getStorageClassFromPvc(ctx, c.client, claim)
+		if err != nil {
+			return false, err
+		}
+
+		if !c.volumeReady(claim, storageClass) {
 			return false, nil
 		}
 	case *corev1.Service:
@@ -263,7 +273,13 @@ func (c *ReadyChecker) serviceReady(s *corev1.Service) bool {
 	return true
 }
 
-func (c *ReadyChecker) volumeReady(v *corev1.PersistentVolumeClaim) bool {
+func (c *ReadyChecker) volumeReady(v *corev1.PersistentVolumeClaim, storageClass *storagev1.StorageClass) bool {
+	if storageClass != nil &&
+		storageClass.VolumeBindingMode != nil &&
+		*storageClass.VolumeBindingMode == storagev1.VolumeBindingWaitForFirstConsumer {
+		return true
+	}
+
 	if v.Status.Phase != corev1.ClaimBound {
 		c.log("PersistentVolumeClaim is not bound: %s/%s", v.GetNamespace(), v.GetName())
 		return false
@@ -409,3 +425,26 @@ func getPods(ctx context.Context, client kubernetes.Interface, namespace, select
 	})
 	return list.Items, err
 }
+
+func getStorageClassFromPvc(ctx context.Context, client kubernetes.Interface, pvc *corev1.PersistentVolumeClaim) (*storagev1.StorageClass, error) {
+	if pvc.Spec.StorageClassName != nil && *pvc.Spec.StorageClassName != "" {
+		storageClass, err := client.StorageV1().StorageClasses().Get(ctx, *pvc.Spec.StorageClassName, metav1.GetOptions{})
+		if err != nil {
+			return nil, err
+		}
+		return storageClass, nil
+	}
+
+	storageClasses, err := client.StorageV1().StorageClasses().List(ctx, metav1.ListOptions{})
+	if err != nil {
+		return nil, fmt.Errorf("could not list storage classes: %w", err)
+	}
+
+	for _, storageClass := range storageClasses.Items {
+		if isDefaultClass, ok := storageClass.GetAnnotations()[defaultStorageClassAnnotation]; ok && isDefaultClass == "true" {
+			return &storageClass, nil
+		}
+	}
+
+	return nil, fmt.Errorf("could not get associated storage class for PVC %s", pvc.Name)
+}

pkg/kube/ready_test.go

@@ -23,10 +23,12 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
+	storagev1 "k8s.io/api/storage/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/kubernetes/fake"
+	"k8s.io/utils/pointer"
 )
 
 const defaultNamespace = metav1.NamespaceDefault
@@ -320,7 +322,8 @@ func Test_ReadyChecker_jobReady(t *testing.T) {
 
 func Test_ReadyChecker_volumeReady(t *testing.T) {
 	type args struct {
-		v *corev1.PersistentVolumeClaim
+		v            *corev1.PersistentVolumeClaim
+		storageClass *storagev1.StorageClass
 	}
 	tests := []struct {
 		name string
@@ -330,28 +333,136 @@ func Test_ReadyChecker_volumeReady(t *testing.T) {
 		{
 			name: "pvc is bound",
 			args: args{
-				v: newPersistentVolumeClaim("foo", corev1.ClaimBound),
+				v:            newPersistentVolumeClaim("foo", corev1.ClaimBound),
+				storageClass: newStorageClass("foo", storagev1.VolumeBindingImmediate, nil),
 			},
 			want: true,
 		},
 		{
 			name: "pvc is not ready",
 			args: args{
-				v: newPersistentVolumeClaim("foo", corev1.ClaimPending),
+				v:            newPersistentVolumeClaim("foo", corev1.ClaimPending),
+				storageClass: newStorageClass("foo", storagev1.VolumeBindingImmediate, nil),
 			},
 			want: false,
 		},
+		{
+			name: "pvc is not ready but storage claim bind is WaitForConsumer",
+			args: args{
+				v:            newPersistentVolumeClaim("foo", corev1.ClaimPending),
+				storageClass: newStorageClass("foo", storagev1.VolumeBindingWaitForFirstConsumer, nil),
+			},
+			want: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			c := NewReadyChecker(fake.NewSimpleClientset(), nil)
-			if got := c.volumeReady(tt.args.v); got != tt.want {
+			if got := c.volumeReady(tt.args.v, tt.args.storageClass); got != tt.want {
 				t.Errorf("volumeReady() = %v, want %v", got, tt.want)
 			}
 		})
 	}
 }
 
+func Test_getStorageClassFromPvc(t *testing.T) {
+	type args struct {
+		pvc *corev1.PersistentVolumeClaim
+		scs *storagev1.StorageClassList
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    string
+		wantErr bool
+	}{
+		{
+			name: "pvc has storage class and it exists",
+			args: args{
+				pvc: func() *corev1.PersistentVolumeClaim {
+					claim := newPersistentVolumeClaim("foo", corev1.ClaimPending)
+					claim.Spec.StorageClassName = pointer.String("default-sc")
+					return claim
+				}(),
+				scs: &storagev1.StorageClassList{
+					Items: []storagev1.StorageClass{
+						*newStorageClass("default-sc", storagev1.VolumeBindingImmediate, nil),
+					},
+				},
+			},
+			want:    "default-sc",
+			wantErr: false,
+		},
+		{
+			name: "pvc has storage class and it does not exist",
+			args: args{
+				pvc: func() *corev1.PersistentVolumeClaim {
+					claim := newPersistentVolumeClaim("foo", corev1.ClaimPending)
+					claim.Spec.StorageClassName = pointer.String("default-sc")
+					return claim
+				}(),
+				scs: &storagev1.StorageClassList{
+					Items: []storagev1.StorageClass{},
+				},
+			},
+			want:    "",
+			wantErr: true,
+		},
+		{
+			name: "pvc has default storage class and it does exist",
+			args: args{
+				pvc: func() *corev1.PersistentVolumeClaim {
+					claim := newPersistentVolumeClaim("foo", corev1.ClaimPending)
+					return claim
+				}(),
+				scs: &storagev1.StorageClassList{
+					Items: []storagev1.StorageClass{
+						*newStorageClass("default-sc", storagev1.VolumeBindingImmediate, map[string]string{
+							defaultStorageClassAnnotation: "true",
+						}),
+						*newStorageClass("other-sc", storagev1.VolumeBindingImmediate, nil),
+					},
+				},
+			},
+			want:    "default-sc",
+			wantErr: false,
+		},
+		{
+			name: "pvc has default storage class and it does not exist",
+			args: args{
+				pvc: func() *corev1.PersistentVolumeClaim {
+					claim := newPersistentVolumeClaim("foo", corev1.ClaimPending)
+					return claim
+				}(),
+				scs: &storagev1.StorageClassList{
+					Items: []storagev1.StorageClass{
+						*newStorageClass("default-sc", storagev1.VolumeBindingImmediate, map[string]string{
+							defaultStorageClassAnnotation: "false",
+						}),
+						*newStorageClass("other-sc", storagev1.VolumeBindingImmediate, nil),
+					},
+				},
+			},
+			want:    "",
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			client := fake.NewSimpleClientset(tt.args.scs)
+
+			got, err := getStorageClassFromPvc(context.Background(), client, tt.args.pvc)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("getStorageClassFromPvc() returns err %v, but wantErr is %t", err, tt.wantErr)
+			}
+
+			if got != nil && got.Name != tt.want {
+				t.Errorf("getStorageClassFromPvc() has name %v, want %v", got.Name, tt.want)
+			}
+		})
+	}
+}
+
 func newDaemonSet(name string, maxUnavailable, numberReady, desiredNumberScheduled, updatedNumberScheduled int) *appsv1.DaemonSet {
 	return &appsv1.DaemonSet{
 		ObjectMeta: metav1.ObjectMeta{
@@ -530,6 +641,16 @@ func newPersistentVolumeClaim(name string, phase corev1.PersistentVolumeClaimPha
 	}
 }
 
+func newStorageClass(name string, bindingMode storagev1.VolumeBindingMode, annotations map[string]string) *storagev1.StorageClass {
+	return &storagev1.StorageClass{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        name,
+			Annotations: annotations,
+		},
+		VolumeBindingMode: &bindingMode,
+	}
+}
+
 func newJob(name string, backoffLimit int, completions *int32, succeeded int, failed int) *batchv1.Job {
 	return &batchv1.Job{
 		ObjectMeta: metav1.ObjectMeta{