-
Notifications
You must be signed in to change notification settings - Fork 669
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
ansible: Install AVA as a systemd service #151
Conversation
This playbook - Installs Gecko dependencies - Clones & builds ava-build/gecko - Creates an ava user - Installs Gecko in /usr/bin - Creates and installs a staking certificate - Installs Gecko as a Systemd service called "ava" - Configures /var/lib/ava/db as the database - Configures /var/log/ava as the log destination - Starts the service
|
Codecov Report
@@ Coverage Diff @@
## master #151 +/- ##
===================================================
- Coverage 61.05467% 60.17373% -0.88095%
===================================================
Files 253 246 -7
Lines 17029 16577 -452
===================================================
- Hits 10397 9975 -422
+ Misses 5734 5695 -39
- Partials 898 907 +9 |
|
@moreati Hey, thanks for making this PR. We haven't forgotten it, we're just really busy 😅. Thanks for your patience |
No problem, it needs some work for the new staking key location
…On Tue, 12 May 2020 at 21:25, danlaine ***@***.***> wrote:
@moreati <https://github.com/moreati> Hey, thanks for making this PR. We
haven't forgotten it, we're just really busy 😅. Thanks for your patience
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#151 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABKS4WSZQJDVS52TDYUCS3RRGWDHANCNFSM4M47EQ5A>
.
--
Alex Willmer <alex@moreati.org.uk>
|
The Ansible equivalent of `set -o errexit` in bash
This restores the default ssh_args for Ansible. When present Ansible instructs ssh to keep a connection open for a short period after exiting. Subsequent ssh processes can then skip TCP & SSH hand shake.
Example 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.
@moreati thank you very much for this wonderful PR. We appreciate your help in making our product better. In fact configuring ansible to run ava
via systemd
was in our plans so your work is very welcomed.
I recently joined the team and I will be in charge to take over continuous integration and delivery. My experience with ansible is still limited so I would appreciate you addressing my - for now non-authoritative - comments in order to better understand how this fits into our setup. Some of the comments are meant merely for my colleagues in order to draw attention on a potential issue.
So yet again - thank you!
transport = ssh | ||
deprecation_warnings = false | ||
host_key_checking = false |
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.
Do we really want this at the global config level?
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'm happy to remove it, but note I haven't changed the setting from what is master (as ssh_args = ... o StrictHostKeyChecking=no
), merely the method by which it's configured.
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.
You're right.
-o ControlMaster=auto | ||
-o ControlPersist=60s | ||
-o ForwardAgent=yes | ||
-o UserKnownHostsFile=/dev/null |
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 am not sure about this entry either at the global level?
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.
Again, I haven't added UserKnownHostsFile=/dev/null
, I just happened to reformat that line.
Perhaps your concern is that this ansible.cfg has any effect outside this directory? It is only in effect when this directory is the working directory.
- curl | ||
- g++ | ||
- golang-go # Assumes Ubuntu 20.04, where this installs Go 1.13 | ||
- libssl-dev |
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.
Note: These work only on ubuntu.
- cmake | ||
- curl | ||
- g++ | ||
- golang-go # Assumes Ubuntu 20.04, where this installs Go 1.13 |
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.
Can we safely make this config choice of Ubuntu 20.04 already?
@@ -0,0 +1,8 @@ | |||
ava_daemon_bin_dir: "/usr/bin" | |||
ava_daemon_data_dir: "/var/lib/{{ ava_daemon_user }}" | |||
ava_daemon_db_dir: "{{ ava_daemon_data_dir }}/db" |
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.
@StephenButtolph just recently had defined that the data dir would be ~/.gecko
. But maybe for ansible deployments this is fine here?
In any case, note that due to ava_daemon_user: ava
, this means that only one user will be able to run ava
on such a deployment.
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 would advocate that having the directory /var/lib/ava/.gecko/{staking,db}
is not what a user would expect`. Hence why I overrode it.
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 worth pointing out that ava_daemon_user: ava
and others are only default values. Ansible is very flexible about overriding variables. For instance one could do
- hosts: localhost
become: true
roles:
- {name: ava-user, vars: {ava_daemon_user: snoopy}
- {name: ava-user, vars: {ava_daemon_user: woodstock}
Hey @moreati thanks for putting this together, it looks great! I had some concerns but most of them appear to have all been addressed. You have "Install under /usr/local" listed as a todo, is this still in progress? Do you have any other changes you're planning on for this PR? I would really like to split the OS and OS-version (Ubuntu/20.x) requirements out by putting them in their own files and including them conditionally based on the platform, but this could also be done as a follow up PR IMO. @holisticode, I'll let you make the call on that since you're using this right now and I'm not sure if the hard Ubuntu 20.x requirement works for you or not. |
If there is no general opposition, I myself am in favor of using ubuntu 20.x right away because it ships right away with a @tyler-smith not sure what you mean by conditionally based on platform, the platform is ubuntu and we use these ansible to just deploy our own nodes, so why would we conditionally support 18.x and 20.x instances? |
Yes, probably this weekend
Not that I can think of. It's already large enough. |
This allows them to be run as scripts. They take the same options and arguments as ansible-playbook.
This makes it easier to skip, if Go has been installed by other means.
Following discussion in ava-labs#151 it was decided that /usr (and by implication /var) should be reserved for OS package managers (e.g. apt, yum).
I added support for (and defaulted to) |
To remove any doubt: I believe I've addressed all comments, and this is ready for review. |
Added some optimistic logic to prevent un-needed traversals.
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.
@moreati thank you so much for this wonderful PR, sorry for the looooong delay.
We are going to merge this.
However, it is now slightly out of date - for example, we moved away from salticidae and thus some dependencies got obsolete.
Also, we are committed to support ubuntu 18.04 LTS, while this PR only works with 20.04. Also, we will want to support rpm
platforms, and thus the installation commands would be failing there.
And the migrating certs
scripts for 0.2.0 we should maybe not even bother by now.
Nevertheless we will not burden this PR to address these things, I made myself a note and we will integrate fixes in follow-up PRs.
In short: thank you for this great contribution, highly appreciated!
And...further ones always welcomed, of course.
Thanks, I'll take a look at the other requirements this weekend. I want to add Raspberry Pi OS as well. |
This playbook
/usr/bin
/var/lib/ava/db
as the database/var/log/ava
as the log destinationAn example run (click to expand)
TODO
/usr/local