This has never actually been used -- and does not make sense with the
check-all-queues-at-once model switched to in 88a123d5e0. The
Tornado processes are the only ones we expect to be non-1, and since
they were added in 3f03dcdf5e the right number has been read from
config, not passed as an argument.
`postgresql-14.4` is a notable upgrade in the PostgreSQL series, as it
fixes potential database corruption from `CREATE INDEX CONCURRENTLY`
statements which are run while rows are modified[1]. However, it also
requires an upgrade from `libllvm9` to `libllvm10`, which means it is
not installed by a mere `apt-get upgrade`.
Add the `--with-new-pkgs` flag to all of the potentially relevant
`apt-get upgrade` calls, so that this (and similar) packages are
upgraded successfully.
[1]: https://www.postgresql.org/docs/release/14.4/
30457ecd02 removed the `--mirror` from
initial clones, but did not add back `--bare`, which `--mirror`
implies. This leads to `/srv/zulip.git` having a working tree in it,
with a `/srv/zulip.git/.git` directory.
This is mostly harmless, and since the bug was recent, not worth
introducing additional complexity into the upgrade process to handle.
Calling `git clone --bare`, however, would clone the refs into
`refs/heads/`, not the `refs/remotes/origin/` we want. Instead, use
`git init --bare`, followed by `git remote add origin`. The remote
will be fetched by the usual `git fetch --all --prune` which is below.
While the `remote.origin.mirror` boolean being set is a very good
proxy for having been cloned with `--mirror`, is technically only used
when pushing into the remote[1]. What we care about is if fetches
from this remote will overwrite `refs/heads/`, or all of `refs/` --
the latter of which is most likely, from having run `git clone
--bare`.
Detect either of these fetch refspecs, and not the mirror flag. We
let the upgrade process error out if `remote.origin.fetch` is unset,
as that represents an unexpected state. We ignore failures to unset
the `remote.origin.mirror` flag, in case it is not set already.
[1]: https://git-scm.com/docs/git-config#Documentation/git-config.txt-remoteltnamegtmirror
The local `/srv/zulip.git` directory has been cloned with `--mirror`
since it was first created as a local cache in dc4b89fb08. This
made some sense at the time, since it was purely a cache of the
remote, and not a home to local branches of its own.
That changed in 3f83b843c2, when we began using `git worktree`,
which caused the `deployment-...` branches to begin being stored in
`/src/zulip.git`. This caused intermixing of local and remote
branches.
When 02582c6956 landed, the addition of `--prune` caused all but the
most recent deployment branch to be deleted upon every fetch --
leaving previous deployments with non-existent branches checked out:
```
zulip@example-prod-host:~/deployments/last$ git status
On branch deployment-2022-04-15-23-07-55
No commits yet
Changes to be committed:
(use "git rm --cached <file>..." to unstage)
new file: .browserslistrc
new file: .codecov.yml
new file: .codespellignore
new file: .editorconfig
[...snip list of every file in repo...]
```
Switch `/srv/zulip.git` to no longer be a `--mirror` cache of the
origin. We reconfigure the remote to drop `remote.origin.mirror`, and
delete all refs under `refs/pulls/` and `refs/heads/`, while
preserving any checked-out branches. `refs/pulls/`, if the remote is
the canonical upstream, contains _tens of thousands_ of refs, so
pruning those refs trims off 20% of the repository size.
Those savings require a `git gc --prune=now`, otherwise the dangling
objects are ejected from the packfiles, which would balloon the
repository up to more than three times its previous size. Repacking
the repository is reasonable, in general, after removing such a large
number of refs -- and the `--prune=now` is safe and will not lose
data, as the `--mirror` was good at ensuring that the repository could
not be used for any local state.
The refname in the upgrade process was previously resolved from the
union of local and remote refs, since they were in the same namespace.
We instead now only resolve arguments as tags, then origin branches;
this means that stale local branches will be skipped. Users who want
to deploy from local branches can use `--remote-url=.`.
Because the `scripts/lib/upgrade-zulip-from-git` file is "stage 1" and
run from the old version's code, this will take two invocations of
`upgrade-zulip-from-git` to take effect.
Fixes#21901.
This adds a --skip-restart which makes `deployments/next` in a state
where it can be restarted into, but holds off on conducting that
restart.
This requires many of the same guarantees as `--skip-tornado`, in
terms of there being no Puppet or database schema changes between the
versions. Enforce those with `--skip-restart`, and also broaden both
flags to prevent other, less common changes which nonetheless
potentially might affect the other deploy.
Because Tornado and Django use memcached as a shared cache for
checking session information, they must agree on the prefix used to
store those values.
Subsequent commits will work to ensure that it is always _safe_ to
share that cache.
These are expensive, and moving them to one explicit call early has
considerable time savings in the critical period:
```
$ hyperfine './manage.py fill_memcached_caches' './manage.py fill_memcached_caches --skip-checks'
Benchmark #1: ./manage.py fill_memcached_caches
Time (mean ± σ): 5.264 s ± 0.146 s [User: 4.885 s, System: 0.344 s]
Range (min … max): 5.119 s … 5.569 s 10 runs
Benchmark #2: ./manage.py fill_memcached_caches --skip-checks
Time (mean ± σ): 3.090 s ± 0.089 s [User: 2.853 s, System: 0.214 s]
Range (min … max): 2.950 s … 3.204 s 10 runs
Summary
'./manage.py fill_memcached_caches --skip-checks' ran
1.70 ± 0.07 times faster than './manage.py fill_memcached_caches'
```
Treating the restart as a start is important in reducing the critical
period during upgrades -- we call restart even when we suspect the
services are stopped, because puppet has a small possibility of
placing them in indeterminate state. However, restart orders the
workers first, then tornado/django, which prolongs the outage.
Recognize when no services are currently started, and switch to acting
like a start, not a restart, which places tornado/django first.
This hides ugly output if the services were already stopped:
```
2022-03-25 23:26:04,165 upgrade-zulip-stage-2: Stopping Zulip...
process-fts-updates: ERROR (not running)
zulip-django: ERROR (not running)
zulip_deliver_scheduled_emails: ERROR (not running)
zulip_deliver_scheduled_messages: ERROR (not running)
Zulip stopped successfully!
```
Being able to skip having to shell out to `supervisorctl`, if all
services are already stopped is also a significant performance
improvement.
These have more accurate timestamps, and have user information --
but are harder to parse, and will not show requests when Django or
Tornado is stopped.
This is a script to search nginx log files by server hostname or
client IP address, and output matching lines, all while skipping
common and less-interesting request lines.
As a consequence:
• Bump minimum supported Python version to 3.8.
• Move Vagrant environment to Ubuntu 20.04, which has Python 3.8.
• Move CI frontend tests to Ubuntu 20.04.
• Move production build test to Ubuntu 20.04.
• Move 3.4 upgrade test to Ubuntu 20.04.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We previously used restart-server if puppet was run, as a nod to the
fact that `supervisor reread && supervisor update` will _start_
service groups that were modified, even if they were previously
stopped; this is because they are marked as `autostart=true`, which is
honored on service change.
However, upgrades want to run while there are no services running. If
puppet is run, explicitly set the server as potentially being "up", so
that a `shutdown_server()` before migrations, if they exist, will stop
services.
7c4293a7d3 switched to checking if the
service was already running, and use `supervisorctl start` if it was
not.
Unfortunately, `list_supervisor_processes("zulip-tornado:*")` did not
include `zulip-tornado`, and as such a non-sharded process was always
considered to _not_ be running, and was thus started, not restarted.
Starting an already-started service is a no-op, and thus non-sharded
tornado processes were never restarted.
The observed behaviour is that requests to the tornado process attempt
to load the user from the cache, with a different prefix from Django,
and immediately invalidate the session and eject the user back to the
login page.
Fix the `list_supervisor_processes` logic to match without the
trailing `:*`.
We had skipped these in #14693 so we could keep generating a friendly
error on Python 3.5, but we gave that up in #19801.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Since wal-g does not provide binaries for aarch64, build them from
source. While building them from source for arm64 would better ensure
that build process is tested, the build process takes 7min and 700M of
temp files, which is an unacceptable cost; we thus only build on
aarch64.
Since the wal-g build process uses submodules, which are not in the
Github export, we clone the full wal-g repository. Because the
repository is relatively small, we clone it anew on each new version,
rather than attempt to manage the remotes.
Fixes#21070.
stop-server and restart-server address all services which talk to the
database, and are thus more correct than restarting or stopping
everything in supervisor.
This is possible now that the previous commit ensures that the zulip
user can read the zulip installation directory during
`create-database`; previously, that directory was still owned by root
when `create-database` was run, whereas now it is in
`~zulip/deployments/`.
Move database creation to immediately before database initialization;
this means it happens in a directory readable by the `zulip` user, as
well as placing it alongside similar operations. It removes the check
for the `zulip::postgresql_common` Puppet class; instead it keeps the
check for `--no-init-db`, and switches to require
`zulip::app_frontend_base`. This is a behavior change for any install
of `zulip::postgresql_common`-only classes, but that is not a common
form -- and such installs likely already pass `--no-init-db` because
they are warm spare replicas.
As a result, all non-`zulip::app_frontend_base` installs now skip
database initialization, even without `--no-init-db`. This is clearly
correct for, e.g. Redis-only hosts, and makes clearer that the
frontend, not the database host, is responsible for database
initialization.
rabbitmqctl ping only checks that the Erlang process is registered
with epmd. There’s a window after that where the rabbit app is still
starting inside it.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This was added in c770bdaa3a, and we
have not added any realm-internal bots since
c770bdaa3a.
Speed up the critical period during upgrades by skipping this step.
Using an absolute `ZULIP_SCRIPTS` path when computing sha245sums
results in a set of hashes which varies based on the path that the
script is called as. This means that each deploy _always_ has
`setup-apt-repo --verify` fail, since it is a different base path.
Make all paths passed to sha256sum be relative to the repository root,
ensuring they can be compared across runs.
In some instances (e.g. during upgrades) we run `restart-server` and
not `start-server`, even though we expect the server to most likely
already be stopped. `supervisorctl restart servicename` if the
service is stopped produces the perhaps-alarming message:
```
restart-server: Restarting servicename
servicename: ERROR (not running)
servicename: started
```
This may cause operators to worry that something is broken, when it is
not.
Check if the service is already running, and switch from "restart" to
"start" in cases where it is not.
The race condition here is safe -- if the service transitions from
stopped to started between the check and the `start` call, it will
merely output:
```
servicename: ERROR (already started)
```
...and continue, as that has exit status 0.
If the service transitions from started to stopped between the check
and the `restart` call, we are merely back in the current case, where
it outputs:
```
servicename: ERROR (not running)
servicename: started
```
In none of these cases does a call to "restart" fail to result in the
service being stopped and then started.
Ubuntu 22.04 pushed a post-feature-freeze update to Python 3.10,
breaking virtual environments in a Debian patch
(https://bugs.launchpad.net/ubuntu/+source/python3.10/+bug/1962791).
Also, our antique version of Tornado doesn’t work in 3.10, and we’ll
need to do some work to upgrade that.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We can safely ignore the presence of the extra tables that could be
left behind in the database from when we had these installed (before
Zulip 1.7.0 and 2.0.0, respectively).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This was not needed for OpenSSL ≥ 1.1.1 (all our supported platforms),
and breaks with OpenSSL ≥ 3.0.0 (Ubuntu 22.04). It was removed from
the upstream configuration file too: https://bugs.debian.org/990228.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This was only used for upgrading from Zulip < 1.9.0, which is no
longer possible because Zulip < 2.1.0 had no common supported
platforms with current main.
If we ever want this optimization for a future migration, it would be
better implemented using Django merge migrations.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Attempting to "upgrade" from `main` to 4.x should abort; Django does
not prevent running old code against the new database (though it
likely errors at runtime), and `./manage.py migrate` from the old
version during the "upgrade" does not downgrade the database, since
the migrations are entirely missing in that directory, so don't get
reversed.
Compare the list of applied migrations to the list of on-disk
migrations, and abort if there are applied migrations which are not
found on disk.
Fixes: #19284.
Services like go-camo and smokescreen are not stopped in stop-server,
since they are upgraded and restarted by puppet application. As such,
they also do not appear in start-server, despite the server relying on
them to be running to function properly.
Ensure those services are started, by starting them in start-server,
if they are configured in supervisor on the host.
For many uses, shelling out to `supervisorctl` is going to produce
better error messages. However, for instances where we wish to parse
the output of `supervisorctl`, using the API directly is less brittle.
The RabbitMQ docs state ([1]):
RabbitMQ nodes and CLI tools (e.g. rabbitmqctl) use a cookie to
determine whether they are allowed to communicate with each
other. [...] The cookie is just a string of alphanumeric
characters up to 255 characters in size. It is usually stored in a
local file.
...and goes on to state (emphasis ours):
If the file does not exist, Erlang VM will try to create one with
a randomly generated value when the RabbitMQ server starts
up. Using such generated cookie files are **appropriate in
development environments only.**
The auto-generated cookie does not use cryptographic sources of
randomness, and generates 20 characters of `[A-Z]`. Because of a
semi-predictable seed, the entropy of this password is thus less than
the idealized 26^20 = 94 bits of entropy; in actuality, it is 36 bits
of entropy, or potentially as low as 20 if the performance of the
server is known.
These sizes are well within the scope of remote brute-force attacks.
On provision, install, and upgrade, replace the default insecure
20-character Erlang cookie with a cryptographically secure
255-character string (the max length allowed).
[1] https://www.rabbitmq.com/clustering.html#erlang-cookie
5c450afd2d, in ancient history, switched from `check_call` to
`check_output` and throwing away its result.
Use check_call, so that we show the steps to (re)starting the server.
This is required in order to lock down the RabbitMQ port to only
listen on localhost. If the nodename is `rabbit@hostname`, in most
circumstances the hostname will resolve to an external IP, which the
rabbitmq port will not be bound to.
Installs which used `rabbit@hostname`, due to RabbitMQ having been
installed before Zulip, would not have functioned if the host or
RabbitMQ service was restarted, as the localhost restrictions in the
RabbitMQ configuration would have made rabbitmqctl (and Zulip cron
jobs that call it) unable to find the rabbitmq server.
The previous commit ensures that configure-rabbitmq is re-run after
the nodename has changed. However, rabbitmq needs to be stopped
before `rabbitmq-env.conf` is changed; we use an `onlyif` on an `exec`
to print the warning about the node change, and let the subsequent
config change and notify of the service and configure-rabbitmq to
complete the re-configuration.
`/etc/rabbitmq/rabbitmq-env.conf` sets the nodename; anytime the
nodename changes, the backing database changes, and this requires
re-creating the rabbitmq users and permissions.
Trigger this in puppet by running configure-rabbitmq after the file
changes.
This reverts commit 889547ff5e. It is
unused in the Docker container, as the configurtaion of the `zulip`
user in the rabbitmq node is done via environment variables. The
Zulip host in that context does not have `rabbitmqctl` installed, and
would have needed to know the Erlang cookie to be able to run these
commands.
This addresses the problems mentioned in the previous commit, but for
existing installations which have `authenticator = standalone` in
their configurations.
This reconfigures all hostnames in certbot to use the webroot
authenticator, and attempts to force-renew their certificates.
Force-renewal is necessary because certbot contains no way to merely
update the configuration. Let's Encrypt allows for multiple extra
renewals per week, so this is a reasonable cost.
Because the certbot configuration is `configobj`, and not
`configparser`, we have no way to easily parse to determine if webroot
is in use; additionally, `certbot certificates` does not provide this
information. We use `grep`, on the assumption that this will catch
nearly all cases.
It is possible that this will find `authenticator = standalone`
certificates which are managed by Certbot, but not Zulip certificates.
These certificates would also fail to renew while Zulip is running, so
switching them to use the Zulip webroot would still be an improvement.
Fixes#20593.
Installing certbot with --method=standalone means that the
configuration file will be written to assume that the standalone
method will be used going forward. Since nginx will be running,
attempts to renew the certificate will fail.
Install a temporary self-signed certificate, just to allow nginx to
start, and then follow up (after applying puppet to start nginx) with
the call to setup-certbot, which will use the webroot authenticator.
The `setup-certbot --method=standalone` option is left intact, for use
in development environments.
Fixes part of #20593; it does not address installs which were
previously improperly configured with `authenticator = standalone`.
As a consequence:
• Bump minimum supported Python version to 3.7.
• Move Vagrant environment to Debian 10, which has Python 3.7.
• Move CI frontend tests to Debian 10.
• Move production build test to Debian 10.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This catches nine functional indexes that the previous query didn’t:
upper_preregistration_email_idx
upper_stream_name_idx
upper_subject_idx
upper_userprofile_email_idx
zerver_message_recipient_upper_subject
zerver_mutedtopic_stream_topic
zerver_stream_realm_id_name_uniq
zerver_userprofile_realm_id_delivery_email_uniq
zerver_userprofile_realm_id_email_uniq
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Restarting the uwsgi processes by way of supervisor opens a window
during which nginx 502's all responses. uwsgi has a configuration
called "chain reloading" which allows for rolling restart of the uwsgi
processes, such that only one process at once in unavailable; see
uwsgi documentation ([1]).
The tradeoff is that this requires that the uwsgi processes load the
libraries after forking, rather than before ("lazy apps"); in theory
this can lead to larger memory footprints, since they are not shared.
In practice, as Django defers much of the loading, this is not as much
of an issue. In a very basic test of memory consumption (measured by
total memory - free - caches - buffers; 6 uwsgi workers), both
immediately after restarting Django, and after requesting `/` 60 times
with 6 concurrent requests:
| Non-lazy | Lazy app | Difference
------------------+------------+------------+-------------
Fresh | 2,827,216 | 2,870,480 | +43,264
After 60 requests | 3,332,284 | 3,409,608 | +77,324
..................|............|............|.............
Difference | +505,068 | +539,128 | +34,060
That is, "lazy app" loading increased the footprint pre-requests by
43MB, and after 60 requests grew the memory footprint by 539MB, as
opposed to non-lazy loading, which grew it by 505MB. Using wsgi "lazy
app" loading does increase the memory footprint, but not by a large
percentage.
The other effect is that processes may be served by either old or new
code during the restart window. This may cause transient failures
when new frontend code talks to old backend code.
Enable chain-reloading during graceful, puppetless restarts, but only
if enabled via a zulip.conf configuration flag.
Fixes#2559.
[1]: https://uwsgi-docs.readthedocs.io/en/latest/articles/TheArtOfGracefulReloading.html#chain-reloading-lazy-apps
On a system where ‘apt-get update’ has never been run, ‘apt-cache
policy’ may show no repositories at all. Try to correct this with
‘apt-get update’ before giving up.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
If nginx was already installed, and we're using the webroot method of
initializing certbot, nginx needs to be reloaded. Hooks in
`/etc/letsencrypt/renewal-hooks/deploy/` do not run during initial
`certbot certonly`, so an explicit reload is required.
The certbot package installs its own systemd timer (and cron job,
which disabled itself if systemd is enabled) which updates
certificates. This process races with the cron job which Zulip
installs -- the only difference being that Zulip respects the
`certbot.auto_renew` setting, and that it passes the deploy hook.
This means that occasionally nginx would not be reloaded, when the
systemd timer caught the expiration first.
Remove the custom cron job and `certbot-maybe-renew` script, and
reconfigure certbot to always reload nginx after deploying, using
certbot directory hooks.
Since `certbot.auto_renew` can't have an effect, remove the setting.
In turn, this removes the need for `--no-zulip-conf` to
`setup-certbot`. `--deploy-hook` is similarly removed, as running
deploy hooks to restart nginx is now the default; pass
`--no-directory-hooks` in standalone mode to not attempt to reload
nginx. The other property of `--deploy-hook`, of skipping symlinking
into place, is given its own flog.
We've had a number of unhappy reports of upgrades failing due to
webpack requiring too much memory. While the previous commit will
likely fix this issue for everyone, it's worth improving the error
message for failures here.
We avoid doing the stop+retry ourselves, because that could cause an
outage in a production system if webpack fails for another reason.
Fixes#20105.
Since the upgrade to Webpack 5, we've been seeing occasional reports
that servers with roughly 4GiB of RAM were getting OOM kills while
running webpack.
Since we can't readily optimize the memory requirements for webpack
itself, we should raise the RAM requirements for doing the
lower-downtime upgrade strategy.
Fixes#20231.
Stopping both `zulip-tornado` and `zulip-tornado:*` causes errors on
deploys with tornado sharding, as the plain `zulip-tornado` service
does not exist.
Pass `zulip-tornado:*`, which matches both plain `zulip-tornado`, as
well as the sharded `zulip-tornado:zulip-tornado-port-9800` cases.
The `analyze_new_cluster.sh` script output by `pg_upgrade` just runs
`vacuumdb --all --analyze-in-stages`, which runs three passes over the
database, getting better stats each time. Each of these passes is
independent; the third pass does not require the first two.
`--analyze-in-stages` is only provided to get "something" into the
database, on the theory that it could then be started and used. Since
we wait for all three passes to complete before starting the database,
the first two passes add no value.
Additionally, PosttgreSQL 14 and up stop writing the
`analyze_new_cluster.sh` script as part of `pg_upgrade`, suggesting
the equivalent `vacuumdb --all --analyze-in-stages` call instead.
Switch to explicitly call `vacuumdb --all --analyze-only`, since we do
not gain any benefit from `--analyze-in-stages`. We also enable
parallelism, with `--jobs 10`, in order to analyze up to 10 tables in
parallel. This may increase load, but will accelerate the upgrade
process.
scripts.lib.node_cache expects Yarn to be in /srv/zulip-yarn, so if
it’s installed somewhere else, even if it’s the right version, we need
to reinstall it.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
It recently started failing on Debian 10 (buster). We immediately
follow this by replacing these packages with our own versions from
pip.txt, anyway.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The support for bullseye was added in #17951
but it was not documented as bullseye was
frozen and did not have proper configuration
files, hence wasn't documented.
Since now bullseye is released as a stable
version, it's support can be documented.
The usual output from this command looks like
Notice: Compiled catalog for localhost in environment production in 2.33 seconds
Notice: /Stage[main]/Zulip::Apt_repository/Exec[setup_apt_repo]/returns: current_value 'notrun', should be ['0'] (noop)
Notice: Class[Zulip::Apt_repository]: Would have triggered 'refresh' from 1 event
Notice: Stage[main]: Would have triggered 'refresh' from 1 event
Notice: Applied catalog in 1.20 seconds
which doesn’t seem abnormally alarming, and hiding it makes failures
harder to diagnose.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We previously used `zulip-puppet-apply` with a custom config file,
with an updated PostgreSQL version but more limited set of
`puppet_classes`, to pre-create the basic settings for the new cluster
before running `pg_upgradecluster`.
Unfortunately, the supervisor config uses `purge => true` to remove
all SUPERVISOR configuration files that are not included in the puppet
configuration; this leads to it removing all other supervisor
processes during the upgrade, only to add them back and start them
during the second `zulip-puppet-apply`.
It also leads to `process-fts-updates` not being started after the
upgrade completes; this is the one supervisor config file which was
not removed and re-added, and thus the one that is not re-started due
to having been re-added. This was not detected in CI because CI added
a `start-server` command which was not in the upgrade documentation.
Set a custom facter fact that prevents the `purge` behaviour of the
supervisor configuration. We want to preserve that behaviour in
general, and using `zulip-puppet-apply` continues to be the best way
to pre-set-up the PostgreSQL configuration -- but we wish to avoid
that behaviour when we know we are applying a subset of the puppet
classes.
Since supervisor configs are no longer removed and re-added, this
requires an explicit start-server step in the instructions after the
upgrades complete. This brings the documentation into alignment with
what CI is testing.
These changes are all independent of each other; I just didn’t feel
like making dozens of commits for them.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
For our marketing emails, we want a width that's more appropriate for
newsletter context, vs. the narrow emails we use for transactional
content.
I haven't figured out a cleaner way to do this than duplicating most
of email_base_default.source.html. But it's not a big deal to
duplicate, since we've been changing that base template only about
once a year.
The script is added to upgrade steps for 20.04 and Buster because
those are the upgrades that cross glibc 2.28, which is most
problematic. It will also be called out in the upgrade notes, to
catch those that have already done that upgrade.
Running `supervisorctl stop` or `supervisorctl restart` on a process
name which is not known is an error:
```
$ supervisorctl stop nonexistent-process
nonexistent-process: ERROR (no such process)
$ echo $?
1
```
ef6d0ec5ca moved
zulip_deliver_scheduled_* out of the `workers:` group. Since upgrades
run `stop-server` before applying puppet, the list of processes at
that time is from the previous version of Zulip, so may not have the
new `zulip_deliver_scheduled_*` names -- and the `stop-server` will
hence fail.
If the upgrade is not applying puppet, it will `restart-server`. At
that point, the old names will still be in the configuration, so
relying on the current `superisorctl status` is the best gauge of what
exists to restart.
In short, only ever stop/start/restart the `zulip_deliver_scheduled_*`
processes if `supervisorctl status` knows about them already.
Nonexistent processes and groups passed to `supervisortctl status` are
printed to STDOUT as follows:
```
$ supervisorctl status zulip-django nonexistent-process nonexistent-group:*
nonexistent-process: ERROR (no such process)
nonexistent-group: ERROR (no such group)
zulip-django RUNNING pid 16043, uptime 17:31:31
```
On supervisor 4 and above, this exits with an exit code of 4;
previously, it returned exit code 0. Ubuntu 18.04 has version 3.3.1,
and Ubuntu 20.04 has version 4.1.0.
Skip any lines with `ERROR (no such ...)`, and accept exit code 4 from
`supervisorctl status`.
Mypy can’t follow absolute imports based on directories other than the
root. This was hiding some type errors due to ignore_missing_imports.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
With two space-separated classes in `puppet_classes`, the second one
is silently ignored. With three of more, puppet generates the
following very opaque error message:
```
Error: Could not parse for environment production: This
Name has no effect. A value was produced and then forgotten (one or
more preceding expressions may have the wrong form)
```
Catch when this has happened, and give an error message to the user.
Fixes#18992.
This command is part of a statsd infrastructure that we stopped
supporting years ago. Its only purpose for some time has been to
provide sample code for how the restart script might trigger a
notification to a graphing system, which doesn't justify maintaining
it.
Fixes part of #18898.
The current `upgrade-zulip` and `upgrade-zulip-from-git`
bash scripts exit with a zero status even if the
upgrade commands exit with a non-zero status.
Hence add `set -e` command which exits the script with
the same status as the non-zero command.
For pipe commands however, the net status of a command
is the status of the last command, hence if the other parts
fail, the net status is only determined by the last command.
This is the case with our main /lib/upgrade-zulip* command
in the scripts whose status is determined by the `tee` command
instead. Hence add a small condition to get the status of the
actual upgrade command and exit the script if it fails with
a non-zero command.
We also check whether the script is being run as root, matching the
install script logic.
This parameter is somewhat useful, and adding this also fixes a
regression where purge-old-deployments would crash since the changes
around c5580607a7 because of
inconsistent supported args lists.
Fixes#16659.
If the server is behind a reverse proxy with http_only=True, the
requests made by email-mirror-postfix need to use http, as https
doesn't work.
Staging and other hosts that are `zulip::app_frontend_base` but not
`zulip::app_frontend_once` do not have a
/etc/supervisor/conf.d/zulip/zulip-once.conf and as such do not have
`zulip_deliver_scheduled_emails` or `zulip_deliver_scheduled_messages`
and thus supervisor will fail to reload.
Making the contents of `zulip-workers` contingent on if the server is
_also_ a `-once` server is complicated, and would involve using Concat
fragments, which severely limit readability.
Instead, expel those two from `zulip-workers`; this is somewhat
reasonable, since they are use an entirely different codepath from
zulip_events_*, using the database rather than RabbitMQ for their
queuing.
We currently run the `clean_unused_caches.py` as a
script to clean the unused caches.
This commit replaces that with
`clean_unused_caches.main` function as it would be
faster.
This commit will allow us to pass the arguments in the
'clean...' functions when calling the `main` function (in
`provision`). It also changes args parsing
function location to `if __name__ == "__main__"` block as
we wouldn't need it to parse args when we call the
function.
We convert the `clean-unused-caches` script to a
python file so we can run it in provision by importing it
instead of running the script, hence saving some time.
Appending data back-to-back without serializing it loses the
information about where the breaks between them lie, which can lead to
different inputs having the same hash.
Using puppet modules from the puppet forge judiciously will allow us
to simplify the configuration somewhat; this specifically pulls in the
stdlib module, which we were already using parts of.
This moves the `.asc` files into subdirectories, and writes out the
according `.list` files into them. It moves from templates to
written-out `.list` files for clarity and ease of
implementation (Debian and Ubuntu need different templates for
`zulip`), and as a way of making explicit which releases are supported
for each list. For the special-case of the PGroonga signing key, we
source an additional file within the directory.
This simplifies the process for adding another class of `.list` file.
Add support for custom database names and database users, which can be
set with the `--postgresql-database-name` and
`--postgresql-database-user` install script options. If these
parameters aren't provided, then the defaults remain "zulip".
Fixes#17662.
Co-authored-by: Alex Vandiver <alexmv@zulip.com>
This adds basic support for `postgresql.database_user` and
`postgresql.database_name` settings in `zulip.conf`; the defaults if
unspecified are left as `zulip`.
Co-authored-by: Adam Birds <adam.birds@adbwebdesigns.co.uk>
Add a helper `run_psql_as_postgres` function in
`scripts/lib/zulip_tools.py`. This is preparatory refactoring for the
work to add custom database and user names.
Fixes this error when running the installer from a directory that
isn’t world-readable:
+ su zulip -c 'git config --global user.email anders@zulip.com'
fatal: cannot come back to cwd: Permission denied
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This reverts part of commit 476524c0c1
(#18215), to fix this error when running the installer from a
directory that isn’t world-readable:
+ '[' -e /var/run/supervisor.sock ']'
+++ dirname /root/zulip-server-4.1/scripts/setup/postgresql-init-db
++ dirname /root/zulip-server-4.1/scripts/setup
+ su zulip -c /root/zulip-server-4.1/scripts/stop-server
bash: /root/zulip-server-4.1/scripts/stop-server: Permission denied
Zulip installation failed (exit code 126)!
Signed-off-by: Anders Kaseorg <anders@zulip.com>
When upgrading from a pre-4.0 release, scripts/stop-server logic would
check whether supervisord configuration files were present to
determine what it needed to restart, but only considered paths to
those files that are introduced in Zulip 4.0.
Fixed#18493.
This ensures that the `git describe` queries that we run for caching
Zulip's Git version are guaranteed to include recent releases.
This change ensures that we have accurate output even if we're pointed
at a fork of Zulip that never updates its tags.
Additionally, it will make it possible to record the
`git merge-base upstream/master` in future commits.
Note that because we run this code before unpacking the new version,
the pre-upgrade version of this code runs.
As a result, we cannot assume that the upstream repository exists.
This removes a possible window where an installer error could leave
`nvm` in a state where it had prepended the full path to the
newly-installed `npm` to `$PATH`; we would like to avoid `nvm`
fiddling with path whenever possible (ref ebe930ab2c).
During the upgrade process of a postgresql-only Zulip installation,
(`puppet_classes = zulip::profile::postgresql` in
`/etc/zulip/zulip.conf`) either `scripts/start-server` or
`scripts/stop-server` fail because they try to handle supervisor
services that are not available (e.g. Tornado) since only
`/etc/supervisor/conf.d/zulip/zulip_db.conf` is present and not
`/etc/supervisor/conf.d/zulip/zulip.conf`.
While this wasn't previously supported, it's a pretty reasonable thing
to do, and can be readily supported by just adding a few conditionals.
Matching the full process name (-x without -f) or full command
line (-xf) is less prone to mistakes like matching a random substring
of some other command line or pgrep matching itself.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Thumbor and tc-aws have been dragging their feet on Python 3 support
for years, and even the alphas and unofficial forks we’ve been running
don’t seem to be maintained anymore. Depending on these projects is
no longer viable for us.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The `en_US.UTF-8` locale may not be configured or generated on all
installs; it also requires that the `locales` package be installed.
If users generate the `en_US.UTF-8` locale without adding it to the
permanent set of system locales, the generated `en_US.UTF-8` stops
working when the `locales` package is updated.
Switch to using `C.UTF-8` in all cases, which is guaranteed to be
installed.
Fixes#15819.
In some cases, puppet can end up restarting supervisord services - which
will use code from the old deployment, because when puppet runs,
/home/zulip/deployments/current still points there. Thus restart-server
needs to be used in favor of start-server, unless we know that puppet
has been skipped.
Previous versions of zulip used `nvm alias default ...` to have `nvm`
prepend the full path to the latest `node` install to the `PATH` in
root's shell. Unfortunately, this means that `update-prod-static`,
when called from `upgrade-zulip-stage-2` after an upgrade of node in
`install-node`, would still have the full path to the _old_ `node` at
the start of its PATH, because the PATH of `upgrade-zulip-stage-2`
would still be unchanged.
Bootstrap out of this by setting a known-reasonable PATH during
upgrade, and remove the problematic `nvm alias default` behaviour.
Fixes#18258.
In Debian, becoming root as `su` does not alter the `$PATH`; this can
lead to the root user not having `/usr/sbin` in its path, and thus
the `useradd zulip` step of the installer fails.
Fixes#17441.
Instead of taking the "onion" approach, where all services are
stopped, and then started back up again, default to a rolling restart
across all processes. This draws out how long the overall "restart"
takes, but minimizes the time that any of the services are down. This
minimizes user-visible impact and queue buildup.
In cases where speed is more important than minimal impact (for
example, there is already a current outage), a --less-graceful flag is
provided, which brings the services down more suddenly, and back up in
a still-correct order.
This hits the unauthenticated Github API to get the list of tags,
which is rate-limited to 60 requests per hour. This means that the
tool can only be run 60 times per hour before it starts to exit with
errors, but that seems like a reasonable limit for the moment.
This commit removes redundant yarn cache by removing the old
version directories, i.e. All the directory under `~/.cache/yarn`
except `~/.cache/yarn/v6` (current version directory).
Fixes#15964.
In general, `./scripts/restart-server` will already work in any
circumstance where the server is already stopped and needs to be
started. However, it will output a couple minor warnings, and it is
not readily obvious that it *will* work correctly.
Add an alias for `restart-server` named `start-server`, for
parallelism with `stop-server`, which omits the steps of
`restart-server` which would stop the server first.
Using `supervisorctl stop all` to stop the server is not terribly
discoverable, and may stop services which are not part of Zulip
proper.
Add an explicit tool which only stops the relevant services. It also
more carefully controls the order in which services are stopped to
minimize lost requests, and maximally quiesce the server.
Locations which may be stopping _older_ versions of Zulip (without
this script) are left with using `supervisorctl stop all`.
Fixes#14959.
The path which contains all of the Zulip supervisor files changed in
3ab9b31d2f to make it easier to purge
now-unwanted supervisor configuration files. However, the paths that
the zulip upgrade process, and restart-server, look at were not
adjusted.
Fix the supervisor configuration file paths.
3314fefaec started needing `python3-yaml`, but incorrectly claimed
that it was always an indirect dependency; it is a dependency of
`ubuntu-minimal` on 20.04, but not required on 18.04 or Debian. We
cannot install it in puppet because then is definitionally too late;
it is needed at load time by `zulip-puppet-apply`.
Install `python3-yaml`, but guarded by a simple check so as to not
further slow most installs.
Fixes#18179.
The stacktraces here are seldom useful -- for the calls to
upgrade-stage-2, we know precisely what was run. For the `run`
wrapper, the output contains the command that failed, which is
sufficient to identify where in the upgrade process it was. Showing
more stacktrace below the actual error merely confuses users and
scrolls the real error off of the screen.
For installs which use the `upgrade-zulip-from-git` process, the
deployment directory is a git checkout. This means that an
administrator can, as an emergency tool, run `git revert` and similar
commands -- assuming there is a `~/.gitconfig` set up for the zulip
user.
Add commands to `scripts/lib/install` to create a `~/.gitconfig` file
at installation time. The `user.name` and `user.email` fields are set
to the hostname and passed-in `--email` value, respectively.
Fixes#18039.
0663b23d54 changed zulip-puppet-apply to
use the venv, because it began using `yaml` to parse the output of
puppet to determine if changes would happen.
However, not every install ends with a venv; notably, non-frontend
servers do not have one. Attempting to run zulip-puppet-apply on them
hence now fails.
Remove this dependency on the venv, by installing a system
python3-yaml package -- though in reality, this package is already an
indirect dependency of the system. Especially since pyyaml is quite
stable, we're not using it in any interesting way, and it does not
actually add to the dependencies, it is preferable to parsing the YAML
by hand in this instance.
When exception is raised inside an exception handler, Python 3
helpfully prints both tracebacks separated by “During handling of the
above exception, another exception occurred:”. But when we’re using
an exception handler to retry the same operation, multiple tracebacks
are just noise. Suppress the earlier one using PEP 409 syntax.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This means that in steady-state, `zulip-puppet-apply` is expected to
produce no changes or commands to execute. The verification step of
`setup-apt-repo` is quite fast, so this cleans up the output for very
little cost.
The class names need to be renamed even if we are not about to run
puppet ourselves; otherwise, deployments which rely on running puppet
themselves will still have the wrong class names.
These are respected by `urllib`, and thus also `requests`. We set
`HTTP_proxy`, not `HTTP_PROXY`, because the latter is ignored in
situations which might be running under CGI -- in such cases it may be
coming from the `Proxy:` header in the request.
Using `config_file.write()` only writes out what python stored of the
file; as such, it strips all comments and whitespace.
Use `crudini --set`, which only modifies the line whose contents are
changed.
There is only one PostgreSQL database; the "appdb" is irrelevant.
Also use "postgresql," as it is the name of the software, whereas
"postgres" the name of the binary and colloquial name. This is minor
cleanup, but enabled by the other renames in the previous commit.
The "voyager" name is non-intuitive and not significant.
`zulip::voyager` and `zulip::dockervoyager` stubs are kept for
back-compatibility with existing `zulip.conf` files.
This moves the puppet configuration closer to the "roles and profiles
method"[1] which is suggested for organizing puppet classes. Notably,
here it makes clear which classes are meant to be able to stand alone
as deployments.
Shims are left behind at the previous names, for compatibility with
existing `zulip.conf` files when upgrading.
[1] https://puppet.com/docs/pe/2019.8/the_roles_and_profiles_method
Because the command is part of a pipe sequence, the exitcode defaults
to the last in the sequence, which is not the most important one here.
Set pipefail, which sets the exit status to the exit code of the last
program in the sequence to exit non-zero, or 0 if all succeeded. This
prevents the upgrade from barreling onward and setting
`postgres.version` improperly if the database upgrade step failed.
Fingerprinting the config is somewhat brittle -- it requires either
custom bootstrapping for old (fingerprint-less) configs, and may have
false-positives.
Since generating the config is lightweight, do so into the .tmp files,
and compare the output to the originals to determine if there are
changes to apply.
In order to both surface errors, as well as notify the user in case a
restart is necessary, we must run it twice. The `onlyif`
functionality cannot show configuration errors to the user, only
determine if the command runs or not. We thus run the command once,
judging errors as "interesting" enough to run the actual command,
whose failure will be verbose in Puppet and halt any steps that depend
on it.
Removing the `onlyif` would result in `stage_updated_sharding` showing
up in the output of every Puppet run, which obscures the important
messages it displays when an update to sharding is necessary.
Removing the `command` (e.g. making it an `echo`) would result in
removing the ability to report configuration errors. We thus have no
choice but to run it twice; this is thankfully low-overhead.
The reason higher expected_time_to_clear_backlog were allowed for queues
during "bursts" was, in simpler terms, because those queues to which
this happens, intrinsically have a higher acceptable "time until cleared"
for new events. E.g. digests_email, where it's completely fine to take a
long time to send them out after putting in the queue. And that's
already configurable without a normal/burst distinction.
Thanks to this we can remove a bunch of overly complicated, and
ultimately useless, logic.
The race condition is described in the comment block removed by this
commit. This leaves room for another, remaining race condition
that should be virtually impossible, but nevertheless it seems
worthwhile to have it documented in the code, so we put a new comment
describing it.
As a final note, this is not a new race condition,
it was hypothetically possible with the old code as well.
We can compute the intended number of processes from the sharding
configuration. In doing so, also validate that all of the ports are
contiguous.
This removes a discrepancy between `scripts/lib/sharding.py` and other
parts of the codebase about if merely having a `[tornado_sharding]`
section is sufficient to enable sharding. Having behaviour which
changes merely based on if an empty section exists is surprising.
This does require that a (presumably empty) `9800` configuration line
exist, but making that default explicit is useful.
After this commit, configuring sharding can be done by adding to
`zulip.conf`:
```
[tornado_sharding]
9800 = # default
9801 = other_realm
```
Followed by running `./scripts/refresh-sharding-and-restart`.
Making this include "zulip-tornado" makes it clearer in supervisor
logs. Without this, one only sees:
```
2020-09-14 03:43:13,788 INFO waiting for port-9807 to stop
2020-09-14 03:43:14,466 INFO stopped: port-9807 (exit status 1)
2020-09-14 03:43:14,469 INFO spawned: 'port-9807' with pid 24289
2020-09-14 03:43:15,470 INFO success: port-9807 entered RUNNING state, process has stayed up for > than 1 seconds (startsecs)
```
`supervisorctl` starts and stops its arguments sequentially, in the
order they are passed[1]. Start them in the opposite order from the
order in which they were stopped -- this puts the dependencies first,
and the most core services (`zulip-django`) last.
While the only "dependency" here is currently thumbor, this sets us up
in case others are added later.
[1] https://github.com/Supervisor/supervisor/blob/master/supervisor/supervisorctl.py#L782
This supports running puppet to pick up new sharding changes, which
will warn of the need to finalize them via
`refresh-sharding-and-restart`, or simply running that directly.
The value in the stats file can get outdated if the queue hasn't done
enough iterations to update the stats file for a while. The queue size
output by rabbitmqctl list_queues is more up to date, and empirically
tends to agree with the value in the stats file (when the stats file is
fresh).
There are three functional side effects:
• Correct an insignificant but mathematically offensive bias toward
repeated characters in generate_api_key introduced in commit
47b4283c4b4c70ecde4d3c8de871c90ee2506d87; its entropy is increased
from 190.52864 bits to 190.53428 bits.
• Use the base32 alphabet in confirmation.models.generate_key; its
entropy is reduced from 124.07820 bits to the documented 120 bits, but
now it uses 1 syscall instead of 24.
• Use the base32 alphabet in get_bigbluebutton_url; its entropy is
reduced from 51.69925 bits to 50 bits, but now it uses 1 syscall
instead of 10.
(The base32 alphabet is A-Z 2-7. We could probably replace all of
these with plain secrets.token_urlsafe, since I expect most callers
can handle the full urlsafe_b64 alphabet A-Z a-z 0-9 - _ without
problems.)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Alex Vandiver
Anders Kaseorg
Tim Abbott
rht
Gaurav Pandey
manavdesai27
Mateusz Mandera
Riken Shah
Adam Birds
Robert Imschweiler
Siddharth Asthana
Ganesh Pawar
Sutou Kouhei
Vishnu KS