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

remove cruft from the writable-paths #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented May 10, 2017

It looks like we accumulated some cruft in our writable-path file. This branch removes the bits that were used to make a ubuntu-personal core snap and adds a bunch of FIXMEs around things where it is not entirely clear why they exist in there.

# ssh keys
/root auto persistent transition none
# passwordless sudo
/etc/sudoers.d auto persistent transition none
/etc/hosts auto persistent transition none
/var/lib/extrausers auto persistent transition none
# FIXME: do we still need this?
Copy link

Choose a reason for hiding this comment

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

Would be great to drop this but we may have users who have files in there after they migrated from a 15.04 installation. Not sure what an update would do or if those people are still using them.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would need to stay, snap create-user and other adduser/useradd writes there when adding users (system or otherwise).

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure the FIXME refers to the line below, not above ;)

Give that we only ship netplan (or netwok-manager via the model assertion) and ifupdown is completely gone from the images, i guess that dir can go as well...

/etc/network/interfaces.d auto persistent transition none
# needed to persist ntp enabled/disabled
/etc/network/if-up.d auto persistent transition none
/etc/NetworkManager/system-connections auto persistent none none
Copy link

Choose a reason for hiding this comment

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

Will double check that.

/var/log auto persistent transition none
/var/lib/NetworkManager auto temporary none defaults
Copy link

Choose a reason for hiding this comment

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

Will double check that.

# snap data
/var/snap auto persistent transition none
/var/lib/bluetooth auto persistent none none
Copy link

Choose a reason for hiding this comment

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

Will double check that.

Copy link

Choose a reason for hiding this comment

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

Should be ok. Isn't use by the bluez snap.

/var/lib/dhcp auto persistent none none
/var/lib/logrotate auto persistent none none
/var/lib/sudo auto temporary none defaults,mode=0700
/var/lib/system-image auto persistent none none
/var/lib/upower auto persistent none none
Copy link

Choose a reason for hiding this comment

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

Will double check that.

Copy link

Choose a reason for hiding this comment

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

Should be ok. The upower snap doesn't use this.

/etc/ppp auto persistent transition none
/var/lib/tpm auto persistent transition none
Copy link

Choose a reason for hiding this comment

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

Will double check that.

Copy link

Choose a reason for hiding this comment

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

Should be ok too.

@@ -82,9 +67,8 @@
# dbus bus policy
/etc/dbus-1/system.d auto persistent transition none
/etc/modprobe.d auto synced none none
# FIXME: do we still need ppp on the image?
Copy link

Choose a reason for hiding this comment

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

Yes we do. We use ppp from the core snap in the modem-manager snap. However need to check if we need /etc/ppp writable or not.

Choose a reason for hiding this comment

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

We probably need /etc/ppp writable still, as mm writes there /etc/ppp/resolv.conf

Copy link

Choose a reason for hiding this comment

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

Also the ppp interface allows read/write access to /etc/ppp

/var/lib/dbus auto persistent none none
# FIXME: do we need this in the world of networkd?
/var/lib/dhcp auto persistent none none
Copy link
Contributor

Choose a reason for hiding this comment

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

we still get a dhclient up before console-conf runs. do we want to drop this completely (i was never sure if thats a bug or a feature) ?

@@ -42,35 +33,29 @@
# needed by apparmor - use transition since some core apps are
# pre-installed on the image
/var/cache/apparmor auto persistent transition none
# FIXME: do we still need this?
/var/lib/apparmor auto persistent transition none
Copy link
Contributor

Choose a reason for hiding this comment

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

i see an empty profile/ dir created in there on my installs, as long as nothing fails when it cant create that dir we should be fine

# used for various writable files (timezone, localtime, ...)
/etc/writable auto synced none none
# ureadahead
/var/lib/ureadahead auto persistent transition none
# FIXME: why do we need this?
# required by update-initramfs
/var/lib/initramfs-tools auto persistent transition none
Copy link
Contributor

Choose a reason for hiding this comment

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

can definitely go ... this was solely for development purposes.

@@ -94,7 +78,9 @@
/etc/systemd/system.conf.d auto persistent transition none
/etc/systemd/user.conf.d auto persistent transition none
/etc/systemd/logind.conf.d auto persistent transition none
# FIXME: what is this used for?
/etc/iproute2 auto persistent transition none
Copy link
Contributor

Choose a reason for hiding this comment

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

this is used for adding more complex routing tables than just a default route ... it was added in the course to fix https://bugs.launchpad.net/snappy/+bug/1658298 and is also part of the network-control interface

/etc/iproute2 auto persistent transition none
# FIXME: why do we need this in the systemd age?
/etc/rc0.d auto persistent transition none
Copy link
Contributor

Choose a reason for hiding this comment

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

from debian/changelog:

  • add all /etc/rc*.d directories to writable paths to quieten systemctl
    disable/enable (we dont use these dirs anyway but they cause warnings)

@zyga
Copy link
Collaborator

zyga commented May 10, 2017

I'd like to comment that given the extent of the changes here it should be extensively tested and not coupled with the next snapd release.

@ogra1
Copy link
Contributor

ogra1 commented May 16, 2017

with the dropping of rsyslog, /etc/rsyslog.d/ can be dropped as well (an rsyslogd snap wont use that dir i suppose)

@ogra1
Copy link
Contributor

ogra1 commented May 23, 2017

disregard my last comment, rsyslog was added back, so we should keep /etc/rsyslog.d/ writable

@zyga
Copy link
Collaborator

zyga commented Oct 4, 2017

Hey, looking at this branch I'd -1 it and instead see a release-by-release change of one or a few entries. I don't think we have a way to reliably measure the impact of large changes like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants