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

Handle COPY --from when an argument is used #2522

Merged
merged 1 commit into from
Aug 11, 2020

Conversation

TomSweeneyRedHat
Copy link
Member

When an argument was passed into "COPY --from" command in
a Containerfile like

COPY --from=${toolchainname}

The argument was never resolved to the value that it had been
set to.

Addresses: #2496

It may also address #2404 but I've not yet tested.

Signed-off-by: TomSweeneyRedHat [email protected]

What type of PR is this?

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake
/kind other

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?


Copy link
Member

@rhatdan rhatdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM other then spacing on some of your tests.

tests/bud.bats Outdated
_prefetch debian

run_buildah bud --build-arg base=alpine --build-arg toolchainname=busybox --build-arg destinationpath=/tmp --pull=false --signature-policy ${TESTSDIR}/policy.json -f ${TESTSDIR}/bud/from-with-arg/Containerfile .
[ "$status" -eq 0 ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix spacing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, IDK how I managed to mess that up so badly without seeing it.

@rhatdan
Copy link
Member

rhatdan commented Aug 8, 2020

@QiWang19
Copy link
Contributor

LGTM

tests/bud.bats Outdated
_prefetch debian

run_buildah bud --build-arg base=alpine --build-arg toolchainname=busybox --build-arg destinationpath=/tmp --pull=false --signature-policy ${TESTSDIR}/policy.json -f ${TESTSDIR}/bud/from-with-arg/Containerfile .
[ "$status" -eq 0 ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary: run_buildah already does status checking

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keep forgetting that, thanks for the nudge, removed/fixed.

When an argument was passed into "COPY --from" command in
a Containerfile like

COPY --from=${toolchainname}

The argument was never resolved to the value that it had been
set to.

Addresses: containers#2496

It may also address containers#2404 but I've not yet tested.

Signed-off-by: TomSweeneyRedHat <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Aug 10, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 11, 2020

Build succeeded:

  • cirrus-ci/success

@bors bors bot merged commit 94fa081 into containers:master Aug 11, 2020
@TomSweeneyRedHat TomSweeneyRedHat deleted the dev/tsweeney/from_args branch August 14, 2020 20:42
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants