Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

this shell script's quality is extremely poor #311

Closed
borats opened this issue Feb 2, 2019 · 2 comments
Closed

this shell script's quality is extremely poor #311

borats opened this issue Feb 2, 2019 · 2 comments

Comments

@borats
Copy link

borats commented Feb 2, 2019

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
Copy link

illusionalsagacity commented Feb 11, 2019

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
Copy link
Author

borats commented Feb 11, 2019

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..

mogren pushed a commit to mogren/amazon-vpc-cni-k8s that referenced this issue Feb 11, 2019
* Standardized on .out for output
* Create log directory if it doesn't exist
* Stop referencing `localhost:10255`
* Consistent parameter substitution

Fixes aws#285 and aws#311
mogren pushed a commit to mogren/amazon-vpc-cni-k8s that referenced this issue Feb 11, 2019
* Standardized on .out for output
* Create log directory if it doesn't exist
* Stop referencing `localhost:10255`
* Consistent parameter substitution

Fixes aws#285 and aws#311

(cherry picked from commit ddbb065)
mogren pushed a commit to mogren/amazon-vpc-cni-k8s that referenced this issue Feb 11, 2019
* Standardized on .out for output
* Create log directory if it doesn't exist
* Stop referencing `localhost:10255`
* Consistent parameter substitution

Fixes aws#285 and aws#311

(cherry picked from commit ddbb065)
mogren pushed a commit to mogren/amazon-vpc-cni-k8s that referenced this issue Feb 12, 2019
* Standardized on .out for output
* Create log directory if it doesn't exist
* Stop referencing `localhost:10255`
* Consistent parameter substitution

Fixes aws#285 and aws#311

(cherry picked from commit ddbb065)
mogren pushed a commit to mogren/amazon-vpc-cni-k8s that referenced this issue Feb 28, 2019
* Standardized on .out for output
* Create log directory if it doesn't exist
* Stop referencing `localhost:10255`
* Consistent parameter substitution

Fixes aws#285 and aws#311

(cherry picked from commit ddbb065)
mogren pushed a commit to mogren/amazon-vpc-cni-k8s that referenced this issue Feb 28, 2019
* Standardized on .out for output
* Create log directory if it doesn't exist
* Stop referencing `localhost:10255`
* Consistent parameter substitution

Fixes aws#285 and aws#311

(cherry picked from commit ddbb065)
mogren pushed a commit to mogren/amazon-vpc-cni-k8s that referenced this issue Feb 28, 2019
* Standardized on .out for output
* Create log directory if it doesn't exist
* Stop referencing `localhost:10255`
* Consistent parameter substitution

Fixes aws#285 and aws#311

(cherry picked from commit ddbb065)
mogren pushed a commit to mogren/amazon-vpc-cni-k8s that referenced this issue Mar 1, 2019
* Standardized on .out for output
* Create log directory if it doesn't exist
* Stop referencing `localhost:10255`
* Consistent parameter substitution

Fixes aws#285 and aws#311

(cherry picked from commit ddbb065)
mogren pushed a commit to mogren/amazon-vpc-cni-k8s that referenced this issue Mar 2, 2019
* Standardized on .out for output
* Create log directory if it doesn't exist
* Stop referencing `localhost:10255`
* Consistent parameter substitution

Fixes aws#285 and aws#311

(cherry picked from commit ddbb065)
rudoi pushed a commit to newrelic-forks/amazon-vpc-cni-k8s that referenced this issue Apr 1, 2019
* Standardized on .out for output
* Create log directory if it doesn't exist
* Stop referencing `localhost:10255`
* Consistent parameter substitution

Fixes aws#285 and aws#311

(cherry picked from commit ddbb065)
@mogren mogren closed this as completed Apr 3, 2019
mogren pushed a commit to mogren/amazon-vpc-cni-k8s that referenced this issue Apr 25, 2019
* Standardized on .out for output
* Create log directory if it doesn't exist
* Stop referencing `localhost:10255`
* Consistent parameter substitution

Fixes aws#285 and aws#311

(cherry picked from commit ddbb065)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants