Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow 'cylc reload' to reload global configuration #6509

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
1 change: 1 addition & 0 deletions changes.d/6509.feat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added --global flag to 'cylc reload'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Added --global flag to 'cylc reload'
Added --global flag to 'cylc reload' which also reloads the Cylc global config.

12 changes: 12 additions & 0 deletions cylc/flow/cfgspec/globalcfg.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@

Not to be confused with :cylc:conf:`flow.cylc[scheduling]`.

.. note::
Copy link
Member

Choose a reason for hiding this comment

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

FYI: This note will only appear in the workflow config reference, not the global config reference due to the default_for function which only includes the first paragraph.


The majority of scheduler settings affect the server and cannot be reloaded
with ``cylc reload --global``, the server must be stopped and restarted for
changes to take effect except for the sections [mail] and [events] which
Copy link
Member

Choose a reason for hiding this comment

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

Little bit of Cylc Sphinx magic, use single backquotes (or the log role :cylc:conf) to reference configuration sections:

Suggested change
changes to take effect except for the sections [mail] and [events] which
changes to take effect except for the sections `[mail]` and `[events]` which

This generates links, but also runs the references through the internal consistency checker to make sure they stay valid.

provide workflow defaults.

.. versionchanged:: 8.0.0

{REPLACES}``[cylc]``
Expand Down Expand Up @@ -1171,6 +1178,11 @@ def default_for(
Symlinks from the the standard ``$HOME/cylc-run`` locations will be
created.

.. note::

Once a platform has been installed and symlinks created they
cannot be modified for that run.

.. versionadded:: 8.0.0
"""):
with Conf('<install target>', desc=dedent("""
Expand Down
22 changes: 21 additions & 1 deletion cylc/flow/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
from cylc.flow.parsec.exceptions import ParsecError
from cylc.flow.run_modes import RunMode
from cylc.flow.task_id import TaskID
from cylc.flow.cfgspec.glbl_cfg import glbl_cfg
from cylc.flow.workflow_status import StopMode


Expand Down Expand Up @@ -327,7 +328,7 @@ async def remove_tasks(


@_command('reload_workflow')
async def reload_workflow(schd: 'Scheduler'):
async def reload_workflow(schd: 'Scheduler', reload_global: bool = False):
"""Reload workflow configuration."""
yield
# pause the workflow if not already
Expand Down Expand Up @@ -361,6 +362,25 @@ async def reload_workflow(schd: 'Scheduler'):
# give commands time to complete
sleep(1) # give any remove-init's time to complete

if reload_global:
# Reload global config if requested
LOG.info("Reloading the global configuration.")
Copy link
Member

Choose a reason for hiding this comment

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

This will update the workflow status message (shown in the GUI toolbar) to reflect what Cylc is doing:

Suggested change
LOG.info("Reloading the global configuration.")
LOG.info("Reloading the global configuration.")
schd.reload_pending = 'reloading the global configuration'
schd.update_data_store() # update workflow status msg
await schd.update_data_structure()

It's useful info incase the operation gets stuck with a filesystem issue, or incase loading the config takes a long time for whatever reason.

try:
glbl_cfg(reload=True)
except (ParsecError, CylcConfigError) as exc:
if cylc.flow.flags.verbosity > 1:
# log full traceback in debug mode
LOG.exception(exc)
LOG.critical(
f'Reload failed - {exc.__class__.__name__}: {exc}'
'\nThis is probably due to an issue with the new'
' configuration.'
'\nTo continue with the pre-reload config, un-pause the'
' workflow.'
'\nOtherwise, fix the configuration and attempt to reload'
' again.'
)
Copy link
Member

Choose a reason for hiding this comment

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

The way this is implemented, if the global config fails to reload, we log the error, but continue and reload the workflow config.

I see cylc reload --global as a transaction that reloads both the global config and the workflow config. Ideally, if either of these operations fail, we would rollback the transaction. I.e, either both operations happen or neither happen, the workflow is always left in an intended state.

So I would expect it to bail at this point. I.E, If the global reload fails, I would expect the logic to skip straight to this block of logic at the end of the routine:

    # resume the workflow if previously paused
    schd.reload_pending = False
    schd.update_data_store()  # update workflow status msg
    schd._update_workflow_state()
    if not was_paused_before_reload:
        schd.resume_workflow()
        schd.process_workflow_db_queue()  # see #5593

That does leave us with the situation where the global config does reload, but the workflow config doesn't (e.g. a validation error). Currently, the global config change will not roll back in this situation, but we could get it to do that something along the lines of:

if reload_global:
    ...
    try:
        cfg = glbl_cfg(cached=False)
    except ...:
        ...

...
# reload the workflow definition
...

glbl_cfg().reload(cfg)

(would require a reload method on the GlobalConfig object)


# reload the workflow definition
schd.reload_pending = 'loading the workflow definition'
schd.update_data_store() # update workflow status msg
Expand Down
4 changes: 4 additions & 0 deletions cylc/flow/network/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -1919,6 +1919,10 @@ class Meta:
class Arguments:
workflows = graphene.List(WorkflowID, required=True)

reload_global = Boolean(
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, this is a breaking change that will cause errors when cylc reload is run against workflows using a different version both on the CLI and in the GUI.

Somewhat surprisingly, we don't yet have a working pattern for adding new fields to mutations. It's one of the things on the TODO list that we haven't found time for yet!

  1. If the new field appears in the mutation and the scheduler doesn't know about it -> error.
  2. If the new field doesn't appear in the mutation but the scheduler is expecting it -> error.

I think we can get around both issues by marking this field as optional (required=False) and omitting the new field from the cylc reload mutation unless the --global option is used.

This way the mutation should remain forward and backward compatible so long as the --global option isn't used. And should only fail for older schedulers where the --global option is used (which is correct).

Sadly, we don't have any automated inter-version testing as yet (another one for the TODO list) so this will need to be tested manually for now.

default_value=False,
description="Also reload global config")

Comment on lines +1922 to +1925
Copy link
Member

Choose a reason for hiding this comment

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

FYI, all arguments added here are automatically listed in the GUI. The description text goes into the help bubble.

Here's how this option will appear:

Screenshot from 2025-01-28 14-49-23

result = GenericScalar()


Expand Down
21 changes: 20 additions & 1 deletion cylc/flow/scripts/reload.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,34 @@
from cylc.flow.option_parsers import (
WORKFLOW_ID_MULTI_ARG_DOC,
CylcOptionParser as COP,
OptionSettings,
)
from cylc.flow.terminal import cli_function

if TYPE_CHECKING:
from optparse import Values


RELOAD_OPTIONS = [
OptionSettings(
['-g', '--global'],
help='also reload global configuration.',
action="store_true",
default=False,
dest="reload_global",
sources={'reload'}
),
]


MUTATION = '''
mutation (
$wFlows: [WorkflowID]!
$wFlows: [WorkflowID]!,
$reloadGlobal: Boolean,
) {
reload (
workflows: $wFlows
reloadGlobal: $reloadGlobal
Copy link
Member

Choose a reason for hiding this comment

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

[In relation to above comment]

I think we can avoid inter-version interoperability issues providing we omit the new reloadGlobal field when --global was not specified.

) {
result
}
Expand All @@ -87,6 +102,9 @@ def get_option_parser():
multiworkflow=True,
argdoc=[WORKFLOW_ID_MULTI_ARG_DOC],
)
for option in RELOAD_OPTIONS:
parser.add_option(*option.args, **option.kwargs)

return parser


Expand All @@ -97,6 +115,7 @@ async def run(options: 'Values', workflow_id: str):
'request_string': MUTATION,
'variables': {
'wFlows': [workflow_id],
'reloadGlobal': options.reload_global,
}
}

Expand Down
3 changes: 3 additions & 0 deletions cylc/flow/scripts/validate_reinstall.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
reinstall_cli as cylc_reinstall,
)
from cylc.flow.scripts.reload import (
RELOAD_OPTIONS,
run as cylc_reload
)
from cylc.flow.terminal import cli_function
Expand All @@ -86,6 +87,7 @@
VALIDATE_OPTIONS,
REINSTALL_OPTIONS,
REINSTALL_CYLC_ROSE_OPTIONS,
RELOAD_OPTIONS,
PLAY_OPTIONS,
CYLC_ROSE_OPTIONS,
modify={'cylc-rose': 'validate, install'}
Expand All @@ -102,6 +104,7 @@ def get_option_parser() -> COP:
for option in VR_OPTIONS:
parser.add_option(*option.args, **option.kwargs)
parser.set_defaults(is_validate=True)

return parser


Expand Down
47 changes: 47 additions & 0 deletions tests/functional/reload/29-global.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/usr/bin/env bash
# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE.
# Copyright (C) NIWA & British Crown (Met Office) & Contributors.
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#-------------------------------------------------------------------------------
# Test reloading global configuration
. "$(dirname "$0")/test_header"
set_test_number 9

create_test_global_config "" ""

TEST_NAME="${TEST_NAME_BASE}"
install_workflow "${TEST_NAME}" "${TEST_NAME_BASE}"

# Validate the config
run_ok "${TEST_NAME}-validate" cylc validate "${WORKFLOW_NAME}"

# Run the workflow
workflow_run_ok "${TEST_NAME_BASE}-run" cylc play "${WORKFLOW_NAME}" --no-detach -v

# Reload happened
grep_ok "Reloading the global configuration" "${WORKFLOW_RUN_DIR}/log/scheduler/01-start-01.log"

# Reload hasn't happened in job a, has happened in b and c
grep_fail "global init-script reloaded!" "${WORKFLOW_RUN_DIR}/log/job/1/a/01/job.out"
grep_ok "global init-script reloaded!" "${WORKFLOW_RUN_DIR}/log/job/1/b/01/job.out"
grep_ok "global init-script reloaded!" "${WORKFLOW_RUN_DIR}/log/job/1/c/01/job.out"

# Events are original in job a, updated in b and c
grep_fail "!!EVENT!! succeeded 1/a" "${WORKFLOW_RUN_DIR}/log/scheduler/01-start-01.log"
grep_ok "!!EVENT!! succeeded 1/b" "${WORKFLOW_RUN_DIR}/log/scheduler/01-start-01.log"
grep_ok "!!EVENT!! succeeded 1/c" "${WORKFLOW_RUN_DIR}/log/scheduler/01-start-01.log"

purge
exit
30 changes: 30 additions & 0 deletions tests/functional/reload/29-global/flow.cylc
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
[scheduling]
cycling mode = integer
[[graph]]
R1 = a => reload-global => b & c
[runtime]
[[root]]
script = true
[[a,b]]
platform = localhost
[[c]]
platform = $CYLC_TEST_PLATFORM
[[reload-global]]
platform = localhost
script = """
# Append to global config
cat >> "$CYLC_CONF_PATH/global.cylc" <<EOF
[platforms]
[[localhost,${CYLC_TEST_PLATFORM}]]
global init-script = echo "global init-script reloaded!"
[task events]
handlers = echo "!!EVENT!! %(event)s %(id)s"
handler events = succeeded
EOF

# Log the new config
cylc config

# Reload the global config
cylc reload --global "${CYLC_WORKFLOW_ID}"
"""
Loading
Loading