Raise priority of ParallelCluster configured route tables #2857
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit fixes a bug from aws#2855, which made the route table/metric number larger (meaning lower priority). Thereafter, some unwanted default rules on AL2023 took priority and failed test_multiple_nics integration test on AL2023. This commit makes the number smaller (meaning higher priority) to fix the issue. e.g. Prior to this commit, the number for table for 1,1 is 1001001. After this commit, the number is 101+10=111. The "+10" is to properly handle table for 0,0, which has number 10. Without "+10", the table would conflict with table 0 from OS. FYI: the number of unwanted default AL2023 rule starts with 10101 Signed-off-by: Hanwen <[email protected]>
hgreebe
approved these changes
Dec 24, 2024
hanwen-cluster
added a commit
to hanwen-cluster/aws-parallelcluster-cookbook
that referenced
this pull request
Dec 31, 2024
aws#2855 made the pcluster route table/metric number larger (meaning lower priority). Thereafter, some unwanted default rules on AL2023 ec2-net-utils took priority and failed test_multiple_nics integration test on AL2023. Then, aws#2857 made the number too small, interfering route table configurations from IMDS on AL2. Therefore, this commit tries to imitate the priority prior to these two PRs. This is not the cleanest fix, because it is staying in the lucky priority rand instead of fully resolving the issue (i.e. prevent IMDS and ec2-net-utils from configuring the route tables). However, this commit is the least breaking change. So I propose to go with this commit. Metric number range before the two PRs Network card (0,0): 1000 Network card (0,1): 1000 (which was causing conflicts and the reason for all these PRs) Network card (n,1): 100n (for p5, which has 32 network card, it will be 1000-10031) Metric number range after the first PR: Network card (0,0): 1000000 Network card (0,1): 1000001 (conflict fixed :) ) Network card (n,1): 1000001+n*1000 (for p5, it will be 1000000-1031001) Metric number range after the second PR: Network card (0,0): 10 Network card (0,1): 75 Network card (n,1): 0x(hexadecimal number)n01+10 (for p5, it will be 10-12555. The hexadecimal number was accidentally introduced because bash automatically interpret numbers start with "00" as hexadecimal number) Metric number range after this commit: Network card (0,0): 1000 Network card (0,1): 1001 Network card (n,1): n01+1000 (for p5, it will be 1000-4101) Signed-off-by: Hanwen <[email protected]>
hanwen-cluster
added a commit
that referenced
this pull request
Dec 31, 2024
#2855 made the pcluster route table/metric number larger (meaning lower priority). Thereafter, some unwanted default rules on AL2023 ec2-net-utils took priority and failed test_multiple_nics integration test on AL2023. Then, #2857 made the number too small, interfering route table configurations from IMDS on AL2. Therefore, this commit tries to imitate the priority prior to these two PRs. This is not the cleanest fix, because it is staying in the lucky priority rand instead of fully resolving the issue (i.e. prevent IMDS and ec2-net-utils from configuring the route tables). However, this commit is the least breaking change. So I propose to go with this commit. Metric number range before the two PRs Network card (0,0): 1000 Network card (0,1): 1000 (which was causing conflicts and the reason for all these PRs) Network card (n,1): 100n (for p5, which has 32 network card, it will be 1000-10031) Metric number range after the first PR: Network card (0,0): 1000000 Network card (0,1): 1000001 (conflict fixed :) ) Network card (n,1): 1000001+n*1000 (for p5, it will be 1000000-1031001) Metric number range after the second PR: Network card (0,0): 10 Network card (0,1): 75 Network card (n,1): 0x(hexadecimal number)n01+10 (for p5, it will be 10-12555. The hexadecimal number was accidentally introduced because bash automatically interpret numbers start with "00" as hexadecimal number) Metric number range after this commit: Network card (0,0): 1000 Network card (0,1): 1001 Network card (n,1): n01+1000 (for p5, it will be 1000-4101) Signed-off-by: Hanwen <[email protected]>
hanwen-cluster
added a commit
to hanwen-cluster/aws-parallelcluster-cookbook
that referenced
this pull request
Jan 7, 2025
This commit fixes a bug from aws#2855, which made the route table/metric number larger (meaning lower priority). Thereafter, some unwanted default rules on AL2023 took priority and failed test_multiple_nics integration test on AL2023. This commit makes the number smaller (meaning higher priority) to fix the issue. e.g. Prior to this commit, the number for table for 1,1 is 1001001. After this commit, the number is 101+10=111. The "+10" is to properly handle table for 0,0, which has number 10. Without "+10", the table would conflict with table 0 from OS. FYI: the number of unwanted default AL2023 rule starts with 10101 Signed-off-by: Hanwen <[email protected]>
hanwen-cluster
added a commit
to hanwen-cluster/aws-parallelcluster-cookbook
that referenced
this pull request
Jan 7, 2025
aws#2855 made the pcluster route table/metric number larger (meaning lower priority). Thereafter, some unwanted default rules on AL2023 ec2-net-utils took priority and failed test_multiple_nics integration test on AL2023. Then, aws#2857 made the number too small, interfering route table configurations from IMDS on AL2. Therefore, this commit tries to imitate the priority prior to these two PRs. This is not the cleanest fix, because it is staying in the lucky priority rand instead of fully resolving the issue (i.e. prevent IMDS and ec2-net-utils from configuring the route tables). However, this commit is the least breaking change. So I propose to go with this commit. Metric number range before the two PRs Network card (0,0): 1000 Network card (0,1): 1000 (which was causing conflicts and the reason for all these PRs) Network card (n,1): 100n (for p5, which has 32 network card, it will be 1000-10031) Metric number range after the first PR: Network card (0,0): 1000000 Network card (0,1): 1000001 (conflict fixed :) ) Network card (n,1): 1000001+n*1000 (for p5, it will be 1000000-1031001) Metric number range after the second PR: Network card (0,0): 10 Network card (0,1): 75 Network card (n,1): 0x(hexadecimal number)n01+10 (for p5, it will be 10-12555. The hexadecimal number was accidentally introduced because bash automatically interpret numbers start with "00" as hexadecimal number) Metric number range after this commit: Network card (0,0): 1000 Network card (0,1): 1001 Network card (n,1): n01+1000 (for p5, it will be 1000-4101) Signed-off-by: Hanwen <[email protected]>
hanwen-cluster
added a commit
to hanwen-cluster/aws-parallelcluster-cookbook
that referenced
this pull request
Jan 7, 2025
This commit fixes a bug from aws#2855, which made the route table/metric number larger (meaning lower priority). Thereafter, some unwanted default rules on AL2023 took priority and failed test_multiple_nics integration test on AL2023. This commit makes the number smaller (meaning higher priority) to fix the issue. e.g. Prior to this commit, the number for table for 1,1 is 1001001. After this commit, the number is 101+10=111. The "+10" is to properly handle table for 0,0, which has number 10. Without "+10", the table would conflict with table 0 from OS. FYI: the number of unwanted default AL2023 rule starts with 10101 Signed-off-by: Hanwen <[email protected]>
hanwen-cluster
added a commit
to hanwen-cluster/aws-parallelcluster-cookbook
that referenced
this pull request
Jan 7, 2025
aws#2855 made the pcluster route table/metric number larger (meaning lower priority). Thereafter, some unwanted default rules on AL2023 ec2-net-utils took priority and failed test_multiple_nics integration test on AL2023. Then, aws#2857 made the number too small, interfering route table configurations from IMDS on AL2. Therefore, this commit tries to imitate the priority prior to these two PRs. This is not the cleanest fix, because it is staying in the lucky priority rand instead of fully resolving the issue (i.e. prevent IMDS and ec2-net-utils from configuring the route tables). However, this commit is the least breaking change. So I propose to go with this commit. Metric number range before the two PRs Network card (0,0): 1000 Network card (0,1): 1000 (which was causing conflicts and the reason for all these PRs) Network card (n,1): 100n (for p5, which has 32 network card, it will be 1000-10031) Metric number range after the first PR: Network card (0,0): 1000000 Network card (0,1): 1000001 (conflict fixed :) ) Network card (n,1): 1000001+n*1000 (for p5, it will be 1000000-1031001) Metric number range after the second PR: Network card (0,0): 10 Network card (0,1): 75 Network card (n,1): 0x(hexadecimal number)n01+10 (for p5, it will be 10-12555. The hexadecimal number was accidentally introduced because bash automatically interpret numbers start with "00" as hexadecimal number) Metric number range after this commit: Network card (0,0): 1000 Network card (0,1): 1001 Network card (n,1): n01+1000 (for p5, it will be 1000-4101) Signed-off-by: Hanwen <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit fixes a bug from #2855, which made the route table/metric number larger (meaning lower priority). Thereafter, some unwanted default rules on AL2023 took priority and failed test_multiple_nics integration test on AL2023.
This commit makes the number smaller (meaning higher priority) to fix the issue. e.g.
Prior to this commit, the number for table for 1,1 is 1001001. After this commit, the number is 101+10=111. The "+10" is to properly handle table for 0,0, which has number 10. Without "+10", the table would conflict with table 0 from OS.
FYI: the number of unwanted default AL2023 rule starts with 10101
Tests
Checklist
developadd the branch name as prefix in the PR title (e.g.[release-3.6]).Please review the guidelines for contributing and Pull Request Instructions.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.