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

Feature/Update zync controller #9

Merged
merged 6 commits into from
Jun 29, 2020
Merged

Conversation

slopezz
Copy link
Member

@slopezz slopezz commented Jun 29, 2020

Solves part of https://github.com/3scale/platform/issues/235

  • Remove zync init container (there was an init container which just check basic DB connectivity, just present on zync deployment, while both deployments connect to same postgres DB). We are removing that kind of init containers from 3scale saas, as DB connectivity is always needed, not just on pod startup. Actually, on a possible rolling update process, if there is no DB connection, new pods won't pass liveness/readiness healthchecks, so new pods won't be promoted for example.
  • Remove var dbWaitSleepSeconds (was only used on removed init container)
  • Match default number of replicas to current documentation
  • Document only accepted value on railsEnv var (test/development/production), leave default value to development), otherwise when pod starts it complains about:
onfig.eager_load is set to nil. 
Please update your config/environments/*.rb files accordingly: 
* development - set it to false 
* test - set it to false (unless you use a tool that preloads your test environment) 
* production - set it to true
  • Remove possibility to change railsLogsToStdout: 3scale product already enables it by default and cannot be tuned, it seems it just changes the logger format, and there is no reason to not have it always enabled. In addition, it is only used on production environment (RAILS_ENV=production), which is the only environment value possible for serious zync deployments (staging/production...), because it enables all zync features. Actually, although it is used as a boolean, zync app just checks if var is present, independently from the value.

@slopezz slopezz requested a review from raelga June 29, 2020 13:50
@slopezz slopezz self-assigned this Jun 29, 2020
@slopezz slopezz requested a review from roivaz June 29, 2020 13:50
@miguelsorianod
Copy link

miguelsorianod commented Jun 29, 2020

Remove zync init container (there was an init container which just check basic DB connectivity, just present on zync deployment, while both deployments connect to same postgres DB). We are removing that kind of init containers from 3scale saas, as DB connectivity is always needed, not just on pod startup. Actually, on a possible rolling update process, if there is no DB connection, new pods won't pass liveness/readiness healthchecks, so new pods won't be promoted for example.

Original motivation of the init container was to prevent the pod from crashing at startup time. When deploying 3scale, customers temporarily saw pods crashing (due to DB not available because it's booting up, and zync process crashing when it's not available) and being restarted until DB connectivity was available, which was perceived as "unstability" of the deployment from their given feedback (pods in 'red', crashing and restarting until their dependencies were available, etc.).

Copy link
Contributor

@raelga raelga left a comment

Choose a reason for hiding this comment

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

Nice improvement, lgtm!

@slopezz slopezz merged commit 3f85345 into master Jun 29, 2020
@slopezz slopezz deleted the feature/update-zync-controller branch June 29, 2020 14:56
@raelga raelga added kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple sprints to complete. size/M Requires about a day to complete the PR or the issue. labels Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple sprints to complete. size/M Requires about a day to complete the PR or the issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants