Merge pull request #102 from rfranzke/fix/storageclass

Workaround storage class changes

pkg/controller/controlplane/valuesprovider.go

@@ -326,11 +326,11 @@ func (vp *valuesProvider) GetControlPlaneChartValues(
 	}
 	checksums[openstack.CloudProviderConfigName] = util.ComputeChecksum(cpConfigSecret.Data)
 
-	cpDiskConfnigSecret := &corev1.Secret{}
-	if err := vp.Client().Get(ctx, kutil.Key(cp.Namespace, openstack.CloudProviderDiskConfigName), cpDiskConfnigSecret); err != nil {
+	cpDiskConfigSecret := &corev1.Secret{}
+	if err := vp.Client().Get(ctx, kutil.Key(cp.Namespace, openstack.CloudProviderDiskConfigName), cpDiskConfigSecret); err != nil {
 		return nil, err
 	}
-	checksums[openstack.CloudProviderDiskConfigName] = util.ComputeChecksum(cpDiskConfnigSecret.Data)
+	checksums[openstack.CloudProviderDiskConfigName] = util.ComputeChecksum(cpDiskConfigSecret.Data)
 
 	// TODO: Remove this code in a future version again.
 	if err := vp.deleteLegacyCloudProviderConfigMaps(ctx, cp.Namespace); err != nil {

pkg/controller/worker/machines.go

@@ -26,10 +26,13 @@ import (
 	extensionscontroller "github.com/gardener/gardener/extensions/pkg/controller"
 	"github.com/gardener/gardener/extensions/pkg/controller/worker"
 	genericworkeractuator "github.com/gardener/gardener/extensions/pkg/controller/worker/genericactuator"
+	"github.com/gardener/gardener/extensions/pkg/util"
 	v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants"
 	"github.com/gardener/gardener/pkg/client/kubernetes"
 	machinev1alpha1 "github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1"
+	storagev1beta1 "k8s.io/api/storage/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 // MachineClassKind yields the name of the OpenStack machine class.
@@ -42,8 +45,34 @@ func (w *workerDelegate) MachineClassList() runtime.Object {
 	return &machinev1alpha1.OpenStackMachineClassList{}
 }
 
+// NewClientForShoot is exposed for testing.
+var NewClientForShoot = util.NewClientForShoot
+
 // DeployMachineClasses generates and creates the OpenStack specific machine classes.
 func (w *workerDelegate) DeployMachineClasses(ctx context.Context) error {
+	// TODO: Remove this in a future version. This is a hacky workaround for a problem introduced with
+	// https://github.com/gardener/gardener-extension-provider-openstack/commit/3bf9f686ea6838aef2d87cbe1ff75f459037594b:
+	// The StorageClasses are immutable, hence, for existing clusters the `availability` field cannot be removed. The
+	// only way is to delete the StorageClass and recreate it. The ControlPlane controller is generating the new StorageClass
+	// without the `availability` field. Hence, we have to check if there is an existing StorageClass with the field and
+	// delete it. We cannot do it in the ControlPlane controller itself as the shoot API server might not be up at this
+	// point in time, ref https://github.com/gardener/gardener/blob/master/pkg/gardenlet/controller/shoot/shoot_control_reconcile.go#L213.
+	_, shootClient, err := NewClientForShoot(ctx, w.Client(), w.cluster.ObjectMeta.Name, client.Options{})
+	if err != nil {
+		return err
+	}
+	storageClasses := &storagev1beta1.StorageClassList{}
+	if err := shootClient.List(ctx, storageClasses); err != nil {
+		return err
+	}
+	for _, storageClass := range storageClasses.Items {
+		if (storageClass.Name == "default" || storageClass.Name == "default-class") && storageClass.Parameters != nil && storageClass.Parameters["availability"] != "" {
+			if err := shootClient.Delete(ctx, storageClass.DeepCopy()); err != nil {
+				return err
+			}
+		}
+	}
+
 	if w.machineClasses == nil {
 		if err := w.generateMachineConfig(ctx); err != nil {
 			return err

pkg/controller/worker/machines_test.go

@@ -40,10 +40,12 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
+	storagev1beta1 "k8s.io/api/storage/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -132,6 +134,8 @@ var _ = Describe("Machines", func() {
 				clusterWithoutImages   *extensionscontroller.Cluster
 				cluster                *extensionscontroller.Cluster
 				w                      *extensionsv1alpha1.Worker
+
+				shootClient *mockclient.MockClient
 			)
 
 			BeforeEach(func() {
@@ -315,6 +319,8 @@ var _ = Describe("Machines", func() {
 				workerPoolHash2, _ = worker.WorkerPoolHash(w.Spec.Pools[1], cluster)
 
 				workerDelegate, _ = NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, clusterWithoutImages)
+
+				shootClient = mockclient.NewMockClient(ctrl)
 			})
 
 			Describe("machine images", func() {
@@ -431,6 +437,13 @@ var _ = Describe("Machines", func() {
 					setup(region, machineImage, "")
 					workerDelegate, _ := NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", w, cluster)
 
+					oldNewClientForShoot := NewClientForShoot
+					defer func() { NewClientForShoot = oldNewClientForShoot }()
+					NewClientForShoot = func(_ context.Context, _ client.Client, _ string, _ client.Options) (*rest.Config, client.Client, error) {
+						return nil, shootClient, nil
+					}
+					expectStorageClassDeletionToWork(shootClient)
+
 					expectGetSecretCallToWork(c, openstackDomainName, openstackTenantName, openstackUserName, openstackPassword)
 
 					// Test workerDelegate.DeployMachineClasses()
@@ -477,6 +490,13 @@ var _ = Describe("Machines", func() {
 					setup(regionWithImages, "", machineImageID)
 					workerDelegate, _ := NewWorkerDelegate(common.NewClientContext(c, scheme, decoder), chartApplier, "", workerWithRegion, clusterWithRegion)
 
+					oldNewClientForShoot := NewClientForShoot
+					defer func() { NewClientForShoot = oldNewClientForShoot }()
+					NewClientForShoot = func(_ context.Context, _ client.Client, _ string, _ client.Options) (*rest.Config, client.Client, error) {
+						return nil, shootClient, nil
+					}
+					expectStorageClassDeletionToWork(shootClient)
+
 					expectGetSecretCallToWork(c, openstackDomainName, openstackTenantName, openstackUserName, openstackPassword)
 
 					// Test workerDelegate.DeployMachineClasses()
@@ -601,6 +621,26 @@ func expectGetSecretCallToWork(c *mockclient.MockClient, openstackDomainName, op
 		})
 }
 
+func expectStorageClassDeletionToWork(c *mockclient.MockClient) {
+	var (
+		sc1 = storagev1beta1.StorageClass{ObjectMeta: metav1.ObjectMeta{Name: "default"}}
+		sc2 = storagev1beta1.StorageClass{
+			ObjectMeta: metav1.ObjectMeta{Name: "default-class"},
+			Parameters: map[string]string{
+				"availability": "foo",
+			},
+		}
+	)
+
+	c.EXPECT().List(context.TODO(), gomock.AssignableToTypeOf(&storagev1beta1.StorageClassList{})).DoAndReturn(func(_ context.Context, list runtime.Object, _ ...client.ListOption) error {
+		obj := &storagev1beta1.StorageClassList{Items: []storagev1beta1.StorageClass{sc1, sc2}}
+		obj.DeepCopyInto(list.(*storagev1beta1.StorageClassList))
+		return nil
+	})
+
+	c.EXPECT().Delete(context.TODO(), sc2.DeepCopy())
+}
+
 func useDefaultMachineClass(def map[string]interface{}, key string, value interface{}) map[string]interface{} {
 	out := make(map[string]interface{}, len(def)+1)