Skip to content
This repository was archived by the owner on Mar 6, 2020. It is now read-only.

Changed default cluster size back to 3#37

Open
GoatCodeNL wants to merge 2 commits intokurrent-io:masterfrom
saa-nl:master
Open

Changed default cluster size back to 3#37
GoatCodeNL wants to merge 2 commits intokurrent-io:masterfrom
saa-nl:master

Conversation

@GoatCodeNL
Copy link
Contributor

@GoatCodeNL GoatCodeNL commented Jun 7, 2019

Signed-off-by: Niels van Esch nvanesch@saa.nl

PR Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Chart Version bumped
  • Chart packaged and index updated (you can use ./release)
  • CHANGELOG.md updated

What this PR does / why we need it:
This PR restores the default clustersize to 3. to make it consistent with documentation, with other default values and expected behaviour.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #36

Special notes for your reviewer:

Niels van Esch added 2 commits June 7, 2019 10:29
Signed-off-by: Niels van Esch <nvanesch@saa.nl>
Signed-off-by: Niels van Esch <nvanesch@saa.nl>
@riccardone
Copy link
Contributor

I don't have permission to merge. This PR need to be merged

@ameier38
Copy link
Contributor

We had changed the default cluster size back to 1 so the port forwarding worked out of the box. My mistake to not update the docs. Could we just update the docs to have the default value as 1?

@riccardone
Copy link
Contributor

can you please do that in a new PR and then we can eventually ask to close this one

@GoatCodeNL
Copy link
Contributor Author

Ofcourse I can change it. However I disagree that this would be the better default.

  • 3 is more like production
  • 1 conflicts with current pdb default. This defaults to minAvailable of 2. It will never reach this. This means rolling updates get stuck and terminating instances do an unclean shutdown.
  • Portforwards are a debug situation.

Honstly I would rather make a pr for you to add a note to the portforward docs.

But I leave that up to you. Please let me know your preferred solution and I’ll make the PR.

@ameier38
Copy link
Contributor

I agree that 3 is more like production, but also think that @hnicke raised a good point here that the chart should work out of the box with port-forwarding so it is easy to get started.

What if we added a values-production.yml similar to the postgres chart and added clusterSize: 3 there?

@GoatCodeNL
Copy link
Contributor Author

I'll see if I can create the PR somewhere this week. I'll create a PR with:

  • updated documents
  • changed PDB (defaulting to maxunavailable 1 in stead of minavailable 2
  • values-production.yaml
  • production advisory doc

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Custersize documentation default does not match actual value

3 participants