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

Remove quagga related code #7476

Merged
merged 11 commits into from
Jun 7, 2021
Merged

Remove quagga related code #7476

merged 11 commits into from
Jun 7, 2021

Conversation

shi-su
Copy link
Contributor

@shi-su shi-su commented Apr 29, 2021

Why I did it

Quagga is no longer being used. Remove quagga related code (e.g., docker-fpm-quagga, sonic-quagga, etc.).

How I did it

Remove quagga related code.

How to verify it

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

@shi-su shi-su marked this pull request as ready for review April 29, 2021 07:01
@shi-su shi-su requested a review from lguohan as a code owner April 29, 2021 07:01
@shi-su shi-su marked this pull request as draft April 29, 2021 07:07
@shi-su shi-su marked this pull request as ready for review May 6, 2021 01:32
@shi-su shi-su requested review from qiluo-msft and xumia as code owners May 6, 2021 01:32
@shi-su shi-su changed the title Remove docker-fpm-quagga Remove quagga related code May 6, 2021
@@ -1,31 +0,0 @@
# docker image for p4 sonic docker image

DOCKER_SONIC_P4 = docker-sonic-p4.gz
Copy link
Collaborator

@qiluo-msft qiluo-msft May 6, 2021

Choose a reason for hiding this comment

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

p4

Is this related to quagga? Why remove? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing p4 related part is because it uses quagga regardless of the routing stack selected. Updated the PR description to clarify that.

@shi-su shi-su changed the title Remove quagga related code Remove quagga and p4 related code May 6, 2021
rules/gobgp.mk Outdated
@@ -1,5 +0,0 @@
# gobgp package
Copy link
Collaborator

Choose a reason for hiding this comment

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

gobgp

gobgp itself is not related to quagga. Why remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the reason is that docker-fpm-gobgp is dependent on docker-fpm-quagga. With docker-fpm-quagga removed, I will need to remove docker-fpm-gobgp. After removing docker-fpm-gobgp, the only place that could use gobgp is docker-sonic-vs. I thought the motivation for keeping gobgp only for docker-sonic-vs is limited. I can undo the removal of gobgp if using it for docker-sonic-vs is necessary or if we have any other scenarios in mind where we should keep gobgp.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Undid removal for gobgp.

Copy link
Collaborator

Choose a reason for hiding this comment

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

let's keep gobgp and p4. only remove quagga.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted removal for gobgp and p4. Note that p4 depends on quagga and docker-fpm-gobgp depends on docker-fpm-quagga, we probably need to do something for these two.

Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

LGTM. Please check with other reviewers.

@qiluo-msft
Copy link
Collaborator

@shi-su Could you update the PR title? I think it is not touching p4.

@shi-su shi-su changed the title Remove quagga and p4 related code Remove quagga related code Jun 1, 2021
@shi-su shi-su merged commit 62a4603 into sonic-net:master Jun 7, 2021
@shi-su shi-su deleted the rm_quagga branch June 7, 2021 23:44
qiluo-msft added a commit that referenced this pull request Jun 9, 2021
qiluo-msft added a commit that referenced this pull request Jun 10, 2021
Reverts #7476
It remove bgpd.conf.j2 and zebra.conf.j2, which is still used by sonic-config-engine unit test.
qiluo-msft pushed a commit that referenced this pull request Jun 16, 2021
#7840)

#### Why I did it
The PR checkers do not re-run the sonic-config-engine test cases, caused by some of the config files changes not detected.

https://sonic-jenkins.westus2.cloudapp.azure.com/job/mellanox/job/buildimage-mlnx-all/660/console
…
07:13:24  ======================================================================
07:13:24  ERROR: test_bgpd_quagga (tests.test_j2files.TestJ2Files)
07:13:24  ----------------------------------------------------------------------
…
07:13:24  ======================================================================
07:13:24  ERROR: test_zebra_quagga (tests.test_j2files.TestJ2Files)
07:13:24  ----------------------------------------------------------------------
…
07:13:24  error: Test failed: <unittest.runner.TextTestResult run=161 errors=2 failures=0>
07:13:24  [  FAIL LOG END  ] [ target/python-wheels/sonic_config_engine-1.0-py2-none-any.whl ]
07:13:24  make: *** [slave.mk:603: target/python-wheels/sonic_config_engine-1.0-py2-none-any.whl] Error 1
07:13:24  Makefile.work:292: recipe for target 'target/sonic-mellanox.bin' failed
07:13:24  make[1]: *** [target/sonic-mellanox.bin] Error 2
07:13:24  make[1]: Leaving directory '/data2/johnar/workspace/mellanox/buildimage-mlnx-all'
07:13:24  Makefile:7: recipe for target 'target/sonic-mellanox.bin' failed
07:13:24  make: *** [target/sonic-mellanox.bin] Error 2

See PR: #7476


#### How I did it
Add the depended files.
See src/sonic-config-engine/tests/test_j2files.py
qiluo-msft pushed a commit that referenced this pull request Jun 16, 2021
#7840)

#### Why I did it
The PR checkers do not re-run the sonic-config-engine test cases, caused by some of the config files changes not detected.

https://sonic-jenkins.westus2.cloudapp.azure.com/job/mellanox/job/buildimage-mlnx-all/660/console
…
07:13:24  ======================================================================
07:13:24  ERROR: test_bgpd_quagga (tests.test_j2files.TestJ2Files)
07:13:24  ----------------------------------------------------------------------
…
07:13:24  ======================================================================
07:13:24  ERROR: test_zebra_quagga (tests.test_j2files.TestJ2Files)
07:13:24  ----------------------------------------------------------------------
…
07:13:24  error: Test failed: <unittest.runner.TextTestResult run=161 errors=2 failures=0>
07:13:24  [  FAIL LOG END  ] [ target/python-wheels/sonic_config_engine-1.0-py2-none-any.whl ]
07:13:24  make: *** [slave.mk:603: target/python-wheels/sonic_config_engine-1.0-py2-none-any.whl] Error 1
07:13:24  Makefile.work:292: recipe for target 'target/sonic-mellanox.bin' failed
07:13:24  make[1]: *** [target/sonic-mellanox.bin] Error 2
07:13:24  make[1]: Leaving directory '/data2/johnar/workspace/mellanox/buildimage-mlnx-all'
07:13:24  Makefile:7: recipe for target 'target/sonic-mellanox.bin' failed
07:13:24  make: *** [target/sonic-mellanox.bin] Error 2

See PR: #7476


#### How I did it
Add the depended files.
See src/sonic-config-engine/tests/test_j2files.py
@shi-su shi-su restored the rm_quagga branch June 16, 2021 18:02
Junchao-Mellanox pushed a commit to Junchao-Mellanox/sonic-buildimage that referenced this pull request Jun 24, 2021
sonic-net#7840)

#### Why I did it
The PR checkers do not re-run the sonic-config-engine test cases, caused by some of the config files changes not detected.

https://sonic-jenkins.westus2.cloudapp.azure.com/job/mellanox/job/buildimage-mlnx-all/660/console
…
07:13:24  ======================================================================
07:13:24  ERROR: test_bgpd_quagga (tests.test_j2files.TestJ2Files)
07:13:24  ----------------------------------------------------------------------
…
07:13:24  ======================================================================
07:13:24  ERROR: test_zebra_quagga (tests.test_j2files.TestJ2Files)
07:13:24  ----------------------------------------------------------------------
…
07:13:24  error: Test failed: <unittest.runner.TextTestResult run=161 errors=2 failures=0>
07:13:24  [  FAIL LOG END  ] [ target/python-wheels/sonic_config_engine-1.0-py2-none-any.whl ]
07:13:24  make: *** [slave.mk:603: target/python-wheels/sonic_config_engine-1.0-py2-none-any.whl] Error 1
07:13:24  Makefile.work:292: recipe for target 'target/sonic-mellanox.bin' failed
07:13:24  make[1]: *** [target/sonic-mellanox.bin] Error 2
07:13:24  make[1]: Leaving directory '/data2/johnar/workspace/mellanox/buildimage-mlnx-all'
07:13:24  Makefile:7: recipe for target 'target/sonic-mellanox.bin' failed
07:13:24  make: *** [target/sonic-mellanox.bin] Error 2

See PR: sonic-net#7476

#### How I did it
Add the depended files.
See src/sonic-config-engine/tests/test_j2files.py
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
Why I did it
Quagga is no longer being used. Remove quagga-related code (e.g., docker-fpm-quagga, sonic-quagga, etc.).

How I did it
Remove quagga-related code.
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
Reverts sonic-net#7476
It remove bgpd.conf.j2 and zebra.conf.j2, which is still used by sonic-config-engine unit test.
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
sonic-net#7840)

#### Why I did it
The PR checkers do not re-run the sonic-config-engine test cases, caused by some of the config files changes not detected.

https://sonic-jenkins.westus2.cloudapp.azure.com/job/mellanox/job/buildimage-mlnx-all/660/console
…
07:13:24  ======================================================================
07:13:24  ERROR: test_bgpd_quagga (tests.test_j2files.TestJ2Files)
07:13:24  ----------------------------------------------------------------------
…
07:13:24  ======================================================================
07:13:24  ERROR: test_zebra_quagga (tests.test_j2files.TestJ2Files)
07:13:24  ----------------------------------------------------------------------
…
07:13:24  error: Test failed: <unittest.runner.TextTestResult run=161 errors=2 failures=0>
07:13:24  [  FAIL LOG END  ] [ target/python-wheels/sonic_config_engine-1.0-py2-none-any.whl ]
07:13:24  make: *** [slave.mk:603: target/python-wheels/sonic_config_engine-1.0-py2-none-any.whl] Error 1
07:13:24  Makefile.work:292: recipe for target 'target/sonic-mellanox.bin' failed
07:13:24  make[1]: *** [target/sonic-mellanox.bin] Error 2
07:13:24  make[1]: Leaving directory '/data2/johnar/workspace/mellanox/buildimage-mlnx-all'
07:13:24  Makefile:7: recipe for target 'target/sonic-mellanox.bin' failed
07:13:24  make: *** [target/sonic-mellanox.bin] Error 2

See PR: sonic-net#7476


#### How I did it
Add the depended files.
See src/sonic-config-engine/tests/test_j2files.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants