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

use libnetwork from c/common #12642

Merged
merged 7 commits into from
Jan 13, 2022
Merged

use libnetwork from c/common #12642

merged 7 commits into from
Jan 13, 2022

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Dec 17, 2021

The libpod/network packages were moved to c/common so that buildah can
use it as well. To prevent duplication use it in podman as well and
remove it from here.

Signed-off-by: Paul Holzinger [email protected]

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 17, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 17, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99

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 Dec 17, 2021
@Luap99 Luap99 mentioned this pull request Dec 17, 2021
Merged
@mheon
Copy link
Member

mheon commented Dec 17, 2021

LGTM once the c/common PR merges.

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 26, 2021
@Luap99 Luap99 mentioned this pull request Jan 3, 2022
Merged
@junnplus junnplus mentioned this pull request Jan 3, 2022
Merged
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 3, 2022
@Luap99 Luap99 force-pushed the libnetwork branch 2 times, most recently from 7c85073 to 26f1bdc Compare January 3, 2022 19:22
@Luap99 Luap99 mentioned this pull request Jan 4, 2022
Merged
@Luap99 Luap99 force-pushed the libnetwork branch 3 times, most recently from fecbafb to 2103d75 Compare January 6, 2022 20:04
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 7, 2022
@openshift-ci openshift-ci bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 7, 2022
@rhatdan
Copy link
Member

rhatdan commented Jan 11, 2022

Containers/common has been merged, could you rebase.

@Luap99
Copy link
Member Author

Luap99 commented Jan 11, 2022

There is no point, I sill need the buildah PR.

This was referenced Jan 12, 2022
Closed
Closed
@Luap99
use libnetwork from c/common
495884b 
The libpod/network packages were moved to c/common so that buildah can
use it as well. To prevent duplication use it in podman as well and
remove it from here.

Signed-off-by: Paul Holzinger <[email protected]>
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 12, 2022
@Luap99 Luap99 changed the title [WIP] use libnetwork from c/common use libnetwork from c/common Jan 12, 2022
@Luap99 Luap99 marked this pull request as ready for review January 12, 2022 16:10
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 12, 2022
@edsantiago
Copy link
Member

Aw, phooey. I just looked at the podman-remote failures, and there are a whole lot more than in local. I'm working on those right now.

edsantiago added a commit to edsantiago/libpod that referenced this pull request Jan 12, 2022
@edsantiago
Manual fixes for PR containers#12642:
267e993 
 - reenable git:// tests
 - git command fails with (EVIL) status 128. Deal with it.
 - skip two failing tests that I don't know how to fix:
   a) --unsetenv is not passed on to buildah (should be easy)
   b) something manifest-related

 - skip a bunch more podman-remote tests. Filed an issue for
   one of them (containers#12838), the others may not be fixable.

Signed-off-by: Ed Santiago <[email protected]>
@edsantiago
Copy link
Member

Okay, I've force-pushed updates to Luap99#2 which I think cover all the errors.

Please do not just blindly merge that (unless this PR is time-critical). Please, someone buildah-savvy, look at all these new added skips and either (a) fix them or (b) move the skips to a different section of the script, with an explanation of why these won't be fixed.

HTH

TomSweeneyRedHat
TomSweeneyRedHat reviewed Jan 12, 2022
View reviewed changes
docs/source/markdown/podman-build.1.md
@@ -54,6 +54,10 @@ Add a custom host-to-IP mapping (host:ip)
Add a line to /etc/hosts. The format is hostname:ip. The **--add-host** option
can be set multiple times.

#### **--all-platforms**

Instead of building for a set of platforms specified using the **--platform** option, inspect the build's base images, and build for all of the platforms for which they are all available. Stages that use *scratch* as a starting point can not be inspected, so at least one non-*scratch* stage must be present for detection to work usefully.
Copy link
Member

Choose a reason for hiding this comment

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

I'm struggling with the first sentence. I think this is better, but I'm not sure it's right or that I like it a lot. Maybe "found" instead of "available" in my suggestion?

Suggested change
Instead of building for a set of platforms specified using the **--platform** option, inspect the build's base images, and build for all of the platforms for which they are all available. Stages that use *scratch* as a starting point can not be inspected, so at least one non-*scratch* stage must be present for detection to work usefully.
Instead of building for a set of platforms specified using the **--platform** option, inspect the build's base images, and build for all of the platforms that are available. Stages that use *scratch* as a starting point can not be inspected, so at least one non-*scratch* stage must be present for detection to work usefully.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can fix this here but this is copy paste from buildah-bud so someone should fix it there as well.

@Luap99
podman build enable --all-platforms and --unsetenv
92e7875 
Make sure we add support for allplatforms and unsetenv to both local and
remote podman.

Signed-off-by: Paul Holzinger <[email protected]>
@edsantiago @Luap99
Manual fixes for PR containers#12642:
ece768f 
 - reenable git:// tests
 - git command fails with (EVIL) status 128. Deal with it.
 - skip a bunch more podman-remote tests. Filed an issue for
   one of them (containers#12838), the others may not be fixable.

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

Luap99 commented Jan 13, 2022

OK all tests were green now except one podman-remote build test bud with containerfile env secret priority which I now also added to skip list since it secrets do not work on remote

@Luap99
Copy link
Member Author

Luap99 commented Jan 13, 2022

@baude @mheon @rhatdan @edsantiago PTAL This should go green now

@mheon
Copy link
Member

mheon commented Jan 13, 2022

LGTM!

@mheon
Copy link
Member

mheon commented Jan 13, 2022

/lgtm
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 13, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 13, 2022
@edsantiago
Copy link
Member

LGTM

@Luap99
Copy link
Member Author

Luap99 commented Jan 13, 2022

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 13, 2022
@openshift-merge-robot openshift-merge-robot merged commit eeb76db into containers:main Jan 13, 2022
@Luap99 Luap99 deleted the libnetwork branch January 13, 2022 17:06
@flouthoc flouthoc mentioned this pull request Jan 14, 2022
Merged
2 tasks
@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 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@TomSweeneyRedHat TomSweeneyRedHat TomSweeneyRedHat left review comments

@edsantiago edsantiago edsantiago left review comments

Assignees

@mheon mheon

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.
6 participants
@Luap99 @mheon @rhatdan @edsantiago @TomSweeneyRedHat @openshift-merge-robot