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

All operator Pods will now reach the Ready state. #201

Merged
merged 3 commits into from
Jun 26, 2018

Conversation

ewoutp
Copy link
Contributor

@ewoutp ewoutp commented Jun 26, 2018

This PR changes the behavior of the Ready state of the Pods that run the operators.

Before such a Pod was only marked Ready when it has won the leader election.
In this PR, such a Pod is marked Ready as soon as a new leader has been detected.

Other changes are:

  • Leader election lifecycle adds events to the Deployment that owns the operator.
  • Pod is exited with exitcode 1 when Pod that won the leader election at some point cannot renew its leader status.

fixes #198

@ghost ghost assigned ewoutp Jun 26, 2018
@ghost ghost added the 2 - Working label Jun 26, 2018
@ewoutp ewoutp requested a review from neunhoef June 26, 2018 09:08
Copy link
Member

@neunhoef neunhoef left a comment

Choose a reason for hiding this comment

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

LGTM. Please adjust one comment, see below.

// runLeaderElection performs a leader election on a lock with given name in
// the namespace that the operator is deployed in.
// When the leader election is won, the given callback is called.
// When the leader election is lost (even after it was won once), the process is killed.
Copy link
Member

Choose a reason for hiding this comment

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

This comment confused me. If a new pod tries to get elected and "loses" the election, then the process remains lingering around and will try again. It is only if leadership is lost that the process exits. Please adjust the comment. Code is OK, as far as I see.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@ewoutp ewoutp merged commit cf338bd into master Jun 26, 2018
@ghost ghost removed the 4 - Review label Jun 26, 2018
@ewoutp ewoutp deleted the bugfix/operator-ready-state branch June 26, 2018 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix operators Deployment spec wrt minimum availability
2 participants