-
Notifications
You must be signed in to change notification settings - Fork 11
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
cleanup test namespace #42
Conversation
set +e | ||
|
||
ns_cleanup() { | ||
echo "Performing namespace ${NAMESPACE} cleanup..." | ||
${KUBECTL} --kubeconfig=${KUBECONFIG} --cluster=${CLUSTER} --user=${USER} delete namespace --wait=false ${NAMESPACE} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The set +e
command disables the immediate exit on error, which can lead to the script continuing execution even if critical commands fail. This can cause unexpected behavior and make debugging difficult. Consider handling errors explicitly where needed and re-enabling set -e
after the critical section.
@@ -71,6 +72,8 @@ else | |||
exit 1 | |||
fi | |||
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The while loop that generates a random namespace does not have a termination condition if the namespace creation keeps failing. This can lead to an infinite loop. Consider adding a maximum retry limit to avoid potential infinite loops.
go func() { | ||
err := stern.Run(ctx, *namespace, clientset, allowErrors, disablePodLogs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goroutine starting at line 374 calls stern.Run
but does not handle any potential errors returned by this function. If stern.Run
fails, the error will be lost, making debugging difficult.
Recommendation: Handle the error returned by stern.Run
within the goroutine, possibly by logging it or sending it to a channel for further processing.
kubectl failure in integration test setup script causes the script to exit prematurely so it_sidecar does not have a chance to perform the cleanup.
This PR adds a trap that would be executed on exit of the script whether it normal exit or by error. This also make the cleanup part of the it_sidecar obsolete.