Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions teuthology/dispatcher/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,7 @@ def main(args):
log.exception(error_message)
if 'targets' in job_config:
node_names = job_config["targets"].keys()
lock_ops.unlock_safe(
node_names,
job_config["owner"],
job_config["name"],
job_config["job_id"]
)
lock_ops.unlock_safe(node_names, job_config["owner"])
report.try_push_job_info(job_config, dict(
status='fail',
failure_reason=error_message))
Expand Down
2 changes: 1 addition & 1 deletion teuthology/dispatcher/supervisor.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ def unlock_targets(job_config):
return
if job_config.get("unlock_on_failure", True):
log.info('Unlocking machines...')
lock_ops.unlock_safe(locked, job_config["owner"], job_config["name"], job_config["job_id"])
lock_ops.unlock_many(locked, job_config["owner"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we switch to unlock_many? It doesn't seem to have any checks before unlocking the nodes.



def run_with_watchdog(process, job_config):
Expand Down
4 changes: 2 additions & 2 deletions teuthology/kill.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def kill_run(run_name, archive_base=None, owner=None, machine_type=None,
if owner is not None:
targets = find_targets(run_name)
names = list(targets.keys())
lock_ops.unlock_safe(names, owner, run_name)
lock_ops.unlock_safe(names, owner)
report.try_mark_run_dead(run_name)


Expand Down Expand Up @@ -103,7 +103,7 @@ def kill_job(run_name, job_id, archive_base=None, owner=None, skip_unlock=False)
log.warn(f"Job {job_id} has no machine_type; cannot report via Prometheus")
if not skip_unlock:
targets = find_targets(run_name, job_id)
lock_ops.unlock_safe(list(targets.keys()), owner, run_name, job_id)
lock_ops.unlock_safe(list(targets.keys()), owner)


def find_run_info(serializer, run_name):
Expand Down
15 changes: 6 additions & 9 deletions teuthology/lock/ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,35 +174,32 @@ def lock_one(name, user=None, description=None):
return response


def unlock_safe(names: List[str], owner: str, run_name: str = "", job_id: str = ""):
def unlock_safe(names: List[str], owner: str):
with teuthology.parallel.parallel() as p:
for name in names:
p.spawn(unlock_one_safe, name, owner, run_name, job_id)
p.spawn(unlock_one_safe, name, owner)
return all(p)


def unlock_one_safe(name: str, owner: str, run_name: str = "", job_id: str = "") -> bool:
def unlock_one_safe(name: str, owner: str) -> bool:
node_status = query.get_status(name)
if node_status.get("locked", False) is False:
log.warn(f"Refusing to unlock {name} since it is already unlocked")
log.debug(f"Refusing to unlock {name} since it is already unlocked")
return False
maybe_job = query.node_active_job(name, node_status)
if not maybe_job:
return unlock_one(name, owner, node_status["description"], node_status)
if run_name and job_id and maybe_job.endswith(f"{run_name}/{job_id}"):
log.error(f"Refusing to unlock {name} since it has an active job: {run_name}/{job_id}")
return False
log.warning(f"Refusing to unlock {name} since it has an active job: {maybe_job}")
return False


def unlock_many(names, user):
def unlock_many(names: List[str], owner: str):
fixed_names = [misc.canonicalize_hostname(name, user=None) for name in
names]
names = fixed_names
uri = os.path.join(config.lock_server, 'nodes', 'unlock_many', '')
data = dict(
locked_by=user,
locked_by=owner,
names=names,
)
with safe_while(
Expand Down
11 changes: 8 additions & 3 deletions teuthology/util/time.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,23 @@

# When we're not using ISO format, we're using this
TIMESTAMP_FMT = "%Y-%m-%d_%H:%M:%S"
# Or this, in the case of paddles timestamps
TIMESTAMP_FMT_ALT = "%Y-%m-%d %H:%M:%S.%f"

def parse_timestamp(timestamp: str) -> datetime:
"""
timestamp: A string either in ISO 8601 format or TIMESTAMP_FMT.
If no timezone is specified, UTC is assumed.
timestamp: A string either in ISO 8601 format, TIMESTAMP_FMT or
TIMESTAMP_FMT_ALT. If no timezone is specified, UTC is assumed.

:returns: a datetime object
"""
try:
dt = datetime.fromisoformat(timestamp)
except ValueError:
dt = datetime.strptime(timestamp, TIMESTAMP_FMT)
try:
dt = datetime.strptime(timestamp, TIMESTAMP_FMT)
except ValueError:
dt = datetime.strptime(timestamp, TIMESTAMP_FMT_ALT)
if dt.tzinfo is None:
dt = dt.replace(tzinfo=timezone.utc)
return dt
Expand Down
Loading