-
Notifications
You must be signed in to change notification settings - Fork 104
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
CNF-13193: Update NTO Hugepages size validation #1086
base: master
Are you sure you want to change the base?
CNF-13193: Update NTO Hugepages size validation #1086
Conversation
@fontivan: This pull request references CNF-13193 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.17.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
Skipping CI for Draft Pull Request. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: fontivan The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
pkg/apis/performanceprofile/v2/performanceprofile_validation.go
Outdated
Show resolved
Hide resolved
pkg/apis/performanceprofile/v2/performanceprofile_validation.go
Outdated
Show resolved
Hide resolved
pkg/apis/performanceprofile/v2/performanceprofile_validation.go
Outdated
Show resolved
Hide resolved
@fontivan: This pull request references CNF-13193 which is a valid jira issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
I like the direction, we should make sure we are not adding too many repeated API server requests though (too many node listings for example). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far
pkg/apis/performanceprofile/v2/performanceprofile_validation.go
Outdated
Show resolved
Hide resolved
pkg/apis/performanceprofile/v2/performanceprofile_validation.go
Outdated
Show resolved
Hide resolved
pkg/apis/performanceprofile/v2/performanceprofile_validation.go
Outdated
Show resolved
Hide resolved
pkg/apis/performanceprofile/v2/performanceprofile_validation.go
Outdated
Show resolved
Hide resolved
79447eb
to
481fee5
Compare
pkg/apis/performanceprofile/v2/performanceprofile_validation.go
Outdated
Show resolved
Hide resolved
pkg/apis/performanceprofile/v2/performanceprofile_validation.go
Outdated
Show resolved
Hide resolved
pkg/apis/performanceprofile/v2/performanceprofile_validation_test.go
Outdated
Show resolved
Hide resolved
481fee5
to
82cb4f4
Compare
cf9e1ed
to
ef5883b
Compare
/test e2e-gcp-pao |
32b9049
to
b70eeaf
Compare
/retest |
1 similar comment
/retest |
b70eeaf
to
f6ee429
Compare
/retest |
f6ee429
to
f18d396
Compare
/test e2e-gcp-pao |
3677c6f
to
2ae47bd
Compare
/retest |
- Include valid sizes for both x86 and aarch64 validation check - Included notes for aarch64 where each kernel page sizes includes only a single valid hugepage size - Fetch a node list once and use it for multiple validation checks - Ensure node list is valid before using it - Clarify preconditions on GetArchitecture helper function - Use separate methods for checking architecture and cpu capacity - Add additional validation to enforce that all nodes that match the selector have the same cpu capacity and architecture - Add additional validation to enforce that the specified hugepage size is valid for the architecture in use
- Update various tests to include architecture specific details where necessary - Add a variety of new tests to cover the newly added code
- It is allowed for a node selector to match zero nodes, this should not return an error - Nodes can be added after the profile is created - If no nodes are currently present it does limit the validation we can perform (No architecture validation without nodes present) - Update profile validation and associated test cases with these changes
2ae47bd
to
7d3b6bc
Compare
/retest |
1 similar comment
/retest |
@fontivan: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
-- Verify all nodes in the list have the same cpu architecture
-- Verify all nodes in the list have the same cpu capacity
-- Verify that the selected hugepage size is valid for the cpu architecture