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

Python Script for JSON Config Files #218

Merged
merged 48 commits into from
Jul 31, 2020

Conversation

catherinemeadows
Copy link
Contributor

Related to issue #184. Created a python script that parses a JSON config file to start up a chain of NFs.

Summary:

This feature makes it easier for the user to launch a chain of NFs by running a python script that parses a JSON Config file containing the desired NFs and parameters. To run the script (within the /examples folder): python config.py [JSON Config file name]. Normal shutdown of NFs occurs on user termination (ctrl+c). Shutdown of NFs also occurs if any NF terminates unexpectedly or fails to start up.

Documentation for using this feature was added here and here.

Usage:

This PR includes
Resolves issues
Breaking API changes
Internal API changes
Usability improvements 👍
Bug fixes
New functionality
New NF/onvm_mgr args
Changes to starting NFs
Dependency updates
Web stats updates

Merging notes:

  • Dependencies: None

TODO before merging :

  • PR is ready for review

Test Plan:

To test, I ran the python script to start up chains of NFs. Screenshots from running the script on the example_chain.json file in /examples:

test_1

While running, processes are killed on user shutdown (Ctrl + C):

user_exit

I also tested the script in the case that all NFs do not start (i.e. one or more NF has some error). The script shuts down all processes in this case and outputs the error from the failed NF(s). For this test case, the same config file was used but with a '-d' flag purposefully omitted from the second set of parameters for the speed_tester.
error_exit

koolzz and others added 27 commits June 4, 2019 14:45
This PR releases OpenNetVM v19.05
This PR adds the following bug fix to the master branch:
[Bug Fix] Fix Typo in Console Stats Header (sdnfv#142)
[Bug Fix] Fix Stats Header in Release Notes (sdnfv#145)
This PR releases OpenNetVM v19.07
@onvm
Copy link

onvm commented Jun 4, 2020

In response to PR creation

CI Message

Your results will arrive shortly

onvm
onvm previously approved these changes Jun 4, 2020
Copy link

@onvm onvm left a comment

Choose a reason for hiding this comment

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

In response to PR creation

CI Message

Run successful see results:
Test took: 5 minutes, 32 seconds
✔️ Pktgen performance check passed
✔️ Speed Test performance check passed

[Results from nimbnode23]

  • Median TX pps for Pktgen: 7721721
    Performance rating - 100.28% (compared to 7700000 average)

  • Median TX pps for Speed Tester: 42065821
    Performance rating - 100.16% (compared to 42000000 average)

@kevindweb
Copy link
Contributor

@onvm again pls

@onvm
Copy link

onvm commented Jul 4, 2020

@onvm again pls

CI Message

Your results will arrive shortly

Copy link

@onvm onvm left a comment

Choose a reason for hiding this comment

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

@onvm again pls

CI Message

Run successful see results:
Test took: 4 minutes, 3 seconds
✔️ Pktgen performance check passed
✔️ Speed Tester performance check passed
❌ NF chain failed with errors, see below

[Results from nimbnode23]

  • Median TX pps for Pktgen: 7754261
    Performance rating - 100.00% (compared to 7700000 average)

  • Median TX pps for Speed Tester: 42247377
    Performance rating - 100.59% (compared to 42000000 average)

NF Chain Results

Simple Forward 2
NF tag: simple_forward
NF instance ID: 5
NF service ID: 2
NF assigned core: 6
----------------------------------------------------
RX total: 47793406
RX total dropped: 0
TX total: 47778368
TX total dropped: 0
NF sent out: 0
NF sent to NF: 47778368
NF dropped: 0
NF next: 0
NF tx buffered: 0
NF tx returned: 0
Speed Tester 1 (errors)
APP: Finished Process Init.
Sending NF_READY message to manager...
Creating 16000 packets to send to 2
Failed to allocate packets
�[2J�[1;1HTotal packets:  10000000 
TX pkts per second:     27666 
Initial packets created: 15038
Speed Tester 3
NF tag: speed_tester
NF instance ID: 3
NF service ID: 3
NF assigned core: 4
----------------------------------------------------
RX total: 237022880
RX total dropped: 0
TX total: 237022880
TX total dropped: 0
NF sent out: 0
NF sent to NF: 237018450
NF dropped: 0
NF next: 0
NF tx buffered: 0
NF tx returned: 16000

@WilliamMaa
Copy link
Contributor

@catherinemeadows on line 190, can you change the print to print("Starting %s %s" % (nf, cmd), flush=True). Sorry but I need this to make the python server be able to properly close the nf chain.

@dennisafa
Copy link
Member

This looks good to go from my end but I'll let @WilliamMaa make the call since you were assigned

@twood02
Copy link
Member

twood02 commented Jul 22, 2020

@catherinemeadows and @WilliamMaa will discuss the NF shutdown issue today. @catherinemeadows will post example JSON file

@twood02 twood02 added this to the ONVM 20.07 milestone Jul 22, 2020
@catherinemeadows
Copy link
Contributor Author

catherinemeadows commented Jul 22, 2020

example JSON config file (this is the one that will be included in the /examples folder:

{
        "globals": [
                {
                        "TTL": 10
                },
                {
                        "directory": "output_files"
                }
        ],
        "simple_forward": [
                {
                        "parameters": "2 -d 1"
                }
        ],
        "speed_tester": [
                {
                        "parameters": "1 -d 2 -c 16000"
                },
		{
			"parameters": "3 -d 3 -c 16000"
                }
        ]
}

@twood02
Copy link
Member

twood02 commented Jul 22, 2020

Are there any other global parameters and if so are they defined in the Readme?

Could you easily add a directory-prefix option, which would let you give a dir name which would then be appended with the date/time? This would be helpful for organizing different experiment runs.

@catherinemeadows
Copy link
Contributor Author

Those are the only two globals I have now. I can add a directory-prefix option. I wrote documentation for the globals and how to run the script here: https://github.com/catherinemeadows/openNetVM/blob/configScript/docs/NF_Dev.md

Copy link
Contributor

@WilliamMaa WilliamMaa left a comment

Choose a reason for hiding this comment

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

Can you briefly explain what the changes are in simple_tb.c? Does it need a separate test plan for it? @catherinemeadows

@catherinemeadows
Copy link
Contributor Author

@WilliamMaa that was just a git mess up on my end. I'll get rid of those changes

WilliamMaa
WilliamMaa previously approved these changes Jul 27, 2020
Copy link
Contributor

@WilliamMaa WilliamMaa left a comment

Choose a reason for hiding this comment

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

The rest seems good to me, I think this is ready for merge!

docs/NF_Dev.md Outdated
@@ -99,17 +103,58 @@ values.
}
```

Additionally, a developer can run the [config python script](../examples/config.py) to deploy multiple network functions, including linear or circular chains of multiple NFs, from a JSON config file. An example config file can be found [here](../examples/example_nf_deploy.json). In the config file, a user can specify a number of seconds for the NFs to run before shutdown with the global "TTL". If no timeout is specified, the NFs will run until a raised error or a manual shutdown (Ctrl + C).
When the NFs are launched, if the user specifies a directory name with the global "directory", a directory will be created (if it does not already exist) with the user-specified name. If a global "directory-prefix" is specified, a directory will be created with the specified prefix + timestamp. If no directory name or prefix is specified, the default name of the created directory will the be the timestamp. Output from each NF will be continuously written to the corresponding log text file within the created or pre-existing directory. Format of the log file name is as follows: "log-NF name-instance ID". To track the output of a NF:
Copy link
Member

Choose a reason for hiding this comment

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

This paragraph is pretty dense. Can you split it up? Or give a paragraph description and then a few bullet points explaining specific details?

One important point not mentioned is that the "NF Name" must match the name of a folder in the examples directory (right?)

docs/NF_Dev.md Outdated
@@ -99,17 +103,58 @@ values.
}
```

Additionally, a developer can run the [config python script](../examples/config.py) to deploy multiple network functions, including linear or circular chains of multiple NFs, from a JSON config file. An example config file can be found [here](../examples/example_nf_deploy.json). In the config file, a user can specify a number of seconds for the NFs to run before shutdown with the global "TTL". If no timeout is specified, the NFs will run until a raised error or a manual shutdown (Ctrl + C).
Copy link
Member

Choose a reason for hiding this comment

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

Put a heading above this like ## Running Groups of NFs since this isn't really that tightly connected to the config file library (totally different other than using json, in fact)


```
python3 config.py <config file name>
```
Copy link
Member

Choose a reason for hiding this comment

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

Let's rename config.py to run_group.py or something similar. Currently it could be mistaken for a tool that helps configure ONVM. It is really more about running things than configuring them.

(config.py is a default name for configuring python packages)

docs/NF_Dev.md Show resolved Hide resolved
@twood02 twood02 added the ready-for-gatekeeper 🚪 PRs that have been approved and are ready for a final review/merge label Jul 29, 2020
Copy link
Contributor

@EthanBaron14 EthanBaron14 left a comment

Choose a reason for hiding this comment

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

Just tested this. Looks good! Noticed one error when you try to give bad JSON input but otherwise works really well. If we can get some nice error messages, maybe like a checkJSON() method or something like that, we should be good to merge.

Copy link
Contributor

@EthanBaron14 EthanBaron14 left a comment

Choose a reason for hiding this comment

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

Talked to Cat about the questions I had previously. Everything else looks good. Approving, ready to merge.

@dennisafa dennisafa merged commit c33c230 into sdnfv:develop Jul 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🚀 ready-for-gatekeeper 🚪 PRs that have been approved and are ready for a final review/merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants