Skip to content
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

docs(orc8r): Added docs for simple gateway registration #12263

Merged
merged 1 commit into from Mar 29, 2022

Conversation

christinewang5
Copy link
Contributor

Added docs for simple gateway registration

@christinewang5 christinewang5 requested review from hcgatewood and a team March 24, 2022 06:44
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines. label Mar 24, 2022
@github-actions
Copy link
Contributor

Thanks for opening a PR! 💯

A couple initial guidelines

Howto

  • Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
  • Checks. All required CI checks must pass before merge.
  • Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

If this is your first Magma PR, also consider reading

@github-actions github-actions bot added the component: docs Documentation-related issue label Mar 24, 2022
docs/docusaurus/i18n/en.json Outdated Show resolved Hide resolved
docs/docusaurus/i18n/en.json Outdated Show resolved Hide resolved
Copy link
Contributor

@hcgatewood hcgatewood left a comment

Choose a reason for hiding this comment

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

Lgtm! Couple comments, msg when ready to merge

docs/readmes/orc8r/dev_gateway_registration.md Outdated Show resolved Hide resolved
docs/readmes/orc8r/dev_gateway_registration.md Outdated Show resolved Hide resolved
Copy link
Contributor

@Neudrino Neudrino left a comment

Choose a reason for hiding this comment

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

Thanks for great additions! 🚀

If we could make "misspell" happy, that would be nice. 😄

## Overview

The overview of gateway registration is as follows:
![gateway_registration_overview](assets/orc8r/gateway_registration_overview.png)
Copy link
Contributor

Choose a reason for hiding this comment

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

The graphics uploaded above seems incomplete. I.e. the lines touching the bottom of the graphics indicate to me, that there is more to come in the process. Could we add a "termination" there indicating that the registration process is finished?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I was using https://sequencediagram.org/ to create the diagram, it seems to format all its sequence diagrams like so

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about using the bottomparticipants keyword in that tool to somehow work around that issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you created the graphics from some text (similar to mermaid), than you would probably also want to add a hidden section with the respective code to the docs.
In Github this would probably be a <!-- HMTL comment block -->, maybe Docusaurus has something similar?

Copy link
Contributor

Choose a reason for hiding this comment

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

If hidden parts are not possible, you might want to use a details block.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @christinewang5 ,
according to this slack message DocuSaurus does support mermaid diagrams. (Also see the examples for Github and Docusaurus).

Could you at least post the code, used to generate the graphics here in the thread, for a later cleanup, please.

docs/readmes/orc8r/dev_gateway_registration.md Outdated Show resolved Hide resolved
docs/readmes/orc8r/dev_gateway_registration.md Outdated Show resolved Hide resolved
docs/readmes/orc8r/dev_gateway_registration.md Outdated Show resolved Hide resolved
It can optionally set the root CA file with the `--ca-file CA_FILE` flag or disable writing to the control proxy file with the `--no-control-proxy` flag.

```shell
sudo /home/vagrant/build/python/bin/python3 ~/magma/orc8r/gateway/python/scripts/register.py [-h] [--ca-file CA_FILE] [--cloud-port CLOUD_PORT] [--no-control-proxy] DOMAIN_NAME REGISTRATION_TOKEN
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason to use sudo here? I do not see any immediate necessity. 😇

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a sentence to explain why we need sudo permission :)

Comment on lines 42 to 54
```shell
sudo /home/vagrant/build/python/bin/python3 ~/magma/orc8r/gateway/python/scripts/register.py [-h] [--ca-file CA_FILE] [--cloud-port CLOUD_PORT] [--no-control-proxy] DOMAIN_NAME REGISTRATION_TOKEN
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we give context, to where this script is executed? And who does execute it?
From the scripts itself it seems as if this is development-VM specific. However, that it is not necessarily correct for a (bare-metal) production deployment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, good point, added the shell prompt in front MAGMA-VM [/home/vagrant]$ to indicate

@christinewang5 christinewang5 force-pushed the simple-gw-reg-docs branch 2 times, most recently from 793a437 to 4af7af0 Compare March 25, 2022 15:26
Signed-off-by: Christine Wang <christinewang@fb.com>
@pull-request-size pull-request-size bot added size/L Denotes a Pull Request that changes 100-499 lines. and removed size/M Denotes a PR that changes 30-99 lines. labels Mar 25, 2022
@github-actions
Copy link
Contributor

Oops! Looks like you failed the Markdown lint check.

Howto

For example, in a testing environment with the `rootCA.pem` and `control_proxy.yml` configured, the operator could run

```shell
MAGMA-VM [/home/vagrant]$ sudo /home/vagrant/build/python/bin/python3 ~/magma/orc8r/gateway/python/scripts/register.py magma.test reg_t5S4zjhD0tXRTmkYKQoN91FmWnQSK2 --cloud-port 7444 --no-control-proxy
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
MAGMA-VM [/home/vagrant]$ sudo /home/vagrant/build/python/bin/python3 ~/magma/orc8r/gateway/python/scripts/register.py magma.test reg_t5S4zjhD0tXRTmkYKQoN91FmWnQSK2 --cloud-port 7444 --no-control-proxy
MAGMA-VM [/home/vagrant]$ sudo /home/vagrant/build/python/bin/python3 ~/magma/orc8r/gateway/python/scripts/register.py magma.test reg_t5S4zjhD0tXRTmkYKQoN91FmWnQSK2 --cloud-port 7444 --no-control-proxy

@hcgatewood hcgatewood merged commit cadb1d4 into magma:master Mar 29, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Mar 29, 2022

Unit Test Results

157 files  157 suites   2h 4m 6s ⏱️
307 tests 304 ✔️ 3 💤 0

Results for commit cadb1d4.

♻️ This comment has been updated with latest results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: docs Documentation-related issue size/L Denotes a Pull Request that changes 100-499 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants