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

fix(ui): default to main container in Sensor logs. Fixes #9459 #9438

Merged
merged 1 commit into from
Sep 5, 2022
Merged

fix(ui): default to main container in Sensor logs. Fixes #9459 #9438

merged 1 commit into from
Sep 5, 2022

Conversation

jsvk
Copy link
Contributor

@jsvk jsvk commented Aug 25, 2022 •
edited
Loading

Fixes #9459

terrytangyuan
terrytangyuan reviewed Aug 25, 2022
View reviewed changes
Copy link
Member

@terrytangyuan terrytangyuan left a comment

Choose a reason for hiding this comment

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

What problem is this trying to solve?

@jsvk
Copy link
Contributor Author

jsvk commented Aug 25, 2022 •
edited
Loading

sorry still working on the write-up (in the form of Github Issue, hope that's ok), but the short summary is we're injecting a sidecar container via mutating admission controller to the Sensor pod (fluent-bit in our case, for forwarding logs to Splunk).

Without specifying the container, the UI fails to load the logs and instead returns with a 504 - Gateway Timeout.

Error logs in the Server pod say it encountered this error -

time="2022-08-25T14:30:21.960Z" level=error msg="a container name must be specified for pod adobe-platform--[snip], choose one of: [main fluent-bit]" namespace=ns-team-adobe-platform--[snip] podName=adobe-platform--[snip]

I saw the API does support parsing podLogOptions.container, and so by appending ?podLogOptions.container=main to the API call, I confirmed the call returns instantly with the logs we expect, so this is a matter of updating this at the UI level.

Is there a better approach I'm not thinking of?

@jsvk jsvk changed the title Default to 'main' container in Sensor logs fix: default to 'main' container in Sensor logs Aug 28, 2022
@jsvk jsvk mentioned this pull request Aug 28, 2022
Closed
3 tasks
@jsvk jsvk force-pushed the patch-1 branch 3 times, most recently from d1c05cb to 897f338 Compare August 29, 2022 00:47
@jsvk jsvk marked this pull request as ready for review August 29, 2022 06:34
@jsvk jsvk changed the title fix: default to 'main' container in Sensor logs fix: default to 'main' container in Sensor logs. Fixes #9459 Aug 29, 2022
@alexec alexec enabled auto-merge (squash) September 5, 2022 20:24
@alexec alexec merged commit f27475f into argoproj:master Sep 5, 2022
juchaosong pushed a commit to juchaosong/argo-workflows that referenced this pull request Nov 3, 2022
@jsvk @juchaosong
fix: default to 'main' container in Sensor logs. Fixes argoproj#9459 (a…
61db1bd 
…rgoproj#9438)

Signed-off-by: jsvk <[email protected]>
Signed-off-by: juchao <[email protected]>
@sarabala1979 sarabala1979 mentioned this pull request Nov 8, 2022
Closed
@aniro-s aniro-s mentioned this pull request Apr 15, 2024
Closed
4 tasks
@agilgur5 agilgur5 changed the title fix: default to 'main' container in Sensor logs. Fixes #9459 fix(ui): default to main container in Sensor logs. Fixes #9459 Apr 18, 2024
@agilgur5 agilgur5 mentioned this pull request Apr 18, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexec alexec alexec approved these changes

@terrytangyuan terrytangyuan Awaiting requested review from terrytangyuan

Assignees
No one assigned
Labels
area/ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

UI hangs when attempting to view Sensor logs if pod has more than one container
4 participants
@jsvk @alexec @terrytangyuan @agilgur5