[TEP-0135] Purge finalizer and delete PVC

[QuanZhang-William]

Changes

Part of #6740 and partially completes the PVC deletion behavior discussion.

Prior to this commit, the PVCs created from PipelineRun's VolumeClaimTemplate are not auto deleted when the owning PipelineRun is completed.

This commit updates the cleanupAffinityAssistantsAndPVCs function to remove the kubernetes.io/pvc-protection finalizer protection (so that the pvc is allowed to be deleted while the pod consuming it is not deleted). The function then explicitly delete such PVCs when cleaning up the Affinity Assistants at the PipelineRun completion time.

This change is NOT applied to coschedule: workspaces mode as there is backward compatability concern. See more details in this discussion

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

• Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
• Has Tests included if any functionality added or changed
• Follows the commit message standard
• Meets the Tekton contributor standards (including functionality, content, code)
• Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
• Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
• Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

[QuanZhang-William]

/kind feature

[tekton-robot]

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/affinity_assistant.go	97.2%	94.9%	-2.3
pkg/reconciler/volumeclaim/pvchandler.go	90.0%	83.3%	-6.7

[tekton-robot]

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/affinity_assistant.go	97.2%	94.9%	-2.3
pkg/reconciler/volumeclaim/pvchandler.go	90.0%	83.3%	-6.7

[lbernick]

My understanding is that the finalizer prevents PVCs from being deleted when the underlying PVs still exist (not when they are still bound to a pod). If we remove the finalizer and delete the PVC, won't there still be a PV taking up storage space? Can we delete the PV as well?

[jimmyjones2]

@lbernick Kubernetes puts the finalizer on PVCs when there is a pod referencing it (including finished pods) to prevent deletion. Previous to this feature you could delete PVCs that were currently in use by a pod! A number of projects that manage pods themselves remove this finalizer, as they "know" the PVC is no longer needed.

Deleting a PVC with the finalizer removed will cause the underlying PV to be deleted as normal.

The finalizer is also used on a PV to prevent them from being deleted if there is an associated PVC using it, but this PR isn't touching that.

https://kubernetes.io/docs/concepts/storage/persistent-volumes/#storage-object-in-use-protection

[lbernick]

thanks for the explanation @jimmyjones2! Quan, can you double check that PVs are deleted with this PR when the PVCs are deleted?

[QuanZhang-William]

thanks for the explanation @jimmyjones2! Quan, can you double check that PVs are deleted with this PR when the PVCs are deleted?

Thanks for the explanation @jimmyjones2! @lbernick , yes I doubled checked that the PV and PVCs are deleted when the PR is completed:

k get pr -w
NAME                                    SUCCEEDED   REASON    STARTTIME   COMPLETIONTIME
demo-pipeline-run-template-only-kqntk   Unknown     Running   38s         
demo-pipeline-run-template-only-kqntk   True        Succeeded   54s         0s

k get pvc
No resources found in default namespace.

k get pv
No resources found

[tekton-robot]

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/affinity_assistant.go	96.5%	94.4%	-2.1
pkg/reconciler/volumeclaim/pvchandler.go	90.0%	83.3%	-6.7

[tekton-robot]

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/affinity_assistant.go	96.5%	94.4%	-2.1
pkg/reconciler/volumeclaim/pvchandler.go	90.0%	83.3%	-6.7

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lbernick

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [lbernick]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[tekton-robot]

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/affinity_assistant.go	96.5%	94.4%	-2.1
pkg/reconciler/volumeclaim/pvchandler.go	90.0%	83.3%	-6.7

[tekton-robot]

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/affinity_assistant.go	96.5%	94.4%	-2.1
pkg/reconciler/volumeclaim/pvchandler.go	90.0%	83.3%	-6.7

[QuanZhang-William]

/test pull-tekton-pipeline-alpha-integration-tests

due to timeout

[Yongxuanzhang]

/lgtm