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

Provide several sub-commands only on Linux #5381

Merged
merged 4 commits into from
Jan 9, 2025

Conversation

twz123
Copy link
Member

@twz123 twz123 commented Dec 20, 2024

Description

They won't work on non-Linux systems anyways. Use build flags to add platform-specific sub-commands instead of runtime checks. Also add build tag to files that only make sense on specific platforms. This includes Autopilot, as this relies on the status socket, which is not available on Windows at the moment.

Inline setup function for install commands: Configuration loading, validation, and user setup are required only for controllers. Inline this functionality into their commands, making the worker command lighter.

Compile users.LookupUID via build tags. On Windows, there's no numeric UIDs, so using this there is futile.

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 linux-only-subcommands branch 2 times, most recently from f95db7d to 529dae8 Compare December 21, 2024 01:11
@twz123 twz123 marked this pull request as ready for review December 21, 2024 01:31
@twz123 twz123 requested review from a team as code owners December 21, 2024 01:31
Copy link
Contributor

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

@twz123 twz123 force-pushed the linux-only-subcommands branch from 529dae8 to 53c9965 Compare December 24, 2024 13:19
@twz123 twz123 force-pushed the linux-only-subcommands branch 3 times, most recently from c84a9bf to 07ee5e9 Compare December 24, 2024 14:13
Copy link
Contributor

github-actions bot commented Jan 6, 2025

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

@twz123 twz123 force-pushed the linux-only-subcommands branch from 07ee5e9 to 2db976c Compare January 7, 2025 07:48
juanluisvaladas
juanluisvaladas previously approved these changes Jan 7, 2025
jnummelin
jnummelin previously approved these changes Jan 7, 2025
@twz123 twz123 dismissed stale reviews from jnummelin and juanluisvaladas via 3e409de January 7, 2025 11:13
@twz123 twz123 force-pushed the linux-only-subcommands branch 6 times, most recently from d59a517 to cd7b466 Compare January 7, 2025 14:09
Copy link
Contributor

github-actions bot commented Jan 8, 2025

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

twz123 added 4 commits January 8, 2025 12:44
On Windows, there's no numeric UIDs, so using this there is futile.

Signed-off-by: Tom Wieczorek <[email protected]>
...and harmonize the status-socket flag in the status sub-command.

The shared use of the StatusSocket variable, along with the non-empty
default value in the status sub-command, caused the registration order
of sub-commands to change the flag value for each sub-command, even
if it wasn't set from the command line. This broke the configuration
defaulting.

Signed-off-by: Tom Wieczorek <[email protected]>
Configuration loading, validation, and user setup are required only for
controllers. Inline this functionality into their commands, making the
worker command lighter.

Signed-off-by: Tom Wieczorek <[email protected]>
They won't work on non-Linux systems anyways.

Use build flags to add platform-specific sub-commands instead of runtime
checks. Also add build tag to files that only make sense on specific
platforms. This includes Autopilot, as this relies on the status socket,
which is not available on Windows at the moment.

Signed-off-by: Tom Wieczorek <[email protected]>
@twz123 twz123 force-pushed the linux-only-subcommands branch from cd7b466 to 90fb8e2 Compare January 8, 2025 11:47
@twz123 twz123 merged commit bb264cb into k0sproject:main Jan 9, 2025
92 checks passed
@twz123 twz123 deleted the linux-only-subcommands branch January 9, 2025 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants