From 9dd26168d6d7205919867fc537e76340d85f38aa Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Thu, 26 May 2022 18:21:23 +0900 Subject: [PATCH] Fix confusing logs from pv and pvc controllers (#1487) Ref https://github.com/actions-runner-controller/actions-runner-controller/issues/1321#issuecomment-1137431212 --- .../persistent_volume_claim_controller.go | 2 -- controllers/sync_volumes.go | 20 ++++++++++++------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/controllers/persistent_volume_claim_controller.go b/controllers/persistent_volume_claim_controller.go index 461908ab..0405e771 100644 --- a/controllers/persistent_volume_claim_controller.go +++ b/controllers/persistent_volume_claim_controller.go @@ -50,8 +50,6 @@ func (r *RunnerPersistentVolumeClaimReconciler) Reconcile(ctx context.Context, r return ctrl.Result{}, client.IgnoreNotFound(err) } - log.Info("Reconciling runner pvc") - res, err := syncPVC(ctx, r.Client, log, req.Namespace, &pvc) if res == nil { diff --git a/controllers/sync_volumes.go b/controllers/sync_volumes.go index 8e83a3d3..fccd3432 100644 --- a/controllers/sync_volumes.go +++ b/controllers/sync_volumes.go @@ -73,6 +73,8 @@ func syncPVC(ctx context.Context, c client.Client, log logr.Logger, ns string, p return nil, nil } + log.V(2).Info("Reconciling runner PVC") + var sts appsv1.StatefulSet if err := c.Get(ctx, types.NamespacedName{Namespace: ns, Name: stsName}, &sts); err != nil { if !kerrors.IsNotFound(err) { @@ -85,7 +87,7 @@ func syncPVC(ctx context.Context, c client.Client, log logr.Logger, ns string, p return &ctrl.Result{RequeueAfter: retry}, nil } - log = log.WithValues("pvc", pvc.Name, "sts", stsName) + log = log.WithValues("sts", stsName) pvName := pvc.Spec.VolumeName @@ -108,7 +110,7 @@ func syncPVC(ctx context.Context, c client.Client, log logr.Logger, ns string, p } pvCopy.Labels[labelKeyCleanup] = stsName - log.Info("Scheduling to unset PV's claimRef", "pv", pv.Name) + log.V(2).Info("Scheduling to unset PV's claimRef", "pv", pv.Name) // Apparently K8s doesn't reconcile PV immediately after PVC deletion. // So we start a relatively busy loop of PV reconcilation slightly before the PVC deletion, @@ -117,14 +119,18 @@ func syncPVC(ctx context.Context, c client.Client, log logr.Logger, ns string, p return nil, err } + log.Info("Updated PV to unset claimRef") + // At this point, the PV is still Bound - log.Info("Deleting unused pvc") + log.V(2).Info("Deleting unused PVC") if err := c.Delete(ctx, pvc); err != nil { return nil, err } + log.Info("Deleted unused PVC") + // At this point, the PV is still "Bound", but we are ready to unset pv.spec.claimRef in pv controller. // Once the pv controller unsets claimRef, the PV becomes "Released", hence available for reuse by another eligible PVC. } @@ -133,13 +139,11 @@ func syncPVC(ctx context.Context, c client.Client, log logr.Logger, ns string, p } func syncPV(ctx context.Context, c client.Client, log logr.Logger, ns string, pv *corev1.PersistentVolume) (*ctrl.Result, error) { - log.V(2).Info("checking pv claimRef") - if pv.Spec.ClaimRef == nil { return nil, nil } - log.V(2).Info("checking labels") + log.V(2).Info("Reconciling PV") if pv.Labels[labelKeyCleanup] == "" { // We assume that the pvc is shortly terminated, hence retry forever until it gets removed. @@ -162,11 +166,13 @@ func syncPV(ctx context.Context, c client.Client, log logr.Logger, ns string, pv pvCopy := pv.DeepCopy() delete(pvCopy.Labels, labelKeyCleanup) pvCopy.Spec.ClaimRef = nil - log.Info("Unsetting PV's claimRef", "pv", pv.Name) + log.V(2).Info("Unsetting PV's claimRef", "pv", pv.Name) if err := c.Update(ctx, pvCopy); err != nil { return nil, err } + log.Info("PV should be Available now") + // At this point, the PV becomes Available, if it's reclaim policy is "Retain". // I have not yet tested it with "Delete" but perhaps it's deleted automatically after the update? // https://kubernetes.io/docs/concepts/storage/persistent-volumes/#retain