Fix: TaskRuns stuck blocked from deletion due to old chains finaliser

[Liu-ko]

Chains v.0.27.0 started to use Service-Side-Apply mechanism to manage finalisers. SSA mechanism cannot modify finalisers previously added by merge patch method. As a result, deletion of TaskRuns that were triggered before the Chains update and got old finalisers is blocked forever.

This change is updating the existing removeOldFinalizerIfExists function to call removeFinalizerViaMergePatch to cleanup the old finaliser and allow future deletion of a TaskRun.

Fixes: #1755

Changes

Submitter Checklist

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

• Has Docs included if any changes are user facing
• Has Tests included if any functionality added or changed
• Follows the commit message standard
• Meets the Tekton contributor standards (including
  functionality, content, code)
• Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
• Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

[tekton-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign puneetpunamiya after the PR has been reviewed.
You can assign the PR to them by writing /assign @puneetpunamiya in a comment when ready.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[jkhelil]

@enarha Can you review please ?

[enarha]

Thank you for the fix. I'll suggest the following instead. Since finalizer with the old name is guaranteed to be set via merge patch, we can simply use the removeFinalizerViaMergePatch function for it. Something like this will be sufficient IMO

  func (r *Reconciler) removeFinalizerViaMergePatch(ctx context.Context, tr *v1.TaskRun) error {
      const oldFinalizerName = "chains.tekton.dev" // add the const
      var newFinalizers []string
      for _, f := range tr.Finalizers {
          if f != taskRunFinalizerName && f != oldFinalizerName {  // one extra condition
              newFinalizers = append(newFinalizers, f)
          }
      }
      // ... unchanged

This only adds the const and the extra f != oldFinalizerName condition. Then we can drop the removeOldFinalizerIfExists function and its calls entirely. I would also like to have an extra test for this case.
On a separate note, can you please add a short release note as I would like to see it included in the release notes.