diff --git a/api/v1alpha1/nfspvc_webhook.go b/api/v1alpha1/nfspvc_webhook.go index 2183e01..e3ed397 100644 --- a/api/v1alpha1/nfspvc_webhook.go +++ b/api/v1alpha1/nfspvc_webhook.go @@ -34,9 +34,8 @@ import ( var c client.Client const ( - UpdateNfsPvcError = "forbidden: NFSPVC spec is immutable after creation" - PVCAlreadyExists = "a PVC of this name already exists in the namespace. Please rename your NFSPVC" - InvalidAccessModeError = "forbidden: only the following AccessModes are permitted" + pvcAlreadyExists = "a PVC of this name already exists in the namespace. Please rename your NFSPVC" + invalidAccessModeError = "forbidden: only the following AccessModes are permitted" ) var supportedAccessModes = sets.New( @@ -67,11 +66,11 @@ func (r *NfsPvc) ValidateCreate() (admission.Warnings, error) { nfspvclog.Info("validate create", "name", r.Name) if r.doesPVCExist(c) { - return admission.Warnings{PVCAlreadyExists}, fmt.Errorf(PVCAlreadyExists) + return admission.Warnings{pvcAlreadyExists}, fmt.Errorf(pvcAlreadyExists) } if !r.validateAccessMode(r.Spec.AccessModes) { - return admission.Warnings{InvalidAccessModeError}, fmt.Errorf(InvalidAccessModeError+": %v", supportedAccessModes) + return admission.Warnings{invalidAccessModeError}, fmt.Errorf(invalidAccessModeError+": %v", supportedAccessModes) } return nil, nil diff --git a/cmd/main.go b/cmd/main.go index f645c34..d38041b 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -35,7 +35,7 @@ import ( nfspvcv1alpha1 "github.com/dana-team/nfspvc-operator/api/v1alpha1" "github.com/dana-team/nfspvc-operator/internal/controller" - utils "github.com/dana-team/nfspvc-operator/internal/controller/utils" + "github.com/dana-team/nfspvc-operator/internal/controller/utils" //+kubebuilder:scaffold:imports ) diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index 4b4fcd9..9382d89 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -14,53 +14,36 @@ namePrefix: nfspvc-operator- # pairs: # someName: someValue +# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in +# crd/kustomization.yaml +# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. resources: - ../crd - ../rbac - ../manager -# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in -# crd/kustomization.yaml - ../webhook -# [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. - ../certmanager # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. #- ../prometheus -patchesStrategicMerge: # Protect the /metrics endpoint by putting it behind auth. # If you want your controller-manager to expose the /metrics # endpoint w/o any authn/z, please comment the following line. -- manager_auth_proxy_patch.yaml # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml -- manager_webhook_patch.yaml - +patches: + - path: manager_auth_proxy_patch.yaml + - path: manager_webhook_patch.yaml + - path: webhookcainjection_patch.yaml # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. # Uncomment 'CERTMANAGER' sections in crd/kustomization.yaml to enable the CA injection in the admission webhooks. # 'CERTMANAGER' needs to be enabled to use ca injection -- webhookcainjection_patch.yaml # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER' prefix. # Uncomment the following replacements to add the cert-manager CA injection annotations -replacements: - - source: # Add cert-manager annotation to ValidatingWebhookConfiguration, MutatingWebhookConfiguration and CRDs - kind: Certificate - group: cert-manager.io - version: v1 - name: serving-cert # this name should match the one in certificate.yaml - fieldPath: .metadata.namespace # namespace of the certificate CR - targets: - - select: - kind: ValidatingWebhookConfiguration - fieldPaths: - - .metadata.annotations.[cert-manager.io/inject-ca-from] - options: - delimiter: '/' - index: 0 - create: true #- select: # kind: MutatingWebhookConfiguration # fieldPaths: @@ -69,29 +52,6 @@ replacements: # delimiter: '/' # index: 0 # create: true - - select: - kind: CustomResourceDefinition - fieldPaths: - - .metadata.annotations.[cert-manager.io/inject-ca-from] - options: - delimiter: '/' - index: 0 - create: true - - source: - kind: Certificate - group: cert-manager.io - version: v1 - name: serving-cert # this name should match the one in certificate.yaml - fieldPath: .metadata.name - targets: - - select: - kind: ValidatingWebhookConfiguration - fieldPaths: - - .metadata.annotations.[cert-manager.io/inject-ca-from] - options: - delimiter: '/' - index: 1 - create: true #- select: # kind: MutatingWebhookConfiguration # fieldPaths: @@ -100,45 +60,84 @@ replacements: # delimiter: '/' # index: 1 # create: true - - select: - kind: CustomResourceDefinition - fieldPaths: - - .metadata.annotations.[cert-manager.io/inject-ca-from] - options: - delimiter: '/' - index: 1 - create: true - - source: # Add cert-manager annotation to the webhook Service - kind: Service - version: v1 - name: webhook-service - fieldPath: .metadata.name # namespace of the service - targets: - - select: - kind: Certificate - group: cert-manager.io - version: v1 - fieldPaths: - - .spec.dnsNames.0 - - .spec.dnsNames.1 - options: - delimiter: '.' - index: 0 - create: true - - source: - kind: Service - version: v1 - name: webhook-service - fieldPath: .metadata.namespace # namespace of the service - targets: - - select: - kind: Certificate - group: cert-manager.io - version: v1 - fieldPaths: - - .spec.dnsNames.0 - - .spec.dnsNames.1 - options: - delimiter: '.' - index: 1 - create: true +replacements: +- source: + fieldPath: .metadata.namespace + group: cert-manager.io + kind: Certificate + name: serving-cert + version: v1 + targets: + - fieldPaths: + - .metadata.annotations.[cert-manager.io/inject-ca-from] + options: + create: true + delimiter: / + select: + kind: ValidatingWebhookConfiguration + - fieldPaths: + - .metadata.annotations.[cert-manager.io/inject-ca-from] + options: + create: true + delimiter: / + select: + kind: CustomResourceDefinition +- source: + fieldPath: .metadata.name + group: cert-manager.io + kind: Certificate + name: serving-cert + version: v1 + targets: + - fieldPaths: + - .metadata.annotations.[cert-manager.io/inject-ca-from] + options: + create: true + delimiter: / + index: 1 + select: + kind: ValidatingWebhookConfiguration + - fieldPaths: + - .metadata.annotations.[cert-manager.io/inject-ca-from] + options: + create: true + delimiter: / + index: 1 + select: + kind: CustomResourceDefinition +- source: + fieldPath: .metadata.name + kind: Service + name: webhook-service + version: v1 + targets: + - fieldPaths: + - .spec.dnsNames.0 + - .spec.dnsNames.1 + options: + create: true + delimiter: . + select: + group: cert-manager.io + kind: Certificate + version: v1 +- source: + fieldPath: .metadata.namespace + kind: Service + name: webhook-service + version: v1 + targets: + - fieldPaths: + - .spec.dnsNames.0 + - .spec.dnsNames.1 + options: + create: true + delimiter: . + index: 1 + select: + group: cert-manager.io + kind: Certificate + version: v1 +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization + diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index ad13e96..ae6062d 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -5,4 +5,4 @@ kind: Kustomization images: - name: controller newName: controller - newTag: latest + newTag: latest \ No newline at end of file diff --git a/internal/controller/finalizer/finalizer.go b/internal/controller/finalizer/finalizer.go new file mode 100644 index 0000000..a3cafcc --- /dev/null +++ b/internal/controller/finalizer/finalizer.go @@ -0,0 +1,31 @@ +package finalizer + +import ( + "context" + + "github.com/dana-team/nfspvc-operator/internal/controller/utils" + + danaiov1alpha1 "github.com/dana-team/nfspvc-operator/api/v1alpha1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +// Remove removes a finalizer from the nfspvc object. +func Remove(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient client.Client) error { + controllerutil.RemoveFinalizer(&nfspvc, utils.NfsPvcDeletionFinalizer) + if err := k8sClient.Update(ctx, &nfspvc); err != nil { + return err + } + return nil +} + +// Ensure adds a finalizer to the nfspvc object if one does not exist. +func Ensure(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient client.Client) error { + if !controllerutil.ContainsFinalizer(&nfspvc, utils.NfsPvcDeletionFinalizer) { + controllerutil.AddFinalizer(&nfspvc, utils.NfsPvcDeletionFinalizer) + if err := k8sClient.Update(ctx, &nfspvc); err != nil { + return err + } + } + return nil +} diff --git a/internal/controller/nfspvc_controller.go b/internal/controller/nfspvc_controller.go index d1dec9b..15b0b38 100644 --- a/internal/controller/nfspvc_controller.go +++ b/internal/controller/nfspvc_controller.go @@ -18,16 +18,17 @@ package controller import ( "context" + "errors" "fmt" "time" danaiov1alpha1 "github.com/dana-team/nfspvc-operator/api/v1alpha1" - finalizerutils "github.com/dana-team/nfspvc-operator/internal/controller/utils/finalizer" - statusutils "github.com/dana-team/nfspvc-operator/internal/controller/utils/status" - syncutils "github.com/dana-team/nfspvc-operator/internal/controller/utils/sync" + "github.com/dana-team/nfspvc-operator/internal/controller/finalizer" + "github.com/dana-team/nfspvc-operator/internal/controller/resources" + "github.com/dana-team/nfspvc-operator/internal/controller/status" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -68,34 +69,35 @@ func (r *NfsPvcReconciler) SetupWithManager(mgr ctrl.Manager) error { func (r *NfsPvcReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { logger := log.FromContext(ctx).WithValues("NfsPvc", req.Name, "NfsPvcNamespace", req.Namespace) logger.Info("Starting Reconcile") - nfspvc := danaiov1alpha1.NfsPvc{} if err := r.Client.Get(ctx, req.NamespacedName, &nfspvc); err != nil { - if errors.IsNotFound(err) { - logger.Info(fmt.Sprintf("Didn't find NfsPvc: %s, from the namespace: %s", nfspvc.Name, nfspvc.Namespace)) + if apierrors.IsNotFound(err) { + logger.Info("Didn't find NfsPvc") return ctrl.Result{}, nil } return ctrl.Result{}, fmt.Errorf("failed to get NfsPvc: %s", err.Error()) } - - err, deleted := finalizerutils.HandleResourceDeletion(ctx, nfspvc, logger, r.Client) - if err != nil { - if finalizerutils.IsFailedCleanUp(err) { - // this means the error is of type *FailedCleanUpError. - logger.Info(fmt.Sprintf("failed to handle NfsPvc deletion: %s, so trying again in a few seconds", err.Error())) - return ctrl.Result{RequeueAfter: time.Second * RequeueIntervalSeconds}, nil + if nfspvc.ObjectMeta.DeletionTimestamp != nil { + deleted, err := resources.HandleDelete(ctx, nfspvc, r.Client) + if err != nil { + if errors.Is(err, resources.FailedCleanupError) { + logger.Info(fmt.Sprintf("failed to handle NfsPvc deletion: %s, so trying again in a few seconds", err.Error())) + return ctrl.Result{RequeueAfter: time.Second * RequeueIntervalSeconds}, nil + } + return ctrl.Result{}, fmt.Errorf("failed to handle NfsPvc deletion: %s", err.Error()) + } + if deleted { + if err := finalizer.Remove(ctx, nfspvc, r.Client); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } - return ctrl.Result{}, fmt.Errorf("failed to handle NfsPvc deletion: %s", err.Error()) - } - if deleted { - return ctrl.Result{}, nil } - if err := finalizerutils.EnsureFinalizer(ctx, nfspvc, r.Client, logger); err != nil { + + if err := finalizer.Ensure(ctx, nfspvc, r.Client); err != nil { return ctrl.Result{}, fmt.Errorf("failed to ensure finalizer in NfsPvc: %s", err.Error()) } - - // now sync the objects to the nfspvc object. - if err := SyncNfsPvc(ctx, nfspvc, logger, r.Client); err != nil { + if err := r.Update(ctx, nfspvc); err != nil { return ctrl.Result{}, fmt.Errorf("failed to sync NfsPvc: %s", err.Error()) } @@ -132,16 +134,15 @@ func (r *NfsPvcReconciler) enqueueRequestsFromPersistentVolumeClaim(ctx context. return requests } -func SyncNfsPvc(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, log logr.Logger, k8sClient client.Client) error { +// Update handles any update to an NFSPVC. +func (r *NfsPvcReconciler) Update(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc) error { if nfspvc.ObjectMeta.DeletionTimestamp == nil { - if err := syncutils.CreateOrUpdateStorageObjects(ctx, nfspvc, log, k8sClient); err != nil { + if err := resources.HandleStorageObjectState(ctx, nfspvc, r.Client); err != nil { return err } } - - if err := statusutils.SyncNfsPvcStatus(ctx, nfspvc, log, k8sClient); err != nil { + if err := status.Update(ctx, nfspvc, r.Client); err != nil { return err } - return nil } diff --git a/internal/controller/resources/resources.go b/internal/controller/resources/resources.go new file mode 100644 index 0000000..61dcc61 --- /dev/null +++ b/internal/controller/resources/resources.go @@ -0,0 +1,104 @@ +package resources + +import ( + "context" + "errors" + "fmt" + + "github.com/dana-team/nfspvc-operator/internal/controller/utils" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + danaiov1alpha1 "github.com/dana-team/nfspvc-operator/api/v1alpha1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var FailedCleanupError = errors.New("failed nfspvc cleanup") + +// HandleDelete ensures the deletion of the nfspvc. +func HandleDelete(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient client.Client) (bool, error) { + if controllerutil.ContainsFinalizer(&nfspvc, utils.NfsPvcDeletionFinalizer) { + pvcDeleted, pvDeleted, err := areResourceDeleted(ctx, nfspvc, k8sClient) + if err != nil { + return false, err + } + if pvDeleted && pvcDeleted { + return true, nil + } else if !pvDeleted && pvcDeleted { + if err := cleanup(ctx, nfspvc.Name, nfspvc.Namespace, k8sClient); err != nil { + return false, err + } + pvName := nfspvc.Name + "-" + nfspvc.Namespace + "-pv" + return false, fmt.Errorf("the pv %s is not deleted yet, %w", pvName, FailedCleanupError) + } + if err := cleanup(ctx, nfspvc.Name, nfspvc.Namespace, k8sClient); err != nil { + return false, err + } + } + return false, nil +} + +// deleteResource gets resource to delete and delete that resource from the cluster. +func deleteResource(ctx context.Context, resource client.Object, k8sClient client.Client) error { + if err := k8sClient.Delete(ctx, resource); client.IgnoreNotFound(err) != nil { + return err + } + return nil +} + +// cleanup deletes the pvc and the pv that related to the nfspvc. +func cleanup(ctx context.Context, nfsPvcName string, nfsPvcNamespace string, k8sClient client.Client) error { + pvc := &corev1.PersistentVolumeClaim{} + pvcDeleted, err := isDeleted(ctx, k8sClient, pvc, types.NamespacedName{Name: nfsPvcName, Namespace: nfsPvcNamespace}) + if err != nil { + return err + } + if !pvcDeleted { + if err := deleteResource(ctx, pvc, k8sClient); err != nil { + return fmt.Errorf("failed to delete pvc - %s: %s", nfsPvcName, err.Error()) + } + } + pvName := nfsPvcName + "-" + nfsPvcNamespace + "-pv" + pv := &corev1.PersistentVolume{} + pvDeleted, err := isDeleted(ctx, k8sClient, pv, types.NamespacedName{Name: pvName}) + if err != nil { + return err + } + if !pvDeleted { + if err := deleteResource(ctx, pv, k8sClient); err != nil { + return fmt.Errorf("failed to delete pv - %s: %s", pvName, err.Error()) + } + } + return nil +} + +// areResourcesDeleted checks if the underlying PV and PVC of an nfspvc are deleted. +func areResourceDeleted(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient client.Client) (bool, bool, error) { + pvName := nfspvc.Name + "-" + nfspvc.Namespace + "-pv" + pvc := corev1.PersistentVolumeClaim{} + pv := corev1.PersistentVolume{} + pvcDeleted, err := isDeleted(ctx, k8sClient, &pvc, types.NamespacedName{Namespace: nfspvc.Namespace, Name: nfspvc.Name}) + if err != nil { + return false, false, err + } + + pvDeleted, err := isDeleted(ctx, k8sClient, &pv, types.NamespacedName{Name: pvName}) + if err != nil { + return false, false, err + } + + return pvcDeleted, pvDeleted, nil +} + +// isDeleted checks if the given object exists in the cluster. +func isDeleted(ctx context.Context, k8sClient client.Client, k8sObject client.Object, namespacedName types.NamespacedName) (bool, error) { + if err := k8sClient.Get(ctx, namespacedName, k8sObject); err != nil { + if apierrors.IsNotFound(err) { + return true, nil + } + return false, err + } + return false, nil +} diff --git a/internal/controller/resources/storage.go b/internal/controller/resources/storage.go new file mode 100644 index 0000000..2040853 --- /dev/null +++ b/internal/controller/resources/storage.go @@ -0,0 +1,97 @@ +package resources + +import ( + "context" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/util/retry" + + danaiov1alpha1 "github.com/dana-team/nfspvc-operator/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + nfsPvcDanaLabel = "nfspvc.dana.io/nfspvc-owner" +) + +// PreparePVC returns a PVC with the given storageclass. +func PreparePVC(nfspvc danaiov1alpha1.NfsPvc, StorageClass string) corev1.PersistentVolumeClaim { + storageClass := StorageClass + return corev1.PersistentVolumeClaim{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: nfspvc.Name, + Namespace: nfspvc.Namespace, + Labels: map[string]string{ + nfsPvcDanaLabel: nfspvc.Name, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + StorageClassName: &storageClass, + VolumeName: nfspvc.Name + "-" + nfspvc.Namespace + "-pv", + AccessModes: nfspvc.Spec.AccessModes, + Resources: corev1.VolumeResourceRequirements{ + Requests: nfspvc.Spec.Capacity, + }, + }, + } +} + +// PreparePV returns a PV with the given storageclass and reclaimpolicy. +func PreparePV(nfspvc danaiov1alpha1.NfsPvc, StorageClass string, ReclaimPolicy string) corev1.PersistentVolume { + var pvName = nfspvc.Name + "-" + nfspvc.Namespace + "-pv" + return corev1.PersistentVolume{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: pvName, + Labels: map[string]string{ + nfsPvcDanaLabel: nfspvc.Name, + }, + }, + Spec: corev1.PersistentVolumeSpec{ + StorageClassName: StorageClass, + Capacity: nfspvc.Spec.Capacity, + AccessModes: nfspvc.Spec.AccessModes, + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimPolicy(ReclaimPolicy), + ClaimRef: &corev1.ObjectReference{ + Name: nfspvc.Name, + Namespace: nfspvc.Namespace, + Kind: corev1.ResourcePersistentVolumeClaims.String(), + }, + PersistentVolumeSource: corev1.PersistentVolumeSource{ + NFS: &corev1.NFSVolumeSource{ + Server: nfspvc.Spec.Server, + Path: nfspvc.Spec.Path, + }, + }, + }, + } +} + +// UpdatePV updates the PV claim reference when the NFSPVC is updated. +func UpdatePV(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient client.Client, pv corev1.PersistentVolume) error { + claimRefForPv := &corev1.ObjectReference{ + Name: nfspvc.Name, + Namespace: nfspvc.Namespace, + Kind: corev1.ResourcePersistentVolumeClaims.String(), + } + var pvName = nfspvc.Name + "-" + nfspvc.Namespace + "-pv" + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + if err := k8sClient.Get(ctx, types.NamespacedName{Name: pvName}, &pv); err != nil { + return err + } + + pv.Spec.ClaimRef = claimRefForPv + updateErr := k8sClient.Update(ctx, &pv) + if errors.IsConflict(updateErr) { + if getErr := k8sClient.Get(ctx, types.NamespacedName{Name: pvName}, &pv); getErr != nil { + return getErr + } + } + return updateErr + }) + return err +} diff --git a/internal/controller/utils/sync/sync.go b/internal/controller/resources/sync.go similarity index 50% rename from internal/controller/utils/sync/sync.go rename to internal/controller/resources/sync.go index cc505cb..049df69 100644 --- a/internal/controller/utils/sync/sync.go +++ b/internal/controller/resources/sync.go @@ -1,34 +1,27 @@ -package sync +package resources import ( "context" "fmt" - "os" + "github.com/dana-team/nfspvc-operator/internal/controller/utils" danaiov1alpha1 "github.com/dana-team/nfspvc-operator/api/v1alpha1" - "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/retry" "sigs.k8s.io/controller-runtime/pkg/client" ) const ( - NfsPvcDanaLabel = "nfspvc.dana.io/nfspvc-owner" - PvcBindStatusAnnotation = "pv.kubernetes.io/bind-completed" - - StorageClassEnv = "STORAGE_CLASS" - ReclaimPolicyEnv = "RECLAIM_POLICY" + pvcBindStatusAnnotation = "pv.kubernetes.io/bind-completed" + desiredBindStatus = "yes" ) -var StorageClass = os.Getenv(StorageClassEnv) -var ReclaimPolicy = os.Getenv(ReclaimPolicyEnv) - -func CreateOrUpdateStorageObjects(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, log logr.Logger, k8sClient client.Client) error { - if err := handlePVState(ctx, nfspvc, log, k8sClient); err != nil { +// HandleStorageObjectState handles the underlying PV and PVC when an NFSPVC is updated. +func HandleStorageObjectState(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient client.Client) error { + if err := handlePVState(ctx, nfspvc, k8sClient); err != nil { return err } @@ -40,14 +33,13 @@ func CreateOrUpdateStorageObjects(ctx context.Context, nfspvc danaiov1alpha1.Nfs } -func handlePVState(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, log logr.Logger, k8sClient client.Client) error { +func handlePVState(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient client.Client) error { pv := corev1.PersistentVolume{} if err := k8sClient.Get(ctx, types.NamespacedName{Name: nfspvc.Name + "-" + nfspvc.Namespace + "-pv"}, &pv); err != nil { if !errors.IsNotFound(err) { - log.Error(err, "unable to get pv - "+nfspvc.Name+"-"+nfspvc.Namespace+"-pv") return err } - pvFromNfsPvc := preparePV(nfspvc) + pvFromNfsPvc := PreparePV(nfspvc, utils.StorageClass, utils.ReclaimPolicy) if err := k8sClient.Create(ctx, &pvFromNfsPvc); err != nil { return fmt.Errorf("failed to create pv: %s", err.Error()) } @@ -59,28 +51,7 @@ func handlePVState(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, log logr.L return fmt.Errorf("failed to fetch claimRef PVC: %s", err.Error()) } if isClaimRefPVCDeleted { - claimRefForPv := &corev1.ObjectReference{ - Name: nfspvc.Name, - Namespace: nfspvc.Namespace, - Kind: corev1.ResourcePersistentVolumeClaims.String(), - } - // Use retry on conflict to update the PV. - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - if err := k8sClient.Get(ctx, types.NamespacedName{Name: nfspvc.Name + "-" + nfspvc.Namespace + "-pv"}, &pv); err != nil { - return err - } - - pv.Spec.ClaimRef = claimRefForPv - updateErr := k8sClient.Update(ctx, &pv) - if errors.IsConflict(updateErr) { - // Conflict occurred, let's re-fetch the latest version of PV and retry the update. - if getErr := k8sClient.Get(ctx, types.NamespacedName{Name: nfspvc.Name + "-" + nfspvc.Namespace + "-pv"}, &pv); getErr != nil { - return getErr - } - } - return updateErr - }) - return err + return UpdatePV(ctx, nfspvc, k8sClient, pv) } return nil } @@ -89,7 +60,7 @@ func handlePVCState(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient pvc := corev1.PersistentVolumeClaim{} if err := k8sClient.Get(ctx, types.NamespacedName{Namespace: nfspvc.Namespace, Name: nfspvc.Name}, &pvc); err != nil { if errors.IsNotFound(err) { - pvcFromNfsPvc := preparePVC(nfspvc) + pvcFromNfsPvc := PreparePVC(nfspvc, utils.StorageClass) if err := k8sClient.Create(ctx, &pvcFromNfsPvc); err != nil { return fmt.Errorf("failed to create pvc: %s", err.Error()) } @@ -99,82 +70,32 @@ func handlePVCState(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient } } - if pvc.Status.Phase == corev1.ClaimLost { // if the pvc's phase is 'lost', so probably the associated pv was deleted. In order to fix that the "bind" annotation needs to be deleted. - bindStatus, ok := pvc.ObjectMeta.Annotations[PvcBindStatusAnnotation] - if ok && bindStatus == "yes" { - // Use retry on conflict to update the PVC. - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - if err := k8sClient.Get(ctx, types.NamespacedName{Namespace: nfspvc.Namespace, Name: nfspvc.Name}, &pvc); err != nil { - return err - } - delete(pvc.ObjectMeta.Annotations, PvcBindStatusAnnotation) - updateErr := k8sClient.Update(ctx, &pvc) - if errors.IsConflict(updateErr) { - // Conflict occurred, let's re-fetch the latest version of PVC and retry the update. - if getErr := k8sClient.Get(ctx, types.NamespacedName{Namespace: nfspvc.Namespace, Name: nfspvc.Name}, &pvc); getErr != nil { - return getErr - } - } - return updateErr - }) - return err - } - return nil + if pvc.Status.Phase == corev1.ClaimLost { // if the pvc's phase is 'lost', so probably the associated pv was deleted + return deletePVCBindAnnotation(ctx, nfspvc, k8sClient, pvc) } return nil } -func preparePVC(nfspvc danaiov1alpha1.NfsPvc) corev1.PersistentVolumeClaim { - storageClass := StorageClass - pvc := corev1.PersistentVolumeClaim{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Name: nfspvc.Name, - Namespace: nfspvc.Namespace, - Labels: map[string]string{ - NfsPvcDanaLabel: nfspvc.Name, - }, - }, - Spec: corev1.PersistentVolumeClaimSpec{ - StorageClassName: &storageClass, - VolumeName: nfspvc.Name + "-" + nfspvc.Namespace + "-pv", - AccessModes: nfspvc.Spec.AccessModes, - Resources: corev1.VolumeResourceRequirements{ - Requests: nfspvc.Spec.Capacity, - }, - }, - } - return pvc -} - -func preparePV(nfspvc danaiov1alpha1.NfsPvc) corev1.PersistentVolume { - pv := corev1.PersistentVolume{ - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Name: nfspvc.Name + "-" + nfspvc.Namespace + "-pv", - Labels: map[string]string{ - NfsPvcDanaLabel: nfspvc.Name, - }, - }, - Spec: corev1.PersistentVolumeSpec{ - StorageClassName: StorageClass, - Capacity: nfspvc.Spec.Capacity, - AccessModes: nfspvc.Spec.AccessModes, - PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimPolicy(ReclaimPolicy), - ClaimRef: &corev1.ObjectReference{ - Name: nfspvc.Name, - Namespace: nfspvc.Namespace, - Kind: corev1.ResourcePersistentVolumeClaims.String(), - }, - PersistentVolumeSource: corev1.PersistentVolumeSource{ - NFS: &corev1.NFSVolumeSource{ - Server: nfspvc.Spec.Server, - Path: nfspvc.Spec.Path, - }, - }, - }, +// deletePVCBindAnnotation deletes the "bind" annotation from a pvc. +func deletePVCBindAnnotation(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient client.Client, pvc corev1.PersistentVolumeClaim) error { + bindStatus, ok := pvc.ObjectMeta.Annotations[pvcBindStatusAnnotation] + if ok && bindStatus == desiredBindStatus { + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + if err := k8sClient.Get(ctx, types.NamespacedName{Namespace: nfspvc.Namespace, Name: nfspvc.Name}, &pvc); err != nil { + return err + } + delete(pvc.ObjectMeta.Annotations, pvcBindStatusAnnotation) + updateErr := k8sClient.Update(ctx, &pvc) + if errors.IsConflict(updateErr) { + if getErr := k8sClient.Get(ctx, types.NamespacedName{Namespace: nfspvc.Namespace, Name: nfspvc.Name}, &pvc); getErr != nil { + return getErr + } + } + return updateErr + }) + return err } - return pv + return nil } // isConnectedPVCDeleted returns if the connected PVC is deleted, diff --git a/internal/controller/status/status.go b/internal/controller/status/status.go new file mode 100644 index 0000000..0a5dfaf --- /dev/null +++ b/internal/controller/status/status.go @@ -0,0 +1,67 @@ +package status + +import ( + "context" + + danaiov1alpha1 "github.com/dana-team/nfspvc-operator/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/retry" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + phaseUnknown = "Unknown" + phaseNotFound = "NotFound" +) + +// Update fetches the phase of the pv and the pvc that is created by the nfspvc and updates the nfspvc status. +func Update(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient client.Client) error { + pvcPhase := getPVCStatus(ctx, nfspvc, k8sClient) + pvPhase := getPVStatus(ctx, nfspvc, k8sClient) + if pvcPhase != nfspvc.Status.PvcPhase || pvPhase != nfspvc.Status.PvPhase { + return ensure(ctx, pvcPhase, pvPhase, nfspvc, k8sClient) + } + return nil +} + +// ensure updates the status of the nfspvc to match the state of the underlying PVC. +func ensure(ctx context.Context, pvcPhase string, pvPhase string, nfspvcObject danaiov1alpha1.NfsPvc, k8sClient client.Client) error { + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + nfspvcObject.Status.PvcPhase = pvcPhase + nfspvcObject.Status.PvPhase = pvPhase + updateErr := k8sClient.Status().Update(ctx, &nfspvcObject) + if errors.IsConflict(updateErr) { + if getErr := k8sClient.Get(ctx, types.NamespacedName{Name: nfspvcObject.Name, Namespace: nfspvcObject.Namespace}, &nfspvcObject); getErr != nil { + return getErr + } + } + return updateErr + }) + return err +} + +// getPVCStatus returns the phase of the pvc. +func getPVCStatus(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient client.Client) string { + pvc := corev1.PersistentVolumeClaim{} + if err := k8sClient.Get(ctx, types.NamespacedName{Namespace: nfspvc.Namespace, Name: nfspvc.Name}, &pvc); err != nil { + if errors.IsNotFound(err) { + return phaseNotFound + } + return phaseUnknown + } + return string(pvc.Status.Phase) +} + +// getPVStatus returns the phase of the pv. +func getPVStatus(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient client.Client) string { + pv := corev1.PersistentVolume{} + if err := k8sClient.Get(ctx, types.NamespacedName{Name: nfspvc.Name + "-" + nfspvc.Namespace + "-pv"}, &pv); err != nil { + if errors.IsNotFound(err) { + return phaseNotFound + } + return phaseUnknown + } + return string(pv.Status.Phase) +} diff --git a/internal/controller/utils/common.go b/internal/controller/utils/common.go index 1f74ea9..129fe40 100644 --- a/internal/controller/utils/common.go +++ b/internal/controller/utils/common.go @@ -8,42 +8,43 @@ import ( corev1 "k8s.io/api/core/v1" ) +const ( + StorageClassEnv = "STORAGE_CLASS" + ReclaimPolicyEnv = "RECLAIM_POLICY" + + UndefinedEnvironmentVariableMsg = "failed to get configuration environment variable" + InvalidReclaimPolicyMsg = "invalid default Persistent Volume Reclaim Policy" + + NfsPvcDeletionFinalizer = "nfspvc.dana.io/nfspvc-protection" +) + var AllowedReclaimPolicies = []corev1.PersistentVolumeReclaimPolicy{ corev1.PersistentVolumeReclaimRecycle, corev1.PersistentVolumeReclaimDelete, corev1.PersistentVolumeReclaimRetain, } - -const ( - STORAGE_CLASS_ENV = "STORAGE_CLASS" - RECLAIM_POLICY_ENV = "RECLAIM_POLICY" - - UNDEFINED_ENVIRONMENT_VARIABLE_MSG = "failed to get configuration environment variable" - INVALID_RECLAIM_POLICY_MSG = "invalid default Persistent Volume Reclaim Policy" -) +var ReclaimPolicy string +var StorageClass string func VerifyEnvironmentVariables() (bool, string) { - if !doesEnvironmentVariableExist() { - return false, UNDEFINED_ENVIRONMENT_VARIABLE_MSG + storageClass, ok := os.LookupEnv(StorageClassEnv) + if !ok { + return false, UndefinedEnvironmentVariableMsg } - - if !doesReclaimPolicyValid(os.Getenv(RECLAIM_POLICY_ENV)) { - return false, INVALID_RECLAIM_POLICY_MSG + StorageClass = storageClass + reclaimPolicy, ok := os.LookupEnv(ReclaimPolicyEnv) + if !ok { + return false, UndefinedEnvironmentVariableMsg + } + if !isReclaimPolicyValid(reclaimPolicy) { + return false, InvalidReclaimPolicyMsg } + ReclaimPolicy = reclaimPolicy return true, "" } -func doesEnvironmentVariableExist() bool { - storageClass := os.Getenv(STORAGE_CLASS_ENV) - reclaimPolicy := os.Getenv(RECLAIM_POLICY_ENV) - if storageClass == "" || reclaimPolicy == "" { - return false - } - return true -} - -func doesReclaimPolicyValid(reclaimPolicy string) bool { +func isReclaimPolicyValid(reclaimPolicy string) bool { policy := corev1.PersistentVolumeReclaimPolicy(reclaimPolicy) return slices.Contains(AllowedReclaimPolicies, policy) } diff --git a/internal/controller/utils/finalizer/finalizer.go b/internal/controller/utils/finalizer/finalizer.go deleted file mode 100644 index cb85bab..0000000 --- a/internal/controller/utils/finalizer/finalizer.go +++ /dev/null @@ -1,133 +0,0 @@ -package finalizer - -import ( - "context" - "fmt" - - danaiov1alpha1 "github.com/dana-team/nfspvc-operator/api/v1alpha1" - "github.com/go-logr/logr" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" -) - -const NfsPvcDeletionFinalizer = "nfspvc.dana.io/nfspvc-protection" - -type FailedCleanUpError struct { - Message string -} - -// Error implements the error interface for FailedCleanUpError. -func (e *FailedCleanUpError) Error() string { - return e.Message -} - -// IsFailedCleanUp returns true if the specified error is FailedCleanUpError. -func IsFailedCleanUp(err error) bool { - if _, ok := err.(*FailedCleanUpError); ok { - // This means the error is of type *FailedCleanUpError. - return true - } - return false -} - -// HandleResourceDeletion ensures the deletion of the nfspvc. -func HandleResourceDeletion(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, log logr.Logger, k8sClient client.Client) (error, bool) { - if nfspvc.ObjectMeta.DeletionTimestamp != nil { - if controllerutil.ContainsFinalizer(&nfspvc, NfsPvcDeletionFinalizer) { - // check if the pv and the pvc are deleted. - pvcDeleted := false - pvc := corev1.PersistentVolumeClaim{} - if err := k8sClient.Get(ctx, types.NamespacedName{Namespace: nfspvc.Namespace, Name: nfspvc.Name}, &pvc); err != nil { - if !errors.IsNotFound(err) { - log.Error(err, "unable to get pvc - "+nfspvc.Name) - return err, false - } - pvcDeleted = true - } - pvDeleted := false - pv := corev1.PersistentVolume{} - if err := k8sClient.Get(ctx, types.NamespacedName{Name: nfspvc.Name + "-" + nfspvc.Namespace + "-pv"}, &pv); err != nil { - if !errors.IsNotFound(err) { - log.Error(err, "unable to get pv - "+nfspvc.Name+"-"+nfspvc.Namespace+"-pv") - return err, false - } - pvDeleted = true - } - - if pvDeleted && pvcDeleted { // if the pv and the pvc were successfully deleted, remove the finalizer. - return removeFinalizer(ctx, nfspvc, log, k8sClient), true - } else if !pvDeleted && pvcDeleted { // if only the pvc was deleted successfully, the reconcile function will be triggered again for the pv cleanup. - // if the pv still exists, delete it. - if err := nfsPvcCleanUp(ctx, nfspvc, log, k8sClient); err != nil { - return err, false - } - pvName := nfspvc.Name + "-" + nfspvc.Namespace + "-pv" - return &FailedCleanUpError{Message: "the pv " + pvName + " is not deleted yet"}, false - } - - // if the pv and the pvc still exist, delete them. - if err := nfsPvcCleanUp(ctx, nfspvc, log, k8sClient); err != nil { - return err, false - } - } else { - return nil, true - } - } - return nil, false -} - -// nfsPvcCleanUp cleanup the pvc and the pv that related to the nfspvc. -func nfsPvcCleanUp(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, log logr.Logger, k8sClient client.Client) error { - pvc := &corev1.PersistentVolumeClaim{} - if err := deleteResource(ctx, types.NamespacedName{Name: nfspvc.Name, Namespace: nfspvc.Namespace}, pvc, log, k8sClient); err != nil { - return fmt.Errorf("failed to delete pvc - %s: %s", nfspvc.Name, err.Error()) - } - - pv := &corev1.PersistentVolume{} - if err := deleteResource(ctx, types.NamespacedName{Name: nfspvc.Name + "-" + nfspvc.Namespace + "-pv"}, pv, log, k8sClient); err != nil { - return fmt.Errorf("failed to delete pv - %s: %s", nfspvc.Name+"-"+nfspvc.Namespace+"-pv", err.Error()) - } - - return nil -} - -// deleteResource get resource to delete and delete that resource from the cluster. -func deleteResource(ctx context.Context, namespacedName types.NamespacedName, resource client.Object, log logr.Logger, k8sClient client.Client) error { - if err := k8sClient.Get(ctx, namespacedName, resource); err != nil { - if !errors.IsNotFound(err) { - log.Error(err, "unable to get resource") - return err - } - return nil - } - if err := k8sClient.Delete(ctx, resource); client.IgnoreNotFound(err) != nil { - log.Error(err, "unable to delete resource") - return err - } - return nil -} - -// removeFinalizer remove the dana finalizer from the nfspvc object. -func removeFinalizer(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, log logr.Logger, k8sClient client.Client) error { - controllerutil.RemoveFinalizer(&nfspvc, NfsPvcDeletionFinalizer) - if err := k8sClient.Update(ctx, &nfspvc); err != nil { - log.Error(err, "unable to remove the finalizer from the NfsPvc - "+nfspvc.Name) - return err - } - return nil -} - -// EnsureFinalizer ensures the nfspvc has the finalizer. -func EnsureFinalizer(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient client.Client, log logr.Logger) error { - if !controllerutil.ContainsFinalizer(&nfspvc, NfsPvcDeletionFinalizer) { - controllerutil.AddFinalizer(&nfspvc, NfsPvcDeletionFinalizer) - if err := k8sClient.Update(ctx, &nfspvc); err != nil { - log.Error(err, "unable to add the finalizer to the nfspvc - "+nfspvc.Name) - return err - } - } - return nil -} diff --git a/internal/controller/utils/status/status.go b/internal/controller/utils/status/status.go deleted file mode 100644 index 527b1ce..0000000 --- a/internal/controller/utils/status/status.go +++ /dev/null @@ -1,88 +0,0 @@ -package status - -import ( - "context" - "fmt" - - danaiov1alpha1 "github.com/dana-team/nfspvc-operator/api/v1alpha1" - "github.com/go-logr/logr" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/util/retry" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -const ( - StoragePhaseUnknown = "Unknown" - StoragePhaseNotFound = "NotFound" -) - -// SyncNfsPvcStatus fetches the phase of the pv and the pvc that is created by the nfspvc and updates the nfspvc status. -func SyncNfsPvcStatus(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, log logr.Logger, k8sClient client.Client) error { - nfspvcObject := danaiov1alpha1.NfsPvc{} - if err := k8sClient.Get(ctx, types.NamespacedName{Namespace: nfspvc.Namespace, Name: nfspvc.Name}, &nfspvcObject); err != nil { - if errors.IsNotFound(err) { - log.Info(fmt.Sprintf("Didn't find NfsPvc: %s, from the namespace: %s", nfspvc.Name, nfspvc.Namespace)) - return nil - } - return fmt.Errorf("failed to get NfsPvc: %s", err.Error()) - } - - pvcPhase := getPvcStatus(ctx, nfspvc, k8sClient) - pvPhase := getPvStatus(ctx, nfspvc, k8sClient) - - if pvcPhase != nfspvc.Status.PvcPhase || pvPhase != nfspvc.Status.PvPhase { - // Use retry on conflict to update the nfspvc status. - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - nfspvcObject.Status.PvcPhase = pvcPhase - nfspvcObject.Status.PvPhase = pvPhase - - updateErr := k8sClient.Status().Update(ctx, &nfspvcObject) - if errors.IsConflict(updateErr) { - // Conflict occurred, let's re-fetch the latest version of NFSPVC and retry the update. - if getErr := k8sClient.Get(ctx, types.NamespacedName{Name: nfspvcObject.Name, Namespace: nfspvcObject.Namespace}, &nfspvcObject); getErr != nil { - return getErr - } - } - return updateErr - }) - return err - } - - return nil -} - -// getPvcStatus return the Pvc's Phase. -func getPvcStatus(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient client.Client) string { - pvc := corev1.PersistentVolumeClaim{} - pvcPhase := "" - if err := k8sClient.Get(ctx, types.NamespacedName{Namespace: nfspvc.Namespace, Name: nfspvc.Name}, &pvc); err != nil { - if errors.IsNotFound(err) { - pvcPhase = StoragePhaseNotFound - } else { - pvcPhase = StoragePhaseUnknown - } - } else { - pvcPhase = string(pvc.Status.Phase) - } - - return pvcPhase -} - -// getPvStatus return the Pv's Phase. -func getPvStatus(ctx context.Context, nfspvc danaiov1alpha1.NfsPvc, k8sClient client.Client) string { - pv := corev1.PersistentVolume{} - pvPhase := "" - if err := k8sClient.Get(ctx, types.NamespacedName{Name: nfspvc.Name + "-" + nfspvc.Namespace + "-pv"}, &pv); err != nil { - if errors.IsNotFound(err) { - pvPhase = StoragePhaseNotFound - } else { - pvPhase = StoragePhaseUnknown - } - } else { - pvPhase = string(pv.Status.Phase) - } - - return pvPhase -}