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

Config can be reloaded with conjurctl #2171

Closed
jtuttle opened this issue May 14, 2021 · 3 comments · Fixed by #2189
Closed

Config can be reloaded with conjurctl #2171

jtuttle opened this issue May 14, 2021 · 3 comments · Fixed by #2189

Comments

@jtuttle
Copy link
Contributor

jtuttle commented May 14, 2021

Add a conjurctl configuration apply command that validates configuration by instantiating a Conjur::ConjurConfig object. If validation fails, it should print the exception message to sderr and exit with code 1. If validation succeeds, it should restart the puma server using a phased restart using the following steps:

  • get the puma process id: ps -ef | grep puma | grep -v grep | grep -v cluster | awk '{print $2}' | tr -d '\n'
  • send signal w/ Process.kill('USR1', <pid>) (see docs)

Unit tests

  • Success case (not sure how to test this yet)
  • Invalid configuration displays message that includes which fields failed validation
  • Invalid configuration causes exit w/ code 1

Integration tests

  • Given a valid trusted proxies configuration, When conjur configuration apply is run, Then the correct IP is displayed in an audit event based on the new config
@jtuttle jtuttle changed the title Config can be reloaded w/ conjurctl Config can be reloaded with conjurctl May 14, 2021
@jtuttle jtuttle self-assigned this May 18, 2021
@jtuttle
Copy link
Contributor Author

jtuttle commented May 19, 2021

As it turns out, we can't currently use phased restart because we are preloading the application. From puma docs:

Preloading can’t be used with phased restart, since phased restart kills and restarts workers one-by-one, and preload_app! copies the code of master into the workers.

So we have to decide whether we want to:

  1. drop preloading so that we can do a phased restart
  2. just do a normal restart

This is what the docs say preloading does:

In clustered mode, Puma can "preload" your application. This loads all the application code prior to forking. Preloading reduces total memory usage of your application via an operating system feature called copy-on-write (Ruby 2.0+ only).

So in theory, if we remove preloading we will end up with additional memory usage in our puma process.

@jtuttle
Copy link
Contributor Author

jtuttle commented May 19, 2021

After looking at memory usage in both scenarios, it doesn't look like the preload is actually saving us any memory. Both cluster workers appear to consume the same amount of memory with or without preload.

with preload:

root@0968f9b1b509:/src/conjur-server# ps afu | awk 'NR==1 {o=$0; a=match($0,$11);}; NR>1 {o=$0;$5=int(10*$5/1024)/10"M";}{ printf "%-8s %6s %-5s %-5s %9s %9s %-8s %-4s %-6s %-5s %s\n", $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, substr(o, a);}'
...
root       1974 0.0   0.1       21.2M      3896 pts/0    Ss   07:54  0:00  bash
root       3060 0.2   1.1      103.3M     22936 pts/0    Sl+  19:41  0:00   \_ ruby /usr/local/bin/conjurctl server
root       3066 0.0   0.0        4.5M       784 pts/0    S+   19:41  0:00       \_ sh -c          rails server -p '3000' -b '0.0.0.0'
root       3075 6.7   6.0        253M    124116 pts/0    Sl+  19:41  0:09       |   \_ puma 3.12.6 (tcp://0.0.0.0:3000) [conjur-server]
root       3086 0.0   5.7      976.8M    116988 pts/0    Sl+  19:42  0:00       |       \_ puma: cluster worker 0: 3075 [conjur-server]
root       3093 0.0   5.7      976.8M    117284 pts/0    Sl+  19:42  0:00       |       \_ puma: cluster worker 1: 3075 [conjur-server]
root       3069 5.5   5.5      236.8M    112136 pts/0    Sl+  19:41  0:07       \_ ruby /var/lib/ruby/bin/rake authn_local:run
root       3072 6.3   5.8      244.3M    119472 pts/0    Sl+  19:41  0:08       \_ ruby /var/lib/ruby/bin/rake expiration:watch
...

without preload:

root@0968f9b1b509:/src/conjur-server# ps afu | awk 'NR==1 {o=$0; a=match($0,$11);}; NR>1 {o=$0;$5=int(10*$5/1024)/10"M";}{ printf "%-8s %6s %-5s %-5s %9s %9s %-8s %-4s %-6s %-5s %s\n", $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, substr(o, a);}'
...
root       1974 0.0   0.1       21.2M      3896 pts/0    Ss   07:54  0:00  bash
root       3132 0.3   1.1      103.3M     23040 pts/0    Sl+  19:46  0:00   \_ ruby /usr/local/bin/conjurctl server
root       3140 0.0   0.0        4.5M       780 pts/0    S+   19:46  0:00       \_ sh -c          rails server -p '3000' -b '0.0.0.0'
root       3149 8.9   6.0      252.8M    123972 pts/0    Sl+  19:46  0:09       |   \_ puma 3.12.6 (tcp://0.0.0.0:3000) [conjur-server]
root       3160 0.0   5.7      976.7M    117176 pts/0    Sl+  19:46  0:00       |       \_ puma: cluster worker 0: 3149 [conjur-server]
root       3165 0.0   5.7      976.7M    117172 pts/0    Sl+  19:46  0:00       |       \_ puma: cluster worker 1: 3149 [conjur-server]
root       3143 7.1   5.5      237.7M    113124 pts/0    Sl+  19:46  0:07       \_ ruby /var/lib/ruby/bin/rake authn_local:run
root       3146 8.0   5.8      244.2M    118924 pts/0    Sl+  19:46  0:08       \_ ruby /var/lib/ruby/bin/rake expiration:watch
...

@jtuttle
Copy link
Contributor Author

jtuttle commented May 21, 2021

It's worth noting that if we remove the preload, it does take a little bit longer for the main process to fork out the two workers. However, this is only a matter of a few seconds so it's fairly negligible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant