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

Use Stratum device config binary format #118

Merged
merged 9 commits into from
Oct 23, 2020
Merged

Conversation

Yi-Tseng
Copy link
Collaborator

@Yi-Tseng Yi-Tseng commented Oct 22, 2020

This pull request change the pipeline config format for all profiles and platforms

Parts modified:

  • build.sh script: use pipeline builder container to create the pipeline config instead of the tarball.
  • PTF tests: use the new pipeline, and set the device to stratum-bfrt for all cases since both target can use that format.
  • Pipeconf: both stratum_bf and stratum_bfrt uses the new pipeline config format.

TODO:

  • Should we merge stratum_bf and stratum_bfrt pipeconf? Both targets can now use the same pipeline config format
  • Should we use the latest version/tag of the stratum-bf-pipeline-builder container image? (not harmful to just use the latest version with shasum for now)

Closes #39

p4src/build.sh Outdated Show resolved Hide resolved
p4src/build.sh Outdated Show resolved Hide resolved
ptf/tests/ptf/ptf_runner.py Outdated Show resolved Hide resolved
ptf/run/tm/Makefile Outdated Show resolved Hide resolved
ptf/tests/ptf/ptf_runner.py Outdated Show resolved Hide resolved
ptf/tests/ptf/ptf_runner.py Outdated Show resolved Hide resolved
Yi Tseng and others added 4 commits October 22, 2020 15:03
 - Merge profiles for stratum_bf and stratum_bfrt
Co-authored-by: Carmelo Cascone <carmelo@opennetworking.org>
@Yi-Tseng Yi-Tseng changed the title [WIP] Use Stratum device config binary format Use Stratum device config binary format Oct 22, 2020
Copy link
Member

@pudelkoM pudelkoM left a comment

Choose a reason for hiding this comment

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

Considering the bfrt branch is still under active development, pinning the pipeline builder by checksum is probably not a bad idea. We can switch to a release-tagged version once we have a Stratum release with the builder in it.

@codecov
Copy link

codecov bot commented Oct 22, 2020

Codecov Report

Merging #118 into master will increase coverage by 1.10%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #118      +/-   ##
============================================
+ Coverage     68.59%   69.70%   +1.10%     
  Complexity      224      224              
============================================
  Files            17       17              
  Lines          1640     1614      -26     
  Branches        135      133       -2     
============================================
  Hits           1125     1125              
+ Misses          440      414      -26     
  Partials         75       75              
Impacted Files Coverage Δ Complexity Δ
.../org/stratumproject/fabric/tna/PipeconfLoader.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 155c142...bd145fc. Read the comment docs.

@Yi-Tseng Yi-Tseng merged commit 5ab1073 into master Oct 23, 2020
@Yi-Tseng Yi-Tseng deleted the new-pipeline-format branch October 23, 2020 22:28
bocon13 pushed a commit that referenced this pull request Dec 7, 2020
* Use Stratum device config binary format

* Simplify the pipeconf

 - Merge profiles for stratum_bf and stratum_bfrt

* remove device parameter from ptf_runner

* fix tests

* Apply suggestions from code review

Co-authored-by: Carmelo Cascone <carmelo@opennetworking.org>

* Minor cleanup to build.sh

* add pipeline config build image sha

* remove stratum_bf from pipeconf name

Co-authored-by: Carmelo Cascone <carmelo@opennetworking.org>
bocon13 pushed a commit that referenced this pull request Dec 7, 2020
* Use Stratum device config binary format

* Simplify the pipeconf

 - Merge profiles for stratum_bf and stratum_bfrt

* remove device parameter from ptf_runner

* fix tests

* Apply suggestions from code review

Co-authored-by: Carmelo Cascone <carmelo@opennetworking.org>

* Minor cleanup to build.sh

* add pipeline config build image sha

* remove stratum_bf from pipeconf name

Co-authored-by: Carmelo Cascone <carmelo@opennetworking.org>
bocon13 pushed a commit that referenced this pull request Dec 8, 2020
* Use Stratum device config binary format

* Simplify the pipeconf

 - Merge profiles for stratum_bf and stratum_bfrt

* remove device parameter from ptf_runner

* fix tests

* Apply suggestions from code review

Co-authored-by: Carmelo Cascone <carmelo@opennetworking.org>

* Minor cleanup to build.sh

* add pipeline config build image sha

* remove stratum_bf from pipeconf name

Co-authored-by: Carmelo Cascone <carmelo@opennetworking.org>
bocon13 pushed a commit that referenced this pull request Dec 8, 2020
* Use Stratum device config binary format

* Simplify the pipeconf

 - Merge profiles for stratum_bf and stratum_bfrt

* remove device parameter from ptf_runner

* fix tests

* Apply suggestions from code review

Co-authored-by: Carmelo Cascone <carmelo@opennetworking.org>

* Minor cleanup to build.sh

* add pipeline config build image sha

* remove stratum_bf from pipeconf name

Co-authored-by: Carmelo Cascone <carmelo@opennetworking.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use bfrt DeviceConfigBuilder tool
3 participants