[BUG] BuildStrategy: Cannot Use Context Dir as Working Directory

adambkaplan commented 3 months ago

Is there an existing issue for this?

[X] I have searched the existing issues

Kubernetes Version

k8s: 1.28.7 Tekton Pipelines: 0.56.2

Shipwright Version

0.12.0

Current Behavior

When authoring a build strategy, builds risk failure if a build strategy step has its workingDir set to a sub-directory and source is cloned from git. Git expects the target directory of any clone action to be empty.

Setting workingDir to a sub-directory of the source root (ex: contextDir) results in errors like the following:

2024/04/11 16:39:28 /usr/bin/git -c safe.directory=/workspace/source clone -h
2024/04/11 16:39:28 /usr/bin/git -c safe.directory=/workspace/source submodule -h
2024/04/11 16:39:28 /usr/bin/git -c safe.directory=/workspace/source clone --quiet --no-tags --single-branch --depth 1 -- https://github.com/redhat-developer-demos/quinoa-wind-turbine.git /workspace/source
2024/04/11 16:39:28 fatal: destination path '/workspace/source' already exists and is not an empty directory. (exit code 128)

Expected Behavior

Ideally build steps succeed if the directory is a subPath of the working directory. However, this may prove difficult due to the way Tekton, Kubernetes, and potentially the underlying container runtime operate (everything runs in a single TaskRun/Pod today).

Steps To Reproduce

Create a build strategy that has a step workingDir set to a sub-path of $(params.shp-source-root)
Run a Build with this strategy that clones source from git.

Anything else?

This is perhaps something that we document as a known issue - ex: in a guide for Build Strategy authors.

adambkaplan commented 2 months ago

Refinement - this is likely a limitation of how Tekton creates containers in a TaskRun. A guide for build strategy authors should call this out.

SaschaSchwarze0 commented 2 months ago

@adambkaplan I would consider this a bug in our Git step implementation. The step could check the existing sub-directories and if there are any, clone into a temporary directory and then move content. Or delete the sub-directories (would that break the parallel steps) and after clone finished verify that they were recreated.

adambkaplan commented 2 months ago

The step could check the existing sub-directories and if there are any, clone into a temporary directory and then move content.

This feels like a lot of extra work, and can be error/risk prone. I also think this encourages "bad" behavior of expecting additional content to exist alongside source code as part of a build process. Things like dependency caches IMO should be configurable and located outside of the source code directory. I'm personally fine keeping this as a known issue/limitation, as this really only impacts strategy authors/platform teams.

Or delete the sub-directories (would that break the parallel steps) and after clone finished verify that they were recreated.

I think that would break the other steps in the build. IIRC Tekton has an "entrypoint" mechanism that starts all TaskRun containers at the same time, then effectively pauses/sleeps them to execute in the right order.

SaschaSchwarze0 commented 2 months ago

I'm personally fine keeping this as a known issue/limitation, as this really only impacts strategy authors/platform teams.

I would say we document it but would still try to resolve it. It is a not so nice limitation and we have own build strategies that work around it like https://github.com/shipwright-io/build/blob/v0.13.0/samples/v1beta1/buildstrategy/ko/buildstrategy_ko_cr.yaml#L104. So, I can understand that build strategy authors run into that issue.

What I would be interested is your opinion on the Tekton behavior. Tekton could easily remove the workingDir from the container and start the step command in the workingDir from their entrypoint (or fail if at that time the directory does not exist).

shipwright-io / build