Use non-login shell for `launch_FV3LAM_wflow.sh`; remove support for `WCOSS_CRAY`; fix cron capability for `tcsh` users on Cheyenne #675

gsketefian · 2022-02-07T20:48:04Z

DESCRIPTION OF CHANGES:

This PR:

Makes the launch_FV3LAM_wflow.sh script run in a non-login shell to avoid sourcing of user-specific login files (e.g ~/.bash_profile, ~/.bash_login, and ~/.profile).
Generalizes the loading of system scripts to initialize not only the module command but others (by sourcing /etc/profile). This is necessary if not using a login shell in launch_FV3LAM_wflow.sh.
Adds an optional argument to launch_FV3LAM_wflow.sh that specifies whether it is being called from a cron job. This is necessary because on Cheyenne, the version of crontab to use depends on whether or not the parent script is being called from a cron job.
Removes support for WCOSS_CRAY since it will be decommissioned soon and is difficult to support.
Removes machine-dependent case statements from ex-scripts.

One of the consequences of this use of non-login shell for launch_FV3LAM_wflow.sh is that the workflow's cron capability now works for tcsh users on Cheyenne.

TESTS CONDUCTED:

Ran the two WE2E tests deactivate_tasks and grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15p2 on Hera, Jet, Cheyenne, and Orion. All passed.

Note that I used tcsh as my login shell on Cheyenne for the purposes of testing this PR (and the tests succeeded).

ISSUE (optional):

Resolves Issue #603.

CONTRIBUTORS:

@chan-hoo, @willmayfield, @mkavulich

…calls since that's the proper calling method; edit comments.

* Add a wrapper (source_machine_file.sh) for sourcing the machine file that allows other commands common to all machines to be called. * Change the scalar variable MODULE_INIT_PATH in the machine files to the array variable ENV_INIT_SCRIPTS_FPS that specifies the list of system scripts that need to be sourced (e.g. to make the "module" command available in a given script). This is needed because on Cheyenne, at least two system scripts need to be sourced (to enable "module" and "qsub"). * Move the "ulimit" commands at the ends of the machine files into the new variable PRE_TASK_CMDS so that they are not called every time the machine file is sourced. They will be called only if a given script issues an "eval ${PRE_TASK_CMDS}" (which all the ex-scripts will do).

…se the wrapper source_machine_file.sh; (2) Use "eval" to evaluate the contents of PRE_TASK_CMDS.

…apper source_machine_file.sh.

… can be sourced and the valid values for a boolean can be made available to any other script.

…or tcsh users). Details: * Introduce new function get_crontab_contents() that takes as input whether or not the calling script is itself being called from a cron job and returns (1) the path to the appropriate crontab command and (2) the contents of the user's cron table. * Such a function is needed because on Cheyenne, the location of the crontab command is different depending on whether or not the script that's calling crontab is itself called from a cron job (because on Cheyenne, "crontab" is containerized, and that complicates things). * Use get_crontab_contents() in generate_FV3LAM_wflow.sh and launch_FV3LAM_wflow.sh (instead of simply calling "crontab" because the latter approach doesn't work on Cheyenne, at least not with users whose login shell is tcsh). * Add "called_from_cron" as an optional argument to launch_FV3LAM_wflow.sh [so that it can then be passed on to get_crontab_contents()]. This argument is only used in the cron job that relaunches the workflow (which is created only if USE_CRON_TO_RELAUNCH is set to "TRUE"). * Having an optional argument like this seems to be the best way to tell launch_FV3LAM_wflow.sh whether or not it is running from a cron job. * launch_FV3LAM_wflow.sh can still be called from the command line without any arguments (since the default value of "called_from_cron" is "FALSE").

…tem scripts can be sourced in a given script (currently, only "module" is initialized). Details: * Introduce the new function init_env() that initializes the envrionment of a script by sourcing necessary system scripts. The full paths to these system scripts are specified in the array ENV_INIT_SCRIPTS_FPS in the machine files. * This function is needed because (1) this sourcing needs to be done in a couple of different scripts in the SRW App and (2) on some machines (e.g. Cheyenne), more than one system script may need to be sourced. * Use the new init_env() function in launch_FV3LAM_wflow.sh and load_modules_run_task.sh. * In load_modules_run_task.sh, init_env() replaces sourcing of only the system script that defines the "module" command. That is because on Cheyenne, in addition to the "module" command, the "qsub" command needs to be defined/initialized (by sourcing a second system script named pbs.sh).

gsketefian · 2022-02-07T20:56:14Z

ush/machine/cheyenne.sh

-MODULE_INIT_PATH=${MODULE_INIT_PATH:-/glade/u/apps/ch/opt/lmod/8.1.7/lmod/8.1.7/init/sh}
+# System scripts to source to initialize various commands within workflow
+# scripts (e.g. "module").
+if [ -z ${ENV_INIT_SCRIPTS_FPS:-""} ]; then


I generalized the MODULE_INIT_PATH variable to the array ENV_INIT_SCRIPTS_FPS (in all the macine files) because on some machines, more than one system script may need to be sourced to enable the module and other commands.

gsketefian · 2022-02-07T20:57:17Z

ush/machine/cheyenne.sh

+fi
+
+# Commands to run at the start of each workflow task.
+PRE_TASK_CMDS='{ ulimit -s unlimited; ulimit -a; }'


Placed the commands that were previously called at the end of the machine files in the new variable PRE_TASK_COMMANDS so they aren't run every time the machine file is sourced but only as necessary.

gsketefian · 2022-02-09T17:48:06Z

scripts/exregional_make_grid.sh

@@ -107,8 +107,8 @@ case "$MACHINE" in
    ;;

  *)
-    source ${MACHINE_FILE}
-    ;;
+    source $USHDIR/source_machine_file.sh


Added a wrapper around sourcing of the machine file in order to include actions we may want to do for all those files (e.g. do set -x depending on the DEBUG variable). See the new file source_machine_file.sh below.

gsketefian · 2022-02-09T17:49:22Z

scripts/exregional_make_grid.sh

-    source ${MACHINE_FILE}
-    ;;
+    source $USHDIR/source_machine_file.sh
+    eval ${PRE_TASK_CMDS}


I moved the commands (ulimit ...) at the end of the machine files into the new variable PRE_TASK_CMDS so that they are not automatically executed when the machine file is sourced. Execute them here though.

gsketefian · 2022-02-09T17:51:18Z

ush/constants.sh

+# Other.
+#
+#-----------------------------------------------------------------------
+#


Convenience variable since this array shows up in multiple places. Will use it in valid_param_vals.sh in a future PR.

gsketefian · 2022-02-09T17:54:35Z

ush/generate_FV3LAM_wflow.sh

@@ -59,6 +59,7 @@ ushdir="${scrfunc_dir}"
 #-----------------------------------------------------------------------
 #
 . $ushdir/source_util_funcs.sh
+. $ushdir/get_crontab_contents.sh


This new function is needed because of the way the crontab function is set up on Cheyenne. The path to the command depends on whether or not it is being called from a cron job. Also, on Cheyenne, crontab adds header lines to its output. This function strips out those headers so the user's cron table doesn't unnecessarily grow.

gsketefian · 2022-02-09T17:57:49Z

ush/launch_FV3LAM_wflow.sh

+#
+#-----------------------------------------------------------------------
+#
+valid_args=( \


FV3LAM_wflow.sh now needs an optional argument (called_from_cron) in order to tell whether or not it is being called from a cron job. This is needed on Cheyenne because the path to the crontab command (which is called in this script) depends on this distinction.

gsketefian · 2022-02-09T18:00:02Z

ush/load_modules_run_task.sh

-#
-esac
+env_init_scripts_fps_str="( "$(printf "\"%s\" " "${ENV_INIT_SCRIPTS_FPS[@]}")")"
+init_env env_init_scripts_fps="${env_init_scripts_fps_str}"


Replaced case statement with the new init_env() function that sources the necessary system scripts to make module and other commands (e.g. qsub on Cheyenne) available. I created a separate function because the block of code is also needed in launch_FV3LAM_wflow.sh.

gsketefian · 2022-02-09T18:07:08Z

ush/launch_FV3LAM_wflow.sh

+#-----------------------------------------------------------------------
+#
+env_init_scripts_fps_str="( "$(printf "\"%s\" " "${ENV_INIT_SCRIPTS_FPS[@]}")")"
+init_env env_init_scripts_fps="${env_init_scripts_fps_str}"


The new init_env() function sources one or more system scripts (now specified by the array ENV_INIT_SCRIPTS_FPS in the machine files) that enable use of certain commands like module (and qsub on Cheyenne). These commands are not defined by default if not using a login shell.

gsketefian · 2022-02-09T20:54:43Z

ush/launch_FV3LAM_wflow.sh

@@ -1,4 +1,4 @@
-#!/bin/bash -l
+#!/bin/bash


Removed the -l flag so that this script runs in a non-login shell. This means a user's specific environment doesn't get loaded (e.g. the files ~/.bash_profile, ~/.bash_login, and ~/.profile aren't sourced). This is the only remaining workflow script in which the -l is (was) still used. But removing -l also means /etc/profile isn't sourced, which means certain commands (like module) will have to be initialized (which is what the init_env function and ENV_INIT_SCRIPTS_FPS variable now do).

…. Fix comments and informational messages.

…gfix/crontab_cheyenne

…sourcing "/etc/profile" is enough to make both the "module" and "qsub" commands (and probably all other system-supported commands) available in non-login scripts.

…" (and other commands) to work.

… for-loop so it's different than the variable i used (and unset) by the system script on Hera.

…gfix/crontab_cheyenne

chan-hoo · 2022-02-23T13:59:42Z

@gsketefian, this pr works on the wcoss dell and cray well now. However, it doesn't work on hera: ./generate_FV3LAM_wflow.sh: line 509: crontab_contents: unbound variable

JeffBeck-NOAA · 2022-02-24T01:33:01Z

@gsketefian, this pr works on the wcoss dell and cray well now. However, it doesn't work on hera: ./generate_FV3LAM_wflow.sh: line 509: crontab_contents: unbound variable

@chan-hoo, thanks for getting this working on the WCOSS Cray and Dell. I think @gsketefian was able to get this to run on Hera today without any problem, so I'll let him comment.

A note on the WCOSS Cray, I spoke with Jacob today regarding the move to WCOSS2 that's coming soon. Since the WCOSS Cray will be retired this summer, we should be able to wind down our support for that platform at this point.

gsketefian · 2022-02-24T11:27:49Z

@chan-hoo I'm going to include parts of our email exchange here (and my responses) so others understand the background.

On Wed, Feb 23, 2022 at 7:01 AM Chan-Hoo Jeon - NOAA Affiliate <chan-hoo.jeon@noaa.gov> wrote:
Gerard,

The problem was
  if [ "$MACHINE" = "WCOSS_DELL_P3" ]; then
    grep_output=$( grep "^${crontab_line_esc_astr}$" "/u/$USER/cron/mycrontab" )
  else
    grep_output=$( echo "${crontab_contents}" | grep "^${crontab_line_esc_astr}$" )
  fi

  exit_status=$?

  if [ "${exit_status}" -eq 0 ]; then
Even in case grep_output="" (nothing), exit_status=0 (It is not supposed to be zero).
So, nothing copies to the crontab file.

Do you have an idea of how to fix this?

Thanks,
Chan-Hoo

It's strange that on the cray, using

grep_output=$( echo "${crontab_contents}" | grep "^${crontab_line_esc_astr}$" )

doesn't work (i.e. the exit status of grep is 0 even if the line is not found in the cron table) but

grep_output=$( crontab -l | grep "^${crontab_line_esc_astr}$" )

works (i.e. returns a non-zero exit status if the line is not found). Since both commands use grep, if the line is not in the cron table, then the exit status should be non-zero regardless of whether the input to grep came from echo or from crontab -l.

On Wed, Feb 23, 2022 at 5:36 AM Chan-Hoo Jeon - NOAA Affiliate chan-hoo.jeon@noaa.gov wrote:
The original expression works well.
So I will add it to the if statement.
  elif [ "$MACHINE" = "WCOSS_CRAY" ]; then
    grep_output=$( crontab -l | grep "^${crontab_line_esc_astr}$" )
Chan-Hoo

I am reluctant to use this fix because it defeats the purpose of having the new function get_crontab_contents.sh that deals with the oddities of getting the contents of the cron table on each machine all in one place instead of having to put if-statements in various places in the scripts. An alternative fix might be to look at the contents of grep_output instead of exit_status in determining what to do in the if-statement that follows these lines.

But -- since it is such a hassle going back and forth to debug this on the cray (because I don't have access to it) and since @JacobCarley-NOAA gave the ok to remove support for the cray from the SRW App, unless you have an objection I am going to go ahead and remove the cray in my next commit.

Btw, I did test this latest version on Hera and it worked for me. I will re-test once I make the latest changes to remove the cray.

chan-hoo · 2022-02-24T12:00:10Z

@gsketefian, I don't have any objection for the change at all. It will make my work easier than before :)

… whether the RUN_CMD_... variable is empty, i.e. -z "${RUN_CMD_...}". This is needed because on Cheyenne, not having the double quotes generates an error when RUN_CMD_... consists of a command that contains spaces (e.g. "mpirun -np ...").

gsketefian · 2022-02-24T17:35:19Z

@chan-hoo Ok, great! Makes life a lot easier! I just pushed a commit that removes support for WCOSS_CRAY. Do you mind testing on WCOSS_DELL_P3 one more time?

@willmayfield Do you mind testing the latest (hash 930f4cc) on Cheyenne? I tested with tcsh and it worked for me, but it would be great if you can as well. It would be nice to also test with bash. Can someone else test with bash on Cheyenne (since it takes a day to switch login shells)? @mkavulich, do you happen to have time?

I tested on Hera and it works. Currently testing on Jet. I can't test on Orion today since it's down for maintenance, so I will try tomorrow.

Thanks all.

gsketefian · 2022-02-24T17:40:02Z

scripts/exregional_make_ics.sh


 nprocs=$(( NNODES_MAKE_ICS*PPN_MAKE_ICS ))

-if [ -z ${RUN_CMD_UTILS:-} ] ; then
+if [ -z "${RUN_CMD_UTILS:-}" ] ; then


Need to have this in (double) quotes because on Cheyenne, if RUN_CMD_UTILS contains spaces (e.g. mpirun -np ...), the -z operator will fail with a too many arguments error. Same in all the other ex-scripts.

gsketefian · 2022-02-24T17:46:05Z

ush/templates/FV3LAM_wflow.xml

@@ -177,7 +177,7 @@ MODULES_RUN_TASK_FP script.

    &RSRV_DEFAULT;
    <command>&LOAD_MODULES_RUN_TASK_FP; "&MAKE_GRID_TN;" "&JOBSDIR;/JREGIONAL_MAKE_GRID"</command>
-  {%- if machine in ["WCOSS_DELL_P3", "WCOSS_CRAY"]  %}
+  {%- if machine in ["WCOSS_DELL_P3"]  %}


Removing support for WCOSS_CRAY.

chan-hoo · 2022-02-28T12:40:06Z

@gsketefian, it works well on the dell.

gsketefian · 2022-02-28T16:57:21Z

@gsketefian, it works well on the dell.

Great, thanks @chan-hoo!

JeffBeck-NOAA

Approving based on previous assessment. We need to have a final test run on Cheyenne to confirm things work. @willmayfield or @mkavulich, do either of you have time to test (with tcsh and/or bash)?

willmayfield · 2022-02-28T21:25:23Z

@JeffBeck-NOAA I'll be testing it today.

willmayfield

tested on Cheyenne with bash and tcsh.

gsketefian added 10 commits February 4, 2022 11:20

Remove unneeded sourcing of source_util_funcs.sh; add "%s" to printf …

9dbc04a

…calls since that's the proper calling method; edit comments.

In the relevant ex-scripts: (1) Change sourcing of machine files to u…

922e90b

…se the wrapper source_machine_file.sh; (2) Use "eval" to evaluate the contents of PRE_TASK_CMDS.

In the WE2E script, change sourcing of the machine file to use the wr…

340a4bf

…apper source_machine_file.sh.

Add new variable valid_vals_BOOLEAN to constants.sh so that this file…

0f91092

… can be sourced and the valid values for a boolean can be made available to any other script.

Bug fix.

d66a205

Remove file that was accidentally added in previous commit.

321af10

Merge branch 'develop' into bugfix/crontab_cheyenne

755418b

gsketefian commented Feb 7, 2022

View reviewed changes

gsketefian commented Feb 9, 2022

View reviewed changes

gsketefian added 2 commits February 9, 2022 16:22

Replace calls to "crontab -l" by echoing of already obtained contents…

2d74154

…. Fix comments and informational messages.

Merge remote-tracking branch 'origin/bugfix/crontab_cheyenne' into bu…

5bdddd7

…gfix/crontab_cheyenne

gsketefian changed the title ~~Fix crontab capability for tcsh users on Cheyenne~~ Fix cron capability for tcsh users on Cheyenne Feb 9, 2022

gsketefian added 5 commits February 9, 2022 20:23

For Cheyenne, don't need to source two separate system scripts. Just …

8491c10

…sourcing "/etc/profile" is enough to make both the "module" and "qsub" commands (and probably all other system-supported commands) available in non-login scripts.

Make script exit with an error message if rocoto commands fail.

85db1e3

Fix the system script that needs to be sourced on Hera to get "module…

3441c51

…" (and other commands) to work.

In init_env.sh, declare "local" variables and change the index of the…

865b784

… for-loop so it's different than the variable i used (and unset) by the system script on Hera.

Merge remote-tracking branch 'origin/bugfix/crontab_cheyenne' into bu…

2ffbb74

…gfix/crontab_cheyenne

gsketefian changed the title ~~Fix cron capability for tcsh users on Cheyenne~~ Fix cron capability for tcsh users on Cheyenne; use non-login shell launch_FV3LAM_wflow.sh Feb 10, 2022

gsketefian changed the title ~~Fix cron capability for tcsh users on Cheyenne; use non-login shell launch_FV3LAM_wflow.sh~~ Fix cron capability for tcsh users on Cheyenne; use non-login shell for launch_FV3LAM_wflow.sh Feb 10, 2022

gsketefian requested review from ywangwof, panll, christopherwharrop-noaa, danielabdi-noaa, mark-a-potts, jkbk2004, willmayfield, jwolff-ncar and dmwright526 as code owners February 23, 2022 13:21

Remove support for WCOSS_CRAY.

998564d

gsketefian changed the title ~~Use non-login shell for launch_FV3LAM_wflow.sh; fix cron capability for tcsh users on Cheyenne~~ Use non-login shell for launch_FV3LAM_wflow.sh; remove support for WCOSS_CRAY; fix cron capability for tcsh users on Cheyenne Feb 24, 2022

gsketefian commented Feb 24, 2022

View reviewed changes

chan-hoo approved these changes Feb 28, 2022

View reviewed changes

JeffBeck-NOAA approved these changes Feb 28, 2022

View reviewed changes

willmayfield approved these changes Feb 28, 2022

View reviewed changes

JeffBeck-NOAA merged commit 140a699 into ufs-community:develop Mar 1, 2022

gsketefian mentioned this pull request Mar 2, 2022

Feature/singularity addition #686

Merged

gsketefian mentioned this pull request Mar 15, 2022

crontab automation does not work on Cheyenne with tcsh #603

Closed

mkavulich mentioned this pull request Mar 28, 2022

In launch_FV3LAM_wflow.sh, remove the "-l" flag to the bash command on the first line #496

Closed

gsketefian mentioned this pull request Apr 8, 2022

Adds an alternative python workflow generation path #698

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use non-login shell for `launch_FV3LAM_wflow.sh`; remove support for `WCOSS_CRAY`; fix cron capability for `tcsh` users on Cheyenne #675

Use non-login shell for `launch_FV3LAM_wflow.sh`; remove support for `WCOSS_CRAY`; fix cron capability for `tcsh` users on Cheyenne #675

gsketefian commented Feb 7, 2022 •

edited

Loading

gsketefian Feb 7, 2022 •

edited

Loading

gsketefian Feb 7, 2022 •

edited

Loading

gsketefian Feb 9, 2022

gsketefian Feb 9, 2022 •

edited

Loading

gsketefian Feb 9, 2022

gsketefian Feb 9, 2022 •

edited

Loading

gsketefian Feb 9, 2022 •

edited

Loading

gsketefian Feb 9, 2022 •

edited

Loading

gsketefian Feb 9, 2022 •

edited

Loading

gsketefian Feb 9, 2022 •

edited

Loading

chan-hoo commented Feb 23, 2022

JeffBeck-NOAA commented Feb 24, 2022

gsketefian commented Feb 24, 2022

chan-hoo commented Feb 24, 2022

gsketefian commented Feb 24, 2022

gsketefian Feb 24, 2022

gsketefian Feb 24, 2022

chan-hoo commented Feb 28, 2022

gsketefian commented Feb 28, 2022

JeffBeck-NOAA left a comment •

edited

Loading

willmayfield commented Feb 28, 2022

willmayfield left a comment

		@@ -1,4 +1,4 @@
		#!/bin/bash -l
		#!/bin/bash

Use non-login shell for launch_FV3LAM_wflow.sh; remove support for WCOSS_CRAY; fix cron capability for tcsh users on Cheyenne #675

Use non-login shell for launch_FV3LAM_wflow.sh; remove support for WCOSS_CRAY; fix cron capability for tcsh users on Cheyenne #675

Conversation

gsketefian commented Feb 7, 2022 • edited Loading

DESCRIPTION OF CHANGES:

TESTS CONDUCTED:

ISSUE (optional):

CONTRIBUTORS:

gsketefian Feb 7, 2022 • edited Loading

Choose a reason for hiding this comment

gsketefian Feb 7, 2022 • edited Loading

Choose a reason for hiding this comment

gsketefian Feb 9, 2022

Choose a reason for hiding this comment

gsketefian Feb 9, 2022 • edited Loading

Choose a reason for hiding this comment

gsketefian Feb 9, 2022

Choose a reason for hiding this comment

gsketefian Feb 9, 2022 • edited Loading

Choose a reason for hiding this comment

gsketefian Feb 9, 2022 • edited Loading

Choose a reason for hiding this comment

gsketefian Feb 9, 2022 • edited Loading

Choose a reason for hiding this comment

gsketefian Feb 9, 2022 • edited Loading

Choose a reason for hiding this comment

gsketefian Feb 9, 2022 • edited Loading

Choose a reason for hiding this comment

chan-hoo commented Feb 23, 2022

JeffBeck-NOAA commented Feb 24, 2022

gsketefian commented Feb 24, 2022

chan-hoo commented Feb 24, 2022

gsketefian commented Feb 24, 2022

gsketefian Feb 24, 2022

Choose a reason for hiding this comment

gsketefian Feb 24, 2022

Choose a reason for hiding this comment

chan-hoo commented Feb 28, 2022

gsketefian commented Feb 28, 2022

JeffBeck-NOAA left a comment • edited Loading

Choose a reason for hiding this comment

willmayfield commented Feb 28, 2022

willmayfield left a comment

Choose a reason for hiding this comment

Use non-login shell for `launch_FV3LAM_wflow.sh`; remove support for `WCOSS_CRAY`; fix cron capability for `tcsh` users on Cheyenne #675

Use non-login shell for `launch_FV3LAM_wflow.sh`; remove support for `WCOSS_CRAY`; fix cron capability for `tcsh` users on Cheyenne #675

gsketefian commented Feb 7, 2022 •

edited

Loading

gsketefian Feb 7, 2022 •

edited

Loading

gsketefian Feb 7, 2022 •

edited

Loading

gsketefian Feb 9, 2022 •

edited

Loading

gsketefian Feb 9, 2022 •

edited

Loading

gsketefian Feb 9, 2022 •

edited

Loading

gsketefian Feb 9, 2022 •

edited

Loading

gsketefian Feb 9, 2022 •

edited

Loading

gsketefian Feb 9, 2022 •

edited

Loading

JeffBeck-NOAA left a comment •

edited

Loading