adoroszlai commented on a change in pull request #1778:
URL: https://github.com/apache/ozone/pull/1778#discussion_r556508057
##########
File path: hadoop-ozone/dev-support/checks/hadolint.sh
##########
@@ -17,25 +17,30 @@ DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null
2>&1 && pwd )"
cd "$DIR/../../.." || exit 1
REPO_DIR="$DIR/../../.."
-ERROR=0
-
+REPORT_DIR=${OUTPUT_DIR:-"$DIR/../../../target/hadolint"}
+mkdir -p "$REPORT_DIR"
+REPORT_FILE="$REPORT_DIR/summary.txt"
for Dockerfile in $(find "$REPO_DIR/hadoop-ozone" "$REPO_DIR/hadoop-hdds"
-name Dockerfile | sort); do
- echo "Checking $Dockerfile"
+ echo "Checking $Dockerfile" | tee -a "$REPORT_FILE"
Review comment:
This would cause exit code 1, because even without any errors, at least
`Checking ...` would be present in the report file, hence `-s` evaluates to
true. I think we can remove this line, as hadolint's error messages include
the input file's path.
##########
File path: hadoop-ozone/dev-support/checks/hadolint.sh
##########
@@ -17,25 +17,30 @@ DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null
2>&1 && pwd )"
cd "$DIR/../../.." || exit 1
REPO_DIR="$DIR/../../.."
-ERROR=0
-
+REPORT_DIR=${OUTPUT_DIR:-"$DIR/../../../target/hadolint"}
+mkdir -p "$REPORT_DIR"
+REPORT_FILE="$REPORT_DIR/summary.txt"
for Dockerfile in $(find "$REPO_DIR/hadoop-ozone" "$REPO_DIR/hadoop-hdds"
-name Dockerfile | sort); do
Review comment:
I suggest the following improvement:
```suggestion
for Dockerfile in $(find hadoop-ozone hadoop-hdds -name Dockerfile | sort);
do
```
This would provide cleaner output, eg.:
```
hadoop-ozone/dev-support/docker/Dockerfile
```
instead of
```
/opt/ozone/src/ozone/hadoop-ozone/dev-support/checks/../../../hadoop-ozone/dev-support/docker/Dockerfile
```
##########
File path: hadoop-ozone/dev-support/checks/hadolint.sh
##########
@@ -17,25 +17,30 @@ DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null
2>&1 && pwd )"
cd "$DIR/../../.." || exit 1
REPO_DIR="$DIR/../../.."
-ERROR=0
-
+REPORT_DIR=${OUTPUT_DIR:-"$DIR/../../../target/hadolint"}
+mkdir -p "$REPORT_DIR"
+REPORT_FILE="$REPORT_DIR/summary.txt"
for Dockerfile in $(find "$REPO_DIR/hadoop-ozone" "$REPO_DIR/hadoop-hdds"
-name Dockerfile | sort); do
- echo "Checking $Dockerfile"
+ echo "Checking $Dockerfile" | tee -a "$REPORT_FILE"
result=$( hadolint $Dockerfile )
if [ ! -z "$result" ]
then
- echo "$result"
- echo ""
- ERROR=1
+ echo "$result" | tee -a "$REPORT_FILE"
+ echo "" | tee -a "$REPORT_FILE"
fi
done
-if [ "$ERROR" = 1 ]
+cat "$REPORT_FILE" | egrep -v '^$|^Checking' | wc -l | awk '{print $1}'>
"$REPORT_DIR/failures"
+
+if [ -s "${REPORT_FILE}" ]
then
- echo ""
- echo ""
- echo "Hadolint errors were found. Exit code: 1."
- exit 1
+ echo "" | tee -a "$REPORT_FILE"
+ echo "" | tee -a "$REPORT_FILE"
+ echo "Hadolint errors were found. Exit code: 1." | tee -a "$REPORT_FILE"
+fi
+
+if [[ -s "${REPORT_FILE}" ]]; then
Review comment:
Nit: these two `if`s can be collapsed (no need to check the condition
twice).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
