-
Notifications
You must be signed in to change notification settings - Fork 687
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
[xenial] Add documentation for Xenial upgrade #4140
Conversation
ab0ed81
to
ee1adba
Compare
Codecov Report
@@ Coverage Diff @@
## develop #4140 +/- ##
========================================
Coverage 84.79% 84.79%
========================================
Files 43 43
Lines 2782 2782
Branches 303 303
========================================
Hits 2359 2359
Misses 355 355
Partials 68 68
Continue to review full report at Codecov.
|
fc1ffbf
to
6876a25
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of minor things, otherwise lgtm.
the servers during the OS installation. | ||
|
||
If you are planning to install the Xenial instance on new hardware, make sure | ||
that you have all the necessary hardware to hand and configured. This guide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to hand
-> on hand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed.
- on the order of 8 hours if no issues are encountered, and longer if anything | ||
goes wrong. | ||
|
||
The process is less complex than the alternative |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence is a little grammatically ambiguous. It is not clear what the preferred method actually is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hopefully clearer now!
|
||
.. caution:: | ||
We have tested the upgrade-in-place procedure on officially supported hardware, | ||
but cannot guarantee that the Ubuntu ``do-release-upgrade`` command will |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the first reference to the command do-release-upgrade
. An admin may not understand this. It should either be referenced in the introductory paragraphs or we should just use a phrase "automated upgrade".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instructions for in-place upgrade look good to me, have not gone through the backup instructions yet. I'll let others also take a pass at reviewing these docs. I've left a few minor comments inline.
./securedrop-admin backup | ||
|
||
This will create a backup named ``sd-backup-<date>.tar.gz`` in the | ||
``~/Persistent/securedrop/install_files/ansible-base`` directory. Make a note of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be worth stating that the backups are not encrypted and to be mindful if/when copying them to other devices/storage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a note to that effect.
|
||
cd ~/Persistent/securedrop | ||
git checkout 0.12.0 | ||
git tag -v 0.12.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should validate the tag before checking it out, which is consistent with the updater behavior
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated this snippet in both docs.
.. code:: sh | ||
|
||
cd ~/Persistent | ||
cp app_services/app*ths securedrop/install_files/ansible-base/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the command at L64:
cp securedrop/install_files/ansible-base/app-*ths app_service/
- Will this also copy
ssh-app-aths
? app_services
->app_service
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's app-ssh-aths iirc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed typo
Disconnect the SSH session to the Application Server. You are now ready to move | ||
on to the next step: reinstalling SecureDrop on the Xenial servers. | ||
|
||
Step 3: Reinstall SecureDrop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose changing the title to either re-provision or re-configure SecureDrop servers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
.. code:: sh | ||
|
||
cd ~/Persistent/securedrop | ||
git checkout 0.12.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, git fetch --tags
might be necessary, and it's more prudent to verify the tag before checking it out, and it's consistent with what's done in the updater as well.
``examplenot4real.onion``, then the address you should visit is | ||
``examplenot4real.onion/metadata``. That page should show you key/value pairs, | ||
including ``0.12.0`` for ``sd_version`` and ``16.04`` for ``server_os``. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensuring OSSEC emails are flowing would be another interesting test to validate the upgrade
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a section for this.
Contact us | ||
---------- | ||
If you have questions or comments regarding this process, or if you | ||
encounter any issues, you can always contact us by the following means: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In your experience, how much instance-specific data are in the error messages when upgrades fail? Enough to recommend they be mindful of org-specific data in public forums (forum, github)
|
||
.. code:: sh | ||
|
||
./securedrop-admin setup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For users that have not updated Tails in a while, there will be the install once/install each time prompt when downloading apt package (described in https://docs.securedrop.org/en/release-0.11.0/install.html). Since either option will work, might not be worth mentioning here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gonna leave it out to keep the doc clean
.. code:: sh | ||
|
||
cd ~/Persistent/securedrop | ||
git checkout 0.12.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
git fetch --tags
might be necessary here since it's not cloned
- Use caution/warnings blocks in a couple of places; - Clean up trailing whitespace and wrap to 80 characters; - Follow formatting recommendations as closely as possible; - Small wording changes, reduce direct repetition; - Correct final step: install instead of sdconfig.
- Minimal additional tests (most failures should surface in playbook run) and an end-to-end test for good measure - Clarify what output is expected for `lsb_release -a` - Make it clearer how to resize terminal (H/T @conorsch) in case of ncurses issues - Remove placeholder notes
- Try to distinguish more clearly between new/existing hardware - Explicitly call out location of IP address info
2c8f18e
to
5a01ddb
Compare
updates to Xenial install docs based on review
cd ~/Persistent/ | ||
mkdir app_service | ||
cp securedrop/install_files/ansible-base/app-*ths app_service/ | ||
cp securedrop/install_files/ansible-base/site-specific app_service/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't the path install_files/ansible-base/group_vars/all/site-specific
by default?
cd ~/Persistent/ | ||
mkdir app_service | ||
cp securedrop/install_files/ansible-base/app-*ths app_service/ | ||
cp securedrop/install_files/ansible-base/site-specific app_service/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This path is wrong, the actual path is:
securedrop/install_files/ansible-base/group_vars/all/site-specific
- Choose **Yes** on the "Configuring libssl.0.0:amd64" dialog - when you do so, | ||
the ``sshd`` and ``tor`` daemons will restart, breaking your connection to the | ||
*Monitor Server* | ||
- Close the terminal window, open a new one, and reconnect with the command |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be a note that if someone is connected over local ssh, then it will not be disconnected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tested this scenario, but if sshd is bounced wouldn't they be disconnected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never happened to me.
updated tmux note, fixed path error
- via our `community forums <https://forum.securedrop.org>`_. | ||
|
||
If you encounter problems that are not security-sensitive, we also encourage you | ||
to `file an issue <https://github.com/freedomofpress/securedrop/issues/new/>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this link needs a _
after the final backtick (it's not rendered as a link currently)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Followed the steps for upgradation on Intel NUCs. This is good to merge. 👍
This PR adds instructions for
(The bulk of the work here was done by @zenmonkeykstop with copyediting by yours truly.)
Resolves #4057.
How to test
A tester should follow the instructions as closely as possible, against supported hardware.
Status
Ready for review
make docs-lint
passes