Differential D515

wait for port function
ClosedPublic
Actions

Authored by lbrabec on Aug 19 2015, 4:01 PM.

Details

Reviewers
kparal
tflink
jskladan
Maniphest Tasks
T559: delay between VM spawning and network bringup on clients is causing task execution to hang
Commits
rLTRN19e8f5041287: wait for port function
Summary

function that repeatedly check if given port is open, used for determine
if machine is ready for ssh connection

Test Plan

manual testing

Diff Detail

Repository
rLTRN libtaskotron
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
lbrabec retitled this revision from to wait for port function.Aug 19 2015, 4:01 PM
lbrabec updated this object.
lbrabec edited the test plan for this revision. (Show Details)
lbrabec added reviewers: jskladan, tflink, kparal.
jskladan accepted this revision.Aug 20 2015, 11:22 AM

Just a few nitpicks, the code LGTM

libtaskotron/vm.py
108

Please add docstring

110

This feels like it should be configurable, but we already have so many options in config, that it's probably not worth it. Thoughts @kparal?

This revision is now accepted and ready to land.Aug 20 2015, 11:22 AM
lbrabec updated this revision to Diff 1362.Aug 20 2015, 12:15 PM
  • docs, timeout as kwarg
tflink accepted this revision.Aug 21 2015, 8:38 PM

LGTM, works on a simple rpmlint run on my local machine

libtaskotron/vm.py
110

I'm OK with it being like it is for now. To a certain point, configurable is better but this might be a bit on the low-level side. If we find problems with the value or a reason to make it variable, we can make it configurable later.

kparal added a comment.Aug 24 2015, 8:08 AM

As nirik mentioned last week on IRC, unfortunately this might not solve issues with cloud-init. According to him, cloud-init does not block sshd service until it is configured, it often configures it after it has been started. So if this is intended to solve purely socket issue, it's fine, but we will have to have additional re-connection attempts also at the ssh level, until proper user certificate is installed (or maybe we can join both into a single place and retry the connection directly at the ssh level?).

libtaskotron/vm.py
116

If would be nice to mention here (probably as DEBUG) what machine:port we're waiting for and for how long.

126

I'd mention all of ipaddr, port and timeout in the error.

lbrabec updated this revision to Diff 1377.Aug 25 2015, 8:12 AM
  • code polishing
kparal accepted this revision.Aug 25 2015, 10:14 AM
Closed by commit rLTRN19e8f5041287: wait for port function (authored by Lukas Brabec <lbrabec@redhat.com>). · Explain WhyAug 25 2015, 10:30 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathPackages
Mlibtaskotron/runner.py (2 lines)
Mlibtaskotron/vm.py (27 lines)

Diff 1380

View Options

libtaskotron/runner.py

Show First 20 Lines • Show All 129 Lines • ▼ Show 20 Line(s)126 else:
130 task_vm = vm.TestCloudMachine(self.arg_data['uuid']) 130​ task_vm = vm.TestCloudMachine(self.arg_data['uuid'])
131 task_vm.prepare() 131​ task_vm.prepare()
132 self.machine = task_vm.ipaddr 132​ self.machine = task_vm.ipaddr
133 self.port = 22 133​ self.port = 22
134 self.user = task_vm.username 134​ self.user = task_vm.username
135 135​
136 log.info("Running Task over SSH on machine %s" % task_vm.instancename) 136​ log.info("Running Task over SSH on machine %s" % task_vm.instancename)
137 137​
138​ task_vm.wait_for_port(self.port)
139​
138 with remote_exec.RemoteExec(self.machine, self.port, self.user, 140​ with remote_exec.RemoteExec(self.machine, self.port, self.user,
139 self.ssh_privkey, stdio_filename) as ssh: 141​ self.ssh_privkey, stdio_filename) as ssh:
140 task_runner = RemoteRunner(ssh, self.task_data, self.arg_data) 142​ task_runner = RemoteRunner(ssh, self.task_data, self.arg_data)
141 try: 143​ try:
142 task_runner.prepare_task() 144​ task_runner.prepare_task()
143 task_runner.run() 145​ task_runner.run()
144 except exc.TaskotronRemoteError, e: 146​ except exc.TaskotronRemoteError, e:
145 log.exception(e) 147​ log.exception(e)
▲ Show 20 Lines • Show All 392 Lines • Show Last 20 Lines
View Options

libtaskotron/vm.py

1# -*- coding: utf-8 -*- 1​# -*- coding: utf-8 -*-
2# Copyright 2009-2015, Red Hat, Inc. 2​# Copyright 2009-2015, Red Hat, Inc.
3# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+> 3​# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+>
4# See the LICENSE file for more details on Licensing 4​# See the LICENSE file for more details on Licensing
5 5​
6 6​
7"""Interface to locally spawned virtual machines that are used as disposable 7​"""Interface to locally spawned virtual machines that are used as disposable
8clients for executing Taskotron tasks.""" 8​clients for executing Taskotron tasks."""
9 9​
10from time import sleep 10​from time import sleep, time
11​import socket
11 12​
12from libtaskotron.logger import log 13​from libtaskotron.logger import log
13import libtaskotron.exceptions as exc 14​import libtaskotron.exceptions as exc
14 15​
15from testcloud import instance, image, cli 16​from testcloud import instance, image, cli
16from testcloud.exceptions import TestcloudInstanceError, TestcloudImageError 17​from testcloud.exceptions import TestcloudInstanceError, TestcloudImageError
17 18​
18 19​
▲ Show 20 Lines • Show All 80 Lines • ▼ Show 20 Line(s)98 '''
99 tc_image = self._prepare_image() 100​ tc_image = self._prepare_image()
100 101​
101 self._check_existing_instance(should_exist=False) 102​ self._check_existing_instance(should_exist=False)
102 103​
103 self._prepare_instance(tc_image) 104​ self._prepare_instance(tc_image)
104 105​
105 self.ipaddr = cli.find_vm_ip(self.instancename) 106​ self.ipaddr = cli.find_vm_ip(self.instancename)
106 107​
108​ def wait_for_port(self, port, timeout=60):
jskladanUnsubmitted
Not Done

Please add docstring

109​ '''Wait until port is open. Repeatedly tries to socket.connect on given port.
110​
jskladanUnsubmitted
Not Done

This feels like it should be configurable, but we already have so many options in config, that it's probably not worth it. Thoughts @kparal?

tflinkUnsubmitted
Not Done

I'm OK with it being like it is for now. To a certain point, configurable is better but this might be a bit on the low-level side. If we find problems with the value or a reason to make it variable, we can make it configurable later.

111​ :param port: port to check
112​ :param timeout: timeout in seconds
113​ :raises TaskotronInstanceError: when timeouted
114​ '''
115​ s = socket.socket()
116​ start_time = time()
kparalUnsubmitted
Not Done

If would be nice to mention here (probably as DEBUG) what machine:port we're waiting for and for how long.

117​ log.debug('Waiting up to %d seconds for %s:%s to open.' % (timeout, self.ipaddr, port))
118​ while True:
119​ try:
120​ s.connect((self.ipaddr, port))
121​ except socket.error:
122​ pass
123​ else:
124​ s.close()
125​ break
126​ if (start_time + timeout) < time():
kparalUnsubmitted
Not Done

I'd mention all of ipaddr, port and timeout in the error.

127​ raise TestcloudInstanceError("Waiting for %s:%s to open timed out (%ds)" %
128​ (self.ipaddr, port, timeout))
129​
130​ sleep(0.1)
131​
107 def _stop_instance(self, tc_instance): 132​ def _stop_instance(self, tc_instance):
108 '''Stop (shut down) a virtual machine instance 133​ '''Stop (shut down) a virtual machine instance
109 134​
110 :raises TaskotronRemoteError: if stopping the instance fails''' 135​ :raises TaskotronRemoteError: if stopping the instance fails'''
111 136​
112 log.debug("Stopping instance {}".format(self.instancename)) 137​ log.debug("Stopping instance {}".format(self.instancename))
113 try: 138​ try:
114 tc_instance.stop() 139​ tc_instance.stop()
Show All 38 Lines