Results doesn't work on Windows

wilstdu commented 2 years ago

Hey, I'm not able to reference results from the task on Windows nodes, in Linux it works without any problems.

Tekton Pipeline version: v0.35.1

Pipeline definition:

apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
name: results-win
labels:
framework: tekton-framework
spec:
tasks:
- name: get-commit-hash
taskSpec:
  results:
  - name: commit-hash
  steps:
  - image: "mcr.microsoft.com/windows/servercore:1809"
    name: doit
    script: |
      #!win powershell.exe -File
      "Dummy message" | Out-File -FilePath $(results.commit-hash.path)

- name: print-commit-hash
params:
  - name: hash
    value: $(tasks.get-commit-hash.results.commit-hash)
taskSpec:
  params:
    - name: hash
      type: string
  steps:
  - image: "mcr.microsoft.com/windows/servercore:1809"
    name: doit
    script: |
      #!win powershell.exe -File
      Write $(params.hash)
runAfter:
- get-commit-hash

Getting this error in PipelineRun:

Results documentation (https://tekton.dev/docs/pipelines/tasks/#emitting-results) states that results files are created in hardcoded /tekton/results directory, which won't work in Windows.

Am I doing something wrong, or this is actually a bug?

imjasonh commented 2 years ago

@aiden-deloryn as one of the folks who's used Tekton on Windows the most, do you have any experience or insight to share here? I've never tried to use results on Windows, it's possible this is just broken.

Even worse, without automated tests to ensure it works end-to-end, even if it works today, as we make changes to how results are stored we might accidentally break Windows in the future. 😢

aiden-deloryn commented 2 years ago

@imjasonh I haven't actually used results on Windows either unfortunately. I'm not surprised that it does not work. I don't know much about the implementation of results, but I'll have a look into it a bit more when I have some spare time.

E2E tests for Windows would be very helpful...

imjasonh commented 2 years ago

Did some digging about termination message paths on Windows, and found https://github.com/kubernetes/kubernetes/issues/68791 and this comment in the code:

// adding TerminationMessagePath on Windows is only allowed if ContainerD is used. Individual files cannot // be mounted as volumes using Docker for Windows.

So that's one thing to be aware of -- termination messages (which results currently rely on) only work on Windows if you're using containerd.

Other than that I'm not sure why writing to /tekton/results won't work -- if there's some Windows-specific path we need we can probably add that to the entrypointer with a build constraint for Windows, or a runtime check for if runtime.GOOS == "windows".

aiden-deloryn commented 2 years ago

@imjasonh That's an interesting find regarding terminationMessagePath only working for ContainerD. Unfortunately I only have a Windows node running Docker so I was not able to test whether results work in that environment.

Could you clarify what you mean by "writing to /tekton/results won't work"? I had a look through the source code and as far as I can tell (please correct me if I am wrong) Tekton doesn't write user-defined results to that directory.

I can confirm that in the example above, "Dummy message" | Out-File -FilePath $(results.commit-hash.path) does result in a file being created at C:\tekton\results\commit-hash with the value Dummy message.

The process seems to be something like this:

A task writes a value to a file (e.g. C:\tekton\results\commit-hash)
Tekton collects these results and stores them in a file at terminationMessagePath
Kubernetes populates the termination message status.containerStatuses.state.terminated.message using the value from terminationMessagePath
The receiving task reads the results from PipelineRunState (I'm a bit unsure about this part)

imjasonh commented 2 years ago

You're right, and thanks for digging into this. (1) and (2) seem to work, I wonder if it's (3) that's reliant on containerd and not working in our case.

As for (4), the TaskRun controller reads the Pod's .terminated.message and writes that to the TaskRun's .status.results. The PipelineRun controller reads those and passes those into downstream TaskRuns' .inputs if any, which are replaced inline when the Pod spec is generated from that downstream TaskRun. That's a lot of detail, but the upshot is that all of that happens in Tekton's controllers, which currently always run on Linux nodes, and it isn't dependent on the underlying OS anyway. So (3) is the likely culprit I think.

Can someone demonstrate that a Windows Pod writing to /dev/termination-log (or any configured .spec.terminationMessagePath results in that value showing up in .termination.message?

edit: more docs about termination messages, in case it's useful: https://kubernetes.io/docs/tasks/debug/debug-application/determine-reason-pod-failure/

tekton-robot commented 2 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale with a justification. Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with /close with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

tekton-robot commented 2 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten with a justification. Rotten issues close after an additional 30d of inactivity. If this issue is safe to close now please do so with /close with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

tekton-robot commented 1 year ago

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen with a justification. Mark the issue as fresh with /remove-lifecycle rotten with a justification. If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

tekton-robot commented 1 year ago

@tekton-robot: Closing this issue.

In response to [this](https://github.com/tektoncd/pipeline/issues/5102#issuecomment-1362928248): >Rotten issues close after 30d of inactivity. >Reopen the issue with `/reopen` with a justification. >Mark the issue as fresh with `/remove-lifecycle rotten` with a justification. >If this issue should be exempted, mark the issue as frozen with `/lifecycle frozen` with a justification. > >/close > >Send feedback to [tektoncd/plumbing](https://github.com/tektoncd/plumbing). Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

doctorpangloss commented 1 year ago

This should be reopened, it is a real issue.

jerop commented 1 year ago

/lifecycle frozen

tektoncd / pipeline

Results doesn't work on Windows #5102