fix(agw): updated docker install script for agw by ShubhamTatvamasi · Pull Request #13726 · magma/magma

ShubhamTatvamasi · 2022-08-24T20:10:18Z

Signed-off-by: Shubham Tatvamasi [email protected]

Summary

Test Plan

Additional Information

This change is backwards-breaking

github-actions · 2022-08-24T20:10:31Z

Thanks for opening a PR! 💯

A couple initial guidelines

All commits must be signed off. This is enforced by DCO check.
All PR titles must follow the semantic commits format. This is enforced by Semantic PR.

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

Contributing Conventions for norms around contributed code

If this is your first Magma PR, also consider reading

Developer Onboarding for onboarding as a new Magma developer
Development Workflow for guidance on your first pull request
CI Checks for points of contact for failing or flaky CI checks
GitHub-to-Slack mappings for Magma maintainers for guidance on how to contact maintainers on Slack

github-actions · 2022-08-24T20:34:37Z

feg-workflow

    2 files 203 suites 40s ⏱️
374 tests 374 ✔️ 0 💤 0 ❌
388 runs 388 ✔️ 0 💤 0 ❌

Results for commit ce7769d.

♻️ This comment has been updated with latest results.

github-actions · 2022-08-24T20:46:46Z

dp-workflow

13 tests 13 ✔️ 2m 50s ⏱️
  1 suites   0 💤
  1 files   0 ❌

Results for commit ce7769d.

♻️ This comment has been updated with latest results.

github-actions · 2022-08-24T21:14:30Z

agw-workflow

615 tests 611 ✔️ 5m 9s ⏱️
    2 suites     4 💤
    2 files     0 ❌

Results for commit ce7769d.

♻️ This comment has been updated with latest results.

maxhbr

I do not think, that this is a fix.

maxhbr · 2022-08-25T05:47:27Z

 NEED_REBOOT=0
 WHOAMI=$(whoami)
-MAGMA_VERSION="${MAGMA_VERSION:-v1.7}"
+MAGMA_VERSION="${MAGMA_VERSION:-v1.8}"


your PR is called "updated docker install script for agw", but it also updates debian install?

Reverted this change.

maxhbr · 2022-08-25T05:48:18Z

 x-agw-python-service: &pyservice
  <<: *service
-  image: ${DOCKER_REGISTRY}agw_gateway_python:${IMAGE_VERSION}
+  image: ${DOCKER_REGISTRY}agw_gateway_python${OPTIONAL_ARCH_PREFIX}:${IMAGE_VERSION}


might be my bad, but if it comes in the end it is a POSTFIX and not a PREFIX?

Updated to POSTFIX

maxhbr · 2022-08-25T05:50:08Z

-DOCKER_REGISTRY=
-IMAGE_VERSION=latest
+DOCKER_REGISTRY=docker.artifactory.magmacore.org/
+IMAGE_VERSION=1.8.0


If I understand that correctly, from now on if you call docker-compose build the resulting artifacts will be tagged like docker.artifactory.magmacore.org/agw_gateway_c:1.8.0 locally, instead of agw_gateway_c:latest. Is that desired?

That is not desired for the build. I will revert this change and update the installation script for adding the docker registry.

maxhbr · 2022-08-25T05:53:54Z

-
-# Update this only for ARM node
-sed -i 's/COMPOSE_PROJECT_NAME=agw/COMPOSE_PROJECT_NAME=agw_arm64/' /var/opt/magma/docker/.env
+sudo su - root


sudo su - root is equivalent to sudo -i which is recomended AFAIK.

Instead of becoming root, you can replace the cat below with:

cat << EOF | sudo tee /var/opt/magma/configs/control_proxy.yml

Updated as per your suggestion.

maxhbr · 2022-08-25T05:54:08Z

 ```bash
-sudo su
-cd /root
+sudo su - root


sudo su - root is equivalent to sudo -i which is recomended AFAIK.

Updated this to sudo -i

maxhbr · 2022-08-25T13:27:31Z


 ```bash
-cat << EOF > /var/opt/magma/configs/control_proxy.yml
+cat << EOF | sudo tee /var/opt/magma/configs/control_proxy.yml


I think you need to add sudo in front of the following docker-compose and docker calls.

maxhbr

LGTM

Signed-off-by: Shubham Tatvamasi <[email protected]>

ShubhamTatvamasi · 2022-08-25T15:23:39Z

@tmdzk semantic-pr is stuck, what to do?

tmdzk · 2022-08-25T15:37:40Z

@tmdzk semantic-pr is stuck, what to do?

MagmaCIBot · 2022-08-25T15:54:31Z

💔 All backports failed

Status	Branch	Result
❌	v1.8	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

backport --pr 13726

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

Signed-off-by: Shubham Tatvamasi <[email protected]>

…13737)

ShubhamTatvamasi requested review from a team August 24, 2022 20:10

ShubhamTatvamasi requested review from a team as code owners August 24, 2022 20:10

ShubhamTatvamasi requested review from maxhbr and voisey August 24, 2022 20:10

pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines. label Aug 24, 2022

ShubhamTatvamasi requested review from tmdzk and ulaskozat August 24, 2022 20:10

github-actions bot added component: agw Access gateway-related issue component: ci All updates on CI (Jenkins/CircleCi/Github Action) component: docs Documentation-related issue labels Aug 24, 2022

pull-request-size bot added size/M Denotes a PR that changes 30-99 lines. and removed size/S Denotes a PR that changes 10-29 lines. labels Aug 24, 2022

maxhbr requested changes Aug 25, 2022

View reviewed changes

ShubhamTatvamasi requested a review from maxhbr August 25, 2022 08:31

maxhbr reviewed Aug 25, 2022

View reviewed changes

maxhbr approved these changes Aug 25, 2022

View reviewed changes

ShubhamTatvamasi enabled auto-merge (squash) August 25, 2022 13:39

ShubhamTatvamasi disabled auto-merge August 25, 2022 13:39

ShubhamTatvamasi added 6 commits August 25, 2022 19:10

fix(agw): updated docker install script for agw

a6e7210

Signed-off-by: Shubham Tatvamasi <[email protected]>

updated su command

c45fb01

Signed-off-by: Shubham Tatvamasi <[email protected]>

remvoed extra command

c17d10f

Signed-off-by: Shubham Tatvamasi <[email protected]>

Updated sudo command

f3f8e0c

Signed-off-by: Shubham Tatvamasi <[email protected]>

reverted the 1.7 changes for debian install script

a0898ff

Signed-off-by: Shubham Tatvamasi <[email protected]>

updated prefix to postfix

106c522

Signed-off-by: Shubham Tatvamasi <[email protected]>

ShubhamTatvamasi added 3 commits August 25, 2022 19:10

updated docs for control proxy

95b414b

Signed-off-by: Shubham Tatvamasi <[email protected]>

reverted .env changes

8aa2aed

Signed-off-by: Shubham Tatvamasi <[email protected]>

updated script for adding .env file

ce7769d

Signed-off-by: Shubham Tatvamasi <[email protected]>

ShubhamTatvamasi force-pushed the agw-docker-image-tags-update branch from 131e700 to ce7769d Compare August 25, 2022 13:41

ShubhamTatvamasi enabled auto-merge (squash) August 25, 2022 14:42

sebathomas approved these changes Aug 25, 2022

View reviewed changes

tmdzk approved these changes Aug 25, 2022

View reviewed changes

voisey approved these changes Aug 25, 2022

View reviewed changes

tmdzk disabled auto-merge August 25, 2022 15:38

tmdzk merged commit d122958 into magma:master Aug 25, 2022

ShubhamTatvamasi added v1.8 apply-v1.8 and removed v1.8 labels Aug 25, 2022

ShubhamTatvamasi added a commit to ShubhamTatvamasi/magma that referenced this pull request Aug 25, 2022

fix(agw): updated docker install script for agw (magma#13726)

1cadd07

ShubhamTatvamasi added a commit to ShubhamTatvamasi/magma that referenced this pull request Aug 25, 2022

fix(agw): updated docker install script for agw (magma#13726)

082c1bc

Signed-off-by: Shubham Tatvamasi <[email protected]>

tmdzk pushed a commit that referenced this pull request Aug 25, 2022

fix(agw): updated docker install script for agw (#13726) (#13737)

943249c

crasu pushed a commit to crasu/magma that referenced this pull request Sep 5, 2022

fix(agw): updated docker install script for agw (magma#13726)

c5e22ce

wolfseb pushed a commit to wolfseb/magma that referenced this pull request Oct 3, 2022

fix(agw): updated docker install script for agw (magma#13726) (magma#…

7aaee93

…13737)

jordanvrtanoski pushed a commit to jordanvrtanoski/magma that referenced this pull request Feb 8, 2023

fix(agw): updated docker install script for agw (magma#13726) (magma#…

75b9292

…13737)

Conversation

ShubhamTatvamasi commented Aug 24, 2022

Summary

Test Plan

Additional Information

Uh oh!

github-actions bot commented Aug 24, 2022

Howto

More info

Uh oh!

github-actions bot commented Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

feg-workflow

Uh oh!

github-actions bot commented Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

dp-workflow

Uh oh!

github-actions bot commented Aug 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

agw-workflow

Uh oh!

maxhbr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxhbr left a comment

Choose a reason for hiding this comment

Uh oh!

ShubhamTatvamasi commented Aug 25, 2022

Uh oh!

tmdzk commented Aug 25, 2022

Uh oh!

MagmaCIBot commented Aug 25, 2022

💔 All backports failed

Manual backport

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

github-actions bot commented Aug 24, 2022 •

edited

Loading

github-actions bot commented Aug 24, 2022 •

edited

Loading

github-actions bot commented Aug 24, 2022 •

edited

Loading