first release of support-util.sh #242
Conversation
aantonel-sysdig
left a comment
aantonel-sysdig
left a comment
There was a problem hiding this comment.
Approving first public release
|
|
||
| DEST_DIR=$(pwd) | ||
| SYSDIG_SUPPORT_DIR="sysdig-support" | ||
| [ ! -d "${DEST_DIR}/${SYSDIG_SUPPORT_DIR}" ] && mkdir -p "${DEST_DIR}/${SYSDIG_SUPPORT_DIR}" |
There was a problem hiding this comment.
| [ ! -d "${DEST_DIR}/${SYSDIG_SUPPORT_DIR}" ] && mkdir -p "${DEST_DIR}/${SYSDIG_SUPPORT_DIR}" | |
| mkdir -p "${DEST_DIR}/${SYSDIG_SUPPORT_DIR}" |
There was a problem hiding this comment.
Thanks, this is a quick one and I'll fix it as soon as possible.
| SYSDIG_SUPPORT_DIR="sysdig-support" | ||
| [ ! -d "${DEST_DIR}/${SYSDIG_SUPPORT_DIR}" ] && mkdir -p "${DEST_DIR}/${SYSDIG_SUPPORT_DIR}" | ||
|
|
||
| ACTIVITY_LOG="${DEST_DIR}/${SYSDIG_SUPPORT_DIR}/activity.log" |
There was a problem hiding this comment.
I would suggest adding the date to the filename
There was a problem hiding this comment.
The filename has already the date/time into the content as first row.
| export SYSDIG_CS_DIR | ||
| export IS_AIRGAPPED | ||
|
|
||
| printf "All the data will be saved using the path ${DEST_DIR}/${SYSDIG_SUPPORT_DIR}\n" |
There was a problem hiding this comment.
printf is ok, but why not just an echo?
There was a problem hiding this comment.
Because printf allow a more granular control over the output.
| printf "All the data will be saved using the path ${DEST_DIR}/${SYSDIG_SUPPORT_DIR}\n" | ||
| log_activity "Data directory set to ${DEST_DIR}/${SYSDIG_SUPPORT_DIR}" | ||
|
|
||
| printf "Please put your executable to access your k8s cluster, kubectl or oc\n" |
There was a problem hiding this comment.
ask to add the context if needed to the command
kubectl --context a.b.c
There was a problem hiding this comment.
I'll add that in the README.
| if [[ $podName =~ $SYSDIG_NA_PREFIX ]]; then | ||
| printf "Collecting log for pod ${podName}\n" | ||
| mkdir -p "${DEST_DIR}/${SYSDIG_SUPPORT_DIR}/logs" | ||
| $k8sCmd -n "${namespace}" logs "${podName}" -c "${SYSDIG_KSPMA_CONTAINER_NAME}" > /dev/null 2>&1 && \ |
There was a problem hiding this comment.
You should never need quotes for namespace or podName or containerName. Also please be consistent using variables. Always either ${var} or $var
There was a problem hiding this comment.
I'll review this and I'll do another round of test, thanks for the heads up.
| printf "Collecting log for pod ${podName}\n" | ||
| mkdir -p "${DEST_DIR}/${SYSDIG_SUPPORT_DIR}/logs" | ||
| $k8sCmd -n "${namespace}" logs "${podName}" > "${DEST_DIR}/${SYSDIG_SUPPORT_DIR}/logs/${podName}.log" 2>/dev/null | ||
| if [ $? -eq 0 ]; then |
There was a problem hiding this comment.
For consistency (and this is only a nit), but you end up with fewer issues with if [[ test ]]
Just a quick check if everything is good, do not merge into main