Skip to content

Conversation

@alan-kut
Copy link
Contributor

@alan-kut alan-kut commented Feb 13, 2024

Node watcher is very heavy when watching
all the nodes, not only the current one.
Nodes are regularly updated, e.g. by kubelet.

Watch only the current node all the time and
other nodes only when holding the drain lock.

@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.

Node watcher is very heavy when watching
all the nodes, not only the current one.
Nodes are regularly updated, e.g. by kubelet.

Watch only the current node all the time and
other nodes only when holding the drain lock.

Signed-off-by: Alan Kutniewski <[email protected]>
@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

Pull Request Test Coverage Report for Build 7889385247

Details

  • -38 of 50 (24.0%) changed or added relevant lines in 1 file are covered.
  • 8 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 30.011%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/daemon/daemon.go 12 50 24.0%
Files with Coverage Reduction New Missed Lines %
pkg/daemon/daemon.go 8 35.46%
Totals Coverage Status
Change from base Build 7875814354: -0.2%
Covered Lines: 3538
Relevant Lines: 11789

💛 - Coveralls

@alan-kut alan-kut marked this pull request as draft February 13, 2024 16:16
@alan-kut alan-kut marked this pull request as ready for review February 15, 2024 09:13
@alan-kut
Copy link
Contributor Author

/test-all

@adrianchiris
Copy link
Collaborator

adrianchiris commented Feb 15, 2024

@alan-kut currently there is work around moving the drain logic to a centralized controller[1]

I think that part of this work, we can limit the node informer to just the current node. WDYT ?

@SchSeba ^^ Thoughts?

[1] #555

@SchSeba
Copy link
Collaborator

SchSeba commented Feb 19, 2024

sure we can do it after we merge #555

@SchSeba
Copy link
Collaborator

SchSeba commented Feb 20, 2024

Hi @alan-kut thanks for the PR!

so the idea base on #555 is that we are not going to watch the node anymore the daemon will only watch the nodeState and in the future also the operator will not watch the nodes as we are going to move the drain request annotation to the nodeState only

@SchSeba
Copy link
Collaborator

SchSeba commented Apr 4, 2024

Hi @alan-kut can you take a look on the operator now that we remove the nodes update from the config daemon we have it only on the controller

@nvidia-ci-cd
Copy link

Can one of the admins verify this patch?

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.

5 participants