[WIP] Add loop to retry where antivirus fails in clamav

[kimallen]

Description

Adds a retry in the antivirus script in case definitions not available on clamav. Retries 5 times before failure.

Reviewer Notes

Wondering if the loop should include more (like cleaning up containers and files). Also whether another check should be added for docker exec -it "${CONTAINER_NAME}" chown clamav:clamav "/store/${WHITELIST}"

Setup

run antivirus locally with make antivirus

Code Review Verification Steps

• Request review from a member of a different team.
• Have the Jira acceptance criteria been met for this change?

References

• A previous failure with logs on CircleCI
• How to test virus scanning

[chrisgilmerproj]

I think you need something in here:

Suggested change

	echo "Container not running! Retry number ${RETRY_COUNT}"
	echo "Container not running! Retry number ${RETRY_COUNT}"
	if [ "${RETRY_COUNT}" == "5" ]; then
	exit 1
	fi

What you want to do is exit this process on the fifth time this fails to run. Otherwise the for-loop ends and the script continues on and will fail a bit later.

[kimallen]

Is it correct that 5 would be a string in bash syntax?

[kimallen]

Oh, a little research and I see that bash variables are untyped.

[chrisgilmerproj]

Yeah, for safety its best to quote everything and treat it as a string. Depending on your version of bash the number might be interpreted as a number or a string, which is not great for us.

[chrisgilmerproj]

Turns out this returns the string true or false and not an exit code. So here's an update:

Suggested change

	if ! docker inspect -f '{{.State.Running}}' "${CONTAINER_NAME}"; then
	if [ "$(docker inspect -f '{{.State.Running}}' "${CONTAINER_NAME}")" == "false" ] ; then

[chrisgilmerproj]

@kimallen - this is great work but the more I look into this the more I see some fundamental issues with the script itself. I'm going to see if I can't figure out what's going on there before I send you more comments.

[chrisgilmerproj]

Looks like the warning about "OUTDATED!" has been reported: mko-x/docker-clamav#39

The probably is upstream from the container maintainer so it may require more effort. But our virus definitions should all be up to date when this runs, which ought to mean we're ok.

[chrisgilmerproj]

@kimallen - great work here but I had to overhaul the script. So I've pushed changes here: #3688. I suggest closing this and if you'd like I can give you a tour of the changes I made in the other PR.