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.

Sign up for GitHub

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

Jump to bottom

Register mesh-gateways using the endpoints controller. #1464

Merged
merged 7 commits into from
Sep 21, 2022
Merged

Register mesh-gateways using the endpoints controller. #1464

merged 7 commits into from
Sep 21, 2022

Conversation

thisisnotashwin
Copy link
Contributor

@thisisnotashwin thisisnotashwin commented Aug 30, 2022 •
edited
Loading

Changes proposed in this PR:

  • Update the mesh-gateway deployment to use the endpoints controller to register the mesh-gateway.
    This requires updating the endpoints controller to parse services that are associated with gateway pods and then register those services with Consul as gateway services. Annotations on the gateway pod is used to determine the kind of the gateway as well as any information required to configure the gateway registration.
  • Since the endpoints controller manages registration, we no longer need a pre-delete hook to de-register the service. This is performed by the endpoints controller now. Additionally, we don't need a Consul sidecar when metrics are enabled as metrics will be managed by the consul dataplane.
  • The init-connect has migrated from acl-init to connect-init as we need to wait for the endpoints controller to register the mesh gateway services before we start the dataplane process. This has allowed significant cleanup of the init container.

How I've tested this PR:

  • Added a lot of unit tests to cover gateway registration, parsing the annotations to determine the wan address that the mesh-gateway should get registered with as well as updating the BATS tests.
  • Was able to verify that the mesh-gateway acceptance tests run successfully against GKE (they do not run against kind ATM)

How I expect reviewers to test this PR:

  • Code review
  • Call out places that could potentially need more detailed comments.
  • Call out test cases that might be missing.
  • Cleaning up for read-abilty as well.
    This PR is a little complicated so feel free to pull me into a meeting to ask any questions or understand the changes made here.

Checklist:

  • Tests added
  • CHANGELOG entry added

    HashiCorp engineers only, community PRs should not add a changelog entry.
    Entries should use present tense (e.g. Add support for...)

@thisisnotashwin thisisnotashwin force-pushed the ashwin/register-mesh-gateways branch 6 times, most recently from 6508cc4 to 8ad1b3d Compare September 1, 2022 18:24
@thisisnotashwin thisisnotashwin force-pushed the ashwin/register-mesh-gateways branch 4 times, most recently from b212ca0 to db72e56 Compare September 14, 2022 22:39
@thisisnotashwin thisisnotashwin changed the base branch from agentless-base to main September 16, 2022 14:35
@thisisnotashwin thisisnotashwin changed the base branch from main to agentless-base September 16, 2022 14:36
@thisisnotashwin thisisnotashwin changed the base branch from agentless-base to main September 16, 2022 14:38
@thisisnotashwin thisisnotashwin changed the base branch from main to agentless-base September 16, 2022 14:39
@thisisnotashwin thisisnotashwin force-pushed the ashwin/register-mesh-gateways branch 9 times, most recently from 8f626da to 921e379 Compare September 19, 2022 20:08
@thisisnotashwin thisisnotashwin changed the title [WIP] Register mesh-gateways using the endpoints controller. Register mesh-gateways using the endpoints controller. Sep 19, 2022
@thisisnotashwin thisisnotashwin marked this pull request as ready for review September 19, 2022 20:18
ishustava and others added 5 commits September 20, 2022 12:12
@ishustava @thisisnotashwin
Initial support for agentless (#1267)
cc083ff 
Add initial support for the basic service mesh running with agentless.

* Update connect-inject deployment to talk to consul servers (currently only in-cluster servers are supported; external servers support will come in a later PR)
* Update endpoints controller to register and deregister services and health checks in the catalog and stop using agent APIs
* Update connect-init command to take the -node-name flag so that it can search for services within a given node using catalog APIs
* Add k8s probes to the envoy container
* Provide -node-name flag to the consul connect envoy command so that we can generate correct configuration
* Selectively disable acceptance tests that don't yet work in this configuration
* Disable consul clients by default
@ishustava @thisisnotashwin
Enable connect-injector to talk to external servers. (#1323)
b77cd7a
@ishustava @thisisnotashwin
Enable CRD controller to talk to Consul servers (#1326)
0a0053b
@ishustava @thisisnotashwin
agentless: initial integration with consul-dataplane (#1470)
b27925c
@thisisnotashwin
Register mesh-gateways using the endpoints controller.
8784c23 
- Use consul-dataplane to configure the mesh-gateway proxy and remove
  envoy container.
- Remove instances of client and auto-encrypt from the deployment.
@thisisnotashwin thisisnotashwin force-pushed the ashwin/register-mesh-gateways branch 2 times, most recently from 8ebdab2 to edc466d Compare September 20, 2022 21:10
ishustava
ishustava approved these changes Sep 20, 2022
View reviewed changes
Copy link
Contributor

@ishustava ishustava left a comment

Choose a reason for hiding this comment

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

Looks good!!! Thank you so much for doing so much hard work to make it super readable!

There is a couple more fixes I think we need (left in-line) but I won't block the approval on that. To fix acceptance tests, I think we need to remove server restarts from tests because consul-dataplane image we're currently using doesn't support server upgrades. That fix is also probably not relevant anymore, so it's ok to remove it permanently.

charts/consul/values.yaml Outdated Show resolved Hide resolved
control-plane/connect-inject/endpoints_controller.go Show resolved Hide resolved
control-plane/connect-inject/endpoints_controller.go Outdated Show resolved Hide resolved
acceptance/tests/mesh-gateway/mesh_gateway_test.go Outdated Show resolved Hide resolved
acceptance/tests/mesh-gateway/mesh_gateway_test.go Outdated Show resolved Hide resolved
@thisisnotashwin thisisnotashwin force-pushed the ashwin/register-mesh-gateways branch 2 times, most recently from 1b98c7e to be6a786 Compare September 20, 2022 23:16
t-eckert
t-eckert approved these changes Sep 21, 2022
View reviewed changes
Copy link
Contributor

@t-eckert t-eckert left a comment

Choose a reason for hiding this comment

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

👍🏻

@thisisnotashwin thisisnotashwin merged commit 949257d into agentless-base Sep 21, 2022
@thisisnotashwin thisisnotashwin deleted the ashwin/register-mesh-gateways branch September 21, 2022 14:47
jmurret pushed a commit that referenced this pull request Sep 27, 2022
@thisisnotashwin @jmurret
Register mesh-gateways using the endpoints controller. (#1464)
51c3585 
* Register mesh-gateways using the endpoints controller.
- Use consul-dataplane to configure the mesh-gateway proxy and remove envoy container.
- Remove instances of client and auto-encrypt from the deployment.
* Replace ioutil.ReadFile with os.ReadFile
t-eckert pushed a commit that referenced this pull request Sep 28, 2022
@thisisnotashwin
Register mesh-gateways using the endpoints controller. (#1464)
2f126de 
* Register mesh-gateways using the endpoints controller.
- Use consul-dataplane to configure the mesh-gateway proxy and remove envoy container.
- Remove instances of client and auto-encrypt from the deployment.
* Replace ioutil.ReadFile with os.ReadFile
t-eckert pushed a commit that referenced this pull request Sep 28, 2022
@thisisnotashwin
Register mesh-gateways using the endpoints controller. (#1464)
f357a02 
* Register mesh-gateways using the endpoints controller.
- Use consul-dataplane to configure the mesh-gateway proxy and remove envoy container.
- Remove instances of client and auto-encrypt from the deployment.
* Replace ioutil.ReadFile with os.ReadFile
t-eckert pushed a commit that referenced this pull request Sep 28, 2022
@thisisnotashwin
Register mesh-gateways using the endpoints controller. (#1464)
0db0d64 
* Register mesh-gateways using the endpoints controller.
- Use consul-dataplane to configure the mesh-gateway proxy and remove envoy container.
- Remove instances of client and auto-encrypt from the deployment.
* Replace ioutil.ReadFile with os.ReadFile
ishustava pushed a commit that referenced this pull request Sep 28, 2022
@thisisnotashwin @ishustava
Register mesh-gateways using the endpoints controller. (#1464)
42fabd0 
* Register mesh-gateways using the endpoints controller.
- Use consul-dataplane to configure the mesh-gateway proxy and remove envoy container.
- Remove instances of client and auto-encrypt from the deployment.
* Replace ioutil.ReadFile with os.ReadFile
@natitomattis natitomattis mentioned this pull request Jun 20, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t-eckert t-eckert t-eckert approved these changes

@ishustava ishustava ishustava approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thisisnotashwin @ishustava @t-eckert