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

feat!: use composition and inheritance to build service managers #8

Merged
merged 8 commits into from
Jul 10, 2024

Conversation

NucciTheBoss
Copy link
Member

@NucciTheBoss NucciTheBoss commented Jul 9, 2024

This pull request enhances the API of slurm_ops to use composition and inheritance to build the Slurm service operations managers. This makes it easier for us to construct managers around slurmctld, slurmd, slurmdbd, and slurmrestd and share logic for configuration and service management between them.

Some use cases

bulk update the configuration of slurmctld:

from charms.hpc_libs.v0.slurm_ops import SlurmManagerBase, ServiceType

slurmctld = SlurmManagerBase(ServiceType.SLURMCTLD)
slurmctld.config.set({"option1": "value1", "option2": "value2"})

set a new munge key:

from charms.hpc_libs.v0.slurm_ops import SlurmManagerBase, ServiceType

slurmd = SlurmManagerBase(ServiceType.SLURMD)
slurmd.munge.set_key("encodedmungekey")
slurmd.munge.restart()

start slurmrestd:

from charms.hpc_libs.v0.slurm_ops import SlurmManagerBase, ServiceType

slurmrestd = SlurmManagerBase(ServiceType.SLURMRESTD)
slurmrestd.enable()

add additional, charm-specific functionality:

from charms.hpc_libs.v0.slurm_ops import SlurmManagerBase, ServiceType

class SlurmctldManager(SlurmManagerBase)

    def __init__(self) -> None:
        super().__init__(ServiceType.SLURMCTLD)

        # Compose additional manager functionality.
        self.cgroup = ...

Misc.

Adds kebabizer from @jedel1043. The function format_key(...) will be used to format Slurm configuration keys before passing off to the Slurm configuration editor.

BREAKING CHANGES: Changes the current API to use composition
to enable configuration modification and munge service management.

Signed-off-by: Jason C. Nucciarone <[email protected]>
Decided that these should be defined locally within the Slurm
operators themselves rather than bloat the charm library with
code that will only be used by a specific operator.

Signed-off-by: Jason C. Nucciarone <[email protected]>
@NucciTheBoss NucciTheBoss added the enhancement New feature or request label Jul 9, 2024
@NucciTheBoss NucciTheBoss requested a review from jedel1043 July 9, 2024 21:19
@NucciTheBoss NucciTheBoss self-assigned this Jul 9, 2024
Copy link
Contributor

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Looks great! I just have some questions around needing Pyfakefs.

lib/charms/hpc_libs/v0/slurm_ops.py Show resolved Hide resolved
tests/unit/test_slurm_ops.py Outdated Show resolved Hide resolved
tests/unit/test_slurm_ops.py Outdated Show resolved Hide resolved
@NucciTheBoss NucciTheBoss requested a review from jedel1043 July 10, 2024 13:46
@NucciTheBoss
Copy link
Member Author

@jedel1043 good catch with pyfakefs! Ready for review again 🔎

@NucciTheBoss
Copy link
Member Author

Also, for future reference before we merge, we need to bump the lib patch version so that the updated lib will get published to Charmhub

Copy link
Contributor

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

LGTM!

Signed-off-by: Jason C. Nucciarone <[email protected]>
@NucciTheBoss NucciTheBoss merged commit 9e1d375 into charmed-hpc:main Jul 10, 2024
4 checks passed
@NucciTheBoss NucciTheBoss deleted the experimentation branch July 10, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants