aws / amazon-vpc-cni-k8s

Networking plugin repository for pod networking in Kubernetes using Elastic Network Interfaces on AWS
Apache License 2.0
2.26k stars 737 forks source link

this shell script's quality is extremely poor #311

Closed borats closed 5 years ago

borats commented 5 years ago

Amazon AWS, please improve your bash quality. Honestly, this script looks like someone cobbled it in couple of minutes. Worst yet, internal CR failed to review it with a careful set of eyes.

--- aws-cni-support-aws.sh  2019-02-03 01:23:31.000000000 +1100
+++ aws-cni-support-mine.sh 2019-02-03 01:22:41.000000000 +1100
@@ -1,4 +1,6 @@
-#!/bin/bash
+#!/usr/bin/env bash
+#XXX
+
 # Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License"). You may
@@ -17,9 +19,13 @@
 # Set language to C to make sorting consistent among different environments.
 export LANG=C

-set -e
+# OMG! was it this hard
+set -euf -o pipefail
 LOG_DIR="/var/log/aws-routed-eni"

+#XXX
+[[ ! -d ${LOG_DIR} ]] && mkdir -p ${LOG_DIR}
+
 # collecting L-IPAMD introspection data
 curl http://localhost:61678/v1/enis         > ${LOG_DIR}/eni.output
 curl http://localhost:61678/v1/pods         > ${LOG_DIR}/pod.output
@@ -33,11 +39,13 @@
 # collecting kubelet introspection data
 curl http://localhost:10255/pods  > ${LOG_DIR}/kubelet.output

+#XXX keep the output filename consistent.. nah.. its really hard..
+
 # ifconfig
-ifconfig > ${LOG_DIR}/ifconig.output
+ifconfig > ${LOG_DIR}/ifconfig.out #XXX typo in production code. great.

 # ip rule show
-ip rule show > ${LOG_DIR}/iprule.output
+ip rule show > ${LOG_DIR}/iprule.out

 # iptables-save
 iptables-save > $LOG_DIR/iptables-save.out
@@ -56,7 +64,7 @@
 cp /var/log/messages $LOG_DIR/

 # dump out route table
-ROUTE_OUTPUT="route.output"
+ROUTE_OUTPUT="route.out" #XXX typo in production code. great.
 echo "=============================================" >> ${LOG_DIR}/${ROUTE_OUTPUT}
 echo "ip route show table all" >> $LOG_DIR/$ROUTE_OUTPUT
 ip route show table all >> $LOG_DIR/$ROUTE_OUTPUT
illusionalsagacity commented 5 years ago

If this is your proposed diff it looks like this (LOGDIR instead of LOG_DIR) is a typo:

+[[ ! -d ${LOG_DIR} ]] && mkdir -p ${LOGDIR}

Happens to the best of us

borats commented 5 years ago

If this is your proposed diff it looks like this (LOGDIR instead of LOG_DIR) is a typo:

+[[ ! -d ${LOG_DIR} ]] && mkdir -p ${LOGDIR}

Happens to the best of us

Here

+[[ ! -d ${LOG_DIR} ]] && mkdir -p ${LOG_DIR}

Fixed. Last I counted, AWS had one of the largest devs in the world... and counting..