Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom Go-based dataplane for xDS control plane APIs #58

Merged
merged 112 commits into from
Oct 17, 2023

Conversation

praveingk
Copy link
Collaborator

Re-creating based on #44
@elevran : Addressed most of the comments from the previous PR, except the dataplane API refactoring (using controller) suggested by @orozery which would be done as another PR after removing the old dataplane code.

praveingk and others added 30 commits September 25, 2023 17:40
Co-authored-by: Or Ozeri <[email protected]>
@orozery
Copy link
Collaborator

orozery commented Oct 10, 2023

@praveingk Can you add an e2e test for this?
Perhaps similar to the tests/k8s.sh.
I plan to write a go test suite for setting up kind clusters with controlplane / dataplane (and ditch the current shell script test).
But anyhow, I think you may need to add a cl-adm flag for using the go-dataplane.

@praveingk
Copy link
Collaborator Author

praveingk commented Oct 10, 2023

@praveingk Can you add an e2e test for this? Perhaps similar to the tests/k8s.sh. I plan to write a go test suite for setting up kind clusters with controlplane / dataplane (and ditch the current shell script test). But anyhow, I think you may need to add a cl-adm flag for using the go-dataplane.

I have added dataplane type in cl-adm, and edited docker script. Please check c63110c and 7ac823f and feel free to edit it accordingly.
The e2e tests can be run as ./tests/k8s.sh clink default is envoy.

Copy link
Collaborator

@orozery orozery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also change the github actions workflow to run tests/k8s.sh with the go dataplane?

cmd/cl-adm/cmd/create/create_peer.go Outdated Show resolved Hide resolved
cmd/cl-adm/templates/docker.go Outdated Show resolved Hide resolved
Signed-off-by: Pravein-Govindan-Kannan <[email protected]>
Signed-off-by: Pravein-Govindan-Kannan <[email protected]>
Signed-off-by: Pravein-Govindan-Kannan <[email protected]>
@praveingk
Copy link
Collaborator Author

Done in 48f9ac1

Signed-off-by: Pravein-Govindan-Kannan <[email protected]>
tests/k8s.sh Outdated Show resolved Hide resolved
cmd/cl-adm/templates/config.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@orozery orozery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@praveingk Can you also add a switch case check to PeerOptions::Run() to verify that DataplaneType has a valid value?

Signed-off-by: Pravein-Govindan-Kannan <[email protected]>
@praveingk
Copy link
Collaborator Author

@praveingk Can you also add a switch case check to PeerOptions::Run() to verify that DataplaneType has a valid value?

Its done here : 0faaeb2

@elevran
Copy link
Collaborator

elevran commented Oct 17, 2023

@orozery reading through the conversation it seems all requests have been addressed. Is there anything else that needs to be done? If not - please mark approval so it can be merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants