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

Add health-sync command to ECS #210

Merged
merged 8 commits into from
Dec 26, 2023
Merged

Add health-sync command to ECS #210

merged 8 commits into from
Dec 26, 2023

Conversation

Ganeshrockz
Copy link
Contributor

Changes proposed in this PR:

  • Adds a new command health-sync to the command factory.
  • Adds back most of the code removed in Offload tons of responsibilities from control-plane #207 but in the healthsync package.
  • Health sync container now does the following
    • Performs Consul login with the connection manager library
    • Sets up the Consul client
    • Accumulates all the health checks from the catalog for the service/proxy registered by mesh-init
    • Enters into a long running reconciliation loop where it (Most of the code is a copy paste of the reconciliation logic that was previously present in Control plane removed in Offload tons of responsibilities from control-plane #207)
      • Periodically syncs back ECS container health checks into Consul.
      • Marks all the checks as critical on receiving SIGTERM
      • Implements a server watch where it listens for changes to the Consul servers and reconfigures the Consul client when such a change happens
      • Implements graceful shutdown where it waits for Consul dataplane to terminate. Once dataplane terminates, it deregisters the service and proxy and finally performs a Consul logout.

How I've tested this PR:

How I expect reviewers to test this PR:

Checklist:

  • Tests added
  • CHANGELOG entry added

@Ganeshrockz Ganeshrockz requested review from a team and roncodingenthusiast and removed request for a team December 22, 2023 12:04
Ganeshrockz
Ganeshrockz commented Dec 22, 2023
View reviewed changes
subcommand/health-sync/checks.go

// fetchHealthChecks fetches the Consul health checks for both the service
// and proxy registrations
func (c *Command) fetchHealthChecks(consulClient *api.Client, taskMeta awsutil.ECSTaskMeta) (map[string]*api.HealthCheck, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a newly added function. Others in this PR are a copy paste of code removed in #207

subcommand/health-sync/command.go
<-c.proceedChan
}

for {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The entire reconciliation loop was just a copy paste from #207

subcommand/health-sync/dataplane_monitor.go
@@ -0,0 +1,81 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: MPL-2.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is also a copy paste of the existing implementation removed in #209

@Ganeshrockz Ganeshrockz temporarily deployed to dockerhub/hashicorpdev December 22, 2023 12:17 — with GitHub Actions Inactive
@Ganeshrockz Ganeshrockz temporarily deployed to dockerhub/hashicorpdev December 22, 2023 12:58 — with GitHub Actions Inactive
@Ganeshrockz Ganeshrockz temporarily deployed to dockerhub/hashicorpdev December 22, 2023 15:59 — with GitHub Actions Inactive
@Ganeshrockz Ganeshrockz temporarily deployed to dockerhub/hashicorpdev December 22, 2023 16:42 — with GitHub Actions Inactive
@Ganeshrockz Ganeshrockz merged commit 7ecc890 into ganeshrockz/rearch-0.8.0 Dec 26, 2023
20 checks passed
@Ganeshrockz Ganeshrockz deleted the net-6660/health-sync-logic branch December 26, 2023 09:47
@Ganeshrockz Ganeshrockz mentioned this pull request Dec 26, 2023
Merged
2 tasks
Ganeshrockz added a commit that referenced this pull request Dec 26, 2023
@Ganeshrockz
Add health-sync container to ECS (#211)
2ebf3fc 
* Offload tons of responsibilities from control-plane

* Pass dynamic login config to Dataplane instead of static credentials (#208)

* Pass dynamic login credentials to dataplane

* Fix tests

* Fix lint

* Rename Control plane to mesh-init (#209)

* Add health-sync command to ECS (#210)

* Add health-sync command to ECS

* Fix enterprise tests by creating namespaces and partitions

* Fix partition bug

* Try fix tests

* Timeout

* Remove timeout

* Still fixing tests :(

* Fix nil panic

* Add changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@absolutelightning absolutelightning absolutelightning approved these changes

@roncodingenthusiast roncodingenthusiast Awaiting requested review from roncodingenthusiast

Assignees
No one assigned
Labels
consul-india
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Ganeshrockz @absolutelightning