cylc vr on a stopped workflow doesn't pass all arguments through to cylc play

[ColemanTom]

Description

If you have an installed but stopped workflow, doing cylc vr on it will try to play the workflow, but will not pass through all options that it should, e.g. it does not pass through -S, -O options.

NOTE: Limited testing, but this does not happen in Cylc=8.2.3, but I can confirm it happens in Cylc 8.2.6, 8.3.0, 8.3.1. I have not checked 8.2.4 or 8.2.5. I'm guessing it was introduced when fixes were made for -S and -O a few months ago.

Reproducible Example

flow.cylc:

#!jinja2
[scheduling]
    initial cycle point = 20200101
    [[graph]]
        P1D = b[-P2D] => b

[runtime]
    [[b]]
        script = "echo {{ FOO }}; sleep 50"

Now, start, stop, vr it

CYLC_VERSION=8.3.0 cylc vip --no-run-name --host=localhost -S FOO=1 .
cylc stop --kill basic-workflow
yes | CYLC_VERSION=8.3.0 cylc vr --host=localhost -S FOO=2 basic-workflow

Error is:

REINSTALLED basic-workflow from .../basic-workflow
Successfully reinstalled.
$ cylc play --host=localhost basic-workflow

 ▪ ■  Cylc Workflow Engine 8.3.0
 ██   Copyright (C) 2008-2024 NIWA
▝▘    & British Crown (Met Office) & Contributors

INFO - Extracting job.sh to ...
2024-07-09T03:05:48Z INFO - Workflow: basic-workflow
2024-07-09T03:05:48Z INFO - LOADING workflow parameters
2024-07-09T03:05:48Z INFO - + workflow UUID = 985f617c-a1be-4e10-b7e6-4b656305932c
2024-07-09T03:05:48Z INFO - + UTC mode = True
2024-07-09T03:05:48Z INFO - + run mode = None
2024-07-09T03:05:48Z INFO - + cycle point time zone = Z
2024-07-09T03:05:48Z ERROR - Workflow shutting down - Jinja2Error: 'FOO' is undefined
    File .../flow.cylc
      [runtime]
          [[b]]
              script = "echo {{ FOO }}; sleep 50"   <-- UndefinedError

Expected Behaviour

Options should pass through from cylc vr to cylc play.

[wxtim]

@ColemanTom - Thank you for the nice reproduceable recipe: It made replication really easy. 😸

Workaround

For the record you can continue without VR using

cylc play my-workflow  # yes the copy of it that failed with cylc vr

Investigation - So far

It looks like there is a failure by the play command called by VR to pick up the changed rose-suite-cylc-install.conf, which is changed correctly by the VR command.

i.e.

# Schematic test, not actual code
> ROSECONF=${HOME}/cylc-run/myworkflow/runN/opt/rose-suite-cylc-install.conf"
> LOGCONF=${HOME}/myworkflow/runN/log/config/

# First time around
> cylc vip -S 'FOO=1' .

> grep FOO ${CONF}
FOO=1
> grep echo ${LOGCONF}/01-start-01.cylc
script = echo 1; sleep 50

# VR
> cylc vr myworkflow -S 'FOO=2'
> grep FOO ${CONF}
FOO=2
> grep echo ${LOGCONF}/02-restart-01.cylc
grep: Path: no such file or directory

# We know that the reinstall has worked, but not the restart:
> cylc play myworkflow
> grep echo ${LOGCONF}/02-restart-01.cylc
script = echo 2; sleep 50

Which suggests that for whatever reason the play command inside VR isn't picking up the optional config.

I will investigate.

[wxtim]

Cause:

Cylc VR uses the validate --against-source option provided for cylc validate. It doesn't turn it off again after validating the workflow source. So that the path passed to cylc rose by the play step is the source, and not the installed workflow.

[MetRonnie]

It sounds like --against-source should not be an option at all for cylc vr? Also what about cylc vip?

[wxtim]

--against-source is a cylc validate option specifically for Cylc VR to check whether re-installing will create a validation failure. It shouldn't be an option that is respected by any other script.

[MetRonnie]

But it is always automatically turned on for the validate step, so I see no reason why it should be exposed at all.

[wxtim]

Because someone might want to use it on validate when validate is standalone.

[MetRonnie]

I mean do not expose it in cylc vr (obviously keep it for cylc validate)

[wxtim]

I mean do not expose it in cylc vr (obviously keep it for cylc validate)

You're right - I'm not sure how easy that is to do, but you are right.

[ColemanTom]

I believe this can be closed (and shifted back to 8.3.3 milestone)?

[wxtim]

I believe this can be closed (and shifted back to 8.3.3 milestone)?

You believe correctly. 😄

[jmancell]

I've been following along with this ticket as believe it was causing a problem when I was trying to restart and retrigger. Can I just clarify that this issue has been fixed by #6213? And fix will be available in 8.3.3?

[MetRonnie]

@jmancell Correct