Skip to content
This repository has been archived by the owner on May 30, 2022. It is now read-only.

Add new documentation about the runner component #191

Merged
merged 5 commits into from Sep 8, 2021

Conversation

arbulu89
Copy link
Contributor

@arbulu89 arbulu89 commented Aug 5, 2021

New entry in the README file about the usage of the trento runner.

I would appreciate if anyway follows the quickstart to see if it works fine

@rtorrero
Copy link
Contributor

Hey @arbulu89, I think you forgot to update the index on the top of the document :-) Appart from that, lgtm

Copy link
Contributor

@rtorrero rtorrero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add a index entry in line 29

Copy link
Member

@fabriziosestito fabriziosestito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I left just some comments about the ansible/ara installation process.

Also, I tried to follow the steps to setup a k3s /helm chart installation of the runner and it looks good!

README.md Outdated

# Enable the SSH connections to the agents machines
# Update the /etc/hosts file to identify the agents hosts
# Authorize SSH connection for all the agents hosts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit confused about this part, even if it works fine by adding the entries to the /etc/hosts files I wouldn't ask the user to do so. We already have a service discovery in place, we might want to use that.The consul server DNS should be possibly used to resolve the nodes.

EDIT:
Just found out that we can add ansible_host=<node_ip> to the ansible inventory to allow ansible to "resolve" the hosts, this is quite easy to do in the ansible template generation, we tested it and it works as expected. See: https://docs.ansible.com/ansible/latest/reference_appendices/special_variables.html#connection-variables

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code updated to work without editing the host file.
I will update the comments here to make sure that the unique mandatory part is the ssh authorization

@arbulu89
Copy link
Contributor Author

@rtorrero @fabriziosestito Content updated with your suggestions

README.md Outdated
ssh-copy-id 192.168.100.1

# Start the Trento runner
./trento runner start --ara-server http://araIP:port --consul-add consulIP:port -i 5
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo here...
--consul-addr

@arbulu89
Copy link
Contributor Author

arbulu89 commented Sep 2, 2021

@fabriziosestito @rtorrero This is ready to be reviewed too!

@arbulu89
Copy link
Contributor Author

arbulu89 commented Sep 7, 2021

@fabriziosestito @rtorrero @stefanotorresi
Pinging again! Needing a review here

README.md Show resolved Hide resolved
@arbulu89 arbulu89 dismissed rtorrero’s stale review September 8, 2021 12:11

Changes done and approved by Fabrizio

@arbulu89 arbulu89 merged commit 736fe45 into trento-project:runner Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants