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

services: un-mark group services as deregistered if restart hook runs #16905

Merged
merged 4 commits into from
Apr 24, 2023

Conversation

shoenig
Copy link
Member

@shoenig shoenig commented Apr 17, 2023

This PR may fix a bug where group services will never be deregistered if the group undergoes an alloc restart.

Fixes #16616

Adds an e2e test case covering the repro in ^

This PR may fix a bug where group services will never be deregistered if the
group undergoes a task restart.
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

LGTM

must.NoError(t, err)

// make sure our service is no longer registered
services, _, err = cc.Service("alloc-restart-http", "", nil)
Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure by the time the alloc is marked running the service will be registered. But service deregistration isn't synchronous with the Job.Deregister RPC, right? I think we need to must.Wait for this API call.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, will add that

@shoenig shoenig added backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line labels Apr 24, 2023
@shoenig shoenig merged commit 889c5aa into main Apr 24, 2023
@shoenig shoenig deleted the group-service-deregister branch April 24, 2023 19:24
shoenig added a commit that referenced this pull request Apr 24, 2023
…#16905)

* services: un-mark group services as deregistered if restart hook runs

This PR may fix a bug where group services will never be deregistered if the
group undergoes a task restart.

* e2e: add test case for restart and deregister group service

* cl: add cl

* e2e: add wait for service list call
shoenig added a commit that referenced this pull request Apr 24, 2023
…#16905)

* services: un-mark group services as deregistered if restart hook runs

This PR may fix a bug where group services will never be deregistered if the
group undergoes a task restart.

* e2e: add test case for restart and deregister group service

* cl: add cl

* e2e: add wait for service list call
shoenig added a commit that referenced this pull request Apr 24, 2023
…#16905) (#16975)

* services: un-mark group services as deregistered if restart hook runs

This PR may fix a bug where group services will never be deregistered if the
group undergoes a task restart.

* e2e: add test case for restart and deregister group service

* cl: add cl

* e2e: add wait for service list call

Co-authored-by: Seth Hoenig <[email protected]>
shoenig added a commit that referenced this pull request Apr 24, 2023
…#16905) (#16976)

* services: un-mark group services as deregistered if restart hook runs

This PR may fix a bug where group services will never be deregistered if the
group undergoes a task restart.

* e2e: add test case for restart and deregister group service

* cl: add cl

* e2e: add wait for service list call

Co-authored-by: Seth Hoenig <[email protected]>
shoenig added a commit that referenced this pull request May 16, 2023
…#16905)

* services: un-mark group services as deregistered if restart hook runs

This PR may fix a bug where group services will never be deregistered if the
group undergoes a task restart.

* e2e: add test case for restart and deregister group service

* cl: add cl

* e2e: add wait for service list call
shoenig added a commit that referenced this pull request May 16, 2023
…#16905) (#17212)

* services: un-mark group services as deregistered if restart hook runs

This PR may fix a bug where group services will never be deregistered if the
group undergoes a task restart.

* e2e: add test case for restart and deregister group service

* cl: add cl

* e2e: add wait for service list call
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Services not unregistered
2 participants