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

bf: ZENKO-408 no zookeeper provisioning for conductor #298

Conversation

jonathan-gramain
Copy link
Contributor

@jonathan-gramain jonathan-gramain commented May 24, 2018

Disable zookeeper provisioning for lifecycle conductors, as kubernetes is able to spawn the required number of instances (i.e. 1) and respawn on another machine whenever one goes down. The zookeeper provisioning was used for about the same purpose.

tmacro
tmacro previously approved these changes May 24, 2018
philipyoo
philipyoo previously approved these changes May 24, 2018
@ironman-machine ironman-machine dismissed stale reviews from tmacro and philipyoo May 24, 2018 22:13

Do it again human slave!:point_right: :runner: (Oh and the pull request has been updated, by the way.)

@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

philipyoo
philipyoo previously approved these changes May 24, 2018
Copy link
Contributor

@philipyoo philipyoo left a comment

Choose a reason for hiding this comment

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

nice!

tmacro
tmacro previously approved these changes May 24, 2018
Copy link
Contributor

@tmacro tmacro left a comment

Choose a reason for hiding this comment

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

That squeaky clean code!

@jonathan-gramain jonathan-gramain force-pushed the bf/ZENKO-408-noZookeeperProvisioningForLifecycleConductor branch from 9018cfe to bda281f Compare May 29, 2018 20:51
@ironman-machine ironman-machine dismissed stale reviews from philipyoo and tmacro May 29, 2018 20:51

Do it again human slave!:point_right: :runner: (Oh and the pull request has been updated, by the way.)

@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

philipyoo
philipyoo previously approved these changes May 29, 2018
Copy link
Contributor

@philipyoo philipyoo left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@@ -63,7 +61,6 @@ class LifecycleConductor {
this._zkClient = null;
this._started = false;
this._isActive = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

can also remove this variable i think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, just removed

Disable zookeeper provisioning for lifecycle conductors, unless the
option useZookeeperDispatcher is set to true. This will only be set
for 7.4.x.
@jonathan-gramain jonathan-gramain force-pushed the bf/ZENKO-408-noZookeeperProvisioningForLifecycleConductor branch from bda281f to 060e18f Compare May 29, 2018 21:09
@ironman-machine ironman-machine dismissed philipyoo’s stale review May 29, 2018 21:09

Do it again human slave!:point_right: :runner: (Oh and the pull request has been updated, by the way.)

@ironman-machine
Copy link
Contributor

PR has been updated. Reviewers, please be cautious.

@jonathan-gramain jonathan-gramain merged commit 319de78 into z/1.0 May 29, 2018
@jonathan-gramain jonathan-gramain deleted the bf/ZENKO-408-noZookeeperProvisioningForLifecycleConductor branch May 29, 2018 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants