Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doc: How to use generate name to replay Task #685

Closed
skeeey opened this issue Mar 27, 2019 · 23 comments
Closed

Doc: How to use generate name to replay Task #685

skeeey opened this issue Mar 27, 2019 · 23 comments
Labels
design This task is about creating and discussing a design help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/question Issues or PRs that are questions around the project or a particular feature meaty-juicy-coding-work This task is mostly about implementation!!! And docs and tests of course but that's a given

Comments

@skeeey
Copy link

skeeey commented Mar 27, 2019

Expected Behavior

It should be possible to use generateName like functionality to create TaskRun yaml definitions such that the definition in a yaml file can be reapplied.

Actual Behavior

I cannot reapply a taskrun when the taskrun fails

Steps to Reproduce the Problem

  1. apply a taskrun with a broken task reference
apiVersion: tekton.dev/v1alpha1
kind: TaskRun
metadata:
  name: echo-hello-world-task-run
  namespace: test
spec:
  taskRef:
    name: hello-world # this task does not exist
  trigger:
    type: manual

then check the taskrun status by kubectl get taskrun echo-hello-world-task-run -o yaml

...
status:
  conditions:
  - lastTransitionTime: 2019-03-27T05:18:49Z
    message: error when listing tasks for taskRun echo-hello-world-task-run task.tekton.dev
      "hello-world" not found
...
  1. correct the task reference in taskrun and apply it again
apiVersion: tekton.dev/v1alpha1
kind: TaskRun
metadata:
  name: echo-hello-world-task-run
  namespace: test
spec:
  taskRef:
    name: echo-hello-world # this task exists
  trigger:
    type: manual
kubectl apply -f helloworld-taskrun.yaml

taskrun.tekton.dev/echo-hello-world-task-run configured
  1. the taskrun still failed, note: the task reference has been changed
...

spec:
  Status: ""
  inputs: {}
  outputs: {}
  taskRef:
    kind: Task
    name: echo-hello-world # a right task reference
  trigger:
    type: manual
status:
  conditions:
  - lastTransitionTime: 2019-03-27T05:18:49Z
    message: error when listing tasks for taskRun echo-hello-world-task-run task.tekton.dev
      "nonexistent-hello-world" not found
    reason: TaskRunResolutionFailed
    status: "False"
    type: Succeeded

Additional Info

@hrishin
Copy link
Member

hrishin commented Mar 27, 2019

Thanks for reporting this issue @skeeey. One way to solve this issue is to delete the taskrun and create it again or create the taskrun with a new name (could document it for the time being if its not done yet) 👍 . Totally agree with the pain of it. However, this is how in general kubrenetes works, right? I mean unless a resource is not changed it doesn't accept any modification. :(

It may be addressed in tekton CLI, where it says tc run task-name, it could start new taskrun instance or we can discuss the design proposal for it.

Any other suggestions or thoughts @vdemeester @bobcatfish ?

@vdemeester
Copy link
Member

Agreeing with @hrishin 👼

That said, there is a concept of generateName in kubernetes (on ObjectMeta) that we could maybe use and support in tekton — what would happen is each time you apply the yaml, it creates a new taskrun from the prefix you specified in generatedName.

GenerateName is an optional prefix, used by the server, to generate a unique name ONLY IF the Name field has not been provided. If this field is used, the name returned to the client will be different than the name passed. This value will also be combined with a unique suffix. The provided value has the same validation rules as the Name field, and may be truncated by the length of the suffix required to make the value unique on the server. If this field is specified and the generated name exists, the server will NOT return a 409 - instead, it will either return 201 Created or 500 with Reason ServerTimeout indicating a unique name could not be found in the time allotted, and the client should retry (optionally after the time indicated in the Retry-After header).

cc @bobcatfish

@chmouel
Copy link
Member

chmouel commented Mar 27, 2019

I think it may be better to have an incremental number (ie -1 -2 -3 etc..) than random suffix,

@skeeey
Copy link
Author

skeeey commented Mar 28, 2019

@hrishin

I mean unless a resource is not changed it doesn't accept any modification.

I think in this case, the resource is changed, I change the taskRef from

taskRef:
    name: hello-world # this task does not exist

to

taskRef:
    name: echo-hello-world # this task exists

from the code, the problem is the judgement IsDone(), this function check the taskrun condition status, if the condition status is not Unknown the reconciler will think taskrun is done (Whether the taskrun is success or failure)

In my case, at the first round, the reconciler handles the taskrun, because the taskrun has a wrong taskRef, so it failed, and the reconciler set the taskrun condition status to False, then, at the second round, I correct the taskRef and reapply it, the k8s send update event, the taskrun controller handle this event, but because the taskrun condition status has been set to False at the first round, the reconciler think the updated taskrun still is done, the reconciler does not handle the taskrun any more.

I understand the IsDone() judgement is used to avoid to update the same taskrun when it is running (because the taskrun status is changed), but for my case, I change the spec of a taskrun, I think for a user this means the user want to update the taskrun, so for reconciler, it should restart the taskrun to update

So I think we may need to enhance the taskrun update handler, currently, the code always pass a new taskrun to reconcile, I think we may need to control the update, e.g. if a taskrun sepc is changed, then we update the taskrun, the code may be

taskRunInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
		AddFunc: impl.Enqueue,
		UpdateFunc: func(old, new interface{}) {
                        oldtaskrun := old.(*v1alpha1.TaskRun)
			newtaskrun := new.(*v1alpha1.TaskRun)
			if !reflect.DeepEqual(oldtaskrun.Spec, newtaskrun.Spec) {
			       impl.Enqueue(new)
                        }
		},
	})

then we need remove the IsDone() judgement in reconcile (if the taskrun is not success done, we may need to cancel it)

Any suggestions or thoughts @hrishin @vdemeester @bobcatfish @chmouel?

@vdemeester
Copy link
Member

My thought is : a TaskRun is "immutable" (in its weird way), meaning once a taskrun has been successfully created, it shouldn't be updated (or allowed to be updated). I would make the parallel with docker containers : you can run as many containers from the same image, but they will all, in the end, have a different id (and names potentially).

That's why I think we should look into supporting generateName (as it's a standard construction in the k8s ecosystem).

@skeeey
Copy link
Author

skeeey commented Mar 28, 2019

@vdemeester thanks for your clarify, If we don't want to update a taskrun, I think the generatedName is a good solution

@zxDiscovery
Copy link

If we use generatedName, the command kubectl apply should be replaced by kubectl create.
Do we need to update the Document? @vdemeester

root@inflict1:~/ICP/sert-test/pipeline# kubectl apply -f pipelinerun.yaml
error: error when retrieving current configuration of:
Resource: "tekton.dev/v1alpha1, Resource=pipelineruns", GroupVersionKind: "tekton.dev/v1alpha1, Kind=PipelineRun"
Name: "", Namespace: "default"
Object: &{map["apiVersion":"tekton.dev/v1alpha1" "kind":"PipelineRun" "metadata":map["generateName":"demo-pipeline-run-" "namespace":"default" "annotations":map["kubectl.kubernetes.io/last-applied-configuration":""]] "spec":map["trigger":map["type":"manual"] "pipelineRef":map["name":"demo-pipeline"] "resources":[map["name":"source-repo" "resourceRef":map["name":"sert-git"]]] "serviceAccount":"test-build-robot-git-ssh"]]}
from server for: "pipelinerun.yaml": resource name may not be empty

@runzexia
Copy link

runzexia commented Apr 3, 2019

How about refusing a user to modify a Run that has ended by admission webhooks?

@vdemeester vdemeester added design This task is about creating and discussing a design kind/question Issues or PRs that are questions around the project or a particular feature labels Apr 5, 2019
@vdemeester
Copy link
Member

@runzexia that could be an option. Right now our admission webhooks are pretty simple though (aka they don't use the k8s api)

@zxDiscovery maybe yes 👼 This works as is with generatedName or do we need to implement something on our end ?

@dlorenc
Copy link
Contributor

dlorenc commented Apr 23, 2019

How about refusing a user to modify a Run that has ended by admission webhooks?

Yeah this makes sense to me. If we're going to stray from k8s and prevent modifications, we should enforce that somehow.

@bobcatfish
Copy link
Collaborator

@skeeey thanks for the detailed explanation of the UpdateFunc solution, made it really clear 🎉 !! I agree with the overall decision to make Runs immutable and use generateName

How about refusing a user to modify a Run that has ended by admission webhooks?

Agree! If someone wants to write this up in an issue I'd be super grateful! :D

@bobcatfish bobcatfish added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. meaty-juicy-coding-work This task is mostly about implementation!!! And docs and tests of course but that's a given labels May 9, 2019
@bobcatfish
Copy link
Collaborator

I've updated the "expected behavior" in the description to reflect our discussion

@bobcatfish bobcatfish changed the title Cannot reapply a taskrun when the taskrun fails Doc: How to use generate name to replay Task Jul 9, 2019
@guillermobandres
Copy link

Hi all, I was deploying Tekton in our AKS to do CI/CD, and I ran into the problem to replay taskRuns and PipelineRuns. I understand what is the current behavior, but can someone tell me how is doing the CI/CD with this behavoir?? Is it mandatory to delete the TaskRuns or PipelineRuns and recreate? Are you creating Runs with differents names?

Thank you

@hrishin
Copy link
Member

hrishin commented Aug 27, 2019

Is it mandatory to delete the TaskRuns or PipelineRuns and recreate?

Yes

Are you creating Runs with different names?

Yes, some ways are already mentioned in the above thread. Other ways to start the pipeline is to use tkn start command in case you haven't come across. Does that answers your question?

@guillermobandres
Copy link

Thank you @hrishin, yes it does!

Is there any way to automatize this process or anyone has this process automatized?

@vdemeester
Copy link
Member

@guillermobandres so, tektoncd/pipeline is one component of a full CI/CD system (being jenkins-x or something else).

An example is the use of tektoncd/triggers, that, for each events, generate a unique name for your runtime types (PipelineRun, TaskRun), with a prefix (usually the name of the pipeline, can be something else). This is also how the tkn cli does it with tkn pipeline start or (soon to be) tkn task start.

@bobcatfish bobcatfish added this to Needs triage in Tekton Pipelines Feb 26, 2020
@vdemeester
Copy link
Member

I am willing to close this issue as, as of today you can use generateName.

/cc @bobcatfish @dibyom @sbwsg

@ghost
Copy link

ghost commented Mar 16, 2020

👍 sgtm

@vdemeester
Copy link
Member

/close

Tekton Pipelines automation moved this from Needs triage to Closed Mar 16, 2020
@tekton-robot
Copy link
Collaborator

@vdemeester: Closing this issue.

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@rdelpret
Copy link

rdelpret commented Sep 26, 2020

kubectl rollout restart taskrun/mytaskrun would be pretty sweet.

@tuananh
Copy link

tuananh commented Oct 4, 2021

is the workaround with generateName documneted somewhere?

@jonashackt
Copy link

@tuananh I don't know if it's documented in the official docs, but I wrote an Q&A here https://stackoverflow.com/a/69880097/4964553

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design This task is about creating and discussing a design help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/question Issues or PRs that are questions around the project or a particular feature meaty-juicy-coding-work This task is mostly about implementation!!! And docs and tests of course but that's a given
Projects
No open projects
Tekton Pipelines
  
Closed
Development

No branches or pull requests