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

flux-accounting guide: add setup instructions for new flux-accounting service #216

Merged
merged 2 commits into from
Apr 24, 2024

Conversation

cmoussa1
Copy link
Member

@cmoussa1 cmoussa1 commented Mar 1, 2023

Background

flux-accounting has a new service that accepts flux account commands that interact with the flux-accounting database and can be controlled with systemd, but there are no setup instructions on how to get it going in the flux-accounting guide on our docs site.


This PR adds a couple of small fixes along with a set of notes on installing and setting up the flux-accounting service in the flux-accounting guide. Since this new addition to documentation will be required as a result of the new flux-accounting release coming up in a few days, I will leave this is as [WIP] until the release has been finalized. Feel free to take a look at my changes proposed here in the meantime, though, in case there is any feedback. 👍

Fixes #215

@cmoussa1 cmoussa1 added the documentation Improvements or additions to documentation label Mar 1, 2023
@vsoch
Copy link
Member

vsoch commented Mar 2, 2023

I added a few notes to the flux-operator multi-tenant tutorial I think were helpful for accounting, namely:

  • creating the database flux account create-db
  • adding root bank flux account add-bank root 1
  • adding user bank flux account add-bank --parent-bank=root user_bank 1
  • adding users flux account add-user --username=peenut --bank=user_bank
  • also showing how to add the user (to the system, probably first) e.g., adduser
  • showing how to get the status of a user with view-user flux account view-user greatuser`

And a note about database ownership / permissions if it's not there yet! The complete guide with the examples above starts here: https://flux-framework.org/flux-operator/getting_started/tutorials/multi-tenancy.html#view-logs maybe it would make sense to add a few of these points? At least I think I checked that guide when I was making this tutorial and I came to the slack to bother everyone still with questions 😆

@cmoussa1 cmoussa1 changed the title [WIP] flux-accounting guide: add setup instructions for new flux-accounting service flux-accounting guide: add setup instructions for new flux-accounting service Mar 7, 2023
@cmoussa1 cmoussa1 marked this pull request as ready for review March 7, 2023 18:01
@cmoussa1
Copy link
Member Author

cmoussa1 commented Mar 7, 2023

@vsoch good suggestions! I think that would also be very helpful to add some of these pointers to the flux-accounting guide. I think adding these might be a little out of scope for this PR, but I'd be happy to open a separate issue and a near-future PR to add it. Does that sound okay?

With that, now that flux-accounting v0.22.0 has landed, I'll take this out of [WIP] and mark it as ready for review whenever convenient.

@cmoussa1 cmoussa1 requested a review from grondo March 7, 2023 18:05
@vsoch
Copy link
Member

vsoch commented Mar 7, 2023

Yep!

Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

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

Oof, so sorry this sat around for so long!


.. code-block:: console

$ sudo systemctl enable /path/to/flux-accounting.service
Copy link
Member

Choose a reason for hiding this comment

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

The path should not be necessary here if the unit file is installed in the right place, which will be the case when installing from packages. sudo systemctl enable flux-accounting should be sufficient.

Comment on lines +83 to +84
2. An active Flux system instance is running. The flux-accounting service will
only run after the system instance is started.
Copy link
Member

Choose a reason for hiding this comment

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

Rather than characterize these as assumptions, maybe "prerequisites"?

@@ -11,7 +11,7 @@ Flux Accounting Guide
documented in this guide may change with regularity.

This document is in DRAFT form and currently applies to flux-accounting
version 0.18.1.
version 0.30.0.
Copy link
Member

Choose a reason for hiding this comment

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

Needs to be bumped again.

Suggestion for later: move this document to the flux accounting project like we did with the admin guide and flux core. and stop mentioning the version in the guide, since it's then more obvious that the guide applies to the current version.

(Side benefit: if you set up readthedocs infrastructure in flux-accounting then man pages for the commands would be easy to add)

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a good suggestion, thanks! I'll open an issue to move this document to flux-accounting 👍

Problem: The version number mentioned in the flux-accounting guide is
outdated.

Update the version number in the flux-accounting guide.
Add instructions to the flux-accounting guide on installing and setting
up the flux-accounting service to be run alongside a Flux system
instance.
@cmoussa1
Copy link
Member Author

Thanks for the review @garlick! I just pushed up some fixes based on your suggestions and opened an issue in flux-accounting to go ahead and move this document in the near future. Since you approved already, I can set MWP here shortly :-)

@cmoussa1 cmoussa1 added the merge-when-passing mark PR for auto-merging by mergify.io bot label Apr 24, 2024
@mergify mergify bot merged commit 24b57fe into flux-framework:master Apr 24, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation merge-when-passing mark PR for auto-merging by mergify.io bot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

flux-accounting: update set up and installation instructions
3 participants