Skip to content

Treat any number of default IPAM pools as auto-assigned in docker_network.present - #15

Open
mitar wants to merge 1 commit into
salt-extensions:mainfrom
mitar:docker-network-default-ipam-pools
Open

mitar wants to merge 1 commit into
salt-extensions:mainfrom
mitar:docker-network-default-ipam-pools

Conversation

@mitar

@mitar mitar commented Sep 21, 2026

Copy link
Copy Markdown

What does this PR do?

docker_network.present skips comparing IPAM config when the state defines no IPAM pools and the existing network only has pools Docker assigned automatically (just a subnet and a gateway). This check only worked when the existing network had exactly one pool.

With enable_ipv6: True and no ipam_pools, Docker assigns two default pools: one IPv4 and one IPv6. The check failed for those networks, so the existing network's two pools were compared with the desired config, which has no pools. The comparison always reported a difference.

This PR accepts any number of existing pools as long as every one of them looks like a Docker-assigned default pool.

What issues does this PR fix or reference?

Fixes:

Previous Behavior

With this state:

mynet:
  docker_network.present:
    - enable_ipv6: True

every run after the first reported Network would be recreated with new config and recreated the network. Connected containers were disconnected and reconnected each time.

New Behavior

A second run with the same state reports no changes: Network 'mynet' already exists, and is configured as specified.

Merge requirements satisfied?

  • Docs
  • Changelog
  • Tests written/updated: added the functional test test_present_enable_ipv6_is_idempotent

Commits signed with GPG?

No

@codecov

codecov Bot commented Sep 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.19%. Comparing base (cebd4c0) to head (5565d64).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #15      +/-   ##
==========================================
+ Coverage   42.10%   42.19%   +0.08%     
==========================================
  Files          26       26              
  Lines        6545     6555      +10     
  Branches     1138     1138              
==========================================
+ Hits         2756     2766      +10     
  Misses       3542     3542              
  Partials      247      247              
Flag Coverage Δ
Linux 42.18% <100.00%> (+0.08%) ⬆️
project 37.71% <ø> (ø)
py310 42.13% <100.00%> (+0.08%) ⬆️
py311 42.13% <100.00%> (+0.08%) ⬆️
py314 42.13% <100.00%> (+0.08%) ⬆️
salt_3006_27 42.13% <100.00%> (+0.08%) ⬆️
salt_3008_2 42.13% <100.00%> (+0.08%) ⬆️
tests 50.59% <100.00%> (+0.21%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@m-czernek

Copy link
Copy Markdown
Contributor

Same question here as in PR#14 - is this a cherry-pick or new commits? These changes are missing on 3006.x if I'm looking correctly.

@whytewolf

Copy link
Copy Markdown

so i can answer as i remember this conversation. This was new code for 3008 that wasn't able to be released because well docker went bye bye in 3008. so yeah this isn't in the older versions. however the 4 fixes he wants are badly needed. the docker suite had some nasty bugs around networking.

@mitar

mitar commented Sep 24, 2026

Copy link
Copy Markdown
Author

Answered here: so this is new code never merged to salt itself (but I did mention some of those fixes there in the past, maybe even made PRs, but as @whytewolf said, it was not really possible to merge them there).

For cherry-picking commits from salt, I just listed those in this comment. They should also be merged, but I didn't do PRs for them. I can if you would prefer that.

@mitar

mitar commented Sep 24, 2026

Copy link
Copy Markdown
Author

Just to add, all those commits (including those which were back-ported to salt master) we are using in production for some time now and made things smooth for us around Docker.

This branch has not been deployed

No deployments
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.

3 participants