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

Module network fixes #29237

Merged
merged 11 commits into from Nov 30, 2015

Conversation

Projects
None yet
4 participants
@sjorge
Contributor

sjorge commented Nov 26, 2015

Since it was actually giving me some issues when testing some stuff... I fixed them all up so I could continue.

Fixes the following:

Partially fixes:

Similar fixes can probably be done for *BSD also but I do not have test systems for those at hand.

[root@core /opt/local/salt_dev/_mod]# /opt/local/salt/bin/salt-call --local network.active_tcp
local:
    ----------
    1:
        ----------
        local_addr:
            2001:6f8:xxxx:yyyy::120
        local_port:
            22
        remote_addr:
            2001:6f8:xxxx:yyyy::1
        remote_port:
            3463
    2:
        ----------
        local_addr:
            2001:6f8:xxxx:yyyy::120
        local_port:
            22
        remote_addr:
            2001:6f8:xxxx:yyyy::1
        remote_port:
            19744
[root@core /opt/local/salt_dev/_mod]# /opt/local/salt/bin/salt-call --local network.default_route inet
local:
    |_
      ----------
      addr_family:
          inet
      destination:
          default
      flags:
          UG
      gateway:
          172.16.yy.1
      interface:
          igb0
      netmask:
@jfindlay

View changes

Show outdated Hide outdated salt/modules/network.py Outdated
@jfindlay

View changes

Show outdated Hide outdated salt/modules/network.py Outdated
@@ -1309,6 +1331,8 @@ def default_route(family=None):
for route in _routes:
if family:
if route['destination'] in default_route[family]:
if __grains__['kernel'] == 'SunOS' and route['addr_family'] != family:

This comment has been minimized.

@jfindlay

jfindlay Nov 30, 2015

Contributor

@sjorge, is there a reason the second condition is predicated upon the platform being SunOS?

@jfindlay

jfindlay Nov 30, 2015

Contributor

@sjorge, is there a reason the second condition is predicated upon the platform being SunOS?

This comment has been minimized.

@sjorge

sjorge Nov 30, 2015

Contributor

_routes contain the output of netstat, the values depends on the platform.

As far as I know only SunOS variants have the addr_family field which is either inet or inet6.
In the case of SunOS de default route is identified by 'default' for both inet and inet6.

If family is set we need to filter on them, for SunOS we still get both inet and inet6 by just filtering on destination. So for SunOS we also check the addr_family property.

@sjorge

sjorge Nov 30, 2015

Contributor

_routes contain the output of netstat, the values depends on the platform.

As far as I know only SunOS variants have the addr_family field which is either inet or inet6.
In the case of SunOS de default route is identified by 'default' for both inet and inet6.

If family is set we need to filter on them, for SunOS we still get both inet and inet6 by just filtering on destination. So for SunOS we also check the addr_family property.

@jfindlay

This comment has been minimized.

Show comment
Hide comment
@jfindlay

jfindlay Nov 30, 2015

Contributor

@sjorge, there is a unit test that needs to be updated.

Contributor

jfindlay commented Nov 30, 2015

@sjorge, there is a unit test that needs to be updated.

@sjorge

This comment has been minimized.

Show comment
Hide comment
@sjorge

sjorge Nov 30, 2015

Contributor

@jfindlay It seems that grains is not populated when the unit test runs.
What is the preferred fix for this?

Contributor

sjorge commented Nov 30, 2015

@jfindlay It seems that grains is not populated when the unit test runs.
What is the preferred fix for this?

@sjorge

This comment has been minimized.

Show comment
Hide comment
@sjorge

sjorge Nov 30, 2015

Contributor

Ok, I think I got it... lets see if jenkis is happy with the test now.

Contributor

sjorge commented Nov 30, 2015

Ok, I think I got it... lets see if jenkis is happy with the test now.

@jfindlay

This comment has been minimized.

Show comment
Hide comment
@jfindlay

jfindlay Nov 30, 2015

Contributor

@sjorge, that is the correct fix.

Contributor

jfindlay commented Nov 30, 2015

@sjorge, that is the correct fix.

cachedout added a commit that referenced this pull request Nov 30, 2015

@cachedout cachedout merged commit f813508 into saltstack:develop Nov 30, 2015

4 of 6 checks passed

default Merged build finished.
Details
jenkins/salt-pr-rs-cent6-n Salt PR - RS CentOS 6 #271 — FAILURE
Details
jenkins/salt-pr-clone Salt PR - Clone Repository #11559 — SUCCESS
Details
jenkins/salt-pr-linode-ubuntu14.04-n Salt PR - Linode Ubuntu 14.04 #2626 — SUCCESS
Details
jenkins/salt-pr-lint-n Salt PR - Code Lint #11267 — SUCCESS
Details
jenkins/salt-pr-rs-cent7-n Salt PR - RS CentOS 7 #10096 — SUCCESS
Details

cachedout added a commit that referenced this pull request Dec 2, 2015

@sjorge sjorge deleted the sjorge:module-network-fixes branch Jan 16, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment