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

Improve process handling and show stdout/stderr in portforward-monitoring-satellite-harvester.sh #9754

Merged
merged 1 commit into from
May 4, 2022

Conversation

mads-hartmann
Copy link
Contributor

Description

This improves process handling and the debugability of the dev/preview/portforward-monitoring-satellite-harvester.sh script by relying on xargs for process management.

It also simplifies the output slightly.

Related Issue(s)

Fixes https://github.com/gitpod-io/ops/issues/1947

How to test

I ran the following script in a workspace on a branch that had a VM /dev/preview/portforward-monitoring-satellite-harvester.sh and tested how it dealt with

  1. The Prometheus pod not running. Previously the script would just silently fail, now it shows an error message you can use to debug:
    E0504 09:07:39.940182   52166 portforward.go:406] an error occurred forwarding 9090 -> 32001: error forwarding port 32001 to pod 49f246b3afe6a6c7eccf0b2c62d8f2922e534edd41c38f2813654f8452e658ce, uid : failed to execute portforward in network namespace "/var/run/netns/cni-4048ab39-6cee-d3e3-880c-834fa272a0f8": failed to dial 32001: dial tcp4 127.0.0.1:32001: connect: connection refused
    E0504 09:07:39.940541   52166 portforward.go:234] lost connection to pod
    
  2. Running the script in many terminals to ensure it deals with existing processes or zombie processes.

Release Notes

NONE

Documentation

N/A

Copy link
Contributor

@liam-j-bennett liam-j-bennett left a comment

Choose a reason for hiding this comment

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

🛹

@mads-hartmann mads-hartmann force-pushed the mads/improve-monitoring-port-forwarding branch from bb2148d to 1c94dd3 Compare May 4, 2022 12:13
@mads-hartmann
Copy link
Contributor Author

mads-hartmann commented May 4, 2022

@liam-j-bennett Thanks for the review! I had to rebase on main but the diff is the same and I just verified the script so I'll merge once CI passes ☺️

@roboquat roboquat merged commit 97d8eb9 into main May 4, 2022
@roboquat roboquat deleted the mads/improve-monitoring-port-forwarding branch May 4, 2022 12:18
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