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

graphtest: skip tests on insufficient permissions #1780

Merged

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Dec 19, 2023

Just a quick drive-by while working on #1779 - feel free to close if you disagree or feel it's the wrong approach (sorry, I'm new to this codebase) but I wanted to share it for your consideration :)


When a graphdriver cannot be tested because of e.g. ErrIncompatibleFS it is skipped. However this seems not to be the case for permission error and when running the tests as user they fail with:

chown: ... operation not permited

This commit changes the test code to account for this as well and skip when there are insufficient permissions.

@rhatdan
Copy link
Member

rhatdan commented Dec 21, 2023

You have to sign your commits

git commit -a --amend -s
git push --force

When a graphdriver cannot be tested because of e.g. `ErrIncompatibleFS`
it is skipped. However this seems not to be the case for permission
error and when running the tests as user they fail with:
```
chown: ... operation not permited
```
This commit changes the test code to account for this as well and
skip when there are insufficient permissions.

Signed-off-by: Michael Vogt <michael.vogt@gmail.com>
@TomSweeneyRedHat
Copy link
Member

reping @mvo5. Can you tend to the administration as noted by @rhatdan ?

@mvo5
Copy link
Contributor Author

mvo5 commented Jan 5, 2024

@TomSweeneyRedHat Hey, thanks for the reminder and sorry that I did not reply earlier. I did the force push with the --amend -s some days ago, did I do something wrong or miss something?

@TomSweeneyRedHat
Copy link
Member

@mvo5 Ooops, my bad, I scanned the error too quickly last night and missed your update. The PR just needed to be updated, which I've just started, and then reviewed.

LGTM

@edsantiago PTAL

@TomSweeneyRedHat
Copy link
Member

/approve

Copy link
Contributor

openshift-ci bot commented Jan 5, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mvo5, TomSweeneyRedHat

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 label Jan 5, 2024
@rhatdan
Copy link
Member

rhatdan commented Jan 5, 2024

/lgtm
/hold

@rhatdan
Copy link
Member

rhatdan commented Jan 5, 2024

/unhold

@openshift-merge-bot openshift-merge-bot bot merged commit 7ca28f6 into containers:main Jan 5, 2024
18 checks passed
@mvo5 mvo5 deleted the skip-driver-tests-on-eperm branch January 5, 2024 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants