Lower the time we wait for interfaces to appear

[remibergsma]

Waiting for interfaces is tricky. They might never appear.. for example when we have entries in /etc/cloudstack/ips.json that haven't been plugged yet. Waiting this long makes everything horribly slow (every vm, interface, static route, etc, etc, will hit this wait, for every device). We've seen CloudStack send an ip_assoc.json command for eth1 public nic only and then the router goes crazy waiting for all other interfaces that were there before reboot and aren't there. If only the router would return to the mgt server a success of eth1, it would get the command for eth2 etc etc. Obviously, a destroy works much faster because no state services, so no knowledge of previous devices so no waits :-)

After a stop/start the router has state in /etc/cloudstack/ips.json and every commands waits. Eventually hitting the hardcoded 120 sec timeout.

[swill]

This seems like a reasonable change to me. I am guessing 2 seconds is long enough, but I don't really understand what other variables could add contention and could cause the 2 seconds to not be long enough.

Since this code actually changes logic, I will put it in the queue for CI.

[yadvr]

tag:needlove

[nlivens]

@remibergsma, down from 15 to 2 seconds is pretty big in terms of computing time. Is there an internal retry in CS might this script give a timeout? I think this is a good change though, waiting 15 seconds on something to come up is really long.

[jburwell]

@remibergsma you may want to consider cribbing wait_until from Marvin. It allows you check for a condition at a fixed interval and give up after a specified period of time. Therefore, you could have the best of both worlds -- check every 2 seconds and give up after 15 seconds.

[nlivens]

@jburwell, I think that would be a nice solution!

[swill]

@remibergsma can you review @jburwell's comment and follow up in this thread? Thanks...

[remibergsma]

@jburwell Thanks, I'll look into that.

[remibergsma]

@jburwell Had a look at the wait_until method and I don't think it will work well here. The problem is that when the router restarts, it has the full config in /etc/cloudstack/*.json but several interfaces are not yet available as they are plugged later. By using wait_ultil and try every 2s for max of 15s, we get the old behaviour of having to wait 15s in case the interface is not there. When the interface is there, it will go through almost immediately. I propose to keep it like this as I've seen it speedup start times quite a bit.

[swill]

Rerunning tests on this one...

[swill]

CI RESULTS

Tests Run: 85
Skipped: 0
Failed: 1
Errors: 1
Duration: 3h 58m 14s

Summary of the problem(s):

ERROR: test suite for <class 'integration.smoke.test_router_dhcphosts.TestRouterDHCPHosts'>
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 228, in run
self.tearDown()
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 351, in tearDown
self.teardownContext(ancestor)
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/suite.py", line 367, in teardownContext
try_run(context, names)
File "/usr/lib/python2.7/site-packages/nose-1.3.7-py2.7.egg/nose/util.py", line 471, in try_run
return func()
File "/data/git/cs2/cloudstack/test/integration/smoke/test_router_dhcphosts.py", line 169, in tearDownClass
raise Exception("Warning: Exception during cleanup : %s" % e)
Exception: Warning: Exception during cleanup : Job failed: {jobprocstatus : 0, created : u'2016-05-18T02:19:21+0200', jobresult : {errorcode : 530, errortext : u'Failed to delete network'}, cmd : u'org.apache.cloudstack.api.command.user.network.DeleteNetworkCmd', userid : u'12c23347-1c80-11e6-8daf-5254001daa62', jobstatus : 2, jobid : u'2141d60b-4b07-46c0-80c3-c7bd5c92b517', jobresultcode : 530, jobresulttype : u'object', jobinstancetype : u'Network', accountid : u'12c21af5-1c80-11e6-8daf-5254001daa62'}
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/test_network_2NN2EZ/results.txt

FAIL: Check that the /etc/dhcphosts.txt doesn't contain duplicate IPs
----------------------------------------------------------------------
Traceback (most recent call last):
File "/data/git/cs2/cloudstack/test/integration/smoke/test_router_dhcphosts.py", line 355, in test_router_dhcphosts
self.test_ssh_command(self.vm_1, nat_rule1, "natrule1")
File "/data/git/cs2/cloudstack/test/integration/smoke/test_router_dhcphosts.py", line 200, in test_ssh_command
"Ping to outside world from VM should be successful"
AssertionError: Ping to outside world from VM should be successful
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/test_network_2NN2EZ/results.txt

Associated Uploads

/tmp/MarvinLogs/DeployDataCenter__May_18_2016_00_40_40_Y2KEK2:

• dc_entries.obj
• failed_plus_exceptions.txt
• runinfo.txt

/tmp/MarvinLogs/test_network_2NN2EZ:

• failed_plus_exceptions.txt
• results.txt
• runinfo.txt

/tmp/MarvinLogs/test_vpc_routers_YOJPG2:

• failed_plus_exceptions.txt
• results.txt
• runinfo.txt

Uploads will be available until 2016-07-18 02:00:00 +0200 CEST

Comment created by upr comment.

[remibergsma]

@swill Thanks for running the tests again. The error is Failed to delete network so that's cleanup related. The failed one, is that the one that randomly? Can't think of a way that it is related to the change in this PR. It looks OK to me.

[swill]

@remibergsma that failure is not one I commonly see, but I definitely do have periodic failures in my environment which complain about "not being able to connect to the outside world", so I think this falls in the same category. There are MANY checks in this suite for connecting out, so if a single one pops, it will usually be a false negative.

[swill]

Can I get some code review on this one? Thanks...

[nlivens]

@remibergsma, I understand your concern regarding the wait_until method. Perhaps we could use the wait_until with a 2s interval and a max wait of 4s? That's 1 retry only. That means that the max wait time has been drastically reduced from 15s to 4s, but you have that extra window of 2s for potential hiccups to happen

[karuturi]

LGTM 👍(code review only)

[swill]

CI RESULTS

Tests Run: 85
Skipped: 0
Failed: 0
Errors: 0
Duration: 8h 23m 38s

Associated Uploads

/tmp/MarvinLogs/DeployDataCenter__May_26_2016_05_06_20_HB7J97:

• dc_entries.obj
• failed_plus_exceptions.txt
• runinfo.txt

/tmp/MarvinLogs/test_network_PAIXSH:

• failed_plus_exceptions.txt
• results.txt
• runinfo.txt

/tmp/MarvinLogs/test_vpc_routers_2E8S0D:

• failed_plus_exceptions.txt
• results.txt
• runinfo.txt

Uploads will be available until 2016-07-26 02:00:00 +0200 CEST

Comment created by upr comment.

[remibergsma]

@nlivens We shouldn't wait more than 2s, because that means the interface is not there and will not come yet. Currently it's horribly slow because every command send to the router will try to see if the interface is there and wait. All those waits together really add up. Especially when restarting. I really think we should keep it like this since it already tries 2 times now.

[nlivens]

@remibergsma, sounds fair. LGTM

[swill]

Thanks guys. This one is ready...