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

feat: Add cli for simple gateway registration #11195

Merged
merged 1 commit into from Mar 21, 2022

Conversation

christinewang5
Copy link
Contributor

@christinewang5 christinewang5 commented Jan 18, 2022

Summary

register CLI for gateways

Testing

manually tested by running CLIs
image

Note: this PR is dependent on #11169 & #11692

@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines. label Jan 18, 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 component: cwf component: feg FEG-gateway related issues component: orc8r Orchestrator-related issue labels Jan 18, 2022
@christinewang5 christinewang5 changed the title Register gateway script feat: Add cli for simple gateway registration Jan 18, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jan 18, 2022

Oops! Looks like you failed the DCO check. Be sure to sign all your commits.

Howto

♻️ Updated: ✅ The check is passing the DCO check after the last commit.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 18, 2022

Oops! Looks like you failed the Python Format Check.

Howto

♻️ Updated: ✅ The check is passing the Python Format Check after the last commit.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 18, 2022

feg-workflow

    2 files  202 suites   34s ⏱️
363 tests 363 ✔️ 0 💤 0
377 runs  377 ✔️ 0 💤 0

Results for commit b0f7101.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 18, 2022

cloud-workflow

    7 files  351 suites   2m 1s ⏱️
960 tests 960 ✔️ 0 💤 0

Results for commit b0f7101.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 18, 2022

agw-workflow

  28 files    43 suites   4m 23s ⏱️
738 tests 729 ✔️ 9 💤 0
739 runs  730 ✔️ 9 💤 0

Results for commit b0f7101.

♻️ This comment has been updated with latest results.

@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 Feb 4, 2022
@christinewang5 christinewang5 force-pushed the register-gateway-script branch 3 times, most recently from 0dc08d2 to 8403f87 Compare February 4, 2022 19:12
@christinewang5 christinewang5 force-pushed the register-gateway-script branch 2 times, most recently from f2158bd to 9fb1e1d Compare February 15, 2022 20:13
@github-actions github-actions bot removed the component: feg FEG-gateway related issues label Feb 15, 2022
@github-actions github-actions bot added the component: nms NMS-related issue label Feb 15, 2022
@github-actions
Copy link
Contributor

nms-workflow

0 files  0 suites   0s ⏱️
0 tests 0 ✔️ 0 💤 0

Results for commit cc58f43.

@github-actions github-actions bot removed the component: nms NMS-related issue label Feb 22, 2022
@christinewang5 christinewang5 force-pushed the register-gateway-script branch 2 times, most recently from 1a1f771 to c0b4922 Compare February 22, 2022 18:11
@christinewang5 christinewang5 marked this pull request as ready for review February 22, 2022 18:11
Copy link
Contributor

@ardzoht ardzoht left a comment

Choose a reason for hiding this comment

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

lgtm

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.

Looking good! Couple small things

orc8r/gateway/python/magma/common/service_registry.py Outdated Show resolved Hide resolved
Returns:
grpc channel
"""
rootca_path = ROOTCA_PATH if not rootca_path else rootca_path
Copy link
Contributor

Choose a reason for hiding this comment

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

So if no rootca path is passed in, we want to use the default system credentials. I think that means you don't have to pass any ssl creds, but not sure how it works. But if you want, we can leave it like this for now, and I'll test it when I get around to making the Orc8r<>Let'sEncrypt work e2e

orc8r/gateway/python/scripts/register.py Outdated Show resolved Hide resolved
orc8r/gateway/python/scripts/register.py Show resolved Hide resolved
orc8r/gateway/python/scripts/register.py Show resolved Hide resolved
@christinewang5 christinewang5 force-pushed the register-gateway-script branch 4 times, most recently from 8a5cacb to b0f7101 Compare March 9, 2022 16:35
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!

if res.HasField("error"):
raise Exception(res.error)

msg = textwrap.dedent(
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I'd prob put this printed msg in the main func, to keep this func as "stateless" as possible

if __name__ == "__main__":
main()
print("> Running checkin_cli")
checkin_cli.main()
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh dang does this work? Nice good to know

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 looks a bit sketch but it does 🤷🏻‍♀️

@hcgatewood hcgatewood enabled auto-merge (squash) March 17, 2022 20:36
@christinewang5 christinewang5 force-pushed the register-gateway-script branch 2 times, most recently from 813b111 to 34c74be Compare March 21, 2022 17:37
Signed-off-by: Christine Wang <christinewang@fb.com>
@hcgatewood hcgatewood merged commit f09f3fe into magma:master Mar 21, 2022
emakeev pushed a commit to emakeev/magma that referenced this pull request Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: orc8r Orchestrator-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