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.

Sign up for GitHub

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

Jump to bottom

Decouple controller flags from CLIOptions struct #5438

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

Decouple controller flags from CLIOptions struct #5438

wants to merge 1 commit into from
+112 −76

Conversation

twz123
Copy link
Member

@twz123 twz123 commented Jan 15, 2025

Description

This eliminates one more global variable. Make the single and enable-worker flags an implementation detail of flag handling, and provide an enum to the outside world instead. Draw a clear line between controller and worker flags. Instead of accessing the entire controller flags struct in the worker, have a bespoke interface that acts as an integration point between a worker and its embedding controller, if any.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

How Has This Been Tested?

  • Manual test
  • Auto test added

Checklist:

  • My code follows the style guidelines of this project
  • My commit messages are signed-off
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

@twz123 twz123 force-pushed the decouple-controller-flags branch from ebdd6f4 to 2e5c36d Compare January 16, 2025 13:37
@twz123 twz123 marked this pull request as ready for review January 16, 2025 16:38
@twz123 twz123 requested review from a team as code owners January 16, 2025 16:38
@twz123 twz123 requested review from makhov and jnummelin January 16, 2025 16:38
@github-actions GitHub Actions
Copy link
Contributor

This pull request has merge conflicts that need to be resolved.

@twz123
Decouple controller flags from CLIOptions struct
4abf5ce 
This eliminates one more global variable. Make the single and
enable-worker flags an implementation detail of flag handling, and
provide an enum to the outside world instead. Draw a clear line between
controller and worker flags. Instead of accessing the entire controller
flags struct in the worker, have a bespoke interface that acts as an
integration point between a worker and its embedding controller, if any.

Signed-off-by: Tom Wieczorek <[email protected]>
@twz123 twz123 force-pushed the decouple-controller-flags branch from 2e5c36d to 4abf5ce Compare January 17, 2025 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@makhov makhov Awaiting requested review from makhov makhov is a code owner automatically assigned from k0sproject/k0s-maintainers

@jnummelin jnummelin Awaiting requested review from jnummelin jnummelin is a code owner automatically assigned from k0sproject/k0s-maintainers

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area/cli area/controlplane chore
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@twz123