From c4d15f4a3777cfb564da09d045728eb5a09196c8 Mon Sep 17 00:00:00 2001 From: Thomas Fussell Date: Mon, 7 Jun 2021 07:54:34 -0400 Subject: [PATCH] Fix PVC deletion (#1221) * fix pvc deletion * local copy * add pvc list permission * improve pvc reconciliation * fix order * cancel resource * cancel namespace deletion when pvcs remain * fix labels --- helm/kvm-operator/templates/rbac.yaml | 2 + service/controller/cluster_resource_set.go | 2 +- service/controller/key/key.go | 13 +-- .../resource/deployment/master_deployment.go | 4 +- .../controller/resource/namespace/current.go | 21 +++- service/controller/resource/pvc/create.go | 6 +- .../controller/resource/pvc/create_test.go | 32 +++--- service/controller/resource/pvc/current.go | 27 ++--- service/controller/resource/pvc/delete.go | 44 ++++---- .../controller/resource/pvc/delete_test.go | 36 +++---- service/controller/resource/pvc/desired.go | 18 +--- .../pvc/{etcd_pvc.go => desired_master.go} | 24 ++--- .../controller/resource/pvc/desired_test.go | 6 +- .../controller/resource/pvc/desired_worker.go | 100 ++++++++++++++++++ service/controller/resource/pvc/local_pvc.go | 76 ------------- service/controller/resource/pvc/resource.go | 12 +-- 16 files changed, 215 insertions(+), 208 deletions(-) rename service/controller/resource/pvc/{etcd_pvc.go => desired_master.go} (62%) create mode 100644 service/controller/resource/pvc/desired_worker.go delete mode 100644 service/controller/resource/pvc/local_pvc.go diff --git a/helm/kvm-operator/templates/rbac.yaml b/helm/kvm-operator/templates/rbac.yaml index 102e4f334..784d1ff8a 100644 --- a/helm/kvm-operator/templates/rbac.yaml +++ b/helm/kvm-operator/templates/rbac.yaml @@ -88,12 +88,14 @@ rules: - get - create - delete + - list - apiGroups: - "" resources: - persistentvolumes verbs: - list + - update - apiGroups: - "" resources: diff --git a/service/controller/cluster_resource_set.go b/service/controller/cluster_resource_set.go index 53440e9a2..e471cc477 100644 --- a/service/controller/cluster_resource_set.go +++ b/service/controller/cluster_resource_set.go @@ -280,9 +280,9 @@ func newClusterResources(config ClusterConfig) ([]resource.Interface, error) { namespaceResource, serviceAccountResource, configMapResource, + pvcResource, deploymentResource, ingressResource, - pvcResource, serviceResource, nodeControllerResource, } diff --git a/service/controller/key/key.go b/service/controller/key/key.go index 70d9168ec..d66296a3f 100644 --- a/service/controller/key/key.go +++ b/service/controller/key/key.go @@ -109,6 +109,7 @@ const ( LabelCluster = "giantswarm.io/cluster" LabelCustomer = "customer" LabelManagedBy = "giantswarm.io/managed-by" + LabelMountTag = "mount-tag" LabelOrganization = "giantswarm.io/organization" LabelVersionBundle = "giantswarm.io/version-bundle" @@ -626,16 +627,6 @@ func PortMappings(customObject v1alpha1.KVMConfig) []corev1.ServicePort { return ports } -func PVCNames(customObject v1alpha1.KVMConfig) []string { - var names []string - - for i := range customObject.Spec.Cluster.Masters { - names = append(names, EtcdPVCName(ClusterID(customObject), VMNumber(i))) - } - - return names -} - func ReleaseVersion(cr v1alpha1.KVMConfig) string { return cr.GetLabels()[label.ReleaseVersion] } @@ -656,7 +647,7 @@ func ShutdownDeferrerPollPath(customObject v1alpha1.KVMConfig) string { return fmt.Sprintf("%s/v1/defer/", ShutdownDeferrerListenAddress(customObject)) } -func StorageType(customObject v1alpha1.KVMConfig) string { +func EtcdStorageType(customObject v1alpha1.KVMConfig) string { return customObject.Spec.KVM.K8sKVM.StorageType } diff --git a/service/controller/resource/deployment/master_deployment.go b/service/controller/resource/deployment/master_deployment.go index cf13982af..ef51d738f 100644 --- a/service/controller/resource/deployment/master_deployment.go +++ b/service/controller/resource/deployment/master_deployment.go @@ -42,7 +42,7 @@ func newMasterDeployments(customResource v1alpha1.KVMConfig, release *releasev1a return nil, microerror.Maskf(invalidConfigError, "error creating memory quantity: %s", err) } - storageType := key.StorageType(customResource) + storageType := key.EtcdStorageType(customResource) // During migration, some TPOs do not have storage type set. // This specifies a default, until all TPOs have the correct storage type set. @@ -71,7 +71,7 @@ func newMasterDeployments(customResource v1alpha1.KVMConfig, release *releasev1a }, } } else { - return nil, microerror.Maskf(wrongTypeError, "unknown storageType: '%s'", key.StorageType(customResource)) + return nil, microerror.Maskf(wrongTypeError, "unknown storageType: '%s'", key.EtcdStorageType(customResource)) } deployment := &v1.Deployment{ TypeMeta: metav1.TypeMeta{ diff --git a/service/controller/resource/namespace/current.go b/service/controller/resource/namespace/current.go index 9e7c18395..c3032a2ed 100644 --- a/service/controller/resource/namespace/current.go +++ b/service/controller/resource/namespace/current.go @@ -72,18 +72,29 @@ func (r *Resource) GetCurrentState(ctx context.Context, obj interface{}) (interf // we get an empty list here after the delete event got replayed. Then we just // remove the namespace as usual. if key.IsDeleted(&customObject) { - n := key.ClusterNamespace(customObject) + namespace := key.ClusterNamespace(customObject) - lo := metav1.ListOptions{ + deployments, err := r.k8sClient.AppsV1().Deployments(namespace).List(ctx, metav1.ListOptions{ LabelSelector: fmt.Sprintf("%s=%s", label.ManagedBy, key.OperatorName), + }) + if err != nil { + return nil, microerror.Mask(err) } - list, err := r.k8sClient.AppsV1().Deployments(n).List(ctx, lo) + // Ensure PVCs have been deleted so that bound PVs are properly cleaned up. + volumeClaims, err := r.k8sClient.CoreV1().PersistentVolumeClaims(namespace).List(ctx, metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", label.ManagedBy, key.OperatorName), + }) if err != nil { return nil, microerror.Mask(err) } - if len(list.Items) != 0 { - r.logger.Debugf(ctx, "cannot finish deletion of namespace due to existing deployments") + + if len(deployments.Items) != 0 || len(volumeClaims.Items) != 0 { + if len(deployments.Items) != 0 { + r.logger.Debugf(ctx, "cannot finish deletion of namespace due to existing deployments") + } else { + r.logger.Debugf(ctx, "cannot finish deletion of namespace due to existing PVCs") + } resourcecanceledcontext.SetCanceled(ctx) finalizerskeptcontext.SetKept(ctx) r.logger.Debugf(ctx, "canceling resource") diff --git a/service/controller/resource/pvc/create.go b/service/controller/resource/pvc/create.go index 086375d24..8e74bea64 100644 --- a/service/controller/resource/pvc/create.go +++ b/service/controller/resource/pvc/create.go @@ -25,8 +25,8 @@ func (r *Resource) ApplyCreateChange(ctx context.Context, obj, createChange inte r.logger.Debugf(ctx, "creating the PVCs in the Kubernetes API") namespace := key.ClusterNamespace(customObject) - for _, PVC := range pvcsToCreate { - _, err := r.k8sClient.CoreV1().PersistentVolumeClaims(namespace).Create(ctx, PVC, v1.CreateOptions{}) + for _, persistentVolumeClaim := range pvcsToCreate { + _, err := r.k8sClient.CoreV1().PersistentVolumeClaims(namespace).Create(ctx, persistentVolumeClaim.DeepCopy(), v1.CreateOptions{}) if apierrors.IsAlreadyExists(err) { // fall through } else if err != nil { @@ -54,7 +54,7 @@ func (r *Resource) newCreateChange(ctx context.Context, obj, currentState, desir r.logger.Debugf(ctx, "finding out which PVCs have to be created") - var pvcsToCreate []*corev1.PersistentVolumeClaim + var pvcsToCreate []corev1.PersistentVolumeClaim for _, desiredPVC := range desiredPVCs { if !containsPVC(currentPVCs, desiredPVC) { diff --git a/service/controller/resource/pvc/create_test.go b/service/controller/resource/pvc/create_test.go index 4d6233168..6bab3d68c 100644 --- a/service/controller/resource/pvc/create_test.go +++ b/service/controller/resource/pvc/create_test.go @@ -28,8 +28,8 @@ func Test_Resource_PVC_newCreateChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{}, - DesiredState: []*corev1.PersistentVolumeClaim{}, + CurrentState: []corev1.PersistentVolumeClaim{}, + DesiredState: []corev1.PersistentVolumeClaim{}, ExpectedPVCNames: []string{}, }, @@ -43,14 +43,14 @@ func Test_Resource_PVC_newCreateChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{ + CurrentState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", }, }, }, - DesiredState: []*corev1.PersistentVolumeClaim{ + DesiredState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", @@ -70,8 +70,8 @@ func Test_Resource_PVC_newCreateChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{}, - DesiredState: []*corev1.PersistentVolumeClaim{ + CurrentState: []corev1.PersistentVolumeClaim{}, + DesiredState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", @@ -93,8 +93,8 @@ func Test_Resource_PVC_newCreateChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{}, - DesiredState: []*corev1.PersistentVolumeClaim{ + CurrentState: []corev1.PersistentVolumeClaim{}, + DesiredState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", @@ -123,14 +123,14 @@ func Test_Resource_PVC_newCreateChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{ + CurrentState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", }, }, }, - DesiredState: []*corev1.PersistentVolumeClaim{}, + DesiredState: []corev1.PersistentVolumeClaim{}, ExpectedPVCNames: []string{}, }, @@ -145,7 +145,7 @@ func Test_Resource_PVC_newCreateChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{ + CurrentState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", @@ -157,7 +157,7 @@ func Test_Resource_PVC_newCreateChange(t *testing.T) { }, }, }, - DesiredState: []*corev1.PersistentVolumeClaim{}, + DesiredState: []corev1.PersistentVolumeClaim{}, ExpectedPVCNames: []string{}, }, @@ -172,7 +172,7 @@ func Test_Resource_PVC_newCreateChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{ + CurrentState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", @@ -184,7 +184,7 @@ func Test_Resource_PVC_newCreateChange(t *testing.T) { }, }, }, - DesiredState: []*corev1.PersistentVolumeClaim{ + DesiredState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", @@ -231,9 +231,9 @@ func Test_Resource_PVC_newCreateChange(t *testing.T) { t.Fatalf("case %d expected %#v got %#v", i+1, nil, err) } - configMaps, ok := result.([]*corev1.PersistentVolumeClaim) + configMaps, ok := result.([]corev1.PersistentVolumeClaim) if !ok { - t.Fatalf("case %d expected %T got %T", i+1, []*corev1.PersistentVolumeClaim{}, result) + t.Fatalf("case %d expected %T got %T", i+1, []corev1.PersistentVolumeClaim{}, result) } if len(configMaps) != len(tc.ExpectedPVCNames) { diff --git a/service/controller/resource/pvc/current.go b/service/controller/resource/pvc/current.go index 04960d5a6..7de2c0a2c 100644 --- a/service/controller/resource/pvc/current.go +++ b/service/controller/resource/pvc/current.go @@ -2,10 +2,9 @@ package pvc import ( "context" + "fmt" "github.com/giantswarm/microerror" - corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/giantswarm/kvm-operator/service/controller/key" @@ -19,25 +18,15 @@ func (r *Resource) GetCurrentState(ctx context.Context, obj interface{}) (interf r.logger.Debugf(ctx, "looking for PVCs in the Kubernetes API") - var PVCs []*corev1.PersistentVolumeClaim - namespace := key.ClusterNamespace(customObject) - pvcNames := key.PVCNames(customObject) - - for _, name := range pvcNames { - manifest, err := r.k8sClient.CoreV1().PersistentVolumeClaims(namespace).Get(ctx, name, metav1.GetOptions{}) - if apierrors.IsNotFound(err) { - r.logger.Debugf(ctx, "did not find a PVC in the Kubernetes API") - // fall through - } else if err != nil { - return nil, microerror.Mask(err) - } else { - r.logger.Debugf(ctx, "found a PVC in the Kubernetes API") - PVCs = append(PVCs, manifest) - } + pvcs, err := r.k8sClient.CoreV1().PersistentVolumeClaims(namespace).List(ctx, metav1.ListOptions{ + LabelSelector: fmt.Sprintf("%s=%s", key.LegacyLabelCluster, key.ClusterID(customObject)), + }) + if err != nil { + return nil, microerror.Mask(err) } - r.logger.Debugf(ctx, "found %d PVCs in the Kubernetes API", len(PVCs)) + r.logger.Debugf(ctx, "found %d PVCs in the Kubernetes API", len(pvcs.Items)) - return PVCs, nil + return pvcs.Items, nil } diff --git a/service/controller/resource/pvc/delete.go b/service/controller/resource/pvc/delete.go index 34187d13c..746c90a11 100644 --- a/service/controller/resource/pvc/delete.go +++ b/service/controller/resource/pvc/delete.go @@ -25,7 +25,9 @@ func (r *Resource) ApplyDeleteChange(ctx context.Context, obj, deleteChange inte if len(pvcsToDelete) != 0 { r.logger.Debugf(ctx, "deleting the PVCs in the Kubernetes API") - pvsList, err := r.k8sClient.CoreV1().PersistentVolumes().List(ctx, metav1.ListOptions{LabelSelector: LabelMountTag}) + persistentVolumes, err := r.k8sClient.CoreV1().PersistentVolumes().List(ctx, metav1.ListOptions{ + LabelSelector: key.LabelMountTag, + }) if err != nil { return microerror.Mask(err) } @@ -45,27 +47,23 @@ func (r *Resource) ApplyDeleteChange(ctx context.Context, obj, deleteChange inte continue } - var boundPV *corev1.PersistentVolume - for _, pv := range pvsList.Items { - ref := pv.Spec.ClaimRef - if ref.Name != pvc.Name && ref.Namespace != namespace { - continue + // Find PV with claim pointing to the deleted PVC + for _, persistentVolume := range persistentVolumes.Items { + if persistentVolume.Spec.ClaimRef != nil && + persistentVolume.Spec.ClaimRef.Name == pvc.Name && + persistentVolume.Spec.ClaimRef.Namespace == namespace { + persistentVolume := persistentVolume.DeepCopy() + // Remove the claim + persistentVolume.Spec.ClaimRef = nil + _, err = r.k8sClient.CoreV1().PersistentVolumes().Update(ctx, persistentVolume, metav1.UpdateOptions{}) + if err != nil { + return microerror.Mask(err) + } + + r.logger.Debugf(ctx, "unbound PV %#q from PVC %#q", persistentVolume.Name, pvc.Name) + break } - - boundPV = pv.DeepCopy() - } - - if boundPV == nil || boundPV.Spec.ClaimRef == nil { - continue } - - boundPV.Spec.ClaimRef = nil - _, err = r.k8sClient.CoreV1().PersistentVolumes().Update(ctx, boundPV, metav1.UpdateOptions{}) - if err != nil { - return microerror.Mask(err) - } - - r.logger.Debugf(ctx, "unbound PV %s from pvc %s", boundPV.Name, pvc.Name) } r.logger.Debugf(ctx, "deleted the PVCs in the Kubernetes API") @@ -77,13 +75,13 @@ func (r *Resource) ApplyDeleteChange(ctx context.Context, obj, deleteChange inte } func (r *Resource) NewDeletePatch(ctx context.Context, obj, currentState, desiredState interface{}) (*crud.Patch, error) { - delete, err := r.newDeleteChange(ctx, obj, currentState, desiredState) + deleteChange, err := r.newDeleteChange(ctx, obj, currentState, desiredState) if err != nil { return nil, microerror.Mask(err) } patch := crud.NewPatch() - patch.SetDeleteChange(delete) + patch.SetDeleteChange(deleteChange) return patch, nil } @@ -100,7 +98,7 @@ func (r *Resource) newDeleteChange(ctx context.Context, obj, currentState, desir r.logger.Debugf(ctx, "finding out which PVCs have to be deleted") - var pvcsToDelete []*corev1.PersistentVolumeClaim + var pvcsToDelete []corev1.PersistentVolumeClaim for _, currentPVC := range currentPVCs { if containsPVC(desiredPVCs, currentPVC) { diff --git a/service/controller/resource/pvc/delete_test.go b/service/controller/resource/pvc/delete_test.go index baf58b2c4..8a8995c73 100644 --- a/service/controller/resource/pvc/delete_test.go +++ b/service/controller/resource/pvc/delete_test.go @@ -28,8 +28,8 @@ func Test_Resource_PVC_newDeleteChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{}, - DesiredState: []*corev1.PersistentVolumeClaim{}, + CurrentState: []corev1.PersistentVolumeClaim{}, + DesiredState: []corev1.PersistentVolumeClaim{}, ExpectedPVCNames: []string{}, }, @@ -43,14 +43,14 @@ func Test_Resource_PVC_newDeleteChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{ + CurrentState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", }, }, }, - DesiredState: []*corev1.PersistentVolumeClaim{ + DesiredState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", @@ -72,8 +72,8 @@ func Test_Resource_PVC_newDeleteChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{}, - DesiredState: []*corev1.PersistentVolumeClaim{ + CurrentState: []corev1.PersistentVolumeClaim{}, + DesiredState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", @@ -93,8 +93,8 @@ func Test_Resource_PVC_newDeleteChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{}, - DesiredState: []*corev1.PersistentVolumeClaim{ + CurrentState: []corev1.PersistentVolumeClaim{}, + DesiredState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", @@ -119,14 +119,14 @@ func Test_Resource_PVC_newDeleteChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{ + CurrentState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", }, }, }, - DesiredState: []*corev1.PersistentVolumeClaim{}, + DesiredState: []corev1.PersistentVolumeClaim{}, ExpectedPVCNames: []string{}, }, @@ -140,7 +140,7 @@ func Test_Resource_PVC_newDeleteChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{ + CurrentState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", @@ -152,7 +152,7 @@ func Test_Resource_PVC_newDeleteChange(t *testing.T) { }, }, }, - DesiredState: []*corev1.PersistentVolumeClaim{}, + DesiredState: []corev1.PersistentVolumeClaim{}, ExpectedPVCNames: []string{}, }, @@ -167,7 +167,7 @@ func Test_Resource_PVC_newDeleteChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{ + CurrentState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", @@ -179,7 +179,7 @@ func Test_Resource_PVC_newDeleteChange(t *testing.T) { }, }, }, - DesiredState: []*corev1.PersistentVolumeClaim{ + DesiredState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", @@ -218,7 +218,7 @@ func Test_Resource_PVC_newDeleteChange(t *testing.T) { }, }, }, - CurrentState: []*corev1.PersistentVolumeClaim{ + CurrentState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", @@ -240,7 +240,7 @@ func Test_Resource_PVC_newDeleteChange(t *testing.T) { }, }, }, - DesiredState: []*corev1.PersistentVolumeClaim{ + DesiredState: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ Name: "pvc-1", @@ -277,9 +277,9 @@ func Test_Resource_PVC_newDeleteChange(t *testing.T) { t.Fatalf("case %d expected %#v got %#v", i+1, nil, err) } - configMaps, ok := result.([]*corev1.PersistentVolumeClaim) + configMaps, ok := result.([]corev1.PersistentVolumeClaim) if !ok { - t.Fatalf("case %d expected %T got %T", i+1, []*corev1.PersistentVolumeClaim{}, result) + t.Fatalf("case %d expected %T got %T", i+1, []corev1.PersistentVolumeClaim{}, result) } if len(configMaps) != len(tc.ExpectedPVCNames) { diff --git a/service/controller/resource/pvc/desired.go b/service/controller/resource/pvc/desired.go index 1742c3124..483585a5a 100644 --- a/service/controller/resource/pvc/desired.go +++ b/service/controller/resource/pvc/desired.go @@ -5,7 +5,6 @@ import ( "github.com/giantswarm/microerror" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/giantswarm/kvm-operator/service/controller/key" ) @@ -16,12 +15,12 @@ func (r *Resource) GetDesiredState(ctx context.Context, obj interface{}) (interf return nil, microerror.Mask(err) } - var PVCs []*corev1.PersistentVolumeClaim + var PVCs []corev1.PersistentVolumeClaim - if key.StorageType(customObject) == "persistentVolume" { + if key.EtcdStorageType(customObject) == "persistentVolume" { r.logger.Debugf(ctx, "computing the new master PVCs") - etcdPVCs, err := newEtcdPVCs(customObject) + etcdPVCs, err := r.getDesiredMasterPVCs(customObject) if err != nil { return nil, microerror.Mask(err) } @@ -30,20 +29,13 @@ func (r *Resource) GetDesiredState(ctx context.Context, obj interface{}) (interf r.logger.Debugf(ctx, "computed the %d new master PVCs", len(PVCs)) } else { - r.logger.Debugf(ctx, "not computing the new PVCs because storage type is not 'persistentVolume'") + r.logger.Debugf(ctx, "not computing the new master PVCs because storage type is not 'persistentVolume'") } if key.HasHostVolumes(customObject) { r.logger.Debugf(ctx, "computing the new worker PVCs") - // Retrieve the existing Persistent Volume in the management cluster to get the storage size - // and create the Persistent Volume Claims for the workload cluster's workers - pvsList, err := r.k8sClient.CoreV1().PersistentVolumes().List(ctx, metav1.ListOptions{LabelSelector: LabelMountTag}) - if err != nil { - return nil, microerror.Mask(err) - } - - localPVCs, err := newLocalPVCs(customObject, pvsList) + localPVCs, err := r.getDesiredWorkerPVCs(ctx, customObject) if err != nil { return nil, microerror.Mask(err) } diff --git a/service/controller/resource/pvc/etcd_pvc.go b/service/controller/resource/pvc/desired_master.go similarity index 62% rename from service/controller/resource/pvc/etcd_pvc.go rename to service/controller/resource/pvc/desired_master.go index b73f9c5eb..1abd536b9 100644 --- a/service/controller/resource/pvc/etcd_pvc.go +++ b/service/controller/resource/pvc/desired_master.go @@ -7,6 +7,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/giantswarm/kvm-operator/pkg/label" "github.com/giantswarm/kvm-operator/service/controller/key" ) @@ -15,8 +16,8 @@ const ( EtcdPVSize = "15Gi" ) -func newEtcdPVCs(customObject v1alpha1.KVMConfig) ([]*corev1.PersistentVolumeClaim, error) { - var persistentVolumeClaims []*corev1.PersistentVolumeClaim +func (r *Resource) getDesiredMasterPVCs(customObject v1alpha1.KVMConfig) ([]corev1.PersistentVolumeClaim, error) { + var persistentVolumeClaims []corev1.PersistentVolumeClaim for i, masterNode := range customObject.Spec.Cluster.Masters { quantity, err := resource.ParseQuantity(EtcdPVSize) @@ -24,21 +25,20 @@ func newEtcdPVCs(customObject v1alpha1.KVMConfig) ([]*corev1.PersistentVolumeCla return nil, microerror.Mask(err) } - persistentVolumeClaim := &corev1.PersistentVolumeClaim{ - TypeMeta: metav1.TypeMeta{ - Kind: "PersistentVolumeClaim", - APIVersion: "v1", - }, + persistentVolumeClaim := corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ Name: key.EtcdPVCName(key.ClusterID(customObject), key.VMNumber(i)), Labels: map[string]string{ - "app": key.MasterID, - "cluster": key.ClusterID(customObject), - "customer": key.ClusterCustomer(customObject), - "node": masterNode.ID, + label.ManagedBy: key.OperatorName, + key.LabelCustomer: key.ClusterCustomer(customObject), + key.LabelApp: key.MasterID, + key.LabelCluster: key.ClusterID(customObject), + key.LegacyLabelCluster: key.ClusterID(customObject), + key.LabelVersionBundle: key.OperatorVersion(customObject), + "node": masterNode.ID, }, Annotations: map[string]string{ - "volume.beta.kubernetes.io/storage-class": StorageClass, + "volume.beta.kubernetes.io/storage-class": EtcdStorageClass, }, }, Spec: corev1.PersistentVolumeClaimSpec{ diff --git a/service/controller/resource/pvc/desired_test.go b/service/controller/resource/pvc/desired_test.go index 7e7a7c55a..b4658246f 100644 --- a/service/controller/resource/pvc/desired_test.go +++ b/service/controller/resource/pvc/desired_test.go @@ -197,9 +197,9 @@ func Test_Resource_PVC_GetDesiredState(t *testing.T) { t.Fatalf("case %d expected %#v got %#v", i+1, nil, err) } - PVCs, ok := result.([]*corev1.PersistentVolumeClaim) + PVCs, ok := result.([]corev1.PersistentVolumeClaim) if !ok { - t.Fatalf("case %d expected %T got %T", i+1, []*corev1.PersistentVolumeClaim{}, result) + t.Fatalf("case %d expected %T got %T", i+1, []corev1.PersistentVolumeClaim{}, result) } if testGetEtcdCount(PVCs) != tc.ExpectedEtcdCount { @@ -208,7 +208,7 @@ func Test_Resource_PVC_GetDesiredState(t *testing.T) { } } -func testGetEtcdCount(PVCs []*corev1.PersistentVolumeClaim) int { +func testGetEtcdCount(PVCs []corev1.PersistentVolumeClaim) int { var count int for _, i := range PVCs { diff --git a/service/controller/resource/pvc/desired_worker.go b/service/controller/resource/pvc/desired_worker.go new file mode 100644 index 000000000..ce0f22bfb --- /dev/null +++ b/service/controller/resource/pvc/desired_worker.go @@ -0,0 +1,100 @@ +package pvc + +import ( + "context" + + "github.com/giantswarm/apiextensions/v3/pkg/apis/provider/v1alpha1" + "github.com/giantswarm/microerror" + "github.com/giantswarm/operatorkit/v5/pkg/controller/context/finalizerskeptcontext" + "github.com/giantswarm/operatorkit/v5/pkg/controller/context/resourcecanceledcontext" + "github.com/giantswarm/to" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + + "github.com/giantswarm/kvm-operator/pkg/label" + "github.com/giantswarm/kvm-operator/service/controller/key" +) + +func (r *Resource) getDesiredWorkerPVCs(ctx context.Context, customObject v1alpha1.KVMConfig) ([]corev1.PersistentVolumeClaim, error) { + var persistentVolumeClaims []corev1.PersistentVolumeClaim + namespace := key.ClusterNamespace(customObject) + + for i, workerKVM := range customObject.Spec.KVM.Workers { + workerCluster := customObject.Spec.Cluster.Workers[i] + + if key.IsDeleted(&customObject) && len(workerKVM.HostVolumes) > 0 { + deploymentName := key.DeploymentName(key.WorkerID, workerCluster.ID) + _, err := r.k8sClient.AppsV1().Deployments(namespace).Get(ctx, deploymentName, metav1.GetOptions{}) + if errors.IsNotFound(err) { + // the cluster is being deleted and the worker deployment doesn't exist + r.logger.Debugf(ctx, "worker deployment %#q not found, not adding pvcs to desired state", deploymentName) + } else if err != nil { + return nil, microerror.Mask(err) + } else { + // deployment still exists, keep finalizer, cancel, and delete on next reconciliation + r.logger.Debugf(ctx, "keeping finalizer and canceling as deployment %#q still exists", deploymentName) + finalizerskeptcontext.SetKept(ctx) + resourcecanceledcontext.SetCanceled(ctx) + return nil, nil + } + } + + for _, hostVolume := range workerKVM.HostVolumes { + candidateVolumes, err := r.k8sClient.CoreV1().PersistentVolumes().List(ctx, metav1.ListOptions{ + LabelSelector: labels.FormatLabels(map[string]string{ + key.LabelMountTag: hostVolume.MountTag, + }), + }) + if err != nil { + return nil, microerror.Mask(err) + } else if len(candidateVolumes.Items) == 0 { + return nil, microerror.Maskf(notFoundError, "persistent volume with mount tag %#q not found", hostVolume.MountTag) + } else if len(candidateVolumes.Items) > 1 { + return nil, microerror.Maskf(notFoundError, "multiple persistent volumes with mount tag %#q found", hostVolume.MountTag) + } + + pv := candidateVolumes.Items[0] + claimName := key.LocalWorkerPVCName(key.ClusterID(customObject), key.VMNumber(i), hostVolume.MountTag) + + if pv.Spec.ClaimRef != nil && (pv.Spec.ClaimRef.Namespace != namespace || pv.Spec.ClaimRef.Name != claimName) { + // PV already bound to a different PVC + return nil, microerror.Maskf(isAlreadyBound, "persistent volume %#q is already bound to %#q", pv.Name, pv.Spec.ClaimRef.Name) + } + + persistentVolumeClaim := corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: claimName, + Labels: map[string]string{ + label.ManagedBy: key.OperatorName, + key.LabelCustomer: key.ClusterCustomer(customObject), + key.LabelApp: key.WorkerID, + key.LabelCluster: key.ClusterID(customObject), + key.LegacyLabelCluster: key.ClusterID(customObject), + key.LabelVersionBundle: key.OperatorVersion(customObject), + "node": workerCluster.ID, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteOnce, + }, + Resources: corev1.ResourceRequirements{ + Requests: pv.Spec.Capacity, + }, + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + key.LabelMountTag: hostVolume.MountTag, + }, + }, + StorageClassName: to.StringP(LocalStorageClass), + }, + } + + persistentVolumeClaims = append(persistentVolumeClaims, persistentVolumeClaim) + } + } + + return persistentVolumeClaims, nil +} diff --git a/service/controller/resource/pvc/local_pvc.go b/service/controller/resource/pvc/local_pvc.go deleted file mode 100644 index 1222fd14d..000000000 --- a/service/controller/resource/pvc/local_pvc.go +++ /dev/null @@ -1,76 +0,0 @@ -package pvc - -import ( - "github.com/giantswarm/apiextensions/v3/pkg/apis/provider/v1alpha1" - "github.com/giantswarm/microerror" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/giantswarm/kvm-operator/service/controller/key" -) - -const ( - LabelMountTag = "mount-tag" -) - -func newLocalPVCs(customObject v1alpha1.KVMConfig, pvsList *corev1.PersistentVolumeList) ([]*corev1.PersistentVolumeClaim, error) { - var persistentVolumeClaims []*corev1.PersistentVolumeClaim - localStorageClass := LocalStorageClass - - for i, workerKVM := range customObject.Spec.KVM.Workers { - for _, hostVolume := range workerKVM.HostVolumes { - pv := findPV(pvsList, hostVolume.MountTag) - if pv == nil { - return nil, microerror.Maskf(notFoundError, "mount tag %s is not available", hostVolume.MountTag) - } - - // discard the PV if is already bound to an existing PV - if pv.Spec.ClaimRef != nil { - return nil, microerror.Maskf(isAlreadyBound, "persistent volume %s is already bound to %s", pv.Name, pv.Spec.ClaimRef.Name) - } - - persistentVolumeClaim := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: key.LocalWorkerPVCName(key.ClusterID(customObject), key.VMNumber(i), hostVolume.MountTag), - Labels: map[string]string{ - key.LabelCustomer: key.ClusterCustomer(customObject), - key.LabelApp: key.WorkerID, - key.LabelCluster: key.ClusterID(customObject), - key.LabelVersionBundle: key.OperatorVersion(customObject), - "node": customObject.Spec.Cluster.Workers[i].ID, - }, - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{ - corev1.ReadWriteOnce, - }, - Resources: corev1.ResourceRequirements{ - Requests: pv.Spec.Capacity, - }, - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - LabelMountTag: hostVolume.MountTag, - }, - }, - StorageClassName: &localStorageClass, - }, - } - - persistentVolumeClaims = append(persistentVolumeClaims, persistentVolumeClaim) - } - } - - return persistentVolumeClaims, nil -} - -func findPV(pvsList *corev1.PersistentVolumeList, mountTag string) *corev1.PersistentVolume { - for _, pv := range pvsList.Items { - if pv.ObjectMeta.Labels[LabelMountTag] != mountTag { - continue - } - - return &pv - } - - return nil -} diff --git a/service/controller/resource/pvc/resource.go b/service/controller/resource/pvc/resource.go index f2c2cc1bb..c4af8194c 100644 --- a/service/controller/resource/pvc/resource.go +++ b/service/controller/resource/pvc/resource.go @@ -10,9 +10,9 @@ import ( const ( // Name is the identifier of the resource. Name = "pvc" - // StorageClass is the storage class annotation persistent volume claims are + // EtcdStorageClass is the storage class annotation persistent volume claims are // configured with. - StorageClass = "g8s-storage" + EtcdStorageClass = "g8s-storage" LocalStorageClass = "local-storage" ) @@ -63,7 +63,7 @@ func (r *Resource) Name() string { return Name } -func containsPVC(list []*corev1.PersistentVolumeClaim, item *corev1.PersistentVolumeClaim) bool { +func containsPVC(list []corev1.PersistentVolumeClaim, item corev1.PersistentVolumeClaim) bool { for _, l := range list { if l.Name == item.Name { return true @@ -73,14 +73,14 @@ func containsPVC(list []*corev1.PersistentVolumeClaim, item *corev1.PersistentVo return false } -func toPVCs(v interface{}) ([]*corev1.PersistentVolumeClaim, error) { +func toPVCs(v interface{}) ([]corev1.PersistentVolumeClaim, error) { if v == nil { return nil, nil } - PVCs, ok := v.([]*corev1.PersistentVolumeClaim) + PVCs, ok := v.([]corev1.PersistentVolumeClaim) if !ok { - return nil, microerror.Maskf(wrongTypeError, "expected '%T', got '%T'", []*corev1.PersistentVolumeClaim{}, v) + return nil, microerror.Maskf(wrongTypeError, "expected '%T', got '%T'", []corev1.PersistentVolumeClaim{}, v) } return PVCs, nil