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(ipam_driver): do not pass --ipam-driver option when value set to … #851

Merged
merged 1 commit into from
May 6, 2024

Conversation

fccagou
Copy link
Contributor

@fccagou fccagou commented Feb 20, 2024

…default

podman returns unsupported ipam driver "default" when --imap-driver default parameter is passed.

So, when ipam driver is set to "default" in docker-compose.yml, --imap-driver must not be used when podman is called.

@fccagou
Copy link
Contributor Author

fccagou commented Feb 20, 2024

Sorry for the push force.
Nothing changed in patch except commit message to refer to the new issue #852 according to CONTRIBUTING.md.

@fccagou fccagou force-pushed the fix-ipam-driver-default branch 2 times, most recently from 7922049 to 23a45c8 Compare February 20, 2024 22:24
@@ -762,7 +762,7 @@ async def assert_cnt_nets(compose, cnt):
args.extend(("--opt", f"{key}={value}"))
ipam = net_desc.get("ipam", None) or {}
ipam_driver = ipam.get("driver", None)
if ipam_driver:
if ipam_driver and ipam_driver not in ("default"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the code would be clearer if we compared using !=. Also I think there's a bug here, because ("default") is not a tuple. ("default",) would be a tuple.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, the missing ',' is strange because my tests worked ...
Yes, we can compare using !=.
I do a patch with it.

Copy link
Collaborator

@p12tic p12tic left a comment

Choose a reason for hiding this comment

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

Looks good in principle but will need unit tests.

@fccagou fccagou force-pushed the fix-ipam-driver-default branch 3 times, most recently from 7b8cebc to 7de2168 Compare March 9, 2024 13:24
@p12tic
Copy link
Collaborator

p12tic commented May 6, 2024

Rebased and adjusted commit message.

@p12tic
Copy link
Collaborator

p12tic commented May 6, 2024

@fccagou Sorry for the delay. I haven't noticed that you've force pushed with the fixes.

@p12tic p12tic merged commit 33d7d35 into containers:main May 6, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants