-
Notifications
You must be signed in to change notification settings - Fork 51
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
feat(24.04): systemd and dependencies #274
feat(24.04): systemd and dependencies #274
Conversation
4228943
to
27933c6
Compare
Diff of dependencies: |
e71981e
to
d2b90f5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks.
the checks are failing (systemd related I believe).
I also couldn't make the mount_bins
slice work...can you provide a test case for that?
I updated the description in the PR with a testing case |
979fd57
to
35ca3f1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. could you please put that into an integration test?
9d783ef
to
8081645
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. One thing worth noticing: adding the --cap-add SYS_ADMIN
and --security-opt apparmor=confined
would make the test runner vulnerable to malicious test cases. We must carefully review the spread tests from external contributors before we let them run.
b6dfbbe
to
0bdb39d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice slices! Nice tests too!
I left a few comments below -- mostly about the systemd
slices, with a few nitpicks here and there. Please let me know what you think about those.
65b8535
to
551af87
Compare
a0bcce7
to
4f928ca
Compare
@rebornplusplus this PR is ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me overall. Only left a few nitpicks and questions.
Splendid job slicing the systemd package! (Other packages too) Took me a long while to review those systemd slices, I wonder what it must have taken you to write them in the first place.
@rebornplusplus @cjdcordeiro tests are now running with rebased on master with the lxd backend, the tests are restored, can we give this a review again? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thank you very much!
Let's settle on the nice.conf
file in that thread however, before merging.
It seems the file may have been moved. EDIT: added the correct path :-) |
6a06c3b
to
89c7b2e
Compare
c9dde98
to
5b2aef0
Compare
Currently systemd is not working as it's lacking the catalog update functionality, blocked by maybe lack of features in Starlark/Chisel
This reverts commit ce546ec.
5b2aef0
to
c0b09f3
Compare
canonical#274 referenced libssecomp2 copyright instead of libmount1. Use libmount1 copyright.
#274 referenced libssecomp2 copyright instead of libmount1. Use libmount1 copyright.
Proposed changes
Add systemd slice and it's dependencies
Testing
Smoke testing mount
Smoke testing systemd
Checklist