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

docs/operator-notes: Note paths that should not be used b/c of switch_root #1697

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

dlipovetsky
Copy link
Contributor

I learned this the hard way (see flatcar/Flatcar#1152), and thought it would be useful to have in the documentation.

Feedback and edits welcome!

@travier
Copy link
Member

travier commented Aug 16, 2023

In general, you should not directly write to /proc, /dev, /sys and should use sysctl config files or udev rules.

For /tmp and /run you should use tpmfiles.d configs instead.

Maybe we should have a Butane lint that refuses writing files to those locations?

@dlipovetsky
Copy link
Contributor Author

Thanks for the feedback!

In general, you should not directly write to /proc, /dev, /sys and should use sysctl config files or udev rules.

That's fair. Want me to add this to the note?

For /tmp and /run you should use tpmfiles.d configs instead.

Ahh, right, /tmp is another one. IIRC, it's mounted after Ignition finishes, not moved by switch_root. I'll update the note.

Maybe we should have a Butane lint that refuses writing files to those locations?

I haven't used Butane myself (I'm using Ignition indirectly, as the result of booting Flatcar Linux machines with Cluster API. Could someone help with this part?

@travier
Copy link
Member

travier commented Aug 24, 2023

The text as written is not correct.

The /proc /sys /dev paths are very much still accessible after switching from the initramfs. Users should not rely on Ignition to write value to those files because the changes are not persisted beyond the first boot.

I think that whether /tmp or /run are a tmpfs mount is up to the distribution, not a requirement of Ignition.

Overall there is nothing invalid with Ignition configs writing to those paths. They only don't do what you likely want.

So I'd phrase this as a recommendation instead. But I'm still not sure that this is the right place for it.

@dlipovetsky
Copy link
Contributor Author

The /proc /sys /dev paths are very much still accessible after switching from the initramfs.

The text says that the files you write under these paths before switch_root are not accessible after switch_root. It does not say the paths are not accessible. But I can rewrite it to be more clear.

Overall there is nothing invalid with Ignition configs writing to those paths. They only don't do what you likely want.

Any file written under these paths by Ignition is not accessible after switch_root. I can't think of any situation where this is what the user wants. If you can think of one, could you please share it?

So I'd phrase this as a recommendation instead. But I'm still not sure that this is the right place for it.

I want to help other Ignition users avoid a mistake that is easy to make, especially if you are used to cloud-init, which runs after switch_root. Please let me know the right place, in case these docs are not it.

@prestist
Copy link
Collaborator

prestist commented Jan 9, 2024

@travier poke

@travier
Copy link
Member

travier commented Feb 28, 2024

Coming back to this and re-reading it again, I think I now understand what you mean. Instead of phrasing it in a negative form, let's write what we recommend instead in a positive form.

docs/operator-notes.md Outdated Show resolved Hide resolved
@travier
Copy link
Member

travier commented Feb 28, 2024

But I still think that this would be better as a check in Butane as the behavior might be distribution dependent and users are not expected to write Ignition configs directly.

@dlipovetsky
Copy link
Contributor Author

But I still think that this would be better as a check in Butane as the behavior might be distribution dependent and users are not expected to write Ignition configs directly.

I agree with you.

I won't be able to contribute this check to Butane, though.

docs/operator-notes.md Outdated Show resolved Hide resolved
@dlipovetsky dlipovetsky requested a review from travier July 18, 2024 17:16
Copy link
Member

@travier travier left a comment

Choose a reason for hiding this comment

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

Small nits but LGTM

docs/operator-notes.md Outdated Show resolved Hide resolved
docs/operator-notes.md Outdated Show resolved Hide resolved
@travier travier added kind/documentation skip-notes This PR does not need release notes labels Jul 19, 2024
@dlipovetsky
Copy link
Contributor Author

Thanks for reviewing, and for the fixes!

docs/operator-notes.md Outdated Show resolved Hide resolved
docs/operator-notes.md Outdated Show resolved Hide resolved
@dlipovetsky
Copy link
Contributor Author

dlipovetsky commented Jul 23, 2024

I think that, earlier in the review, @travier made a good suggestion to give users guidance about managing files under these mount points--namely, to configure the respective systemd services that manage files under tose mount points.

I think it would be good to keep that text, in addition to the guidance on not using Ignition to directly manage files under these mount points. Looks like that guidance is still there, so no need to change anything 😄 .

@dlipovetsky
Copy link
Contributor Author

Thanks everyone for working together, in good faith, to improve the docs for Ignition users 🙏. It might not be a big change, but the reviews made the text very clear, and I'm sure that will save people from frustration and wasted time.

Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

I just squashed it all to one commit. Thanks!

@@ -62,6 +62,14 @@ If `wipeFilesystem` is set to false, Ignition will then attempt to reuse the exi

When resolving paths, Ignition follows symlinks on all but the last element of a path. This ensures existing symlinks on a filesystem can be overwritten while still following symlinks as expected. When writing files, links, or directories, Ignition does not allow following symlinks outside the specified filesystem. When writing files, links, or directories on the `root` filesystem, Ignition follows symlinks as if it were executing in that root; a symlink to `/etc` is followed to `/etc` on the `root` filesystem. When writing files, links, or directories to any other filesystem, Ignition fails if it tries to follow a symlink outside that filesystem.

## Making changes to `/proc`, `/sys`, `/dev`, `/tmp` or `/run`

To create files, directories or symlinks in `/proc`, `/sys` or `/dev`, you should use [sysctl.d config files](https://www.mankier.com/5/sysctl.d) or [udev rules](https://www.mankier.com/7/udev).
Copy link
Member

Choose a reason for hiding this comment

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

I still find this sentence a bit weird because you wouldn't typically want to create files, directories, or symlinks in there in the first place. But fine as is too.

@jlebon jlebon enabled auto-merge August 9, 2024 19:31
@jlebon jlebon merged commit af00a48 into coreos:main Aug 9, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/documentation skip-notes This PR does not need release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants