fix: prepend agones sidecars before user defined init containers#4240
fix: prepend agones sidecars before user defined init containers#4240stevefan1999-personal wants to merge 8 commits intoagones-dev:mainfrom
Conversation
|
@Sivasankaran25 @0xaravindh sorry for a ping but is this syntax legal in Golang? I see that it is a vararg I'm not sure if ... unpack operator works for slices. Another way to do it: import "slices"
pod.Spec.InitContainers = slices.Concat(sidecars, pod.Spec.InitContainers)but this requires go 1.22+ |
|
@stevefan1999-personal yes, since we're already using Go 1.24, it's safe to use slices.Concat() — no issues there. #slices |
|
Oh that's a really good catch! My only extra thought - might be worth adding a unit test to make sure that the sdk-server sidecar is always first, and we don't ever have a regression. No strong opinions on which slice syntax is used 😄 |
|
/gcbrun |
|
Build Failed 😭 Build Id: b191037d-1958-41e8-9068-ab10a026abe9 Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
/gcbrun |
|
/gcbrun submit-e2e-test-cloud-build is still flaky. |
|
Build Failed 😭 Build Id: 2d520d12-cba1-4178-9e1c-96c819c2e196 Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
/gcbrun |
|
Build Failed 😭 Build Id: ca564f19-822c-4573-a93e-5eb0dc0ae8ec Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Ignore the errors for now - CI is pretty borked |
|
/gcbrun |
|
I'd still love a unit test here so we never regress this change. |
|
Build Failed 😭 Build Id: 404943de-1fd1-4011-a130-0bea2e3f4863 Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
|
Coming back around to this - would love this in the next release, but it does need some unit tests please to make sure the change doesn't regress at any point. (Also working on trying to make the broken CI less broken). |
|
/gcbrun |
|
Build Succeeded 🥳 Build Id: 1f795bd3-95b8-4a57-a876-badb2c27f758 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version: |
Replacement of agones-dev#4240 with an accompanying unit test. As stated in that issue, if you want to use other init container sidecar, they should have access to the SDK server if they want it. Closes agones-dev#4239
|
FYI: I wanted to make sure this is in the next release, so wrote the same fix with accompanying unit test here: #4240 |
Replacement of agones-dev#4240 with an accompanying unit test. As stated in that issue, if you want to use other init container sidecar, they should have access to the SDK server if they want it. Closes agones-dev#4239
Overview
This PR addresses and closes #4239 by fixing a critical initialization order issue with the Agones SDK init container that causes deadlocks in certain deployment configurations.
Problem Description
Discovery Context
I discovered this issue while working with a custom init container setup for our game server deployments. Our initialization workflow includes an init container responsible for:
This initialization process has a critical dependency: it requires the Agones SDK to be fully initialized and accessible before it can proceed with port allocation and server registration tasks.
The Deadlock Scenario
Kubernetes init containers execute sequentially in the order they are defined in the pod specification. Each init container must complete successfully before the next one begins execution. This sequential execution pattern is where our problem manifests:
Root Cause Analysis
The core issue stems from the container injection order. When Agones adds the SDK init container to the final pod specification, it appends it to the existing init containers array rather than prepending it. This breaks the assumption that the Agones SDK will be available for other init containers that depend on it.
Solution
This PR modifies the container injection logic to ensure the Agones SDK init container is placed at the beginning of the init containers array, guaranteeing it initializes before any user-defined init containers that may depend on its functionality.
Impact & Benefits
Testing Performed