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

Address regression causing bootproto=dhcp for manual IP addresses #56376

Open
wants to merge 3 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@tylarb
Copy link

commented May 13, 2019

SUMMARY

Commit b7724fd appears to have caused a regression, where ip4, gw4, ip6, gw6 were converted to ipv4.address, ipv4.gateway etc.

This causes bootproto (or ipv4.method) to remain dhcp, as noted in #36615

This commit only reverts the key-value pairs to the original names, which is in line with both expectation (manual ip addr == no dhcp) and the language used in the playbook, which is, for example, "ip4" not
"ipv4.address"

Co-authored-by: Stuart Pollock spollock@pivotal.io
Co-authored-by: Tyler Ramer tramer@pivotal.io

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

nmcli community module

Address regression causing bootproto=dhcp for manual IP addresses
Commit
b7724fd
appears to have caused a regression, where `ip4`, `gw4`, `ip6`, `gw6`
were converted to `ipv4.address`, `ipv4.gateway` etc.

This causes bootproto (or `ipv4.method`) to remain `dhcp`, as noted in #36615

This commit only reverts the key-value pairs to the original names,
which is in line with both expectation (manual ip addr == no dhcp) and
the language used in the playbook, which is, for example, "ip4" not
"ipv4.address"

Co-authored-by: Stuart Pollock <spollock@pivotal.io>
Co-authored-by: Tyler Ramer <tramer@pivotal.io>
@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 13, 2019

@tylarb

This comment has been minimized.

Copy link
Author

commented May 13, 2019

I'll resolve this small issue: ip4 vs ipv4.address is a critical difference on "connection add" but if the connection is added then it needs to be ipv4.address to modify.

@ansibot ansibot added core_review and removed needs_revision labels May 13, 2019

@tylarb

This comment has been minimized.

Copy link
Author

commented May 13, 2019

From man nmcli:

The alias [ip4] is equivalent to the +ipv4.addresses syntax and also sets ipv4.method to manual. It can be specified multiple times.

So I expect that a "modify" connection should not append an ip address, rather it should edit it. This would be done with ipv4.address, ipv4.gateway, etc instead of ip4. However, for creating a new connection, to ensure the ip address is set and bootproto = none, we should use ip4.

At the least, this final fixup brings the behavior fully in line with that before the regression caused by b7724fd

@samdoran samdoran removed the needs_triage label May 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.