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

Terminate process correctly from reflector thread #525

Merged
merged 2 commits into from
Oct 4, 2021

Conversation

yuvipanda
Copy link
Collaborator

Our reflectors run on their own thread, and sys.exit() from
a thread does not actually exit the process - just the thread.
When the reflector has to shut down because it can not talk
to the k8s master anymore for a while, we had just been stopping
the reflector thread and doing nothing else.

This will instead actually cause the hub to stop, and in z2jh
restart - which seems the right thing to do.

Ref 2i2c-org/infrastructure#680,
where we encountered this during a period of time when the
k8s master was unreachable for about 1 minute.

Our reflectors run on their own thread, and sys.exit() from
a thread does not actually exit the process - just the thread.
When the reflector has to shut down because it can not talk
to the k8s master anymore for a while, we had just been stopping
the reflector thread and doing nothing else.

This will instead actually cause the hub to stop, and in z2jh
restart - which seems the right thing to do.

Ref 2i2c-org/infrastructure#680,
where we encountered this during a period of time when the
k8s master was unreachable for about 1 minute.
@consideRatio
Copy link
Member

Thanks for the bump @yuvipanda! I hesitated with this during an initial glance a while back and forgot about it.

My initial hesitations were

  1. Is this really a desired behavior in the logic?
    Answer: yes - the logic has logging statements indicating that this is really meant to shut down the hub itself, but failed to do so.
  2. Will this happen when there is a brief failure of the k8s api-server?
    Answer: no - this happens after several retries and failures.

Conclusion: this is a straight forward bugfix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants