OCPBUGS-91663: Clean up old temp directories in downloads pod by hongkailiu · Pull Request #1176 · openshift/console-operator

hongkailiu · 2026-06-23T19:14:01Z

Add cleanup logic to remove old download-* temp directories before creating a new one. This prevents accumulation of stale temp directories from previous pod instances.

Changes:

Define TEMP_DIR_PREFIX constant for 'download-' prefix
Add cleanup logic to remove existing download-* directories on startup
Use prefix parameter in tempfile.mkdtemp() for easier identification

Analysis / Root cause:

When the pod is killed with oc -n openshift-console exec <downloads-pod> -- kill 1, it is like simulating A container crashing does not remove a Pod from a node. The data in an emptyDir volume is safe across container crashes. as in https://kubernetes.io/docs/concepts/storage/volumes/#emptydir.

The data from the previous container are reserved and thus could cause out-of-disk issue after a loop of such actions.

Solution description:

See the PR description above.

Test setup:

Test cases:

I will verify manually with the steps in the associated bug.

Browser conformance:

Chrome
Firefox
Safari (or Epiphany on Linux)

Additional info:

Reviewers and assignees:

Summary by CodeRabbit

Bug Fixes
- Downloads service now includes automatic cleanup of temporary directories from previous operations, improving resource management and preventing disk space accumulation.

Add cleanup logic to remove old download-* temp directories before creating a new one. This prevents accumulation of stale temp directories from previous pod instances. Changes: - Define TEMP_DIR_PREFIX constant for 'download-' prefix - Add cleanup logic to remove existing download-* directories on startup - Use prefix parameter in tempfile.mkdtemp() for easier identification Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

coderabbitai · 2026-06-23T19:14:16Z

Walkthrough

The embedded Python startup script inside downloads-deployment.yaml is updated to define a TEMP_DIR_PREFIX constant (download-), scan the system temp directory for stale directories matching that prefix, remove them via shutil.rmtree (logging warnings on failure), and create the new working directory using that prefix.

Changes

Downloads Deployment Startup Script — Temp Dir Cleanup

Layer / File(s)	Summary
Prefixed temp dir cleanup on startup `bindata/assets/deployments/downloads-deployment.yaml`	Import line is updated to include `shutil`. `TEMP_DIR_PREFIX = 'download-'` is introduced. On startup, any existing `download-*` directories in the system temp dir are removed via `shutil.rmtree` with per-item warning logs on exceptions. The working directory creation is switched from unprefixed `tempfile.mkdtemp()` to `tempfile.mkdtemp(prefix=TEMP_DIR_PREFIX)`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 13 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description includes Analysis/Root cause, Solution description, and Test cases sections. However, it lacks the required Jira issue prefix in the title and leaves Test setup and Browser conformance sections incomplete.	Add Jira issue prefix (OCPBUGS-91663) to the PR title and complete the Test setup and Browser conformance sections of the description template.
Microshift Test Compatibility	⚠️ Warning	PR adds 26 new Ginkgo e2e test files. Tests use config.openshift.io/v1 APIs (proxy, infrastructure, ingress, featuregate, etc.) which are NOT available on MicroShift. No [Skipped:MicroShift], [apig...	Add [apigroup:config.openshift.io] tags to tests using config APIs, or guard with IsMicroShiftCluster() checks with g.Skip(). Tests: custom_url_test.go, proxy_test.go, brand_customization_test.go, and affected framework components.

✅ Passed checks (13 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	PR only modifies downloads-deployment.yaml (Python script in YAML), not any Ginkgo test files. No test code changes present in this PR.
Test Structure And Quality	✅ Passed	The PR modifies only a deployment YAML file with embedded Python script, not Ginkgo test code. The custom check for test structure and quality is not applicable.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	This PR does not add any Ginkgo e2e tests; it only modifies the downloads deployment manifest to add temp directory cleanup logic. SNO compatibility check is not applicable.
Topology-Aware Scheduling Compatibility	✅ Passed	PR modifies only the embedded Python startup script for temp directory cleanup; no Kubernetes scheduling constraints (affinity, replicas, nodeSelector, tolerations, PDB, topology spread) are introd...
Ote Binary Stdout Contract	✅ Passed	The PR modifies a Kubernetes deployment YAML for a downloads pod (not an OTE test binary). The Python script's print statements are pod startup logs, not test output that communicates with openshif...
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo e2e tests are added in this PR. All e2e tests use standard Go testing.T package, not Ginkgo. The custom check is not applicable.
No-Weak-Crypto	✅ Passed	PR modifies only temp directory cleanup logic with no weak crypto (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), custom crypto implementations, or insecure comparisons.
Container-Privileges	✅ Passed	The manifest enforces restrictive security: runAsNonRoot, allowPrivilegeEscalation=false, all capabilities dropped, read-only root filesystem. No privileged, hostPID/Network/IPC, or SYS_ADMIN setti...
No-Sensitive-Data-In-Logs	✅ Passed	The logging statements in the cleanup logic only log filesystem paths and generic OS exceptions (e.g., permission denied), not passwords, tokens, keys, PII, session IDs, or sensitive data.
Title check	✅ Passed	The title clearly and specifically describes the main change: adding cleanup logic for old temp directories in the downloads pod, matching the PR's primary objective.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

hongkailiu · 2026-06-23T21:57:22Z

/retest-required

Maybe flaky tests?

hongkailiu · 2026-06-23T22:12:25Z

/retest-required

hongkailiu · 2026-06-23T23:27:39Z

The fix seems working.

Cluster-bot (logs)

launch 5.0.0-0.nightly,console-operator#1176 aws

Then

$ downloads_pod="$(oc get pod -n openshift-console -l app=console,component=downloads -o wide -o jsonpath='{.items[0].metadata.name}')"

$ oc -n openshift-console exec $downloads_pod -- kill 1
$ oc -n openshift-console exec $downloads_pod -- du -h -d 1 /tmp
3.0G	/tmp/download-7o_q2ztx
3.0G	/tmp
$ oc -n openshift-console exec $downloads_pod -- kill 1
$ oc -n openshift-console exec $downloads_pod -- du -h -d 1 /tmp
error: Internal error occurred: unable to upgrade connection: container not found ("download-server")
$ oc -n openshift-console exec $downloads_pod -- du -h -d 1 /tmp
1.9G	/tmp/download-bqem1kf_
1.9G	/tmp
$ oc -n openshift-console exec $downloads_pod -- du -h -d 1 /tmp
3.1G	/tmp/download-bqem1kf_
3.1G	/tmp
$ oc -n openshift-console exec $downloads_pod -- kill 1
$ oc -n openshift-console exec $downloads_pod -- du -h -d 1 /tmp
error: Internal error occurred: unable to upgrade connection: container not found ("download-server")
$ oc -n openshift-console exec $downloads_pod -- du -h -d 1 /tmp
error: Internal error occurred: unable to upgrade connection: container not found ("download-server")
$ oc -n openshift-console exec $downloads_pod -- du -h -d 1 /tmp
3.1G	/tmp/download-mrmmn4gw
3.1G	/tmp
$ oc  -n openshift-console logs $downloads_pod
Starting downloads server...
Cleaning up old temp directories in /tmp...
  Removing old directory: /tmp/download-bqem1kf_
Serving from: /tmp/download-mrmmn4gw
Creating arch directories...
Creating license symlink...
Creating oc binary symlinks (archives will be created asynchronously)...
...

openshift-ci-robot · 2026-06-23T23:28:07Z

@hongkailiu: This pull request references Jira Issue OCPBUGS-91663, which is invalid:

expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Add cleanup logic to remove old download-* temp directories before creating a new one. This prevents accumulation of stale temp directories from previous pod instances.

Changes:

Define TEMP_DIR_PREFIX constant for 'download-' prefix

Add cleanup logic to remove existing download-* directories on startup

Use prefix parameter in tempfile.mkdtemp() for easier identification

Analysis / Root cause:

When the pod is killed with oc -n openshift-console exec <downloads-pod> -- kill 1, it is like simulating A container crashing does not remove a Pod from a node. The data in an emptyDir volume is safe across container crashes. as in https://kubernetes.io/docs/concepts/storage/volumes/#emptydir.

The data from the previous container are reserved and thus could cause out-of-disk issue after a loop of such actions.

Solution description:

See the PR description above.

Test setup:

Test cases:

I will verify manually with the steps in the associated bug.

Browser conformance:

Chrome

Firefox

Safari (or Epiphany on Linux)

Additional info:

Reviewers and assignees:

Summary by CodeRabbit

Bug Fixes

Downloads service now includes automatic cleanup of temporary directories from previous operations, improving resource management and preventing disk space accumulation.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

hongkailiu · 2026-06-23T23:33:10Z

/jira refresh

openshift-ci-robot · 2026-06-23T23:33:16Z

@hongkailiu: This pull request references Jira Issue OCPBUGS-91663, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (5.0.0) matches configured target version for branch (5.0.0)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Details

In response to this:

/jira refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

hongkailiu · 2026-06-23T23:38:05Z

/retest-required

hongkailiu · 2026-06-24T10:26:20Z

/test e2e-aws-console

hongkailiu · 2026-06-24T12:52:55Z

/test e2e-aws-console

openshift-ci · 2026-06-24T15:22:46Z

@hongkailiu: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

jhadvig · 2026-06-25T10:01:32Z

/payload-job periodic-ci-openshift-release-master-nightly-5.0-e2e-aws-ovn-upgrade-fips
/payload-job periodic-ci-openshift-release-master-nightly-5.0-e2e-aws-ovn-upgrade-fips-nat-instance

openshift-ci · 2026-06-25T10:01:37Z

@jhadvig: trigger 0 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

jhadvig · 2026-06-25T10:05:47Z

/lgtm
/approve

openshift-ci · 2026-06-25T10:07:18Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hongkailiu, jhadvig

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [jhadvig]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

hongkailiu · 2026-06-25T10:16:15Z

/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips

openshift-ci · 2026-06-25T10:16:19Z

@hongkailiu: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/e6c311d0-707e-11f1-9bb8-7c12c4bd56bc-0

hongkailiu · 2026-06-25T10:21:25Z

$ rg "5.0.*upgrade-fips-.+-instance"
ci-operator/jobs/openshift/release/openshift-release-main-periodics.yaml
273293:  name: periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips-no-nat-instance

core-services/release-controller/_releases/release-ocp-5.0.json
187:        "name": "periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips-no-nat-instance"

core-services/release-controller/_releases/priv/release-ocp-5.0.json
223:                "name": "periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips-no-nat-instance-priv"

/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips-no-nat-instance

openshift-ci · 2026-06-25T10:21:30Z

@hongkailiu: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-upgrade-fips-no-nat-instance

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/9f990840-707f-11f1-9314-d0d109c503cc-0

Follow up openshift/console-operator#1176 The GoLang implementation uses a fixed folder name `defaultArtifactsDir` to store the files and cleans the folder up [1]. So it should not suffer the issue of accumulating the files over startups. The current unit test ensures already the same `defaultArtifactsDir` is used [2], and we add the logic in the pull to ensure that the file from previous container is removed in the new container after startup. [1]. https://github.com/openshift/console/blob/71f7e03bcfe04f9842f24a8f628660f8f2d4b892/cmd/downloads/config/downloads_config.go#L119-L122 [2]. https://github.com/openshift/console/blob/28f9ca40e41a5b221bcd562194fcc1ff930cc6f4/cmd/downloads/config/downloads_config_test.go#L266-L268

openshift-ci Bot requested review from TheRealJon and jhadvig June 23, 2026 19:14

hongkailiu changed the title ~~Clean up old temp directories in downloads pod~~ OCPBUGS-91663: Clean up old temp directories in downloads pod Jun 23, 2026

openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Jun 23, 2026

openshift-ci-robot removed the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Jun 23, 2026

openshift-ci Bot assigned jhadvig Jun 25, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 25, 2026

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 25, 2026

hongkailiu mentioned this pull request Jun 25, 2026

NO-JIRA: Downloads: Ensure cleanup of the artifact folder on each startup openshift/console#16681

Open

3 tasks

Uh oh!

Conversation

hongkailiu commented Jun 23, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (2 warnings)

Uh oh!

hongkailiu commented Jun 23, 2026

Uh oh!

hongkailiu commented Jun 23, 2026

Uh oh!

hongkailiu commented Jun 23, 2026

Uh oh!

openshift-ci-robot commented Jun 23, 2026

Summary by CodeRabbit

Uh oh!

hongkailiu commented Jun 23, 2026

Uh oh!

openshift-ci-robot commented Jun 23, 2026

Uh oh!

hongkailiu commented Jun 23, 2026

Uh oh!

hongkailiu commented Jun 24, 2026

Uh oh!

hongkailiu commented Jun 24, 2026

Uh oh!

openshift-ci Bot commented Jun 24, 2026

Uh oh!

jhadvig commented Jun 25, 2026

Uh oh!

openshift-ci Bot commented Jun 25, 2026

Uh oh!

jhadvig commented Jun 25, 2026

Uh oh!

openshift-ci Bot commented Jun 25, 2026

Uh oh!

hongkailiu commented Jun 25, 2026

Uh oh!

openshift-ci Bot commented Jun 25, 2026

Uh oh!

hongkailiu commented Jun 25, 2026

Uh oh!

openshift-ci Bot commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hongkailiu commented Jun 23, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 23, 2026 •

edited

Loading