diff --git a/scripts/test_verify_agones_allocation_response.py b/scripts/test_verify_agones_allocation_response.py index 6e4faa1a..2c31b37b 100644 --- a/scripts/test_verify_agones_allocation_response.py +++ b/scripts/test_verify_agones_allocation_response.py @@ -9,15 +9,15 @@ from verify_agones_allocation_response import validate_allocation def response(**overrides): document = { + # Mirrors Agones' real GameServerAllocationStatus, which is flat. + # These fixtures previously encoded a nested "gameServer" object that + # Agones never returns, so the suite agreed with the validator while + # both disagreed with reality. "status": { "state": "Allocated", - "gameServer": { - "metadata": {"name": "cosmic-clash-game-abc"}, - "status": { - "address": "10.0.0.7", - "ports": [{"name": "game", "port": 31001}], - }, - }, + "gameServerName": "cosmic-clash-game-abc", + "address": "10.0.0.7", + "ports": [{"name": "game", "port": 31001}], } } document["status"].update(overrides) @@ -34,28 +34,28 @@ class AgonesAllocationResponseTest(unittest.TestCase): def test_rejects_missing_identity_or_address(self): missing_name = response() - missing_name["status"]["gameServer"]["metadata"] = {} + missing_name["status"]["gameServerName"] = "" with self.assertRaises(ValueError): validate_allocation(missing_name) missing_address = response() - missing_address["status"]["gameServer"]["status"]["address"] = "0.0.0.0" + missing_address["status"]["address"] = "0.0.0.0" with self.assertRaises(ValueError): validate_allocation(missing_address) def test_rejects_ambiguous_or_invalid_game_ports(self): duplicate = response() - duplicate["status"]["gameServer"]["status"]["ports"].append({"name": "game", "port": 31002}) + duplicate["status"]["ports"].append({"name": "game", "port": 31002}) with self.assertRaises(ValueError): validate_allocation(duplicate) wrong_name = response() - wrong_name["status"]["gameServer"]["status"]["ports"] = [{"name": "query", "port": 31001}] + wrong_name["status"]["ports"] = [{"name": "query", "port": 31001}] with self.assertRaises(ValueError): validate_allocation(wrong_name) invalid_port = response() - invalid_port["status"]["gameServer"]["status"]["ports"][0]["port"] = 70000 + invalid_port["status"]["ports"][0]["port"] = 70000 with self.assertRaises(ValueError): validate_allocation(invalid_port) diff --git a/scripts/verify_agones_allocation_response.py b/scripts/verify_agones_allocation_response.py index d93116d8..e2eac9cf 100644 --- a/scripts/verify_agones_allocation_response.py +++ b/scripts/verify_agones_allocation_response.py @@ -11,26 +11,26 @@ def validate_allocation(document: dict[str, Any]) -> tuple[str, int]: if not isinstance(status, dict) or status.get("state") != "Allocated": raise ValueError(f"allocation state is {status.get('state') if isinstance(status, dict) else None!r}, expected 'Allocated'") - game_server = status.get("gameServer") - if not isinstance(game_server, dict): - raise ValueError("allocation did not return a GameServer") - metadata = game_server.get("metadata") - name = metadata.get("name") if isinstance(metadata, dict) else None + # GameServerAllocationStatus is flat: state, gameServerName, address, + # ports, nodeName. It does not embed the allocated GameServer object. This + # validator originally read status.gameServer.metadata.name and + # status.gameServer.status.{address,ports}, and its tests asserted that + # same invented shape, so both agreed with each other and neither agreed + # with Agones -- undetected because the gate never once got far enough to + # allocate anything. + name = status.get("gameServerName") if not isinstance(name, str) or not name.strip(): - raise ValueError("allocation GameServer has no metadata.name") + raise ValueError("allocation did not return a gameServerName") - game_status = game_server.get("status") - if not isinstance(game_status, dict): - raise ValueError("allocation GameServer has no status") - address = game_status.get("address") + address = status.get("address") if not isinstance(address, str) or not address.strip() or any(char.isspace() for char in address): raise ValueError(f"allocation returned an invalid address: {address!r}") if address in {"0.0.0.0", "::"}: raise ValueError(f"allocation returned an unspecified address: {address!r}") - ports = game_status.get("ports") + ports = status.get("ports") if not isinstance(ports, list): - raise ValueError("allocation GameServer has no ports") + raise ValueError("allocation returned no ports") game_ports = [ entry.get("port") for entry in ports diff --git a/scripts/verify_kind_agones.sh b/scripts/verify_kind_agones.sh index d05015a6..661f2c70 100755 --- a/scripts/verify_kind_agones.sh +++ b/scripts/verify_kind_agones.sh @@ -206,4 +206,13 @@ spec: EOF kubectl create -f "$work_dir/allocation.yaml" -o json > "$work_dir/allocation.json" -python3 scripts/verify_agones_allocation_response.py "$work_dir/allocation.json" +# Print the response when validation fails. work_dir is deleted by the EXIT +# trap, so a mismatch between what Agones returns and what the validator +# expects is otherwise unknowable from CI -- which is exactly how a validator +# reading a field Agones never sends survived undetected. +if ! python3 scripts/verify_agones_allocation_response.py "$work_dir/allocation.json"; then + echo "--- allocation response as returned by Agones ---" >&2 + cat "$work_dir/allocation.json" >&2 || true + echo >&2 + exit 1 +fi