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.

Sign up for GitHub

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

Jump to bottom

dispatcher: consider builtinargs and userargs while processing baseImage name #229

Merged
merged 1 commit into from
Jun 16, 2022
Merged

dispatcher: consider builtinargs and userargs while processing baseImage name #229

merged 1 commit into from
Jun 16, 2022

Conversation

flouthoc
Copy link
Contributor

@flouthoc flouthoc commented May 26, 2022 •
edited

We already consider builtinargs and userArgs while computing --from=
flags so use same logic for computing name of baseImage while evaluating
FROM statements.

Allows use cases like

FROM --platform=linux/amd64 alpine as platform-amd64
# do something

FROM --platform=linux/arm64 alpine as platform-arm64
# do something

FROM platform-${TARGETARCH}

also allows FROM ${SOME_DEFAULT_OR_USER_ARG} in general.

Closes: containers/podman#14375

@flouthoc flouthoc mentioned this pull request May 26, 2022
Closed
@flouthoc
Copy link
Contributor Author

@nalind @rhatdan @TomSweeneyRedHat PTAL

@rhatdan
Copy link
Contributor

rhatdan commented May 26, 2022 •
edited

LGTM
Although this does not look like tests ran.
Nevermind, I thought this was in buildah.

nalind
nalind reviewed Jun 15, 2022
View reviewed changes
builder_test.go Outdated Show resolved Hide resolved
@nalind
Copy link
Member

nalind commented Jun 15, 2022

Code change looks fine, but the new test could use a few more entries in its slice of test cases.

@flouthoc flouthoc force-pushed the consider-buildinargs-userargs-from-eval branch from 17bb324 to c4b4155 Compare June 16, 2022 08:20
@flouthoc
dispatcher: consider builtinargs and userargs while processing baseIm…
c009008 
…age name

We already consider builtinargs and userArgs while computing `--from=`
flags so use same logic for computing name of baseImage while evaluating
`FROM` statements.

Allows use cases like
```Dockerfile
FROM --platform=linux/amd64 alpine as platform-amd64

FROM --platform=linux/arm64 alpine as platform-arm64

FROM platform-${TARGETARCH}
```

also allows `FROM ${SOME_DEFAULT_OR_USER_ARG} ` in general.

Closes: containers/podman#14375

Signed-off-by: Aditya R <arajan@redhat.com>
@flouthoc flouthoc force-pushed the consider-buildinargs-userargs-from-eval branch from c4b4155 to c009008 Compare June 16, 2022 08:24
@flouthoc
Copy link
Contributor Author

Code change looks fine, but the new test could use a few more entries in its slice of test cases.

@nalind @TomSweeneyRedHat @rhatdan PTAL added more requested tests.

@flouthoc flouthoc requested a review from nalind June 16, 2022 08:25
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 16, 2022

@flouthoc: all tests passed!

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@nalind
Copy link
Member

nalind commented Jun 16, 2022

LGTM

@rhatdan
Copy link
Contributor

rhatdan commented Jun 16, 2022

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 16, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 16, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 16, 2022
@openshift-ci openshift-ci bot merged commit 0839fa7 into openshift:master Jun 16, 2022
flouthoc added a commit to flouthoc/buildah that referenced this pull request Jun 17, 2022
@flouthoc
vendor: bump openshift/imagebuilder to master@0839fa7
fbfa23c 
PR openshift/imagebuilder#229 adds support for
resolving args in base name, bump openshift/imagebuilder to
master@839fa7 so buildah could use this feature.

Signed-off-by: Aditya R <arajan@redhat.com>
@flouthoc flouthoc mentioned this pull request Jun 17, 2022
Merged
flouthoc added a commit to flouthoc/buildah that referenced this pull request Jun 22, 2022
@flouthoc
vendor: bump openshift/imagebuilder to master@0839fa7
976e179 
PR openshift/imagebuilder#229 adds support for
resolving args in base name, bump openshift/imagebuilder to
master@839fa7 so buildah could use this feature.

Signed-off-by: Aditya R <arajan@redhat.com>
flouthoc added a commit to flouthoc/buildah that referenced this pull request Jun 22, 2022
@flouthoc
vendor: bump openshift/imagebuilder to master@0839fa7
0d377c5 
PR openshift/imagebuilder#229 adds support for
resolving args in base name, bump openshift/imagebuilder to
master@839fa7 so buildah could use this feature.

Signed-off-by: Aditya R <arajan@redhat.com>
flouthoc added a commit to flouthoc/buildah that referenced this pull request Jul 11, 2022
@flouthoc
vendor: bump openshift/imagebuilder to master@0839fa7
5098629 
PR openshift/imagebuilder#229 adds support for
resolving args in base name, bump openshift/imagebuilder to
master@839fa7 so buildah could use this feature.

Signed-off-by: Aditya R <arajan@redhat.com>
flouthoc added a commit to flouthoc/buildah that referenced this pull request Jul 12, 2022
@flouthoc
vendor: bump openshift/imagebuilder to master@4151e43
0125819 
PR openshift/imagebuilder#229 adds support for
resolving args in base name, bump openshift/imagebuilder to
master@4151e43 so buildah could use this feature.

Signed-off-by: Aditya R <arajan@redhat.com>
flouthoc added a commit to flouthoc/buildah that referenced this pull request Jul 12, 2022
@flouthoc
vendor: bump openshift/imagebuilder to master@4151e43
f146c71 
PR openshift/imagebuilder#229 adds support for
resolving args in base name, bump openshift/imagebuilder to
master@4151e43 so buildah could use this feature.

Signed-off-by: Aditya R <arajan@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomSweeneyRedHat TomSweeneyRedHat Awaiting requested review from TomSweeneyRedHat

@nalind nalind Awaiting requested review from nalind

Assignees

@rhatdan rhatdan

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Using ${TARGETARCH} in a FROM stage does not work
4 participants
@flouthoc @rhatdan @nalind @TomSweeneyRedHat