Merging to release-5.6.0: TT-13130 only mark the wg as done when the connection is stablished (#6574)

[buger]

User description

TT-13130 only mark the wg as done when the connection is stablished (#6574)

User description

Description

When the gw is ran as edge we want this to wait until the connection is
established with MDCB before attempting to pull policies and apis, we
should only mark the connection waitgroup as done when the connection is
successful not on every attempt

Related Issue

TT-13130

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing
  functionality to change)
• Refactoring or add test (improvements in base code or adds test
  coverage to functionality)

Checklist

• I ensured that the documentation is up to date
• I explained why this PR updates go.mod in detail with reasoning
  why it's required
• I would like a code coverage CI quality gate exception and have
  explained why

---

PR Type

Bug fix

---

Description

• Fixed the logic in rpc_client.go to ensure the waitgroup is marked
  as done only when a connection is successfully established.
• Removed the Done() call from the defer statement and added it after
  the connection is confirmed.

---

Changes walkthrough 📝

	Relevant files
Bug fix	rpc_client.go Fix waitgroup completion logic in connection establishment rpc/rpc_client.go Removed the Done() call from the defer statement in the Dial function. Added Done() call after the connection is successfully established. Ensures the waitgroup is marked as done only upon successful connection. +2/-1

---

💡 PR-Agent usage: Comment /help "your question" on any pull
request to receive relevant information

Co-authored-by: sredny buitrago [email protected]

---

PR Type

Bug fix

---

Description

• Fixed an issue in rpc_client.go where the waitgroup was marked as done prematurely. Now, it is only marked as done after a successful connection establishment.
• This change ensures that the gateway waits for a successful connection with MDCB before proceeding, preventing potential issues with policy and API pulling.

---

Changes walkthrough 📝

	Relevant files
Bug fix	rpc_client.go Ensure waitgroup is marked done only after successful connection rpc/rpc_client.go Removed premature call to connectionDialingWG.Done(). Added connectionDialingWG.Done() after successful connection establishment. +2/-1

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[github-actions]

API Changes

no api changes detected

[github-actions]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Possible Bug Ensure that the connectionDialingWG.Done() is called in all possible execution paths to prevent deadlocks or goroutine leaks.

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add error handling for conn.Write() to prevent silent failures Consider handling potential errors from conn.Write() calls to ensure robust error handling and avoid silent failures. rpc/rpc_client.go [298-300] -conn.Write([]byte("proto2")) -conn.Write([]byte{byte(len(connID))}) -conn.Write([]byte(connID)) +if _, err := conn.Write([]byte("proto2")); err != nil { + return nil, err +} +if _, err := conn.Write([]byte{byte(len(connID))}); err != nil { + return nil, err +} +if _, err := conn.Write([]byte(connID)); err != nil { + return nil, err +} Suggestion importance[1-10]: 10 Why: Adding error handling for conn.Write() is crucial to prevent silent failures and ensure robust error management, addressing a potential bug.	10
Best practice	Use defer to ensure Done() is called even if an error occurs Ensure that connectionDialingWG.Done() is called in a defer statement to guarantee it executes even if an error occurs earlier in the function. rpc/rpc_client.go [302] -// only mark as done is connection is established -connectionDialingWG.Done() +defer connectionDialingWG.Done() Suggestion importance[1-10]: 9 Why: Using defer for connectionDialingWG.Done() ensures that it is called regardless of where the function exits, which is a best practice for resource management and error handling.	9
Performance	Combine conn.Write() calls into a single call for better performance To improve performance, consider combining the conn.Write() calls into a single call by concatenating the byte slices beforehand. rpc/rpc_client.go [298-300] -conn.Write([]byte("proto2")) -conn.Write([]byte{byte(len(connID))}) -conn.Write([]byte(connID)) +data := append([]byte("proto2"), byte(len(connID))) +data = append(data, connID...) +if _, err := conn.Write(data); err != nil { + return nil, err +} Suggestion importance[1-10]: 7 Why: Combining conn.Write() calls can improve performance by reducing the number of system calls, although the performance gain may be minor in this context.	7
Maintainability	Correct a typo in the comment Correct the typo in the comment to improve code readability and professionalism. rpc/rpc_client.go [301] -// only mark as done is connection is established +// only mark as done if connection is established Suggestion importance[1-10]: 5 Why: Correcting the typo in the comment improves code readability and professionalism, but it is a minor change with no impact on functionality.	5

[sonarcloud]

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud