hack/install.sh doesn't exit if a command fails

[alexellis]

Expected Behaviour

If any command fails, the script should stop and make it obvious to the user that things didn't work. I ran into an error downloading containerd and it was buried in the output.

Current Behaviour

hack/install.sh doesn't exit if a command fails

See also: alexellis/containerd-arm#1

List all Possible Solutions

Perhaps add pipefail if it's not being used already?

Steps to Reproduce (for bugs)

1. Deliberately break one command in the script
2. Run ./hack/install.sh
3. See it continue to run the other steps

Context

This hides errors which may not be picked up in e2e testing due to variations in OS/architecture of different users.

[xrazis]

I had a better look at it, did a bit of testing, and can confirm the above behavior. It seems that we can add set to the beginning of the script like so:

#!/bin/bash
set -e -u -o pipefail

# Copyright OpenFaaS Author(s) 2020

#########################
# Repo specific content #
#########################
...

The script exits successfully if I mess up with the containerd url.

ubuntu@faasd-testing:~/faasd/hack$ bash install.sh
Finding latest version from GitHub
0.14.3
Hit:1 http://archive.ubuntu.com/ubuntu focal InRelease
Hit:2 http://archive.ubuntu.com/ubuntu focal-updates InRelease
Hit:3 http://archive.ubuntu.com/ubuntu focal-backports InRelease
Hit:4 http://security.ubuntu.com/ubuntu focal-security InRelease
Reading package lists... Done
Reading package lists... Done
Building dependency tree       
Reading state information... Done
bridge-utils is already the newest version (1.6-2ubuntu1).
curl is already the newest version (7.68.0-1ubuntu2.7).
runc is already the newest version (1.0.0~rc95-0ubuntu1~20.04.2).
0 upgraded, 0 newly installed, 0 to remove and 15 not upgraded.
net.ipv4.conf.all.forwarding = 1
net.ipv4.conf.all.forwarding=1
./
./ptp
./macvlan
./flannel
./sbr
./host-local
./dhcp
./vlan
./bridge
./tuning
./firewall
./bandwidth
./static
./host-device
./loopback
./ipvlan
./portmap
curl: (22) The requested URL returned error: 404 

gzip: stdin: unexpected end of file
tar: Child returned status 1
tar: Error is not recoverable: exiting now
ubuntu@faasd-testing:~/faasd/hack$ 

Why each set option was used:

• -e will make the script exit immediately if any of the simple commands fail (see set docs for what exactly is a simple command)
• -u will throw an error if an unset variable is used. This wont affect anything for now but may prove useful in the future when editing the script.
• -o pipefail will return the error code of the last failing command in a pipeline expression. By default the pipeline only returns an error if the last command fails.

@alexellis let me know if this seems good and I'll go ahead and submit a PR with extensive testing.

sources:

• set man page
• this great gist explaining set in simple terms

[alexellis]

That looks promising. Let's get a PR up for it?

Out of interest, does arkade's get.sh script also do this?

[xrazis]

Nope, arkade does not use set. I tried to fail that as well but seems to fail successfully with or without set.

[xrazis]

Seems i've run into some problems with set.

This is the output when running the script in its current form

  ubuntu@faasd-testing:~/faasd/hack$ sudo bash install.sh
  Finding latest version from GitHub
  0.14.3
  Hit:1 http://archive.ubuntu.com/ubuntu focal InRelease
  Hit:2 http://archive.ubuntu.com/ubuntu focal-updates InRelease
  Hit:3 http://archive.ubuntu.com/ubuntu focal-backports InRelease
  Hit:4 http://security.ubuntu.com/ubuntu focal-security InRelease
  Reading package lists... Done                         
  Reading package lists... Done
  Building dependency tree       
  Reading state information... Done
  bridge-utils is already the newest version (1.6-2ubuntu1).
  curl is already the newest version (7.68.0-1ubuntu2.7).
  runc is already the newest version (1.0.1-0ubuntu2~20.04.1).
  0 upgraded, 0 newly installed, 0 to remove and 15 not upgraded.
  net.ipv4.conf.all.forwarding = 1
  net.ipv4.conf.all.forwarding=1
  ./
  ./ptp
  ./macvlan
  ./flannel
  ./sbr
  ./host-local
  ./dhcp
  ./vlan
  ./bridge
  ./tuning
  ./firewall
  ./bandwidth
  ./static
  ./host-device
  ./loopback
  ./ipvlan
  ./portmap
  bin/ctr
  bin/containerd-shim
  bin/containerd
  bin/containerd-shim-runc-v2
  bin/containerd-shim-runc-v1
  Finding latest version from GitHub
  0.13.15
  Downloading package https://github.com/openfaas/faas-cli/releases/download/0.13.15/faas-cli as /tmp/faas-cli
  Download complete.
  
  Running with sufficient permissions to attempt to move faas-cli to /usr/local/bin
  New version of faas-cli installed to /usr/local/bin
    ___                   _____           ____
   / _ \ _ __   ___ _ __ |  ___|_ _  __ _/ ___|
  | | | | '_ \ / _ \ '_ \| |_ / _` |/ _` \___ \
  | |_| | |_) |  __/ | | |  _| (_| | (_| |___) |
   \___/| .__/ \___|_| |_|_|  \__,_|\__,_|____/
        |_|
  
  CLI:
   commit:  b562392b12a78a11bcff9c6fca5a47146ea2ba0a
   version: 0.13.15
  curl: (23) Failed writing body (0 != 1345)
  2021/11/04 22:01:04 Writing to: "/var/lib/faasd/secrets/basic-auth-password"
  2021/11/04 22:01:04 Writing to: "/var/lib/faasd/secrets/basic-auth-user"
  Check status with:
    sudo journalctl -u faasd --lines 100 -f
  
  Login with:
    sudo cat /var/lib/faasd/secrets/basic-auth-password | faas-cli login -s
  Skipping caddy installation as FAASD_DOMAIN.

This is the output with set added and no breakpoint

  ubuntu@faasd-testing:~/faasd/hack$ sudo bash install.sh
  Finding latest version from GitHub
  0.14.3
  Hit:1 http://archive.ubuntu.com/ubuntu focal InRelease
  Hit:2 http://archive.ubuntu.com/ubuntu focal-updates InRelease
  Hit:3 http://archive.ubuntu.com/ubuntu focal-backports InRelease
  Hit:4 http://security.ubuntu.com/ubuntu focal-security InRelease
  Reading package lists... Done
  Reading package lists... Done
  Building dependency tree       
  Reading state information... Done
  bridge-utils is already the newest version (1.6-2ubuntu1).
  curl is already the newest version (7.68.0-1ubuntu2.7).
  runc is already the newest version (1.0.1-0ubuntu2~20.04.1).
  0 upgraded, 0 newly installed, 0 to remove and 15 not upgraded.
  net.ipv4.conf.all.forwarding = 1
  net.ipv4.conf.all.forwarding=1
  ./
  ./ptp
  ./macvlan
  ./flannel
  ./sbr
  ./host-local
  ./dhcp
  ./vlan
  ./bridge
  ./tuning
  ./firewall
  ./bandwidth
  ./static
  ./host-device
  ./loopback
  ./ipvlan
  ./portmap
  bin/ctr
  bin/containerd-shim
  bin/containerd
  bin/containerd-shim-runc-v2
  bin/containerd-shim-runc-v1
  Finding latest version from GitHub
  0.13.15
  Downloading package https://github.com/openfaas/faas-cli/releases/download/0.13.15/faas-cli as /tmp/faas-cli
  Download complete.
  
  Running with sufficient permissions to attempt to move faas-cli to /usr/local/bin
  New version of faas-cli installed to /usr/local/bin
    ___                   _____           ____
   / _ \ _ __   ___ _ __ |  ___|_ _  __ _/ ___|
  | | | | '_ \ / _ \ '_ \| |_ / _` |/ _` \___ \
  | |_| | |_) |  __/ | | |  _| (_| | (_| |___) |
   \___/| .__/ \___|_| |_|_|  \__,_|\__,_|____/
        |_|
  
  CLI:
   commit:  b562392b12a78a11bcff9c6fca5a47146ea2ba0a
   version: 0.13.15
  curl: (23) Failed writing body (0 != 1345)

Is this the intended behavior? set catches this error from curl: curl: (23) Failed writing body (0 != 1345), and exits the script. Should we (safely) ignore that error or should i dig more into it?

Relevant answer on SO: Why does cURL return error "(23) Failed writing body"?

[welteki]

/close: resolved in #218