Skip to content

Reenable two skipped tests in run_e2e.go#2328

Closed
mheon wants to merge 1 commit intocontainers:masterfrom
mheon:reenable_run_tests
Closed

Reenable two skipped tests in run_e2e.go#2328
mheon wants to merge 1 commit intocontainers:masterfrom
mheon:reenable_run_tests

Conversation

@mheon
Copy link
Copy Markdown
Member

@mheon mheon commented Feb 13, 2019

I have no idea why these were skipped, and when we started skipping them, but let's see if they started passing at some point.

The ubuntu runc one, if it still fails, we can fix ourselves, AFAIK our PPA packages the runc podman uses.

@openshift-ci-robot
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon

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:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 13, 2019
@baude
Copy link
Copy Markdown
Member

baude commented Feb 13, 2019

lgtm

@TomSweeneyRedHat
Copy link
Copy Markdown
Member

Changes look fine, but looks like a lot of test unhappiness @mheon.

@mheon
Copy link
Copy Markdown
Member Author

mheon commented Feb 13, 2019

The volume test appears to have been disabled for a reason.

It looks like it's not finding a string in /proc/self/mount?

@mheon
Copy link
Copy Markdown
Member Author

mheon commented Feb 13, 2019

The mount now appears to have the relatime flag, in addition to the expected rw and nodev

@mheon mheon force-pushed the reenable_run_tests branch from 56bcf81 to 216159f Compare February 13, 2019 18:43
Signed-off-by: Matthew Heon <matthew.heon@pm.me>
@mheon
Copy link
Copy Markdown
Member Author

mheon commented Feb 13, 2019

Some instances have nodev, some don't. Not sure why yet.

@rhatdan
Copy link
Copy Markdown
Member

rhatdan commented Feb 13, 2019

nodev shows up if you are using the default storage.conf file.

Comment thread test/e2e/run_test.go
host := GetHostDistributionInfo()
if host.Distribution != "rhel" && host.Distribution != "centos" && host.Distribution != "fedora" {
Skip("this test requires a working runc")
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this works only on the Fedora/RHEL package, as we are carrying an extra patch.

upstream doesn't support it yet: opencontainers/runc#1807

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Test passes, though?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see it is failing on Ubuntu

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hm. Fair enough. I'll disable it again and focus on the other tests, which seems to be just changed mount opts.

@mheon
Copy link
Copy Markdown
Member Author

mheon commented Feb 21, 2019

I don't think either of these is getting reenabled easily - we'd need to parse the output of mountinfo more thoroughly to actually determine if mount options are correct.

Closing as I don't have time to push it through.

@mheon mheon closed this Feb 21, 2019
@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 27, 2023
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Sep 27, 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. 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.

6 participants