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

Connect sidecar and check management based on false invariant #10843

Closed
shoenig opened this issue Jul 2, 2021 · 1 comment · Fixed by #10872
Closed

Connect sidecar and check management based on false invariant #10843

shoenig opened this issue Jul 2, 2021 · 1 comment · Fixed by #10872
Labels
stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/consul/connect Consul Connect integration type/bug

Comments

@shoenig
Copy link
Member

shoenig commented Jul 2, 2021

The Consul synchronization path attempts to ignore Connect sidecar service and check definitions. From Nomad's perspective these are implicitly defined via the connect.sidecar_service stanza, and therefor are not explicitly added or removed (i.e. via WorkloadServices methods) - creating a false sense of atomicity between the sidecar service + checks and its parent service + checks. In Consul the sidecar service and the parent service are independent.

The problem is the implementation and use of isNomadSidecar, which refers to Nomad's internal tracking of services to determine if a service or check belongs to a parent service. Nomad may have removed it's tracking of the parent service, but Consul still has a handle on its definition of the sidecar service or checks, which means Nomad could get into a state of never removing them even though Nomad [implicitly] created them.

@shoenig shoenig added type/bug theme/consul/connect Consul Connect integration stage/accepted Confirmed, and intend to work on. No timeline committment though. labels Jul 2, 2021
shoenig added a commit that referenced this issue Jul 8, 2021
…nect proxies

This PR uses regex-based matching for sidecar proxy services and checks when syncing
with Consul. Previously we would check if the parent of the sidecar was still being
tracked in Nomad. This is a false invariant - one which we must not depend when we
make #10845 work.

Fixes #10843
shoenig added a commit that referenced this issue Jul 8, 2021
…nect proxies

This PR uses regex-based matching for sidecar proxy services and checks when syncing
with Consul. Previously we would check if the parent of the sidecar was still being
tracked in Nomad. This is a false invariant - one which we must not depend when we
make #10845 work.

Fixes #10843
shoenig added a commit that referenced this issue Jul 8, 2021
…nect proxies

This PR uses regex-based matching for sidecar proxy services and checks when syncing
with Consul. Previously we would check if the parent of the sidecar was still being
tracked in Nomad. This is a false invariant - one which we must not depend when we
make #10845 work.

Fixes #10843
@github-actions
Copy link

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/consul/connect Consul Connect integration type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant