Skip to content

Trim cruft and harden the Docker gateway idempotency fix - #21

Merged
ycedres merged 2 commits into
mainfrom
backport-6f146d6
Oct 1, 2026
Merged

ycedres merged 2 commits into
mainfrom
backport-6f146d6

Conversation

@m-czernek

@m-czernek m-czernek commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Porting saltstack/salt@6f146d6 which was not picked up by the migration.

  • systemd_service: drop the else-raise added to enable()/disable()'s sysvinit paths. It's unreachable (_get_service_exec() only ever returns a path ending in update-rc.d or chkconfig, or raises first) and untested; its commit rationale actually described the _runlevel() IndexError, which is fixed separately.
  • dockermod.compare_networks: tighten the auto-assigned-Gateway exception so it only ignores a one-sided Gateway when it equals the subnet's Docker- assigned default (network address + 1). Previously any one-sided Gateway was stripped unconditionally, which would mask a real change if a user explicitly set a non-default gateway and then removed it. Add unit tests covering the auto-assigned case, the non-default-gateway-removed case, and a changed-explicit-gateway case, mirroring the existing test_compare_networks_ipam_config_empty_iprange regression test. Add a changelog entry describing the fix accurately: the auto-gateway/no-gateway mismatch is structural (any Docker version that auto-assigns gateways), not specific to a newer Docker engine.

Fixes #22

- systemd_service: drop the else-raise added to enable()/disable()'s sysvinit
  paths. It's unreachable (_get_service_exec() only ever returns a path ending
  in update-rc.d or chkconfig, or raises first) and untested; its commit
  rationale actually described the _runlevel() IndexError, which is fixed
  separately.
- dockermod.compare_networks: tighten the auto-assigned-Gateway exception so
  it only ignores a one-sided Gateway when it equals the subnet's Docker-
  assigned default (network address + 1). Previously any one-sided Gateway
  was stripped unconditionally, which would mask a real change if a user
  explicitly set a non-default gateway and then removed it. Add unit tests
  covering the auto-assigned case, the non-default-gateway-removed case, and
  a changed-explicit-gateway case, mirroring the existing
  test_compare_networks_ipam_config_empty_iprange regression test. Add a
  changelog entry describing the fix accurately: the auto-gateway/no-gateway
  mismatch is structural (any Docker version that auto-assigns gateways), not
  specific to a newer Docker engine.
@codecov

codecov Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 42.85714% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 42.07%. Comparing base (cebd4c0) to head (9680a3c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/saltext/dockermod/modules/dockermod.py 53.33% 11 Missing and 3 partials ⚠️
tests/unit/modules/dockermod/test_module.py 26.31% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #21      +/-   ##
==========================================
- Coverage   42.10%   42.07%   -0.03%     
==========================================
  Files          26       26              
  Lines        6545     6590      +45     
  Branches     1138     1145       +7     
==========================================
+ Hits         2756     2773      +17     
- Misses       3542     3567      +25     
- Partials      247      250       +3     
Flag Coverage Δ
Linux 42.06% <42.85%> (-0.03%) ⬇️
project 37.76% <53.33%> (+0.05%) ⬆️
py310 42.01% <42.85%> (-0.03%) ⬇️
py311 42.01% <42.85%> (-0.03%) ⬇️
py314 42.01% <42.85%> (-0.03%) ⬇️
salt_3006_27 42.01% <42.85%> (-0.03%) ⬇️
salt_3008_2 42.01% <42.85%> (-0.03%) ⬇️
tests 50.17% <26.31%> (-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.

@ycedres
ycedres merged commit e6259d3 into main Oct 1, 2026
12 checks passed
@m-czernek
m-czernek deleted the backport-6f146d6 branch October 5, 2026 13:55
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.

[TECH DEBT] Backport 6f146d6

2 participants