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

Add support for /run/.containerenv #1844

Merged
merged 1 commit into from
Sep 6, 2019
Merged

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Sep 5, 2019

Container processes want to check for the existence of this file
to determine if they are running inside of a container.

Signed-off-by: Daniel J Walsh [email protected]

@rhatdan rhatdan requested a review from nalind September 5, 2019 20:57
@rhatdan
Copy link
Member Author

rhatdan commented Sep 5, 2019

@rhatdan
Copy link
Member Author

rhatdan commented Sep 5, 2019

Fixes: #1843

run_linux.go Outdated
// Empty file, so no need to recreate if it exists
if _, ok := bindFiles["/run/.containerenv"]; !ok {
// Empty string for now, but we may consider populating this later
containerenvPath := filepath.Join(path, "/run.containerenv")
Copy link
Member

Choose a reason for hiding this comment

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

should this be /run/.containerenv? (added a slash after run)

Copy link
Member Author

Choose a reason for hiding this comment

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

I love typos... I guess tests would have found this. Fixed.

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

Besides @TomSweeneyRedHat s question, LGTM.

@TomSweeneyRedHat
Copy link
Member

LGTM
Unfortunately, Red Hat CI seems to be off to a lovely death already. Unrelated to the changes I'm sure.

@rhatdan
Copy link
Member Author

rhatdan commented Sep 6, 2019

@rh-atomic-bot r=TomSweeneyRedHat

@rh-atomic-bot
Copy link
Collaborator

📌 Commit bc99264 has been approved by TomSweeneyRedHat

Container processes want to check for the existence of this file
to determine if they are running inside of a container.

Signed-off-by: Daniel J Walsh <[email protected]>
@TomSweeneyRedHat
Copy link
Member

@rh-atomic-bot r+

@rh-atomic-bot
Copy link
Collaborator

📌 Commit 7f0b60c has been approved by TomSweeneyRedHat

@rhatdan
Copy link
Member Author

rhatdan commented Sep 6, 2019

Hopefully the tests pass this time. Found a couple of other issues with this in the mean time. Was breaking ubuntu images and did not do SELinux correctly. Should be fixed now.

@rhatdan
Copy link
Member Author

rhatdan commented Sep 6, 2019

bot, retest this please
@rh-atomic-bot retry

@rhatdan
Copy link
Member Author

rhatdan commented Sep 6, 2019

Homu is not responding. Manually merging.

@rhatdan rhatdan merged commit f54c965 into containers:master Sep 6, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants