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

Correct network option name ip -> ip6 #22509

Merged

Conversation

sanmai-NL
Copy link
Contributor

@sanmai-NL sanmai-NL commented Apr 25, 2024

Also, properly capitalize.

Does this PR introduce a user-facing change?

None

Also, properly capitalize.

Signed-off-by: Sander Maijers <[email protected]>
@openshift-ci openshift-ci bot added the do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None label Apr 25, 2024
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@openshift-ci openshift-ci bot added release-note-none and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Apr 26, 2024
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

- **ip=**_IPv6_: Specify a static ipv6 address for this container.
- **mac=**_MAC_: Specify a static mac address for this container.
- **ip=**_IPv4_: Specify a static IPv4 address for this container.
- **ip6=**_IPv6_: Specify a static IPv6 address for this container.
Copy link
Member

Choose a reason for hiding this comment

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

technically it does not matter and you can very well set ip=<some ipv6 address>, the backend code mixes them together anyway. But for documentation purposes this makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does matter in my test with Podman Desktop 5.0.2. on MacOS. With ip and an IPv6 address, the container doesn't get the address assigned and with ip6, it does.

Copy link
Member

Choose a reason for hiding this comment

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

If you use a GUI then why does the cli syntax apply at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Who says I use a GUI? I'm using the CLI, just listed the product.

Copy link
Member

Choose a reason for hiding this comment

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

Because if you use the cli you cannot say that ip doesn't work for a ipv6 address, the code is literally the same no matter of ip or ip6

case "ip", "ip6":

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe you can also test it, looks can be deceptive.

Copy link
Contributor

openshift-ci bot commented Apr 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, sanmai-NL

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 Apr 26, 2024
@mheon
Copy link
Member

mheon commented Apr 26, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 26, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 3927a52 into containers:main Apr 26, 2024
89 of 91 checks passed
@stale-locking-app stale-locking-app bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Jul 27, 2024
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Jul 27, 2024
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. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants