fix: allow cli/ui to follow logs #8987
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alexmt
approved these changes
Apr 4, 2022
Codecov Report
@@ Coverage Diff @@
## master #8987 +/- ##
==========================================
- Coverage 45.00% 44.95% -0.05%
==========================================
Files 212 212
Lines 25272 25272
==========================================
- Hits 11373 11362 -11
- Misses 12292 12306 +14
+ Partials 1607 1604 -3
Continue to review full report at Codecov.
|
Signed-off-by: Daniel Helfand <helfand.4@gmail.com>
crenshaw-dev
approved these changes
Apr 4, 2022
There was a problem hiding this comment.
LGTM! @jannfis was there any reason you needed the default to be 32s instead of 0?
|
@crenshaw-dev Thanks for pulling me in. The server side timeout is the timeout passed to Kubernetes API requests. The default used by the K8s API client is 32 seconds, and should imho stay this way. But probably, when set to 0, the API client will use the 32s default anyway. I was just making it explicit. |
|
I think the reason why the follow logs fails is a different tho. It's probably this addition, introduced in my change: argo-cd/pkg/apis/application/v1alpha1/types.go Line 2346 in a5933db EDIT: Hm. That's 5 minutes per default. Shouldn't be the 30s culprit then. |
|
@jannfis Thanks for the perspective on this. I tried configuring the various options to see if any of these were the culprit. Happy to take our time on this and debug a bit further though if you have further questions I can look into. |
Any docs on this? Am only seeing 32 seconds as default for the discovery client. |
3 tasks
|
Following up here, I think having no timeout as the default is the way to go. Looking further into the k8s client set up, I feel pretty confident that no timeout is being set by default and the 32 second timeout is specific to the discovery client. This would make sense since, prior to #8404, the timeout was not configurable or being set at all. |
|
@jannfis is that explanation persuasive / would you be alright with merging this PR? |
|
@danielhelfand I'm not super familiar with this area of the code, but your explanation makes sense to me. We can always revert if there's trouble. |
danielhelfand
added a commit
to danielhelfand/argo-cd
that referenced
this pull request
Apr 11, 2022
Signed-off-by: Daniel Helfand <helfand.4@gmail.com>
crenshaw-dev
pushed a commit
that referenced
this pull request
Apr 11, 2022
wojtekidd
pushed a commit
to wojtekidd/argo-cd
that referenced
this pull request
Apr 25, 2022
Signed-off-by: Daniel Helfand <helfand.4@gmail.com> Signed-off-by: wojtekidd <wojtek.cichon@protonmail.com>
sujeilyfonseca
added a commit
to sujeilyfonseca/argo-cd
that referenced
this pull request
Jul 7, 2022
* fix(ui): Applications page incorrectly resets to tiles view. Fixes argoproj#8702 (argoproj#8718) Signed-off-by: Yuan Tang <terrytangyuan@gmail.com> * fix: correct jsonnet paths resolution (argoproj#8721) Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> * chore: Bump stable version of application set addon (argoproj#8744) Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> * fix: Retry checkbox unchecked unexpectedly; Sync up with YAML (argoproj#8682) (argoproj#8720) Signed-off-by: Keith Chong <kykchong@redhat.com> * Bump version to 2.3.1 * Bump version to 2.3.1 * Merge pull request from GHSA-2f5v-8r3f-8pww * fix: application resource APIs must enforce project restrictions Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> * Fix unit tests Signed-off-by: jannfis <jann@mistrust.net> Co-authored-by: jannfis <jann@mistrust.net> * chore: remove lint-docs CI task (argoproj#8722) (argoproj#8858) * chore: remove lint-docs CI task Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> * chore: remove not longer necessary url-allow-list Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> Co-authored-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> * chore: fix imports (argoproj#8859) Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * Bump version to 2.3.2 * Bump version to 2.3.2 * fix: Set QPS and burst rate for resource ops client (argoproj#8915) * fix: Set QPS and burst rate for resource ops client Signed-off-by: jannfis <jann@mistrust.net> * fix: prevent excessive repo-server disk usage for large repos (argoproj#8845) (argoproj#8897) fix: prevent excessive repo-server disk usage for large repos (argoproj#8845) (argoproj#8897) Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * fix: bump gitops engine version to v0.6.2 Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> * docs: update v2.4+ roadmap items (argoproj#8593) Signed-off-by: ishitasequeira <isequeir@redhat.com> * docs: reflect v2.3 release changes in roadmap.md (argoproj#8747) docs: reflect v2.3 release changes in roadmap.md (argoproj#8747) Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> * Bump version to 2.3.3 * Bump version to 2.3.3 * fix: Fix docs build error (argoproj#8895) * work with specific jinja version Signed-off-by: pashavictorovich <pavel@codefresh.io> * fix: fix broken monaco editor collapse icons (argoproj#8709) Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> * chore: upgrade to go 1.17.8 (argoproj#8866) (argoproj#9004) * chore: upgrade to go 1.17.8 Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: use 1.17 so it's always latest in the series Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * fix: allow cli/ui to follow logs (argoproj#8987) (argoproj#9065) Signed-off-by: Daniel Helfand <helfand.4@gmail.com> * Merge pull request from GHSA-xmg8-99r8-jc2j Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> Co-authored-by: Michael Crenshaw <michael@crenshaw.dev> * Merge pull request from GHSA-6gcg-hp2x-q54h * fix: do not allow symlinks from directory-type applications Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: add new util file Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: lint Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: use t.TempDir for simpler tests Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * address comments Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * Merge pull request from GHSA-r642-gv9p-2wjj Signed-off-by: jannfis <jann@mistrust.net> Co-authored-by: Michael Crenshaw <michael@crenshaw.dev> Co-authored-by: Michael Crenshaw <michael@crenshaw.dev> * Bump version to 2.3.4 * Bump version to 2.3.4 * test: fix ErrorContains (argoproj#9445) Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * fix: missing Helm params (argoproj#9565) (argoproj#9566) * fix: missing Helm params Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * use absolute paths, fix tests Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * fix race in test Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: upgrade golangci-lint to v1.46.2 (argoproj#9448) * chore: upgrade golangci-lint to v1.46.2 Because: * Installation of golangci-lint v1.45.2 is currently broken and fails silently due to a redacted dependency (blizzy78/varnamelen#13) This commit: * Upgrades golangci-lint to v1.46.2 Signed-off-by: Tommaso Sardelli <lacapannadelloziotom@gmail.com> * fix: lint Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * fix: lint Signed-off-by: Tommaso Sardelli <lacapannadelloziotom@gmail.com> Co-authored-by: Michael Crenshaw <michael@crenshaw.dev> Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * fix: test race (argoproj#9469) Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: lint issues Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: update golangci-lint (argoproj#8988) * chore: update golangci-lint Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: remove obsolete repo-server unit test (argoproj#9559) Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> * chore: Make unit tests run on platforms other than amd64 (argoproj#8995) Signed-off-by: jannfis <jann@mistrust.net> Co-authored-by: Michael Crenshaw <michael@crenshaw.dev> Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: eliminate go-mpatch dependency (argoproj#9045) * chore: eliminate go-mpatch dependency Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: abstract out resource list function Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: don't exit the program in anything but the main function Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: better error messages Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: better error messages Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * test: directory app manifest generation (argoproj#9503) * test: directory app manifest generation Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * git doesn't support empty dirs Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * Merge pull request from GHSA-h4w9-6x78-8vrj Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * Merge pull request from GHSA-2m7h-86qq-fp4v Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> fix references Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> use long enough state param for oauth2 Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> typo Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> more entropy Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> fix test Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * Merge pull request from GHSA-q4w5-4gq2-98vm Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * Merge pull request from GHSA-jhqp-vf4w-rpwq Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> defer instead of multiple close calls Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> oops Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> don't count jsonnet against max Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> fix codegen Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> add caveat about 300x ratio Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> fix versions Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> fix tests/lint Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * chore: fix docs gen Signed-off-by: Michael Crenshaw <michael@crenshaw.dev> * Bump version to 2.3.5 * Bump version to 2.3.5 * docs: Changes for v2.3.5 Documented key decision factors to use Argo CD v2.3.5. Contributes to: automation-saas/automation-saas/native-AWS#1972 Signed-off-by: Sujeily Fonseca <sujeily.fonseca@ibm.com> Co-authored-by: Yuan Tang <terrytangyuan@gmail.com> Co-authored-by: Alexander Matyushentsev <AMatyushentsev@gmail.com> Co-authored-by: Keith Chong <kykchong@redhat.com> Co-authored-by: argo-bot <argoproj@gmail.com> Co-authored-by: jannfis <jann@mistrust.net> Co-authored-by: Michael Crenshaw <michael@crenshaw.dev> Co-authored-by: Ishita Sequeira <46771830+ishitasequeira@users.noreply.github.com> Co-authored-by: pasha-codefresh <pavel@codefresh.io> Co-authored-by: Daniel Helfand <helfand.4@gmail.com> Co-authored-by: Tommaso Sardelli <lacapannadelloziotom@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #8947
#8404 introduced a new environment var (
ARGOCD_K8S_TCP_TIMEOUT) with a default of 32 seconds. This is causing requests for logs withfollowconfigured to be timed out. Maybe the default should be no timeout and should be explicitly configured by the user if requested.For any user looking to configure these values, we should consider including these logging considerations in documentation for these env vars. There is an issue open for documenting these vars: #8527.
Additionally, the
maxparam for usingenv.ParseDurationFromEnvfunc is being passed as arg math.MaxInt32, which is converted to2.147483647sas a time.Duration. This then only allows users to configure a value between 0 and 2.147483647s. This pr also updates parse duration funcs using math.MaxInt32 as a max to math.MaxInt32*time.Second to fix this issue.Note on DCO:
If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.
Checklist: