-1

Using bash to write this script. For some reason my second IF condition is not working. I never get the message "FALSE result" when the condition is false. BTW, I need to nest two more if conditions but wanted to get this first one working. Any suggestions?

if [[ condition == "true" ]] ; then
   echo "TRUE result"

   if [[ condition == "false" ]] ; then
      echo "FALSE result"

   fi
fi
John Kugelman
  • 349,597
  • 67
  • 533
  • 578
driverA
  • 3
  • 2
  • That's not why `if-else` conditions are designed for. If `condition == "true"`, what's the need of `condition == "false"`? If you want to print both `TRUE result` & `FALSE result`, then why do you need `if-else` statements? Just print them one after the other.. – harshavmb May 19 '22 at 19:33
  • What is `condition`? If you use it in the code as shown, it's just a string. – choroba May 19 '22 at 19:36
  • Nice one ;). apparently he doesn't trust the shell. He has to ask again in True-block maybe it's False :-) – ufopilot May 19 '22 at 20:36

2 Answers2

3

There are two problems here. The first is that condition is a fixed string, and will never be equal to either "true" or "false". If it's supposed to be a variable, you need to use "$condition" to get its value (the $ is required to get the variable's value, the double-quotes are sometimes needed to avoid weird parsing of the value). So something like if [[ "$condition" == "true" ]] ; then.

The second problem is that since the second if is nested inside the first, it'll never be tested if the first condition is false. That is, if $condition is "false", it'd test whether it's equal to "true", and since it isn't it'll skip everything up to the last fi, and hence never compare it to "false".

What you probably want is an elif (short for "else if") clause instead of a nested if -- that way it'll make the second test only if the first fails, instead of only if it succeeds. Note that an elif clause is not nested, but an extension of the original if statement, so it doesn't take an additional fi to close it. So something like this:

if [[ "$condition" == "true" ]] ; then
   echo "TRUE result"

elif [[ "$condition" == "false" ]] ; then
   echo "FALSE result"

fi

If you're comparing a something against a list of possible strings/patterns, it might be better to use a case statement:

case "$condition" in
   true)
      echo "TRUE result" ;;

   false)
      echo "FALSE result" ;;

   maybe)
      echo "MAYBE result" ;;

   *)
      echo "Unrecognized result" ;;
esac
Gordon Davisson
  • 118,432
  • 16
  • 123
  • 151
  • What kind of weird parsing could leaving the double quotes out lead to? I always thought `[[` was safe in that regard and would have written `if [[ $condition == true ]] ; then` – Ted Lyngmo May 19 '22 at 19:38
  • 1
    @TedLyngmo Omitting the double-quotes is safe in that specific context (and actually `case $condition in` would also be safe), but keeping track of when it's safe and when it's dangerous is difficult -- IMO it's much easier and safer to double-quote unless there's a specific reason *not* to. – Gordon Davisson May 19 '22 at 19:43
  • Ok, you mean that it's safe to leave them out in all `[[` .. `]]` context but not in some other contexts? If so, I get it. – Ted Lyngmo May 19 '22 at 19:45
  • 3
    @TedLyngmo, ...the _left-hand side of_ `[[ $a = $b ]]` is safe to leave unquoted, but the right-hand side is interpreted as a glob unless quoted, so leaving it unquoted can have unexpected behavior when the intent is a direct string comparison. This is in keeping with Gordon's advice of preferring to teach folks to quote by default. :) – Charles Duffy May 19 '22 at 19:45
  • @CharlesDuffy Ah! That one I need to keep in mind. Thanks! – Ted Lyngmo May 19 '22 at 19:46
  • @GordonDavisson Tried your suggestion with no success. Here is my script, do you see anything wrong? ``` for x in $hostname do `nc -z $hostname 22` if [[ "$hostname" == "host1" || "$hostname" == "host2" ]] ; then message1=$(ssh -q -t adminuser@$x /usr/bin/sudo systemctl is-active sc4s.service ) echo "The SC4S service on" $hostname "is" $message1 elif [[ $message1 == "inactive" ]] ; then echo "The SC4S service on" $hostname "is" $message1 "Please run the startsplunk script to restart the service" fi done ``` – driverA May 19 '22 at 20:03
  • @driverA Comments don't allow code blocks, and have a size limit; it'd be better to edit your question to include the full script. But first, I'd recommend running it through [shellcheck.net](https://www.shellcheck.net) and fixing any problems that points out. – Gordon Davisson May 19 '22 at 20:05
  • @GordonDavisson so basically, the ELIF is not being tested and I never get the "...please restart..." message when the service is inactive. – driverA May 19 '22 at 20:17
  • @driverA It looks like in that version, you *do* want to nest the `if` statements. `$message1` is set inside the first `if` section, so if the first condition (on `$hostname`) isn't matched, it won't have any value, so testing it in an `elif` clause doesn't make any sense. In any case, you need to think carefully about what should happen if a condition is true vs if it's false, and arrange the conditions based on that. Also, your quoting is all weird; again, variable references should be *inside* double-quotes, not left outside the double-quoted sections as you have them. – Gordon Davisson May 19 '22 at 21:47
  • @GordonDavisson In the end, I ended up simplifying my script and using the KISS(Keep It Simple Stupid) method and using some of your suggestions. The script works and thanks for the shellcheck.net link, that's awesome! – driverA May 20 '22 at 22:52
0
read -p "Enter hostname(s): " HOSTS

for host in $HOSTS
do
   echo "Test ssh-port on $host"
   nc -zv -w 2 $host 22
   if [ $? -ne 0 ]; then
      echo "$host is not reachable.. See message above. Check hostname/ssh-deamon/firewall"
      continue
   fi
   if [ "$host" = "host1" -o "$host" = "host2" ] ; then
        message=$(ssh -q -t adminuser@$host "/usr/bin/sudo systemctl is-active sc4s.service")
        echo "The SC4S service on $host is $message"
        [ "$message" = "inactive" ] && echo "Please run the startsplunk script to restart the service on $host"
   fi
done
ufopilot
  • 3,269
  • 2
  • 10
  • 12