Change in vdsm[master]: net: source route: don't attempt to configure invalid values

[Dan Kenigsberg]

Dan Kenigsberg has uploaded a new change for review.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

net: source route: don't attempt to configure invalid values

If a DHCP server provides Vdsm with invalid ip/netmask/gateway supervdsm
logs a not-very-useful

"ipaddr, mask or gateway not received"

Instead, this patch logs dhclient's faulty information before passing it
to the source route configurator, which hopefully would provide the
local admin more clues regarding the failure.

Change-Id: I0c4719629bfdc632bbc35171489670062c6c14cb
Signed-off-by: Dan Kenigsberg 
---
M vdsm/network/sourceroute.py
M vdsm/network/sourceroutethread.py
2 files changed, 5 insertions(+), 5 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/16/32416/1

diff --git a/vdsm/network/sourceroute.py b/vdsm/network/sourceroute.py
index da78ac4..5564acd 100644
--- a/vdsm/network/sourceroute.py
+++ b/vdsm/network/sourceroute.py
@@ -64,10 +64,6 @@
                      srcDevice=self.device)]

def configure(self, ipaddr, mask, gateway):
-        if gateway in (None, '0.0.0.0') or not ipaddr or not mask:
-            logging.error("ipaddr, mask or gateway not received")
-            return
-
         self.ipaddr = ipaddr
         self.mask = mask
         self.gateway = gateway
diff --git a/vdsm/network/sourceroutethread.py b/vdsm/network/sourceroutethread.py
index c2f14e6..04e9347 100644
--- a/vdsm/network/sourceroutethread.py
+++ b/vdsm/network/sourceroutethread.py
@@ -48,7 +48,11 @@
                     ip = sourceRouteContents[1]
                     mask = sourceRouteContents[2]
                     gateway = sourceRouteContents[3]
-                    sourceRoute.configure(ip, mask, gateway)
+                    if gateway in (None, '0.0.0.0') or not ip or not mask:
+                        logging.error('invalid DHCP response %s',
+                            sourceRouteContents)
+                    else:
+                        sourceRoute.configure(ip, mask, gateway)
                 else:
                     sourceRoute.remove()
             else:

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 1: Code-Review-1 Verified-1

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11293/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12235/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/190... : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12082/ : UNSTABLE

Assaf Muller has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 1:

So the configure method is called from lots of places. Don't we want to log the bad input everywhere, and not just in the DHCP case?

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 2:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11294/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12236/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/190... : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12083/ : SUCCESS

Antoni Segura Puimedon has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 2: Code-Review+1

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 2: Code-Review-1

There's another call to configure() in configurators/__init__.py

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 3:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12767/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11821/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/201... : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12610/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 4:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12771/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11825/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/201... : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12614/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 5:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12781/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11835/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/201... : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12624/ : SUCCESS

Antoni Segura Puimedon has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 5: Code-Review+1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 6:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12834/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11886/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/202... : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12677/ : SUCCESS

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 7:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12885/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/204... : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12727/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11936/ : FAILURE

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 8:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12904/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/204... : There was an infra issue, please contact [e-mail]

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12746/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11955/ : FAILURE

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 6:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12834/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12677/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/205... : There was an infra issue, please contact [e-mail]

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 6:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12834/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12677/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/205... : There was an infra issue, please contact [e-mail]

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 6:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12834/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12677/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_network_functional_tests_gerrit/206... : There was an infra issue, please contact [e-mail]

Ido Barkan has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 8: Verified+1

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 8: Code-Review+2

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

net: source route: don't attempt to configure invalid values

If a DHCP server provides Vdsm with invalid ip/netmask/gateway supervdsm
logs a not-very-useful

"ipaddr, mask or gateway not received"

Instead, this patch logs dhclient's faulty information before passing it
to the source route configurator, which hopefully would provide the
local admin more clues regarding the failure.

Change-Id: I0c4719629bfdc632bbc35171489670062c6c14cb
Signed-off-by: Dan Kenigsberg 
Reviewed-on: http://gerrit.ovirt.org/32416
Reviewed-by: Antoni Segura Puimedon 
Tested-by: Ido Barkan 
---
M vdsm/network/configurators/__init__.py
M vdsm/network/sourceroute.py
M vdsm/network/sourceroutethread.py
3 files changed, 17 insertions(+), 9 deletions(-)

Approvals:
  Ido Barkan: Verified
  Antoni Segura Puimedon: Looks good to me, but someone else must approve
  Dan Kenigsberg: Looks good to me, approved

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: net: source route: don't attempt to configure invalid values
......................................................................

Patch Set 9:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/4079/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc21-x86_64_merged/65/ : ABORTED

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el6-x86_64_merged/92/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged_test_debug/287/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5919/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el7-x86_64_merged/89/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc20-x86_64_merged/85/ : SUCCESS