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

buildah build --network add support for custom networks #3720

Merged
merged 1 commit into from
Jan 26, 2022

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Jan 19, 2022

What type of PR is this?

/kind feature

What this PR does / why we need it:

The backend logic already supports specifying custom network names. This
only adds the support for the frontend parsing.

How to verify it

buildah bud --network name1,name2 ...

Which issue(s) this PR fixes:

Fixes containers/podman#12282

Special notes for your reviewer:

Does this PR introduce a user-facing change?


@@ -389,7 +389,8 @@ Valid _mode_ values are:
- **none**: no networking;
- **host**: use the host network stack. Note: the host mode gives the container full access to local system services such as D-bus and is therefore considered insecure;
- **ns:**_path_: path to a network namespace to join;
- `private`: create a new namespace for the container (default)
- **private**: create a new namespace for the container (default)
- **\<network name|ID\>**: Join the network with the given name or ID. Only supported for rootful users.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Example for this would be nice, is this --network <networkname>.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean in the description or under the example section at the bottom of the page?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes but its not a blocker for me. I figured cli from tests

@flouthoc
Copy link
Collaborator

Renaming the PR to buildah build instead of buildah bud.

@flouthoc flouthoc changed the title buildah bud --network add support for custom networks buildah build --network add support for custom networks Jan 19, 2022
@@ -798,6 +799,8 @@ buildah build --dns-search=example.com --dns=223.5.5.5 --dns-option=use-vc .

buildah build -f Containerfile.in -t imageName .

buildah build --network mynet .

Copy link
Member

Choose a reason for hiding this comment

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

We'll eventually need similar changes to the man page for podman build if not already there.

@TomSweeneyRedHat
Copy link
Member

LGTM

@flouthoc
Copy link
Collaborator

Something is glitched in CI @Luap99 could you repush please and point to latest c/common which automatically bumps c/storage and c/image.

@Luap99 Luap99 force-pushed the networks branch 3 times, most recently from cc90bbb to b147dca Compare January 21, 2022 14:10
@TomSweeneyRedHat
Copy link
Member

@Luap99 tests are very unhappy and you might need to rebase again.

@rhatdan
Copy link
Member

rhatdan commented Jan 21, 2022

@Luap99 Once this gets in, do you think you are done with Buildah? IE Can we cut a release of Buildah to complete vendoring into Podman.

@Luap99
Copy link
Member Author

Luap99 commented Jan 21, 2022

@Luap99 Once this gets in, do you think you are done with Buildah? IE Can we cut a release of Buildah to complete vendoring into Podman.

Yes, but it is possible that we have to change c/common for more bugfixes.

@TomSweeneyRedHat
Copy link
Member

I'd like to plan to start building the next Buildah version on Wed Jan 26, 2022. @flouthoc @nalind holler if you've things you'd like to make sure get in before we cut that.

@rhatdan
Copy link
Member

rhatdan commented Jan 21, 2022

Sure we can revendor in containers-common as we go through the release.

@rhatdan
Copy link
Member

rhatdan commented Jan 21, 2022

I think Podman 4.0 RC3 should be next Friday, so Wednesday sounds good. We only have three PRs that I see getting in, right now.

tests/bud.bats Outdated
run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json --network nontexists ${TESTSDIR}/bud/network
expect_output --substring "network not found"

run_buildah bud --signature-policy ${TESTSDIR}/policy.json --network podman ${TESTSDIR}/bud/network
Copy link
Member

Choose a reason for hiding this comment

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

Where does the configuration for the network with this name come from?

Copy link
Member Author

Choose a reason for hiding this comment

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

podman is the default network name (can be changed in cotnainers.conf). If it does not exists on disk the default will always be created in memory, this happens in the libnetwork backend.

@Luap99
Copy link
Member Author

Luap99 commented Jan 25, 2022

OK the problem with the in container test is that the isolation is set to chroot where the network namespace is not supported. I added a check to throw an error in this case

@Luap99 Luap99 force-pushed the networks branch 2 times, most recently from 36c3445 to 30b204c Compare January 25, 2022 17:15
The backend logic already supports specifying custom network names. This
only adds the support for the frontend parsing.

Fixes containers/podman#12282

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
@TomSweeneyRedHat
Copy link
Member

Happy green test buttons, but I'm not sure if this needs a rebase or not.

@rhatdan
Copy link
Member

rhatdan commented Jan 26, 2022

/lgtm

@rhatdan
Copy link
Member

rhatdan commented Jan 26, 2022

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 26, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, 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-merge-robot openshift-merge-robot merged commit bed25d0 into containers:main Jan 26, 2022
@Luap99 Luap99 deleted the networks branch January 26, 2022 08:44
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 13, 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.

Podman Machine on Mac: podman build --net doesn't work
6 participants