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

Fix podman build --pull-never #9631

Merged
merged 1 commit into from
Mar 29, 2021
Merged

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Mar 5, 2021

Fixes: #9573

Signed-off-by: Daniel J Walsh [email protected]

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 5, 2021
@rhatdan
Copy link
Member Author

rhatdan commented Mar 5, 2021

This PR Requires containers/buildah#3062

@rhatdan rhatdan changed the title Fix podman build --pull-never [WIP] Fix podman build --pull-never Mar 5, 2021
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 5, 2021
@rhatdan rhatdan force-pushed the pull branch 2 times, most recently from e8b1860 to 8423eef Compare March 5, 2021 20:06
@rhatdan rhatdan changed the title [WIP] Fix podman build --pull-never Fix podman build --pull-never Mar 5, 2021
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 5, 2021
go.mod Outdated
@@ -11,19 +11,19 @@ require (
github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd // indirect
github.com/containernetworking/cni v0.8.1
github.com/containernetworking/plugins v0.9.0
github.com/containers/buildah v1.19.7
github.com/containers/common v0.35.0
github.com/containers/buildah v1.19.2-0.20210305112159-898eea8fbefd
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be 1.20 something? I saw the commit in master, maybe I missed the 1.19 commit?

Copy link
Member Author

Choose a reason for hiding this comment

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

We have not created a 1.20 yet, so I just put in the current commit id. Since we have not updated the main branch beyond v1.19.2 this is what it show up as.
IE I set the version to
898eea8fbefd73993406610311cad8fcab045bad

And then make vendor-in-container changes it to the above. BTW I think it might be time to cut a new buildah release.

@@ -307,7 +323,7 @@ func buildFlagsWrapperToOptions(c *cobra.Command, contextDir string, flags *buil
}

if flags.PullNever {
pullPolicy = imagebuildah.PullIfMissing
pullPolicy = imagebuildah.PullNever
Copy link
Member

Choose a reason for hiding this comment

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

OH OUCH!

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 7, 2021
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 7, 2021
@rhatdan rhatdan force-pushed the pull branch 2 times, most recently from ac627ef to 7d694bd Compare March 8, 2021 17:04
@giuseppe
Copy link
Member

giuseppe commented Mar 9, 2021

@cevich of those failing tests (e.g. podman fedora-33 root host), what are running inside of a privileged container?

@cevich
Copy link
Member

cevich commented Mar 9, 2021

what are running inside of a privileged container?

Should just be the ones that have container at the end of the name. i.e. int podman fedora-33 root container

@rhatdan rhatdan force-pushed the pull branch 6 times, most recently from 4e615b9 to 47fd974 Compare March 13, 2021 10:48
@rhatdan rhatdan force-pushed the pull branch 3 times, most recently from c0b9855 to 21d9a01 Compare March 20, 2021 11:47
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 20, 2021
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 20, 2021
@edsantiago
Copy link
Member

I'm seeing Files changed: 131. Is that deliberate? I don't remember the earlier iteration.

@rhatdan
Copy link
Member Author

rhatdan commented Mar 24, 2021

@edsantiago This is all vendoring in a new version of Buildah. If we had a new release this PR would be much smaller.

@edsantiago
Copy link
Member

OK. I'm confused about the vendor downgrade from buildah 1.19.8 to .2.something, but will leave that for others to worry about.

If you need to repush for any reason, what would you think of deleting these lines from the bud test?

@@ -2023,6 +2028,7 @@ _EOF
}
@test "bud pull never" {
+ skip "FIXME: podman issue #9573"
target=pull
run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json -t ${target} --pull-never ${TESTSDIR}/bud/pull
expect_output --substring "pull policy is \"never\" but \""

@rhatdan
Copy link
Member Author

rhatdan commented Mar 24, 2021

Sure.

@rhatdan rhatdan force-pushed the pull branch 2 times, most recently from 1593df1 to b79d294 Compare March 25, 2021 13:18
@edsantiago
Copy link
Member

bud test failing:

 + git clone -q --branch v1.19.2-0.20210325094324-0c0ce48c77e8 https://github.com/containers/buildah test-buildah-v1.19.2-0.20210325094324-0c0ce48c77e8
 fatal: Remote branch v1.19.2-0.20210325094324-0c0ce48c77e8 not found in upstream origin

This is probably my fault: my bud test assumes that whatever is in go.mod can be checked out as a tag from the buildah repo. It looks like I need some way of detecting magic strings like v<xyz>-something-sha. What should be the exact git clone command for a string like that?

@rhatdan
Copy link
Member Author

rhatdan commented Mar 25, 2021

@cevich We should have a new major release soon, in buildah, so there should be no rush.

@rhatdan rhatdan force-pushed the pull branch 4 times, most recently from 1f6948f to 521bebc Compare March 27, 2021 02:05
Currently pull policy is set incorrectly when users set --pull-never.

Also pull-policy is not being translated correctly when using
podman-remote.

Fixes: containers#9573

Signed-off-by: Daniel J Walsh <[email protected]>
@edsantiago
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 29, 2021
@openshift-merge-robot openshift-merge-robot merged commit ac3499c into containers:master Mar 29, 2021
@umohnani8
Copy link
Member

LGTM

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

podman build: --pull-never is a NOP
8 participants