Permalink
Browse files

multiple refactorings

rational:

reduce complexity and increase readability by making relationships between
objects more specific. It is better to pass sub-objects of minimal scope than
global "god-objects". That way there are less possible interactions.

changes:

- moved SSHKey to Task (it's not really needed inside session for anything.
  Plus, when we recreate Session we don't always need or mean to create a new
  temporary SSH key)

- encapsulated session logs into a narrower sub-class (Session.Logs): that way
  we can pass the narrower class rather than the entire session, which makes it
  clearer what we're doing with it and reduces possible interactions.

- make Watchdog behave as an instance class rather than as a collection of
  classmethods (now that we've narrowed down what we need passing around lots
  of arguments just makes the code more verbose without really reducing
  complexity)
  • Loading branch information...
1 parent 2d6bca4 commit eb64db0c77644bf68a698aea0ae7da427e758c64 @lirazsiri lirazsiri committed Dec 19, 2012
Showing with 144 additions and 152 deletions.
  1. +19 −20 cloudtask/executor.py
  2. +2 −2 cloudtask/reporter.py
  3. +65 −85 cloudtask/session.py
  4. +34 −11 cloudtask/task.py
  5. +24 −34 cloudtask/watchdog.py
View
@@ -68,7 +68,7 @@ def func():
return func
- def __init__(self, session, taskconf, ipaddress=None, destroy=None, event_stop=None, launchq=None):
+ def __init__(self, session_logs, taskconf, sshkey, ipaddress=None, destroy=None, event_stop=None, launchq=None):
self.pid = os.getpid()
@@ -77,9 +77,8 @@ def __init__(self, session, taskconf, ipaddress=None, destroy=None, event_stop=N
self.event_stop = event_stop
- self.wlog = session.wlog
- self.mlog = session.mlog
- self.session_key = session.key
+ self.logs = session_logs
+ self.sshkey = sshkey
self.strikes = taskconf.strikes
self.strike = 0
@@ -121,7 +120,7 @@ def handler(s, f):
def callback():
return not stopped.value
- instance = list(self.hub.launch(1, VerboseLog(session.mlog), callback, **taskconf.ec2_opts))[0]
+ instance = list(self.hub.launch(1, VerboseLog(session_logs.manager), callback, **taskconf.ec2_opts))[0]
if not instance or (event_stop and event_stop.is_set()):
raise self.Terminated
@@ -137,17 +136,17 @@ def callback():
try:
self.ssh = SSH(self.ipaddress,
- identity_file=self.session_key.path,
+ identity_file=self.sshkey.path,
login_name=taskconf.user,
callback=self.handle_stop)
except SSH.Error, e:
self.status("unreachable via ssh: " + str(e))
- traceback.print_exc(file=self.wlog)
+ traceback.print_exc(file=self.logs.worker)
raise self.Error(e)
try:
- self.ssh.copy_id(self.session_key.public)
+ self.ssh.copy_id(self.sshkey.public)
if taskconf.overlay:
self.ssh.apply_overlay(taskconf.overlay)
@@ -157,7 +156,7 @@ def callback():
except Exception, e:
self.status("setup failed")
- traceback.print_exc(file=self.wlog)
+ traceback.print_exc(file=self.logs.worker)
raise self.Error(e)
@@ -168,7 +167,7 @@ def _cleanup(self):
if self.cleanup_command:
self.ssh.command(self.cleanup_command).close()
- self.ssh.remove_id(self.session_key.public)
+ self.ssh.remove_id(self.sshkey.public)
except:
pass
@@ -185,7 +184,7 @@ def _cleanup(self):
raise self.Error("Hub didn't destroy worker instance as requested!")
except:
self.status("failed to destroy worker %s" % self.instanceid)
- traceback.print_exc(file=self.wlog)
+ traceback.print_exc(file=self.logs.worker)
raise
def __getstate__(self):
@@ -198,8 +197,8 @@ def status(self, msg, after_output=False):
timestamp = time.strftime('%Y-%m-%d %H:%M:%S', time.localtime())
c = "\n" if after_output else ""
- self.wlog.status.write(c + "# %s [%s] %s\n" % (timestamp, self.ipaddress, msg))
- self.mlog.write("%s (%d): %s\n" % (self.ipaddress, os.getpid(), msg))
+ self.logs.worker.status.write(c + "# %s [%s] %s\n" % (timestamp, self.ipaddress, msg))
+ self.logs.manager.write("%s (%d): %s\n" % (self.ipaddress, os.getpid(), msg))
def __call__(self, job):
command = job.command
@@ -222,7 +221,7 @@ class WorkerDied(Exception):
def handler(ssh_command, buf):
if buf:
read_timeout.reset()
- self.wlog.write(buf)
+ self.logs.worker.write(buf)
if ssh_command.running and timeout.expired():
raise CommandTimeout
@@ -261,7 +260,7 @@ def handler(ssh_command, buf):
else:
if ssh_command.exitcode == 255 and re.match(r'^ssh: connect to host.*:.*$', ssh_command.output):
self.status("worker unreachable # %s" % command)
- self.wlog.write("%s\n" % ssh_command.output)
+ self.logs.worker.write("%s\n" % ssh_command.output)
raise self.Error(SSH.Error(ssh_command.output))
self.status("exit %d # %s" % (ssh_command.exitcode, command), True)
@@ -303,7 +302,7 @@ class CloudExecutor:
class Error(Exception):
pass
- def __init__(self, session, taskconf):
+ def __init__(self, session_logs, taskconf, sshkey):
ipaddresses = taskconf.workers
split = taskconf.split
@@ -315,7 +314,7 @@ def __init__(self, session, taskconf):
ipaddress = ipaddresses[0]
else:
ipaddress = None
- self._execute = CloudWorker(session, taskconf, ipaddress)
+ self._execute = CloudWorker(session_logs, taskconf, sshkey, ipaddress)
self.results = []
else:
@@ -340,7 +339,7 @@ def callback():
hub = Hub(taskconf.hub_apikey)
i = None
try:
- for i, instance in enumerate(hub.launch(new_workers, VerboseLog(session.mlog), callback, **taskconf.ec2_opts)):
+ for i, instance in enumerate(hub.launch(new_workers, VerboseLog(session_logs.manager), callback, **taskconf.ec2_opts)):
launchq.put(instance)
except Exception, e:
unlaunched_workers = new_workers - (i + 1) \
@@ -351,7 +350,7 @@ def callback():
launchq.put(None)
if not isinstance(e, hub.Stopped):
- traceback.print_exc(file=session.mlog)
+ traceback.print_exc(file=session_logs.manager)
threading.Thread(target=thread).start()
@@ -361,7 +360,7 @@ def callback():
else:
ipaddress = None
- worker = Deferred(CloudWorker, session, taskconf, ipaddress,
+ worker = Deferred(CloudWorker, session_logs, taskconf, sshkey, ipaddress,
event_stop=self.event_stop, launchq=launchq)
workers.append(worker)
View
@@ -121,8 +121,8 @@ def header(title, c):
print >> sio, header("Fallback session log", "=")
print >> sio, taskconf.fmt()
- mlog = file(session.paths.log).read()
- print >> sio, mlog
+ manager_log = file(session.paths.log).read()
+ print >> sio, manager_log
body = sio.getvalue()
View
@@ -15,12 +15,6 @@
import paths
import errno
-import time
-
-from temp import TempFile
-import uuid
-
-import executil
from taskconf import TaskConf
import pprint
@@ -34,24 +28,6 @@ def makedirs(path, mode=0750):
if e.errno != errno.EEXIST:
raise
-class TempSessionKey(TempFile):
- def __init__(self):
- TempFile.__init__(self, prefix='key_')
- os.remove(self.path)
-
- self.uuid = uuid.uuid4()
- executil.getoutput("ssh-keygen -N '' -f %s -C %s" % (self.path, self.uuid))
-
- @property
- def public(self):
- return self.path + ".pub"
-
- def __del__(self):
- if os.getpid() == self.pid:
- os.remove(self.public)
-
- TempFile.__del__(self)
-
class UNDEFINED:
pass
@@ -114,56 +90,84 @@ def update_retry_failed(self):
self.save()
- class WorkerLog(object):
- def fh(self):
- if not self._fh:
- self._fh = file(join(self.path, str(os.getpid())), "a", 1)
+ class Logs:
+ class Worker(object):
+ def fh(self):
+ if not self._fh:
+ self._fh = file(join(self.path, str(os.getpid())), "a", 1)
- return self._fh
- status = fh = property(fh)
+ return self._fh
+ status = fh = property(fh)
+ def __init__(self, path, tee=False):
+ self._fh = None
+ self.path = path
+ self.tee = tee
- def __init__(self, path, tee=False):
- self._fh = None
- self.path = path
- self.tee = tee
+ @staticmethod
+ def _filter(buf):
+ buf = re.sub(r'Connection to \S+ closed\.\r+\n', '', buf)
+ buf = re.sub(r'\r[^\r\n]+$', '', buf)
+ buf = re.sub(r'.*\r(?![\r\n])','', buf)
+ buf = re.sub(r'\r+\n', '\n', buf)
+
+ return buf
+
+ def write(self, buf):
+ if self.tee:
+ sys.stdout.write(buf)
+ sys.stdout.flush()
+
+ # filter progress bars and other return-carriage crap
+ buf = self._filter(buf)
+
+ if buf:
+ self.fh.write(buf)
+ else:
+ os.utime(self.path, None)
- @staticmethod
- def _filter(buf):
- buf = re.sub(r'Connection to \S+ closed\.\r+\n', '', buf)
- buf = re.sub(r'\r[^\r\n]+$', '', buf)
- buf = re.sub(r'.*\r(?![\r\n])','', buf)
- buf = re.sub(r'\r+\n', '\n', buf)
+ def __getattr__(self, attr):
+ return getattr(self.fh, attr)
- return buf
+ class Manager:
+ def __init__(self, path):
+ self.fh = file(path, "a", 1)
- def write(self, buf):
- if self.tee:
+ def write(self, buf):
+ self.fh.write(buf)
sys.stdout.write(buf)
sys.stdout.flush()
- # filter progress bars and other return-carriage crap
- buf = self._filter(buf)
+ def __getattr__(self, attr):
+ return getattr(self.fh, attr)
- if buf:
- self.fh.write(buf)
- else:
- os.utime(self.path, None)
+ def __init__(self, path_session_log, path_workers):
+ self.pid = os.getpid()
+ self.path_session_log = path_session_log
+ self.path_workers = path_workers
- def __getattr__(self, attr):
- return getattr(self.fh, attr)
+ self._worker = None
+ self._manager = None
- class ManagerLog:
- def __init__(self, path):
- self.fh = file(path, "a", 1)
+ @property
+ def worker(self):
+ if self._worker:
+ return self._worker
- def write(self, buf):
- self.fh.write(buf)
- sys.stdout.write(buf)
- sys.stdout.flush()
+ makedirs(self.path_workers)
- def __getattr__(self, attr):
- return getattr(self.fh, attr)
+ worker = self.Worker(self.path_workers, True if os.getpid() == self.pid else False)
+ self._worker = worker
+ return worker
+
+ @property
+ def manager(self):
+ if self._manager:
+ return self._manager
+
+ manager = self.Manager(self.path_session_log)
+ self._manager = manager
+ return manager
@staticmethod
def new_session_id(sessions_path):
@@ -203,11 +207,7 @@ def __init__(self, sessions_path, id=None):
self.paths = Session.Paths(path)
self.jobs = self.Jobs(self.paths.jobs)
- self._wlog = None
- self._mlog = None
-
- self.key = TempSessionKey()
-
+ self.logs = self.Logs(self.paths.log, self.paths.workers)
self.id = id
def taskconf(self, val=UNDEFINED):
@@ -221,23 +221,3 @@ def taskconf(self, val=UNDEFINED):
print >> file(path, "w"), pprint.pformat(d)
taskconf = property(taskconf, taskconf)
- @property
- def wlog(self):
- if self._wlog:
- return self._wlog
-
- makedirs(self.paths.workers)
- wlog = self.WorkerLog(self.paths.workers, False if self.taskconf.split and self.taskconf.split > 1 else True)
-
- self._wlog = wlog
- return wlog
-
- @property
- def mlog(self):
- if self._mlog:
- return self._mlog
-
- mlog = self.ManagerLog(self.paths.log)
-
- self._mlog = mlog
- return mlog
Oops, something went wrong.

0 comments on commit eb64db0

Please sign in to comment.