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

motd: maintain a /etc/motd file with interesting info about the TAC #77

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

KarlK90
Copy link
Member

@KarlK90 KarlK90 commented Aug 6, 2024

This is a refactor of #57

Not anyone looks at the web interface or the display of the TAC. This gives us a third vector to convey information to the user, by showing messages on ssh login.

Use bind-mounts to put the highly volatile file itself to /var/run and point to it from /etc.

@KarlK90 KarlK90 requested a review from hnez August 6, 2024 12:33
@hnez
Copy link
Member

hnez commented Aug 6, 2024

I've had a look at the changes between #57 and this PR:

  • Do not remove the /etc/motd bind mount when the motd::Motd struct is dropped. This simplifies the main.rs (but does mean that we may leave stale information behind. Which is okay though, as it is assumed that there will always be a running tacd).
  • Use impl Display for Motd to format the message instead of a fully-custom method, which is a nice.
  • Inline the color -codes into the writeln!() calls. This makes the lines quite long, but that's okay. (Line length was the reason I did not inline it initially).
  • Use futures::select!() to watch all information sources instead of a lot of tasks and an ad-hoc queue. I do not like the readability and debuggability of macros, but this solution is more compact than the one in motd: maintain a /etc/motd file with interesting info about the TAC #57, which was not very elegant.
  • Use MsFlags::MS_REMOUNT instead of explicit unmount + mount. Neat!

@hnez
Copy link
Member

hnez commented Aug 6, 2024

I will not get to the review of the changes now, but I think this PR is something we will want in the next meta-lxatac / tacos stable release and I will add a corresponding milestone.

@hnez hnez added this to the third stable release milestone Aug 6, 2024
Not anyone looks at the web interface or the display of the TAC.
This gives us a third vector to convey information to the user,
by showing messages on ssh login.

Use bind-mounts to put the highly volatile file itself to /var/run
and point to it from /etc.

Signed-off-by: Stefan Kerkmann <[email protected]>
Co-authored-by: Leonard Göhrs <[email protected]>
Copy link
Member

@hnez hnez left a comment

Choose a reason for hiding this comment

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

I finally got around to having a look at this.
Looks good, thanks!

@hnez hnez merged commit a25b8a7 into linux-automation:main Aug 22, 2024
11 checks passed
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.

2 participants