Skip to content

Add missing --timeout flag to helmfile sync command with documentation#2148

Merged
yxxhero merged 4 commits into
mainfrom
copilot/fix-1543
Aug 27, 2025
Merged

Add missing --timeout flag to helmfile sync command with documentation#2148
yxxhero merged 4 commits into
mainfrom
copilot/fix-1543

Conversation

Copilot AI commented Aug 23, 2025

Copy link
Copy Markdown
Contributor

The helmfile sync command was missing the --timeout flag despite timeout functionality being documented and available in the configuration. Users attempting to use helmfile sync --timeout 900 would encounter an "unknown flag: --timeout" error.

This PR implements the missing CLI flag and adds comprehensive documentation:

Changes Made

  • Added --timeout flag to sync command: The flag accepts an integer value representing seconds and validates input properly
  • Updated configuration structures: Added Timeout field to both SyncOptions and SyncOpts structs
  • Modified timeout precedence logic: Updated timeoutFlags() function to prioritize CLI timeout over release-specific and default timeouts
  • Added comprehensive tests: New test case verifies CLI timeout correctly overrides other timeout settings
  • Enhanced documentation: Added detailed CLI flag documentation with examples and precedence explanation

Documentation Updates

The sync command documentation now includes:

  • New subsection documenting common sync flags (--timeout, --wait, --wait-for-jobs)
  • Practical usage examples showing various flag combinations
  • Clear precedence explanation showing CLI flags override configuration settings
  • Enhanced CLI reference section with notes about command-specific help

Behavior

The timeout flag follows the established priority order:

  1. CLI flag: helmfile sync --timeout 900 (highest priority)
  2. Release config: timeout: 600 in release specification
  3. Helm defaults: helmDefaults.timeout: 300 (lowest priority)

Example Usage

# Override default timeout for all releases in this sync
helmfile sync --timeout 900

# Works with other flags
helmfile sync --timeout 600 --wait --wait-for-jobs

# Target specific releases with custom timeout
helmfile sync --selector tier=backend --timeout 1200

The implementation maintains full backward compatibility and follows existing code patterns. Input validation ensures invalid values like --timeout abc are properly rejected with clear error messages.

Fixes #1543.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

- Add Timeout field to SyncOptions struct in pkg/config/sync.go
- Add --timeout flag to sync command in cmd/sync.go
- Add Timeout field to SyncOpts struct in pkg/state/state.go
- Modify timeoutFlags() function to prioritize CLI timeout over release and default configs
- Add test case to verify CLI timeout overrides other timeout settings
- Follow same pattern as existing --wait and --wait-for-jobs flags

Co-authored-by: yxxhero <[email protected]>
Copilot AI changed the title [WIP] helmfile sync --timeout <timeout_value> returns "unknown flag: --timeout" despite being documented as supported Add missing --timeout flag to helmfile sync command Aug 23, 2025
Copilot AI requested a review from yxxhero August 23, 2025 08:19
@yxxhero yxxhero marked this pull request as ready for review August 23, 2025 08:38
@yxxhero

yxxhero commented Aug 23, 2025

Copy link
Copy Markdown
Member

@copilot please fix lint issues.

@yxxhero

yxxhero commented Aug 23, 2025

Copy link
Copy Markdown
Member

@copilot please update the docs.

Copilot AI commented Aug 23, 2025

Copy link
Copy Markdown
Contributor Author

@copilot please fix lint issues.

Fixed lint issues. The problem was improper struct field formatting in the test file. All golangci-lint checks now pass successfully. (fb8c49e)

Copilot AI commented Aug 23, 2025

Copy link
Copy Markdown
Contributor Author

@copilot please update the docs.

Updated the documentation to include the new --timeout flag. Added comprehensive documentation for the sync command flags including usage examples and precedence explanation. The docs now clearly show how the CLI --timeout flag overrides helmDefaults.timeout and per-release timeout settings. (215ef74)

Copilot AI changed the title Add missing --timeout flag to helmfile sync command Add missing --timeout flag to helmfile sync command with documentation Aug 23, 2025
@yxxhero yxxhero merged commit 135ff63 into main Aug 27, 2025
13 checks passed
@yxxhero yxxhero deleted the copilot/fix-1543 branch August 27, 2025 06:32
KATechDev pushed a commit to KATechDev/helmfile that referenced this pull request Oct 1, 2025
helmfile#2148)

* Initial plan

* Implement --timeout flag for helmfile sync command

- Add Timeout field to SyncOptions struct in pkg/config/sync.go
- Add --timeout flag to sync command in cmd/sync.go
- Add Timeout field to SyncOpts struct in pkg/state/state.go
- Modify timeoutFlags() function to prioritize CLI timeout over release and default configs
- Add test case to verify CLI timeout overrides other timeout settings
- Follow same pattern as existing --wait and --wait-for-jobs flags

Co-authored-by: yxxhero <[email protected]>

* Fix lint issues: format test struct fields properly

Co-authored-by: yxxhero <[email protected]>

* Update docs: Add --timeout flag documentation for helmfile sync command

Co-authored-by: yxxhero <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: yxxhero <[email protected]>
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.

helmfile sync --timeout <timeout_value> returns "unknown flag: --timeout" despite being documented as supported

3 participants