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

Vendor in new opencontainers/selinux #13687

Merged
merged 1 commit into from
Apr 8, 2022
Merged

Vendor in new opencontainers/selinux #13687

merged 1 commit into from
Apr 8, 2022

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Mar 29, 2022 •
edited
Loading

Also update vendor of containers/common,buildah,storage,image

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2069586

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 29, 2022
@Luap99
Copy link
Member

Luap99 commented Mar 29, 2022

Looks like this update breaks podman build/buildah.

@Luap99 Luap99 mentioned this pull request Mar 31, 2022
Closed
@flouthoc flouthoc mentioned this pull request Apr 1, 2022
Closed
This was referenced Apr 5, 2022
Closed
Closed
Closed
@TomSweeneyRedHat
Copy link
Member

Looks like Buildah has some bud functionality that hasn't gotten into podman build yet:

[+1189s] Error running buildah bud tests under podman.
[+1189s] 
[+1189s] It looks like you're vendoring in a new buildah. The likely failure
[+1189s] here is that there's a new test in bud.bats that uses functionality
[+1189s] not (yet) in podman build. You will likely need to 'skip' that test.
[+1189s] 
[+1189s] 
[+1189s] Please see test/buildah-bud/README.md for advice
[13:51:21] END - [+1189s] total duration since 2022-04-05T13:51:21

Failed tests (1):
 - 227 bud - invalid runtime flags test

@Luap99
Copy link
Member

Luap99 commented Apr 6, 2022

The functionality exists in podman, that's why the vendoring broke the first time and I had to patch this in buildah.

The test in buildah are using two arguments, that raises the question why buildah build would even allow that since we can have only one context dir.

Luap99 added a commit to Luap99/buildah that referenced this pull request Apr 6, 2022
@Luap99
buildah build should accept at most one arg
52c3394 
Right now buildah just ignores it when you set more than one argument for
build. Only the first argument is used as context dir. This looks wrong
to me and can lead to users errors.

Fixes test failure in containers/podman#13687

Signed-off-by: Paul Holzinger <[email protected]>
@Luap99 Luap99 mentioned this pull request Apr 6, 2022
Merged
@Luap99
Copy link
Member

Luap99 commented Apr 6, 2022

containers/buildah#3901 should fix it hopefully

Luap99 added a commit to Luap99/buildah that referenced this pull request Apr 6, 2022
@Luap99
buildah build should accept at most one arg
8827050 
Right now buildah just ignores it when you set more than one argument for
build. Only the first argument is used as context dir. This looks wrong
to me and can lead to users errors.

Fixes test failure in containers/podman#13687

Signed-off-by: Paul Holzinger <[email protected]>
Luap99 added a commit to Luap99/buildah that referenced this pull request Apr 6, 2022
@Luap99
buildah build should accept at most one arg
3351617 
Right now buildah just ignores it when you set more than one argument for
build. Only the first argument is used as context dir. This looks wrong
to me and can lead to users errors.

Fixes test failure in containers/podman#13687

Signed-off-by: Paul Holzinger <[email protected]>
@Luap99
Copy link
Member

Luap99 commented Apr 6, 2022

@rhatdan You should be able to update to latest main now and then hopefully everything passes.

@rhatdan
Copy link
Member Author

rhatdan commented Apr 6, 2022

Done, we will see if it works.

@rhatdan rhatdan force-pushed the VENDOR branch 2 times, most recently from 34b5315 to d96418c Compare April 7, 2022 10:03
@rhatdan rhatdan mentioned this pull request Apr 7, 2022
Merged
@Luap99
Copy link
Member

Luap99 commented Apr 7, 2022

Validate is failing because a build flag is missing in the man page

Luap99
Luap99 reviewed Apr 7, 2022
View reviewed changes
docs/source/markdown/podman-build.1.md Outdated
@@ -320,6 +320,10 @@ Print usage statement

Pass through HTTP Proxy environment variables.

#### **--identity-label** *bool-value*
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#### **--identity-label** *bool-value*
#### **--identity-label**

Since this is a bool we should not have a value behind it.

test/buildah-bud/apply-podman-deltas Outdated
Comment on lines 139 to 140
#skip "N/A under podman" \
#
Copy link
Member

Choose a reason for hiding this comment

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

Why the comment, this should be removed?

@Luap99 Luap99 mentioned this pull request Apr 7, 2022
Merged
flouthoc
flouthoc approved these changes Apr 7, 2022
View reviewed changes
Copy link
Collaborator

@flouthoc flouthoc left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 7, 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

@rhatdan
Vendor in new opencontainers/selinux
dc17195 
Also update vendor of containers/common,buildah,storage,image

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2069586

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

Luap99 commented Apr 8, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 8, 2022
@openshift-merge-robot openshift-merge-robot merged commit cdbc331 into containers:main Apr 8, 2022
@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 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Luap99 Luap99 Luap99 left review comments

@flouthoc flouthoc flouthoc approved these changes

Assignees

@Luap99 Luap99

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rhatdan @Luap99 @TomSweeneyRedHat @flouthoc @openshift-merge-robot