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

Removes the errexit shell option from generate_network_config.sh #276

Merged
merged 2 commits into from
Oct 30, 2024

Conversation

nkinkade
Copy link
Contributor

@nkinkade nkinkade commented Oct 30, 2024

The script runs a loop waiting for /sys/class/netc/ethN directories to exist. With errexit enabled, the script would terminate when those directories did not exist, which is counterproductive in this case. I'm sure there is some other way to get around this while still leaving the errexit option enabled, but this seemed pretty simple. Indeed, this script never had any shell options set until I added them just a week or two ago while doing other work.

This PR also modifies the configure_tc_fq.sh script to operate on the /sys/class/net/ethN/carrier file instead of "operstate", which is generate_network_config.sh already does, and was just an oversight on my part to not change it earlier, too.

Also, add set -ux to generate_network_config.sh, just to make debugging easier in the future if we have any issues with the script.


This change is Reviewable

The script runs a loop that checks for a files that may not exist. With -e set,
if it tries to list the files and they don't exist then the script exits. This
commit simply removes the -e shell option. There is likely some other
workaround, but this is pretty simple.
The file /sys/class/net/ethN/carrier is what the generate-network-config script
uses, and is probably more appropriate to the intention, and it was just an
oversight that this file didn't previously get changed too.
Copy link
Contributor

@stephen-soltesz stephen-soltesz left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

@nkinkade nkinkade merged commit ead392a into main Oct 30, 2024
5 checks passed
@nkinkade nkinkade deleted the sandbox-kinkade branch October 30, 2024 19:40
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.

2 participants