Skip to content

feat: refactor cleanup cronjob to use pagination and reduce memory usage #411

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.

Sign up for GitHub

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

Open
davidmogar wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: refactor cleanup cronjob to use pagination and reduce memory usage #411

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 33 additions & 24 deletions ...ts/internal-services/internal-staging/cronjob/cleanup-internal-requests-pipelineruns.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,39 +35,48 @@ spec:
- |
set -o pipefail
PATH="/bin:/usr/bin:/usr/local/bin"
PRUNING_CRS_FILE="/var/tmp/crs-to-be-pruned"
KUBECTL_OUTPUT=$(mktemp -p /var/tmp)
SECONDS_BACK=$(date -d "${OLDER_THAN}" +%s)
DELETED_COUNT=0
ERROR_COUNT=0

echo "INFO: Starting cleanup for ${CR_TYPE} in ${CR_NAMESPACE} older than ${OLDER_THAN}"

# Fetch all resources with chunked pagination (handled by kubectl internally)
KUBECTL_OUTPUT=$(mktemp)
KUBECTL_ERR=$(mktemp)
Comment on lines +45 to +46

@qodo-for-redhat-appstudio qodo-for-redhat-appstudio Bot Jun 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Tempfiles on read-only fs 🐞 Bug ☼ Reliability

The script now uses mktemp without targeting the writable /var/tmp mount while
readOnlyRootFilesystem: true, so temp file creation (and the kubectl get redirect) can fail and
break the cleanup job. This is a regression vs the internal-production manifest which explicitly
places temp files under /var/tmp.
Agent Prompt 
### Issue description
The cronjob creates temp files via `mktemp` with no directory, which defaults to `/tmp`. This CronJob runs with `readOnlyRootFilesystem: true` and only mounts an `emptyDir` at `/var/tmp`, so creating temp files in `/tmp` is likely to fail.

### Issue Context
The manifest already mounts `/var/tmp` specifically for scratch space, and the internal-production variant uses `mktemp -p /var/tmp`.

### Fix Focus Areas
- components/internal-services/internal-staging/cronjob/cleanup-internal-requests-pipelineruns.yaml[36-56]
- components/internal-services/internal-staging/cronjob/cleanup-internal-requests-pipelineruns.yaml[80-99]

### Suggested change
- Set `TMPDIR=/var/tmp` or update both tempfiles to `mktemp -p /var/tmp` (for `KUBECTL_OUTPUT` and `KUBECTL_ERR`).
- (Optional hardening) Add a `trap` to delete temp files on exit.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


if ! kubectl get "${CR_TYPE}" -n "${CR_NAMESPACE}" -l "${LABELS}" \
--template '{{range .items}}{{if .status.completionTime}}{{.metadata.name}}{{"\t"}}{{.metadata.namespace}}{{"\t"}}{{.status.completionTime}}{{"\n"}}{{end}}{{end}}' > $KUBECTL_OUTPUT; then
--chunk-size=100 -o json > "$KUBECTL_OUTPUT" 2> "$KUBECTL_ERR"; then
echo "ERROR: failed to list ${CR_TYPE} resources"
cat "$KUBECTL_ERR" >&2
rm -f "$KUBECTL_OUTPUT" "$KUBECTL_ERR"
exit 1
fi
rm -f "$KUBECTL_ERR"

awk -v since=${SECONDS_BACK} '{
# parsing the completionTime and converting it to epoch
# so we can compute the precise CRs that should be deleted
gsub("[:\\-TZ]", " ", $3)
t=mktime($3)
completionTime=strftime("%s", t)
#
# completionTime should be smaller than `since` seconds so it can be deleted
if(since > completionTime) {
args="%s:%s\n"
printf(args, $1, $2)
}
}' $KUBECTL_OUTPUT > $PRUNING_CRS_FILE
# Process each item to minimize memory usage
while IFS=: read -r cr_name cr_namespace completion_time; do
# Convert completionTime to epoch
if ! completion_epoch=$(date -d "${completion_time}" +%s 2>&1); then
echo "ERROR: Unparseable completionTime for ${cr_name} in ${cr_namespace}: ${completion_time}"
ERROR_COUNT=$((ERROR_COUNT + 1))
continue
fi

while IFS= read -r line; do
cr=${line%:*}
namespace=${line#*:}
if output=$(kubectl delete "${CR_TYPE}" "${cr}" -n "${namespace}" --wait=false --timeout=30s 2>&1); then
echo "INFO: namespace=${namespace} ${output}"
else
echo "ERROR: namespace=${namespace} ${output}"
# Check if old enough to delete
if [ "$SECONDS_BACK" -gt "$completion_epoch" ]; then
if output=$(kubectl delete "${CR_TYPE}" "${cr_name}" -n "${cr_namespace}" --wait=false --timeout=30s 2>&1); then
echo "INFO: namespace=${cr_namespace} ${output}"
DELETED_COUNT=$((DELETED_COUNT + 1))
else
echo "ERROR: namespace=${cr_namespace} ${output}"
ERROR_COUNT=$((ERROR_COUNT + 1))
fi
fi
done < $PRUNING_CRS_FILE
done < <(jq -r '.items[] | select(.status.completionTime != null) | "\(.metadata.name):\(.metadata.namespace):\(.status.completionTime)"' "$KUBECTL_OUTPUT")

rm -f "$KUBECTL_OUTPUT"
echo "INFO: Cleanup complete. Deleted: ${DELETED_COUNT}, Errors: ${ERROR_COUNT}"
imagePullPolicy: IfNotPresent
image: quay.io/konflux-ci/release-service-utils:9089cafbf36bb889b4b73d8c2965613810f13736
volumeMounts:
Expand Down
Loading