Skip to content
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

e2e tmpdir cleanup #12471

Merged

Conversation

edsantiago
Copy link
Member

My initial goal was to allow caching the e2e images in a directory other than /tmp.
In the process of doing so I discovered some very confusing and/or unnecessary code.
I took the liberty of cleaning it up.

These are three separate easily-reviewed commits, I recommend reviewing each separately:

  • Rename CrioRoot as just Root
  • Image caches: allow overriding cache dir
  • remove ARTIFACT_DIR and ArtifactPath

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 1, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 1, 2021
@rhatdan
Copy link
Member

rhatdan commented Dec 1, 2021

LGTM

@rhatdan
Copy link
Member

rhatdan commented Dec 1, 2021

BTW CRIO is just because Podman came out of the original CRIO effort as kpod, and we stole CRIO test suite and never changed the name.

...and remove other uses of "crio". They're confusing
and misleading. (I'm sure it made sense at one time)

Signed-off-by: Ed Santiago <[email protected]>
Images were being cached in /tmp, with no option to
override. Now $PODMAN_TEST_IMAGE_CACHE_DIR can be
used to point to a user-preferred location. If unset,
try $TMPDIR before settling on /tmp.

Also: refactor the logic for determining the tarball name.
Also: include registry name in tarball name.
Also: clean up unused/unnecessary code
Also: do not echo "Restoring..." if we're not actually restoring.

Signed-off-by: Ed Santiago <[email protected]>
...they're not actually used for anything

Signed-off-by: Ed Santiago <[email protected]>
@TomSweeneyRedHat
Copy link
Member

Changes LGTM, now to get the test happy.

@edsantiago
Copy link
Member Author

It's just #9597, one of our most popular flakes

@Luap99
Copy link
Member

Luap99 commented Dec 2, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 2, 2021
@openshift-merge-robot openshift-merge-robot merged commit 6b5ecde into containers:main Dec 2, 2021
@edsantiago edsantiago deleted the e2e_tmpdir_cleanup branch December 2, 2021 13:29
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants