feat: add tcp endpoint for grpc channel #583
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
aneojgurhem
reviewed
Jan 9, 2025
aneojgurhem
reviewed
Jan 9, 2025
- Factorize kestrel options definition in the GrpcChannelProvider class - Apply review suggestions
jfonseca-aneo
force-pushed
the
jf/agent_tcp
branch
from
2014093 to
6afc4dc
Compare
jfonseca-aneo
force-pushed
the
jf/agent_tcp
branch
from
a46e9a1 to
80606a3
Compare
jfonseca-aneo
force-pushed
the
jf/agent_tcp
branch
from
80606a3 to
2b6406b
Compare
jfonseca-aneo
force-pushed
the
jf/agent_tcp
branch
from
765ed45 to
585b499
Compare
7 tasks
aneojgurhem
approved these changes
Jan 17, 2025
jfonseca-aneo
added a commit
to aneoconsulting/ArmoniK.Core
that referenced
this pull request
Jan 20, 2025
# Motivation Java does not support grpc communication between the worker and the agent over unix sockets, this PR builds on the changes introduced in aneoconsulting/ArmoniK.Api#583 to enable the option to use a tcp socket instead. # Description Mimic the changes in aneoconsulting/ArmoniK.Api#583 and enlarged the kestrel configuration options in the `AgentHandler` such that it also accepts to listen on a tcp socket. In addition this PR separates the volumes used for shared data and unix socket. Before the PR both of them where mounted on `\cache`, the updated mount point for shared data is `\comm`. # Testing Terraform deployment and `just` file have been modified to take into account a new variable `socket_type`, that can be set to either `unixdomainsocket` or `tcp` so the deployment configures which socket type should be employed accordingly. With these changes the same set of tests that ran for unix sockets can be executed but using a tcp socket instead . # Impact The desired life cycle behavior in a deployment with the whole infrastructure is that the worker starts before the agent and stops after it. This is currently enforced by checking the absence/existence of the unix socket created by the agent. With the changes of this PR, in the case of choosing a tcp socket for the connection, the unix socket is not created. Hence, the infra should be adapted accordingly to guarantee the desired worker/agent life cycle. One option is to transform the worker container in Kubernetes POD into a sidecar container, but this will require the direct use of Helm charts since the Kubernetes provider we use for terraform does not support this yet. # Additional Information Not applicable # Checklist - [x] My code adheres to the coding and style guidelines of the project. - [x] I have performed a self-review of my code. - [ ] I have commented my code, particularly in hard-to-understand areas. - [ ] I have made corresponding changes to the documentation. - [x] I have thoroughly tested my modifications and added tests when necessary. - [x] Tests pass locally and in the CI. - [ ] I have assessed the performance impact of my modifications.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Java does not support grpc communication between the worker and the agent over unix sockets, this PR adds the option to use a tcp socket instead.
Description
Enlarged the kestrel configuration options in the worker server so it also accepts to listen on a tcp socket.
Testing
Not applicable
Impact
Carry on similar logic on the server configuration on the Agent in Armonik.Core
Additional Information
Not applicable
Checklist