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

Release Pipeline failing #5174

Closed
dibyom opened this issue Jul 19, 2022 · 12 comments · Fixed by #5180
Closed

Release Pipeline failing #5174

dibyom opened this issue Jul 19, 2022 · 12 comments · Fixed by #5180
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@dibyom
Copy link
Member

dibyom commented Jul 19, 2022

Expected Behavior

Release pipeline works

Actual Behavior

Pipeline fails at the run-ko step:

2022/07/19 18:39:13 Unexpected error running "go build": exit status 2
# github.com/tektoncd/pipeline/cmd/entrypoint
cmd/entrypoint/main.go:142:4: unknown field 'stdoutPath' in struct literal of type realRunner
cmd/entrypoint/main.go:143:4: unknown field 'stderrPath' in struct literal of type realRunner

Steps to Reproduce the Problem

Additional Info

Full logs: https://gist.github.com/dibyom/81be5cb43e342cbcb77b9bea0926e0a3

Run: https://dashboard.dogfooding.tekton.dev/#/namespaces/default/pipelineruns/pipeline-release-run-z8vht?pipelineTask=publish-images

  • Tekton Pipeline version: v0.37.1

Go version: 1.17.8

@dibyom dibyom added the kind/bug Categorizes issue or PR as related to a bug. label Jul 19, 2022
@dibyom
Copy link
Member Author

dibyom commented Jul 19, 2022

I tried the failing command ko resolve --platform=linux/amd64,linux/arm,linux/arm64,linux/s390x,linux/ppc64le,windows/amd64 --preserve-import-paths -t v0.38.0 -R -f config/ locally on go version go1.18.3 darwin/amd64 and it worked

@dibyom
Copy link
Member Author

dibyom commented Jul 19, 2022

cc @jerop

@jerop
Copy link
Member

jerop commented Jul 20, 2022

another release pipeline failure reported in #5175

@jerop jerop self-assigned this Jul 20, 2022
@abayer
Copy link
Contributor

abayer commented Jul 20, 2022

Ok, via docker run -it gcr.io/tekton-releases/dogfooding/ko@sha256:aafcfa5c22281f9e882664c3e5f234a2069a0050674c0c8d9908ccae6f0e8457 and manually replicating the commands leading up to the ko resolve and then running ko resolve ... with --push=false, I've reproduced the error, and narrowed it down to having windows/amd64 in the platforms. And more specifically, it's only when .ko.yaml is updated to use gcr.io/tekton-releases/github.com/tektoncd/pipeline/combined-base-image@sha256:cabda55b027488777f06576dcb615779640ea9e768784b2720215b91e14cc9e0 as the base image for cmd/entrypoint, which is done by

COMBINED_BASE_IMAGE=$(go run ./vendor/github.com/tektoncd/plumbing/cmd/combine/main.go \
ghcr.io/distroless/static \
mcr.microsoft.com/windows/nanoserver:ltsc2022 \
${CONTAINER_REGISTRY}/$(params.package)/combined-base-image:latest)
cat <<EOF > ${PROJECT_ROOT}/.ko.yaml
# This matches the value configured in .ko.yaml
defaultBaseImage: ghcr.io/distroless/static
baseImageOverrides:
# Use the combined base image for images that should include Windows support.
$(params.package)/cmd/entrypoint: ${COMBINED_BASE_IMAGE}
$(params.package)/cmd/nop: ${COMBINED_BASE_IMAGE}
$(params.package)/cmd/workingdirinit: ${COMBINED_BASE_IMAGE}
# This matches values configured in .ko.yaml
$(params.package)/cmd/git-init: ghcr.io/distroless/git
EOF

That said, it doesn't even try to build Windows images without that base image override.

I'm trying rebuilding the ko image with Go 1.18.3 (it's based on Go 1.18.1 currently) and will see if I get the same result.

@abayer
Copy link
Contributor

abayer commented Jul 20, 2022

Dang, still failed with 1.18.3.

@abayer
Copy link
Contributor

abayer commented Jul 20, 2022

Ah-ha! cmd/entrypoint/runner.go, where realRunner and its fields are defined, has //go:build !windows - so a0c7d31#diff-d9a50b24352d0a286b3e339692447eb55a219654e06e0fd71ad373de71b8c265R141-R144 broke compilation on Windows, period. I wonder if cmd/entrypoint/main.go ever actually worked on Windows, but changing from

Runner:              &realRunner{},

to

Runner: &realRunner{
	stdoutPath: *stdoutPath,
	stderrPath: *stderrPath,
},

definitely breaks compilation.

I think we may need to revert #4882, if there isn't a quick fix.

@abayer
Copy link
Contributor

abayer commented Jul 20, 2022

cc @tektoncd/core-maintainers to see what people's thoughts are as to how we should move forward.

@afrittoli
Copy link
Member

The windows entrypoint struct has not been changed, so I think an option could be to make the code able to cope with the windows version too, and declare in the release notes that the feature is not available on Windows for now.

Alternatively we could revert the change.
Testing this on Windows would take longer as we don't have windows CI infrastructure.

@imjasonh
Copy link
Member

To get this to build, add stdoutPath and stderrPath to realRunner defined in https://github.com/tektoncd/pipeline/blob/main/cmd/entrypoint/runner_windows.go#L34

Then to @afrittoli's point we probably need to add code specific to Windows to fail if these fields are set with a helpful error message indicating they're not supported on Windows.

abayer added a commit to abayer/tektoncd-pipeline that referenced this issue Jul 20, 2022
closes tektoncd#5174

This fixes the release pipeline issue with building `cmd/entrypoint` on Windows, and returns an error if `StdoutPath` or `StderrPath` are specified in a Windows step.

Signed-off-by: Andrew Bayer <[email protected]>
tekton-robot pushed a commit that referenced this issue Jul 20, 2022
closes #5174

This fixes the release pipeline issue with building `cmd/entrypoint` on Windows, and returns an error if `StdoutPath` or `StderrPath` are specified in a Windows step.

Signed-off-by: Andrew Bayer <[email protected]>
@dibyom
Copy link
Member Author

dibyom commented Jul 20, 2022

Hmmm....I'm surprised we did not catch this in our nightly builds

@abayer
Copy link
Contributor

abayer commented Jul 20, 2022

@dibyom We don’t build Windows images anywhere other than releases, I think. Well, until #5178 at least.

@dibyom
Copy link
Member Author

dibyom commented Jul 21, 2022

I thought the nightly build pipeline and our release pipeline were the exact same..but looks like that's not true
https://dashboard.dogfooding.tekton.dev/#/namespaces/default/pipelines/pipeline-release-nightly?view=yaml
https://dashboard.dogfooding.tekton.dev/#/namespaces/default/pipelines/pipeline-release?view=yaml

Opened #5191

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants