Bash if fuction body is getting executed even if condition is not met

Question

I am tring to write a bash function to check pods in kuebernetes and if all pods are healthy i.e either they are Running or Completed if not I am entering the loop and exiting with error

check_pods_status() {
  # Save the output of the kubectl command in an array
  pods_status=`kubectl get pods -o wide | grep "swarm" |  awk '{print $3}'`
  echo "============================================================================================================"
  echo "checking all the microservices in the swarm cluster are running normally ..."
  kubectl get pods -o wide | grep "swarm"
  echo "============================================================================================================"
  for status in "${pods_status[@]}"; do
    if [[ "$status" != "Running" ]] || [[ "$status" != "Completed" ]]; then
      # Print an error message and return 1
      echo "Error: One or more pods are not running or completed"
      return 1
    fi
  done
  # all pods are healthy at this point and function will return 0 whihc is true
  return 0
}

this if condition

if [[ "$status" != "Running" ]] || [[ "$status" != "Completed" ]]; then

is getting passed even if all the pods are in Running state Any suggestions? thanks

I am expecting this function to check and return error if pod status is other then Running or Completed

Which value of `$status` would be both `Running` _and_ `Completed` at the same time, thus making `[[ "$status" != "Running" ]] || [[ "$status" != "Completed" ]]` false? — Biffen, Feb 02 '23 at 15:55
Have you tried combining the test statements, eg `[[ $a != $b || ... ]]`? I don't think it's doing what you think it's doing — ijustlovemath, Feb 02 '23 at 15:58
based on the text of the error message you're looking to generate, I'm thinking the conditional should be `[[ "$status" != "Running" ]] && [[ "$status" != "Completed" ]]` (note the replacement of `||` with `&&`) — markp-fuso, Feb 02 '23 at 15:59
@Biffen for Pod it can be only one status at a time it can be any of the following Waiting , Running , Terminated, or Completed .. I my test case its entering body for Running pods — Arjun Singh, Feb 02 '23 at 16:02
@ArjunSingh If `$status` is `Running` then `[[ "$status" != "Completed" ]]` is true, and vice versa. There’s no value for `$status` that can make the statement false. — Biffen, Feb 02 '23 at 16:03
when `$status=Running` the conditional evaluates to `false OR true`; when `$status=Completed` you get `true OR false`; in both cases the net result is `true` — markp-fuso, Feb 02 '23 at 16:07
I think this will correct my mistake : ``` for status in "${pods_status[@]}"; do if [ ["$status" = "Running" ]] || [[ "$status" = "Completed" ]]; then return 0 else # Print an error message and return 1 echo "Error: One or more pods are not running or completed" return 1 fi done ``` — Arjun Singh, Feb 02 '23 at 16:25
[Shellcheck](https://www.shellcheck.net/) identifies the problem with the `if` statement, and other problems. [Shellcheck](https://www.shellcheck.net/) can find many problems in shell code. It's a good idea to run it on all new and modified code. — pjh, Feb 02 '23 at 16:29
The `pods_status` assignment (as well as using the archaic backticks syntax instead of `$(...)`) sets it as a string, not an array. `pods_status=( $(kubectl ...) )` may do what you want in this case, but `array=( $(prog ...) )` doesn't work in general because of shell word splitting and pathname expansion. If you run the code through [Shellcheck](https://www.shellcheck.net/) it will provide links to safe ways of doing what you want. Also see [Reading output of a command into an array in Bash](https://stackoverflow.com/q/11426529/4154375). — pjh, Feb 02 '23 at 16:36
Personally, I would be inclined to use a `case` statement here instead of `if X && Y` (or `if X || Y`). This is largely a stylistic preference, but it does make the particular kind of logic error present here somewhat more difficult to commit. — John Bollinger, Feb 02 '23 at 16:47
edited and used OR condition : if [ "$status" = "Running" ] || [ "$status" = "Completed" ]; then but it is still entering the block.. any other suggestions? — Arjun Singh, Feb 02 '23 at 17:00

score 0 · Accepted Answer · answered Feb 02 '23 at 16:58

this if condition

if [[ "$status" != "Running" ]] || [[ "$status" != "Completed" ]]; then

is getting passed even if all the pods are in Running state

Yes, of course it is. If all the pods are in the Running state, then all of them satisfy the condition [[ "$status" != "Completed" ]], and therefore the overall condition evaluates to true for each one. This is an example of a kind of logic error that we see frequently, in all sorts of languages. Presumably, what you meant to write was

if [[ "$status" != "Running" ]] && [[ "$status" != "Completed" ]]; then

Note the change from || to &&. Now the overall condition evaluates true if and only if $status is neither Running nor Completed.

That's easy enough to spot and fix with a little experience, but in cases like this you could make the logic clearer and such mistakes more difficult to commit by using a case statement instead of an if statement:

    case $status in
    Running|Completed)
      # Do nothing in this case
      ;;
    *)
      # Print an error message and return 1
      echo "Error: One or more pods are not running or completed"
      return 1
      ;;
    esac

Bash if fuction body is getting executed even if condition is not met

1 Answers1