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.

Sign up for GitHub

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

Jump to bottom

add sort for the policy on the sriovOperatorConfig controller #721

Merged
merged 1 commit into from
Jun 27, 2024
Merged

add sort for the policy on the sriovOperatorConfig controller #721

merged 1 commit into from
Jun 27, 2024

Conversation

SchSeba
Copy link
Collaborator

@SchSeba SchSeba commented Jun 23, 2024

we need to be consistent with the policy order

@github-actions GitHub Actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@coveralls
Copy link

coveralls commented Jun 23, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9633221770

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 6 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.07%) to 38.686%
Files with Coverage Reduction New Missed Lines %
controllers/sriovnetworkpoolconfig_controller.go 6 63.7%
Totals Coverage Status
Change from base Build 9549902217: 0.07%
Covered Lines: 5187
Relevant Lines: 13408
💛 - Coveralls

ykulazhenkov
ykulazhenkov reviewed Jun 24, 2024
View reviewed changes
Copy link
Collaborator

@ykulazhenkov ykulazhenkov left a comment

Choose a reason for hiding this comment

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

This is required to keep daemon's DS NodeSelector stable?
Can you add a brief comment near the sorting?

zeeke
zeeke reviewed Jun 25, 2024
View reviewed changes
Copy link
Member

@zeeke zeeke left a comment

Choose a reason for hiding this comment

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

LGTM

please consider adding a unit test like this
6a10768

@SchSeba SchSeba force-pushed the fix_policy_order branch from c9a54ac to de0a86e Compare June 26, 2024 05:05
@github-actions GitHub Actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@coveralls
Copy link

coveralls commented Jun 26, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9673856840

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 5 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.005%) to 40.38%
Files with Coverage Reduction New Missed Lines %
controllers/generic_network_controller.go 5 74.53%
Totals Coverage Status
Change from base Build 9667839529: 0.005%
Covered Lines: 5457
Relevant Lines: 13514
💛 - Coveralls

@SchSeba SchSeba force-pushed the fix_policy_order branch from de0a86e to 8b3cbb3 Compare June 26, 2024 15:44
@github-actions GitHub Actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

adrianchiris
adrianchiris approved these changes Jun 27, 2024
View reviewed changes
Copy link
Collaborator

@adrianchiris adrianchiris left a comment

Choose a reason for hiding this comment

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

LGTM. @SchSeba please rebase and we can merge

@SchSeba SchSeba force-pushed the fix_policy_order branch from 8b3cbb3 to 4104cc4 Compare June 27, 2024 14:09
@github-actions GitHub Actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@coveralls
Copy link

coveralls commented Jun 27, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9698175400

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 5 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.08%) to 40.454%
Files with Coverage Reduction New Missed Lines %
controllers/generic_network_controller.go 5 74.53%
Totals Coverage Status
Change from base Build 9691456203: 0.08%
Covered Lines: 5467
Relevant Lines: 13514
💛 - Coveralls

@SchSeba
add sort for the policy on the sriovOperatorConfig controller
91cf86c 
we need to be consistent with the policy order

Signed-off-by: Sebastian Sch <[email protected]>
@SchSeba SchSeba force-pushed the fix_policy_order branch from 4104cc4 to 91cf86c Compare June 27, 2024 15:16
@github-actions GitHub Actions
Copy link

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@coveralls
Copy link

coveralls commented Jun 27, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9699223239

Details

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 17 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.04%) to 40.621%
Files with Coverage Reduction New Missed Lines %
pkg/apply/apply.go 2 67.21%
controllers/helper.go 6 69.47%
controllers/sriovoperatorconfig_controller.go 9 63.76%
Totals Coverage Status
Change from base Build 9699181042: 0.04%
Covered Lines: 5520
Relevant Lines: 13589
💛 - Coveralls

@zeeke zeeke merged commit 10ace35 into k8snetworkplumbingwg:master Jun 27, 2024
13 checks passed
@zeeke zeeke mentioned this pull request Jun 28, 2024
Merged
This was referenced Jul 5, 2024
Merged
Merged
@zeeke zeeke mentioned this pull request Jul 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ykulazhenkov ykulazhenkov ykulazhenkov left review comments

@adrianchiris adrianchiris adrianchiris approved these changes

@zeeke zeeke zeeke approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@SchSeba @coveralls @zeeke @adrianchiris @ykulazhenkov