Skip to content

fix: replace context.TODO with timeout context in config dump#8122

Merged
arkodg merged 3 commits intoenvoyproxy:mainfrom
jaffarkeikei:fix/context-todo-config-dump
Feb 1, 2026
Merged

fix: replace context.TODO with timeout context in config dump#8122
arkodg merged 3 commits intoenvoyproxy:mainfrom
jaffarkeikei:fix/context-todo-config-dump

Conversation

@jaffarkeikei
Copy link
Copy Markdown
Contributor

Replaces context.TODO() with context.WithTimeout() to enable proper cancellation and timeouts.

Before:

pods, err := listPods(context.TODO(), client, cd.Namespace, ...)

After:

ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
pods, err := listPods(ctx, client, cd.Namespace, ...)

Benefits:

  • Prevents indefinite hangs when Kubernetes API is slow
  • Enables proper cancellation
  • Follows Go best practices for context propagation

Fixes #8121

@jaffarkeikei jaffarkeikei requested a review from a team as a code owner January 29, 2026 07:35
@netlify
Copy link
Copy Markdown

netlify bot commented Jan 29, 2026

Deploy Preview for cerulean-figolla-1f9435 ready!

Name Link
🔨 Latest commit 0cf2ac3
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/697b73b7d982b90008c3b9a6
😎 Deploy Preview https://deploy-preview-8122--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

}

func (cd ConfigDump) Collect(_ chan<- interface{}) (tbcollect.CollectorResult, error) {
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would prefer this(30s) be configurable with a default value of 30s.

@jaffarkeikei
Copy link
Copy Markdown
Contributor Author

Good point! I've updated the implementation to make the timeout configurable:

  • Added a Timeout field to the ConfigDump struct
  • Created a DefaultConfigDumpTimeout constant set to 30s
  • Added a getTimeout() helper that returns the configured timeout or falls back to the default

This way, users can customize the timeout when needed, while maintaining the sensible 30s default. Thanks for the review!

@zirain
Copy link
Copy Markdown
Member

zirain commented Jan 29, 2026

Good point! I've updated the implementation to make the timeout configurable:

  • Added a Timeout field to the ConfigDump struct
  • Created a DefaultConfigDumpTimeout constant set to 30s
  • Added a getTimeout() helper that returns the configured timeout or falls back to the default

This way, users can customize the timeout when needed, while maintaining the sensible 30s default. Thanks for the review!

Thanks for your patient, the next step is making it configurable as part of the args in egctl x collect.

@zirain
Copy link
Copy Markdown
Member

zirain commented Jan 29, 2026

BTW, please fix DCO.

@jaffarkeikei jaffarkeikei force-pushed the fix/context-todo-config-dump branch from 3c9be5d to 1b03386 Compare January 29, 2026 08:37
jaffar added 2 commits January 29, 2026 03:48
Uses context.WithTimeout instead of context.TODO() to enable
proper cancellation and prevent indefinite hangs when Kubernetes
API is slow or unavailable.

Fixes envoyproxy#8121

Signed-off-by: jaffar <[email protected]>
- Add Timeout field to ConfigDump struct
- Add DefaultConfigDumpTimeout constant (30s)
- Add getTimeout() helper that returns configured timeout or default
- Update Collect() to use cd.getTimeout() instead of hardcoded value

Signed-off-by: jaffar <[email protected]>
@jaffarkeikei jaffarkeikei force-pushed the fix/context-todo-config-dump branch from 1b03386 to b88453f Compare January 29, 2026 08:48
@jaffarkeikei
Copy link
Copy Markdown
Contributor Author

DCO fixed! Added sign-off to all commits. ✅

@jaffarkeikei
Copy link
Copy Markdown
Contributor Author

Hi @zirain - Both commits have the Signed-off-by trailer:

Commit 1 (5bab0f7):

Signed-off-by: jaffar <[email protected]>

Commit 2 (b88453f):

Signed-off-by: jaffar <[email protected]>

You can verify by checking the commits directly:

The DCO should pass now. Perhaps the DCO bot needs to re-check? ✅

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 29, 2026

Codecov Report

❌ Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.65%. Comparing base (42522ff) to head (0cf2ac3).
⚠️ Report is 23 commits behind head on main.

Files with missing lines Patch % Lines
internal/troubleshoot/collect/config_dump.go 57.14% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8122      +/-   ##
==========================================
- Coverage   73.65%   73.65%   -0.01%     
==========================================
  Files         239      240       +1     
  Lines       36311    36467     +156     
==========================================
+ Hits        26745    26859     +114     
- Misses       7670     7703      +33     
- Partials     1896     1905       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- Test getTimeout() method with various timeout values
- Test DefaultConfigDumpTimeout constant
- Test Timeout field can be set and retrieved
- Increase patch coverage to meet project requirements (60%)

This addresses the Codecov check failure by adding comprehensive
unit tests for the new timeout functionality introduced in the
previous commits.

Signed-off-by: jaffar <[email protected]>
@arkodg arkodg added this to the v1.7.0 Release milestone Jan 30, 2026
Copy link
Copy Markdown
Member

@zirain zirain left a comment

Choose a reason for hiding this comment

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

This's good enough for first contributor.

@arkodg arkodg merged commit ac5749a into envoyproxy:main Feb 1, 2026
36 checks passed
cnvergence pushed a commit to cnvergence/gateway that referenced this pull request Feb 3, 2026
…roxy#8122)

* fix: replace context.TODO with timeout context in config dump

Uses context.WithTimeout instead of context.TODO() to enable
proper cancellation and prevent indefinite hangs when Kubernetes
API is slow or unavailable.

Fixes envoyproxy#8121

Signed-off-by: jaffar <[email protected]>

* Make config dump timeout configurable with 30s default

- Add Timeout field to ConfigDump struct
- Add DefaultConfigDumpTimeout constant (30s)
- Add getTimeout() helper that returns configured timeout or default
- Update Collect() to use cd.getTimeout() instead of hardcoded value

Signed-off-by: jaffar <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
cnvergence added a commit that referenced this pull request Feb 3, 2026
* e2e: speed tracing tests (#8124)

* e2e: speed tracing tests

Signed-off-by: zirain <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* fix(translator): allow single-label backends in host mode (#8123)

Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* ci: release json report (#8107)

Signed-off-by: zirain <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* fix oidc flakiness (#8119)

* fix oidc flakiness

Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* fix: skip_test_workflow doesn't exist (#8116)

This also uses grouped redirects to satisfy shellcheck SC2129.

Signed-off-by: Dylan M. Taylor <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* fix e2e test panic (#8109)

fix e2e test

Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* chore: bump func-e to v1.4.0 (#8105)

bump func-e to v1.4.0

Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* fix: route idle timeout (#8058)

* fix: route idle timeout

Signed-off-by: Huabing (Robin) Zhao <[email protected]>

* address comments

Signed-off-by: Huabing (Robin) Zhao <[email protected]>

* add test

Signed-off-by: Huabing (Robin) Zhao <[email protected]>

---------

Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* docs: add Mirakl to adopters list (#8138)

Signed-off-by: Thierry Wandja <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* docs: add security warning to control plane extensions (#7967)

chore(docs): add warnings about control plane extensions

Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* chore: add lint for release notes filenames (#8137)

* chore: add lint for release notes filenames

Signed-off-by: zirain <[email protected]>

* remove 1.7.0

Signed-off-by: zirain <[email protected]>

* fix lint

Signed-off-by: zirain <[email protected]>

---------

Signed-off-by: zirain <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* fix: remove global logger in message package (#8131)

* fix: remove global logger in message package

Signed-off-by: zirain <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* docs: fix url result of regex rewrite (#7864)

* Update http-urlrewrite.md

Signed-off-by: Sadmi Bouhafs <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* chore: log skipped xds (#8132)

log skipped xds

Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* docs: fixes for OPA sidecar + Unix Domain Socket task (#8142)

Signed-off-by: Matt Miller <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* fix: basic auth validation (#8053)

* fix basic auth validation

Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* fix: controller cache-sync readiness check (#7430)

Signed-off-by: Karol Szwaj <[email protected]>

* fix: replace context.TODO with timeout context in config dump (#8122)

* fix: replace context.TODO with timeout context in config dump

Uses context.WithTimeout instead of context.TODO() to enable
proper cancellation and prevent indefinite hangs when Kubernetes
API is slow or unavailable.

Fixes #8121

Signed-off-by: jaffar <[email protected]>

* Make config dump timeout configurable with 30s default

- Add Timeout field to ConfigDump struct
- Add DefaultConfigDumpTimeout constant (30s)
- Add getTimeout() helper that returns configured timeout or default
- Update Collect() to use cd.getTimeout() instead of hardcoded value

Signed-off-by: jaffar <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* refactor: convert IR map fields to slices to ensure deterministic Dee… (#7953)

* refactor: convert IR map fields to slices to ensure deterministic DeepEqual

Addresses issue #7852.

Signed-off-by: Junnygram <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* fix links in releasing and develop docs (#8141)

* fix links in releasing and develop docs

Signed-off-by: Karol Szwaj <[email protected]>

* update quickstart link

Signed-off-by: Karol Szwaj <[email protected]>

---------

Signed-off-by: Karol Szwaj <[email protected]>

* docs: add provider guide for entra (#7977)

* docs: add provider guide for entra

Signed-off-by: Oliver Bähler <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* chore: clean up test output files (#8154)

clean up test output files

Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* fix: TCPRoute mTLS didn't work (#8152)

* fix: remove auto HTTP config on TCP cluster

Signed-off-by: zirain <[email protected]>

* fix lint

Signed-off-by: zirain <[email protected]>

* add e2e

Signed-off-by: zirain <[email protected]>

* fix e2e

Signed-off-by: zirain <[email protected]>

* fix comment

Signed-off-by: zirain <[email protected]>

* fix

Signed-off-by: zirain <[email protected]>

* fix resource name

Signed-off-by: zirain <[email protected]>

* address Arko's comment

Signed-off-by: zirain <[email protected]>

---------

Signed-off-by: zirain <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>

* v1.7.0-rc2 release notes (#8163)

* v1.7.0-rc2 release notes

Signed-off-by: Karol Szwaj <[email protected]>

* fix the date

Signed-off-by: Karol Szwaj <[email protected]>

---------

Signed-off-by: Karol Szwaj <[email protected]>

---------

Signed-off-by: zirain <[email protected]>
Signed-off-by: Karol Szwaj <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Dylan M. Taylor <[email protected]>
Signed-off-by: Thierry Wandja <[email protected]>
Signed-off-by: Guy Daich <[email protected]>
Signed-off-by: Sadmi Bouhafs <[email protected]>
Signed-off-by: Matt Miller <[email protected]>
Signed-off-by: jaffar <[email protected]>
Signed-off-by: Junnygram <[email protected]>
Signed-off-by: Oliver Bähler <[email protected]>
Co-authored-by: zirain <[email protected]>
Co-authored-by: Adrian Cole <[email protected]>
Co-authored-by: Huabing (Robin) Zhao <[email protected]>
Co-authored-by: Dylan M. Taylor <[email protected]>
Co-authored-by: Thierry Wandja <[email protected]>
Co-authored-by: Guy Daich <[email protected]>
Co-authored-by: Sadmi Bouhafs <[email protected]>
Co-authored-by: Matt Miller <[email protected]>
Co-authored-by: Isaac Wilson <[email protected]>
Co-authored-by: jaffar keikei <[email protected]>
Co-authored-by: Olaleye <[email protected]>
Co-authored-by: Oliver Bähler <[email protected]>
Inode1 pushed a commit to Inode1/gateway that referenced this pull request Feb 23, 2026
…roxy#8122)

* fix: replace context.TODO with timeout context in config dump

Uses context.WithTimeout instead of context.TODO() to enable
proper cancellation and prevent indefinite hangs when Kubernetes
API is slow or unavailable.

Fixes envoyproxy#8121

Signed-off-by: jaffar <[email protected]>

* Make config dump timeout configurable with 30s default

- Add Timeout field to ConfigDump struct
- Add DefaultConfigDumpTimeout constant (30s)
- Add getTimeout() helper that returns configured timeout or default
- Update Collect() to use cd.getTimeout() instead of hardcoded value

Signed-off-by: jaffar <[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.

context.TODO() used in production code (config_dump.go)

3 participants