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
accessability to zk is requirement for a marathon start #1690
Conversation
This repo has @mesosphere-mergebot integration. You can interact with the following commands.
|
This ensures the zk resolution by spartan is possible in order to start marathon. We had an issue https://jira.mesosphere.com/browse/MARATHON-7390 where spartan failed to resolve zk. Should not try to restart unless zk can resolve. It is believed that this will help in future debugging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR looks good to me. The only concern is that if we need to handle the rare case that Spartan is up during start-pre operations but then down when Marathon itself starts up and fails to reach all 5 ZK nodes and gets stuck. I'm good with any decision related to the above case, just want to understand if this has been discussed and what's the underlying reason if we decide not to address this case.
@kensipe - A corresponding EE PR is required for this change to get a ship-it label. |
@mesosphere-mergebot bump-ee |
Your pull request's branch is not based on the most recent version of |
rebased |
@mesosphere-mergebot bump-ee |
Enterprise Bump PR: mesosphere/dcos-enterprise/pull/1134 |
@adamtheturtle, do you know why this was not merged yet? Let me know if I should ping someone else. |
@jeschkies There are a few reasons: This has a This can be done with:
Then, it appears that there are new required tests created since this PR went up. I recommend rebasing this against the latest master. |
@mesosphere-mergebot bump-ee |
Your pull request's branch is not based on the most recent version of |
thanks for the rebase @adamtheturtle |
I did not rebase this branch - I did not touch it.
What I meant was that this branch cannot get a Ship It because there are new "Required" CI builders. This means that there are no CI results for these builders. Rebasing should make this run against those builders, I guess. |
That is to say, I have not done a review of this and so I cannot say whether this needs a test. |
ac01f88
to
fed5a80
Compare
@mesosphere-mergebot bump-ee |
rebased and bumped |
Enterprise Bump mesosphere/dcos-enterprise/pull/1134 updated. |
@mesosphere-mergebot label ready for review |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
16 similar comments
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
👋 This PR has been inactive for 3 days. Please review the status checks to see what needs to be done to move it forward or change the label to Work in Progress. Thank you! |
Let's move forward with this, the change looks reasonable and our cluster tests succeed. |
@mesosphere-mergebot label ship-it |
High Level Description
Adding Required Precondition for marathon startup
Related Issues
Checklist for all PR's