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

Add support for configuring pods as arbiterOnly based on labels #42

Closed
wants to merge 1 commit into from

Conversation

luqasn
Copy link

@luqasn luqasn commented May 3, 2017

Still needs documentation for the new env variable MONGO_SIDECAR_POD_LABELS_ARBITER. This is something we needed in our cluster, maybe it is also of interest to others. What do you think about the approach of additional labels used for selecting a subset of the pods?

Copy link
Author

@luqasn luqasn left a comment

Choose a reason for hiding this comment

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

Needs fixing

@@ -213,7 +213,9 @@ var invalidReplicaSet = function(db, pods, done) {
var podElection = function(pods) {
//Because all the pods are going to be running this code independently, we need a way to consistently find the same
//node to kick things off, the easiest way to do that is convert their ips into longs and find the highest
pods.sort(function(a,b) {
pods
.filter(function(pod) { return !k8s.podContainsLabels(pod, config.mongoPodLabelCollectionArbiter); })
Copy link
Author

Choose a reason for hiding this comment

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

I think this function is currently not working as intended. The caller expects the pods array it passes into the function to be sorted in-place, which the filter prohibits by creating a copy. I need to fix that.

Copy link
Owner

Choose a reason for hiding this comment

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

Yep, you got.

pods = pods.filter(function(pod) { return !k8s.podContainsLabels(pod, config.mongoPodLabelCollectionArbiter); });
//Now you can sort and carry on as you were before.

@cravall
Copy link
Owner

cravall commented May 8, 2017

Nice, looks good.

What do you think about the approach of additional labels used for selecting a subset of the pods?

Initially I thought that it would be confusing to consumers in that they would have to understand arbiters would have to satisfy MONGO_SIDECAR_POD_LABELS and MONGO_SIDECAR_POD_LABELS_ARBITER ... not that hard to understand, but people don't like reading documentation and are use to specific behaviours when seeing different config patterns. If someone saw them and thought that they were completely independent, could you blame them? Is that how you'd expect it to work given past experiences with similar libraries?

But in saying that... if you wanted to make them completely separate, you'd need to change a lot more code. And, really, if you are going to start setting arbiters, you should be reading the documentation to see exactly how it works.

So in short, I'm cool with either approach as long as they are documented to make it clear 😄 ... maybe a row in the configuration table and a separate little blurb on setting up arbiters.

@luqasn
Copy link
Author

luqasn commented Jul 26, 2018

Closing this in favour of #73, thanks @ericbaum, kind of lazied out of this after I stopped using the lib :)

@luqasn luqasn closed this Jul 26, 2018
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.

2 participants