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

Use FireSim sudo scripts if available #304

Merged
merged 10 commits into from
Jun 19, 2024

Conversation

abejgonzalez
Copy link
Contributor

Try to use FireSim's sudo scripts when available (instead of defaulting to guestmount or using cpio with sudo).

jerryz123
jerryz123 previously approved these changes Jun 17, 2024
Copy link
Contributor

@jerryz123 jerryz123 left a comment

Choose a reason for hiding this comment

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

This global stuff is ugly... but I don't think I can think of another solution quickly.

@abejgonzalez abejgonzalez requested a review from jerryz123 June 17, 2024 22:22
@abejgonzalez
Copy link
Contributor Author

I'm unsure why the fed-smoke0 test doesn't work (it runs locally) and all other tests pass. This should be good to merge.

@jerryz123
Copy link
Contributor

Does the CI have the same sudo settings as your machine? I see:

2024-06-17T22:07:36.2258753Z 2024-06-17 15:05:55,794 [run         ] [DEBUG]  DEBUG: cp: cannot create regular file '/scratch/buildbot/firemarshal-ci-shared/firemarshal-a21bc92476b0e8375efbee3bdc86ee567e97de48/disk-mount/etc/shadow': Permission denied

This feels like a related failure

jerryz123
jerryz123 previously approved these changes Jun 17, 2024
Copy link
Contributor

@jerryz123 jerryz123 left a comment

Choose a reason for hiding this comment

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

I didn't realize the tests had been broken... if you judge the CI failures to be unrelated go ahead and merge

@abejgonzalez abejgonzalez force-pushed the use-sudo-scripts-if-available branch from a597627 to c510768 Compare June 18, 2024 20:45
@abejgonzalez
Copy link
Contributor Author

Copying to/from the mountpoint is now more complicated. Now it recursively chmod's all directories in the mountpoint s.t. it can safely copy in/out files (these chmod's are reverted afterwards to keep the same permissions on the directories). This works around an issue where marshal build's mount would leave a sub-dir in the mountpoint with incorrect permissions while running a separate subprocess.run(... mount ...) (virtually the same way FireMarshal does the mount) has the proper permissions on the sub-dir.

Copy link
Contributor

@jerryz123 jerryz123 left a comment

Choose a reason for hiding this comment

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

Thanks for dealing with this

@abejgonzalez abejgonzalez merged commit 87c9fd1 into master Jun 19, 2024
8 checks passed
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.

2 participants