Enable endpoints controller #467
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
ishustava
force-pushed
the
enable-endpoints-controller
branch
3 times, most recently
from
939e904 to
dc6001b
Compare
ishustava
added
theme/tproxy
ishustava
commented
Mar 26, 2021
| @@ -1,131 +0,0 @@ | |||
| package connectinject | |||
There was a problem hiding this comment.
everything here was unused, except for escapeJSONPointer, which was only used in tests. I've moved it to the test that was using it instead.
ishustava
commented
Mar 26, 2021
Comment on lines
+868
to
+869
| DestinationServiceName: "counting", | ||
| DestinationServiceID: "counting-counting", |
There was a problem hiding this comment.
this is to fix errors in consul agent logs
ishustava
requested review from
a team,
kschoche and
thisisnotashwin
and removed request for
a team
ishustava
force-pushed
the
enable-endpoints-controller
branch
from
dc6001b to
3d8c1f3
Compare
kschoche
reviewed
Mar 26, 2021
kschoche
reviewed
Mar 26, 2021
kschoche
reviewed
Mar 26, 2021
kschoche
reviewed
Mar 26, 2021
Co-authored-by: Kyle Schochenmaier <kschoche@gmail.com>
thisisnotashwin
pushed a commit
that referenced
this pull request
Mar 26, 2021
This PR enables the endpoints controller and configures the connect-init container to now search for services. It removes the old functionality where the services were registered in the init container. The full list of changes is below container_init: Remove service registration from the init container template and the corresponding tests. Note that I have removed some of the tests prematurely, even though that logic has not yet been implemented in the endpoints controller. Namely, tests for metrics configuration and namespaces. However, I think that is OK as these tests will be added back when we do implement those. Other removed tests that didn't already have coverage in the endpoints controller have been moved to the endpoints controller. consul-sidecar: Consul sidecar is now only injected if we need to run the metrics merging server. The consulSidecar function now fails if you try to run it when metrics merging isn't enabled, and if it is, we configure it to not register services. envoy-sidecar: Remove the preStop hook from the envoy sidecar container. The hook will no longer work as there is no service.hcl file, plus services should now be deregistered only be the endpoints controller. connect-init: Remove the temporary -skip-service-registration-polling flag since we now always need to poll for services. endpoints-controller: Enable endpoints controller in the inject-connect command.
ishustava
added a commit
that referenced
this pull request
Apr 14, 2021
This PR enables the endpoints controller and configures the connect-init container to now search for services. It removes the old functionality where the services were registered in the init container. The full list of changes is below container_init: Remove service registration from the init container template and the corresponding tests. Note that I have removed some of the tests prematurely, even though that logic has not yet been implemented in the endpoints controller. Namely, tests for metrics configuration and namespaces. However, I think that is OK as these tests will be added back when we do implement those. Other removed tests that didn't already have coverage in the endpoints controller have been moved to the endpoints controller. consul-sidecar: Consul sidecar is now only injected if we need to run the metrics merging server. The consulSidecar function now fails if you try to run it when metrics merging isn't enabled, and if it is, we configure it to not register services. envoy-sidecar: Remove the preStop hook from the envoy sidecar container. The hook will no longer work as there is no service.hcl file, plus services should now be deregistered only be the endpoints controller. connect-init: Remove the temporary -skip-service-registration-polling flag since we now always need to poll for services. endpoints-controller: Enable endpoints controller in the inject-connect command.
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.
Changes proposed in this PR:
This PR enables the endpoints controller and configures the connect-init container to now search for services. It removes the old functionality where the services were registered in the init container. The full list of changes is below
container_init
Remove service registration from the init container template and the corresponding tests.
Note that I have removed some of the tests prematurely, even though that logic has not yet been implemented in the endpoints controller. Namely, tests for metrics configuration and namespaces. However, I think that is OK as these tests will be added back when we do implement those.
For other removed tests, if there wasn't already coverage in the endpoints controller, I've moved them there.
consul-sidecar
Consul sidecar is now only injected if we need to run the metrics merging server. The
consulSidecarfunction now fails if you try to run it when metrics merging isn't enabled, and if it is, we configure it to not register services.envoy-sidecar
Remove the preStop hook from the envoy sidecar container. The hook will no longer work as there is no service.hcl file, plus services should now be deregistered only be the endpoints controller.
connect-init
Remove the temporary
-skip-service-registration-pollingflag since we now always need to poll for services.endpoints-controller
Enable endpoints controller in the inject-connect command.
How I've tested this PR:
Ran connect, controller, mesh-, ingress-, and terminating gateway acceptance tests only in insecure configuration as endpoints controller does not yet support TLS and ACLs link.
Note that I've added an acceptance test for when consul clients are restarted here.
How I expect reviewers to test this PR:
Acceptance tests. However, if you would like or have time, you could also test it manually.
Checklist: