Old resource deletion hangs when `alpha` flag is off

[chuangw6]

Expected Behavior

Users should be able to delete any taskrun object using kubectl delete tr --all no matter if alpha feature flag is enabled.

Actual Behavior

After a new pipeline controller is deployed with alpha feature flag off, kubectl delete tr --all command hangs on the taskruns that worked fine under previous controller with alpha feature flag enabled.

Steps to Reproduce the Problem

1. Clone the pipeline repo. Change stable feature flag to alpha in this file.
2. Run ko apply -R -f config/
3. After the new controller is running, run the example yaml. You can see the pipelinerun will be successfully executed.
4. Back to step 1, change the flag back to stable
5. Rerun step 2
6. Run kubectl delete tr --all to delete all taskruns that were previously created. You will see the deletion is hanging.

Additional Info

• Kubernetes version:

  Output of kubectl version:

  Client Version: version.Info{Major:"1", Minor:"23", GitVersion:"v1.23.1", GitCommit:"86ec240af8cbd1b60bcc4c03c20da9b98005b92e", GitTreeState:"clean", BuildDate:"2021-12-16T11:41:01Z", GoVersion:"go1.17.5", Compiler:"gc", Platform:"linux/amd64"}
  Server Version: version.Info{Major:"1", Minor:"21", GitVersion:"v1.21.12-gke.1500", GitCommit:"6c11aec6ce32cf0d66a2631eed2eb49dd65c89f8", GitTreeState:"clean", BuildDate:"2022-05-11T09:25:37Z", GoVersion:"go1.16.15b7", Compiler:"gc", Platform:"linux/amd64"}
  WARNING: version difference between client (1.23) and server (1.21) exceeds the supported minor version skew of +/-1
  

• Tekton Pipeline version:

  Output of tkn version or kubectl get pods -n tekton-pipelines -l app=tekton-pipelines-controller -o=jsonpath='{.items[0].metadata.labels.version}'

  latest main
  

[vdemeester]

@chuangw6 any additionnal elements you can see ? Can you share the logs of the controller and webhook ?

[abayer]

I can't reproduce this - deletion went through right away for me...

[chuangw6]

Hi @vdemeester ,

• controller log
• webhook log.

If you search the keyworkd alpha in the controller log, you can see the complaints about it. This alpha flag was added here.

[chuangw6]

I can't reproduce this - deletion went through right away for me...

Hi @abayer ,
Instead of following step 4 & 5 to turn off alpha flag, can you try to just apply the latest release to see if you can reproduce? Thanks

kubectl apply -f https://storage.googleapis.com/tekton-releases/pipeline/previous/v0.38.0/release.yaml

[abayer]

@chuangw6 Aaaaah - that's fixed in #5215.

[chuangw6]

@chuangw6 Aaaaah - that's fixed in #5215.

Thanks @abayer for the pointer. I pulled the latest main and followed the above steps.
Now the controller doesn't have the complaints, but deleting the old taskruns still hangs.

Any idea?

[abayer]

I was looking at the webhook logs, which is where I saw the conversion errors. Could you post your new webhook log with latest main?

[abayer]

Aaaah, I see https://gist.github.com/chuangw6/c1907a1d1f10a2962e9718b0c21ca4fd#file-pipeline-webhook-log-L342 - that's a problem for sure, but it's fixed in #5210.

[chuangw6]

I was looking at the webhook logs, which is where I saw the conversion errors. Could you post your new webhook log with latest main?

sure!

• new webhook log
• new controller log

[abayer]

Interesting - https://gist.github.com/chuangw6/7c68774399baaf689a7bc0aa189c5ded#file-new-webhook-log-L276 is there, with references to Chains, though I think that's just service account stuff?

[chuangw6]

Interesting - https://gist.github.com/chuangw6/7c68774399baaf689a7bc0aa189c5ded#file-new-webhook-log-L276 is there,

Interesting. Why can pipeline webhook pick up chains-related stuff? lol

This is chains Controller log. It complaints about alpha flag too.

with references to Chains, though I think that's just service account stuff?

But the line[system:serviceaccounts system:serviceaccounts:tekton-chains system:authenticated] says it's authenticated.

[vdemeester]

It's on an UPDATE event, so it does validation and fails there. Chains tries to update the object, but cannot because some fields are behind alpha and stable is set. I am not sure why it would block deletion, except maybe if it fails to sign, put the object into the queue to retry, making it somehow un-deletable ?

@chuangw6 can you try to disable chains and see if that happens ?

[chuangw6]

It's on an UPDATE event, so it does validation and fails there. Chains tries to update the object, but cannot because some fields are behind alpha and stable is set. I am not sure why it would block deletion, except maybe if it fails to sign, put the object into the queue to retry, making it somehow un-deletable ?

@chuangw6 can you try to disable chains and see if that happens ?

Hi @vdemeester,
I tried two things:

1. Had a new cluster with only pipeline installed. No resource deletion hanging problem at all under either stable or alpha flag.

2. Tried to disable chains in my old cluster where both chains and pipelines are installed.
   I tried to disable chains by two ways

   • scale the controller 0.
   • delete all the resources under chains namespace by running ko delete -R -f config/.

   Neither of them works. And things even get tricker: after chains is disabled, I cannot delete the taskruns after alpha is turn on again.

[wlynch]

It's on an UPDATE event, so it does validation and fails there. Chains tries to update the object, but cannot because some fields are behind alpha and stable is set. I am not sure why it would block deletion, except maybe if it fails to sign, put the object into the queue to retry, making it somehow un-deletable ?

^ @vdemeester this is correct. Chains uses a finalizer to make sure that Run objects don't disappear before they can be attested. Normally, the finalizer is removed once chains has processed the object, but because the alpha flag was disabled, chains can't complete its reconcile (probably when trying to add the chains.tekton.dev/signed annotation) so the finalizer is never removed.

I don't think chains is unique here - any client that tries to update a beta object with alpha fields would fail in a similar way. This kind of unexpected behavior was what I was worried about in #5005 (comment) (cc @lbernick @afrittoli)