If there is a syntax error in `settings.py`, `restart-server` should
provide a reasonable message about this. It did so prior to
af08bcdb3f, becausde any invocation `./manage.py` without
`--skip-checks` will verify `settings.py`, among several other checks.
After af08bcdb3f, there are no `./manage.py` calls in most restarts,
which fa77be6e6c took further.
Add an explicit `./manage.py check` in the default case.
upgrade-zulip-stage-2 overrides this by passing `--skip-checks`, for
performance. This also means that `upgrade-zulip-from-git` itself
picks up the same `--skip-checks` flag, since it inherits the same
flag parsing, though that is perhaps of dubious utility.
Although Node.js 18 is not the active LTS release for another 3 weeks,
the Node.js 16 end-of-life date was moved forward to September 2023,
(https://nodejs.org/en/blog/announcements/nodejs16-eol/), so it seems
prudent to switch now.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Fixes “E713 Test for membership should be `not in`” found by ruff (now
that I’ve fixed it not to ignore scripts lacking a .py extension).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Fixes “E713 Test for membership should be `not in`” found by
ruff (https://github.com/charliermarsh/ruff).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
One should now be able to configure a regex by appending _regex to the
port number:
[tornado_sharding]
9802_regex = ^[l-p].*\.zulipchat\.com$
Signed-off-by: Anders Kaseorg <anders@zulip.com>
https://nginx.org/en/docs/http/ngx_http_map_module.html
Since Puppet doesn’t manage the contents of nginx_sharding.conf after
its initial creation, it needs to be renamed so we can give it
different default contents.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This implements get_mandatory_secret that ensures SHARED_SECRET is
set when we hit zerver.decorator.authenticate_notify. To avoid getting
ZulipSettingsError when setting up the secrets, we set an environment
variable DISABLE_MANDATORY_SECRET_CHECK to skip the check and default
its value to an empty string.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
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.
Anders Kaseorg
Alex Vandiver
Zixuan James Li