Add test to validate codependency bug exists#1126
Conversation
| time.Sleep(15 * time.Second) | ||
| fmt.Println("Proxy signaling self") | ||
| proc, _ := os.FindProcess(os.Getpid()) | ||
| proc.Signal(syscall.SIGABRT) | ||
| time.Sleep(10 * time.Second) |
There was a problem hiding this comment.
is all this sleeping really necessary? Or can we trigger on something?
Also do we normally have fmt.Println in tests?
There was a problem hiding this comment.
These are being done from inside the processes running inside garden, so the println gets captured as part of the garden job output rather than as test output. It probably goes nowhere anyway since we don't look at any of the logged output.
I'll look into these processes being http servers that the test signals after they're running, to trigger them crashing. But since one is the main app, one is a sidecar process, and one is the proxy process, it may be a little weird.
cf-foundation-community-automation
Bot
moved this to Inbox
in Application Runtime Platform Working Group
geofffranks
force-pushed
the
sidecar-codependency-bug
branch
4 times, most recently
from
c82dd2a to
a851c03
Compare
geofffranks
force-pushed
the
sidecar-codependency-bug
branch
from
9f17c6b to
9cf12db
Compare
geofffranks
force-pushed
the
sidecar-codependency-bug
branch
9 times, most recently
from
1be0b0b to
0e6bb2c
Compare
Implements test infrastructure to validate that containers crash when codependent processes exit, rather than leaving processes hanging. Key components: - New fake_app and fake_proxy test assets with controllable exit behavior - Comprehensive unit tests for both fake applications - Integration test matrix covering main/sidecar/proxy exit scenarios - Support for various exit codes including SIGABRT (134) - HTTP-based signaling for deterministic test control - Serial test execution to prevent port conflicts - Proper resource cleanup and error handling The test validates the fix for the sidecar codependency bug where processes would remain hung instead of the container properly exiting when any codependent process fails. Made-with: Cursor
Improves error handling consistency in component cleanup routines. Changes cleanup functions to log warnings instead of panicking on cleanup failures, preventing race conditions during test teardown. This ensures tests can complete gracefully even when temporary files or directories are already cleaned up by other processes. Made-with: Cursor
geofffranks
force-pushed
the
sidecar-codependency-bug
branch
from
0e6bb2c to
f30b6e8
Compare
| Expect(err).ToNot(HaveOccurred()) | ||
| if err := os.RemoveAll(configFile.Name()); err != nil { | ||
| fmt.Fprintf(os.Stderr, "Warning: failed to cleanup route emitter config file: %v\n", err) | ||
| } |
There was a problem hiding this comment.
These cleanups are part of cleaning up the individual processes run from tests, and having ginkgo failures here was causing race failures in the tests, and also may have prevented other cleanups from running if they encountered an error.
|
Ok, i think all your original concerns are finally addressed and passing tests, and all of clod's debugging slop has been removed. Can you please take another look? |
github-project-automation
Bot
moved this from Inbox
to Pending Merge | Prioritized
in Application Runtime Platform Working Group
github-project-automation
Bot
moved this from Pending Merge | Prioritized
to Done
in Application Runtime Platform Working Group
2 participants
Made-with: Cursor
Summary
Adds inigo tests to ensure that the issue found with codependent processes exiting of their own accord not triggering other processes to exit does not regress
Backward Compatibility
Breaking Change? no