Skip to content
29 changes: 18 additions & 11 deletions cylc/flow/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -921,20 +921,27 @@ def manage_remote_init(self):

* Called within the main loop.
* Starts file installation when Remote init is complete.
* Removes complete installations or installations encountering SSH
error (remote init will take place on next job submission).
* Retries remote init/file install on SSH failure (255).
Comment thread
MetRonnie marked this conversation as resolved.
* Removes complete or fatally failed installations.
* The bad_hosts logic already handles unreachable hosts.
"""
for install_target, platform in list(self.incomplete_ri_map.items()):
status = self.task_job_mgr.task_remote_mgr.remote_init_map[
install_target]
remote_mgr = self.task_job_mgr.task_remote_mgr
status = remote_mgr.remote_init_map[install_target]
if status == REMOTE_INIT_DONE:
self.task_job_mgr.task_remote_mgr.file_install(platform)
if status in [REMOTE_FILE_INSTALL_DONE,
REMOTE_INIT_255,
REMOTE_FILE_INSTALL_255,
REMOTE_INIT_FAILED,
REMOTE_FILE_INSTALL_FAILED]:
# Remove install target
remote_mgr.file_install(platform)
elif status == REMOTE_INIT_255:
# Remote init failed due to unreachable host, retry.
del remote_mgr.remote_init_map[install_target]
remote_mgr.remote_init(platform)
elif status == REMOTE_FILE_INSTALL_255:
# File install failed due to unreachable host, retry.
del remote_mgr.remote_init_map[install_target]
remote_mgr.file_install(platform)
Comment thread
samuel-denton marked this conversation as resolved.
Comment on lines +933 to +938

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Have gotta feeling that deleting the remote_init_map entry deletes the state of all prior progress.

I.e, For REMOTE_FILE_INSTALL_255 that would delete the state which indicated successful remote-init.

Do we need to delete the remote_init_map state to force retry, or is it enough to just call the relevant method?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure, ill have a play.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Both remote_init() and file_install() set the entry in the map almost immediately, so I don't think the del is needed before either

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think you are correct. When I tested this I briefly saw some odd behaviour but that might be another bug. It seems to work, but when I clicked on a running task in the GUI, and asked it to show the log, it just gave me an error that it couldn't connect to bad1.
^^ Different issue. we can remove the del lines.

elif status in [REMOTE_FILE_INSTALL_DONE,
REMOTE_INIT_FAILED,
REMOTE_FILE_INSTALL_FAILED]:
# Complete or fatally failed, remove install target.
self.incomplete_ri_map.pop(install_target)

def _load_task_run_times(self, row_idx, row):
Expand Down
81 changes: 81 additions & 0 deletions tests/integration/test_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import asyncio
import logging
from pathlib import Path
from unittest.mock import Mock

import pytest
import re
from signal import SIGHUP, SIGINT, SIGTERM
Comment thread
samuel-denton marked this conversation as resolved.
Outdated
Expand All @@ -26,6 +28,14 @@
from cylc.flow.exceptions import CylcError
from cylc.flow.parsec.exceptions import ParsecError
from cylc.flow.scheduler import Scheduler, SchedulerStop
from cylc.flow.task_remote_mgr import (
REMOTE_FILE_INSTALL_255,
REMOTE_FILE_INSTALL_DONE,
REMOTE_FILE_INSTALL_FAILED,
REMOTE_INIT_255,
REMOTE_INIT_DONE,
REMOTE_INIT_FAILED,
)
from cylc.flow.task_state import (
TASK_STATUS_SUCCEEDED,
TASK_STATUS_WAITING,
Expand Down Expand Up @@ -454,3 +464,74 @@ async def test_set_stall_interaction(flow, scheduler, start):
schd.data_store_mgr.data[schd.tokens.id]['workflow'].status_msg
!= 'stalled'
)


async def test_manage_remote_init_retry_on_255(
flow, scheduler, start, monkeypatch: pytest.MonkeyPatch
):
"""Test that manage_remote_init retries remote init/file install on 255.

See https://github.com/cylc/cylc-flow/issues/7286

On restart, if remote-init fails with exit code 255 (SSH connection
failure), manage_remote_init should retry rather than silently dropping
the install target.
"""
missing_target = 'Alderaan'
fake_platform = {'install target': missing_target, 'name': 'test_plat'}

schd: Scheduler = scheduler(flow('one'), run_mode='live')
async with start(schd):
remote_mgr = schd.task_job_mgr.task_remote_mgr
mock_remote_init = Mock()
monkeypatch.setattr(remote_mgr, 'remote_init', mock_remote_init)
mock_file_install = Mock()
monkeypatch.setattr(remote_mgr, 'file_install', mock_file_install)

# Simulate restart remote-init has been kicked off and is being
# tracked via incomplete_ri_map:
schd.incomplete_ri_map[missing_target] = fake_platform

# REMOTE_INIT_255: retry remote_init, pop from init map, NOT ri map
remote_mgr.remote_init_map[missing_target] = REMOTE_INIT_255
schd.manage_remote_init()
assert missing_target not in remote_mgr.remote_init_map, (
"Stale 255 status should be deleted before retry"
)
mock_remote_init.assert_called_once_with(fake_platform)
assert missing_target in schd.incomplete_ri_map, (
"Install target should remain in incomplete_ri_map for tracking"
)
mock_remote_init.reset_mock()

# REMOTE_INIT_DONE: should trigger file_install
remote_mgr.remote_init_map[missing_target] = REMOTE_INIT_DONE
schd.manage_remote_init()
mock_file_install.assert_called_once_with(fake_platform)
assert missing_target in schd.incomplete_ri_map
mock_file_install.reset_mock()

# REMOTE_FILE_INSTALL_255: should retry file_install
remote_mgr.remote_init_map[missing_target] = REMOTE_FILE_INSTALL_255
schd.manage_remote_init()
assert missing_target not in remote_mgr.remote_init_map
mock_file_install.assert_called_once_with(fake_platform)
assert missing_target in schd.incomplete_ri_map
mock_file_install.reset_mock()

# REMOTE_FILE_INSTALL_DONE: should remove from ri map
remote_mgr.remote_init_map[missing_target] = REMOTE_FILE_INSTALL_DONE
schd.manage_remote_init()
assert missing_target not in schd.incomplete_ri_map

# REMOTE_INIT_FAILED: should remove from ri map
schd.incomplete_ri_map[missing_target] = fake_platform
remote_mgr.remote_init_map[missing_target] = REMOTE_INIT_FAILED
schd.manage_remote_init()
assert missing_target not in schd.incomplete_ri_map

# REMOTE_FILE_INSTALL_FAILED: should remove from ri map
schd.incomplete_ri_map[missing_target] = fake_platform
remote_mgr.remote_init_map[missing_target] = REMOTE_FILE_INSTALL_FAILED
schd.manage_remote_init()
assert missing_target not in schd.incomplete_ri_map
Loading