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

Move NodeManager over to asynchronous IPCache API #20117

Closed
wants to merge 1 commit into from
Closed

Move NodeManager over to asynchronous IPCache API #20117

wants to merge 1 commit into from

Conversation

joestringer
Copy link
Member

@joestringer joestringer commented Jun 8, 2022 •
edited
Loading

Meta: #21142

Depends on #19765

A few WIP notes:

  • NodeManager expects to complete the IPCache push to then trigger
    updates to datapath routes, encrypt state, etc.; New interface doesn't
    provide this guarantee.
  • There's a bit of weirdness in how IPCache just accepts random numeric
    identities as input, meaning that another node could tell us what its
    identity is but we don't yet know what that identity means(!). How
    should we handle this? Seems like something we could route through the
    ipcache to defer until we understand what it means & resolve policy
    etc....
  • Should 'hostIP' for a (pod / node) ipcache entry be part of 'aux'?
  • We could probably fold Identity source info with encryptKey/tunnel key
    / etc. when generating ipcache entries. Would require making identity
    ipcache updates async though. But maybe that's where we should go
    anyway.

@joestringer joestringer added the release-note/misc This PR makes changes that have no direct user impact. label Jun 8, 2022
@maintainer-s-little-helper

This comment was marked as outdated.

Sign in to view
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jun 8, 2022
@joestringer joestringer mentioned this pull request Jun 14, 2022
Open
@joestringer joestringer force-pushed the pr/joe/ipcache-new branch 3 times, most recently from fd71399 to 536453b Compare June 18, 2022 17:49
@maintainer-s-little-helper

This comment was marked as outdated.

Sign in to view
@joestringer joestringer force-pushed the pr/joe/ipcache-new branch 2 times, most recently from 44af332 to f59a16b Compare June 19, 2022 17:08
@github-actions

This comment was marked as outdated.

Sign in to view
@github-actions github-actions bot added stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. and removed stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. labels Jul 24, 2022
@maintainer-s-little-helper

This comment was marked as outdated.

Sign in to view
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Aug 2, 2022
@joestringer joestringer added this to the 1.13 milestone Aug 2, 2022
@joestringer joestringer mentioned this pull request Aug 4, 2022
Merged
@joestringer joestringer force-pushed the pr/joe/ipcache-new branch 2 times, most recently from 3d357c9 to 801e9a8 Compare August 16, 2022 22:24
@joestringer joestringer force-pushed the pr/joe/ipcache-new branch 3 times, most recently from d15ac32 to 77451b7 Compare August 28, 2022 18:21
@joestringer joestringer mentioned this pull request Aug 30, 2022
Open
29 tasks
@maintainer-s-little-helper

This comment was marked as outdated.

Sign in to view
@maintainer-s-little-helper
Copy link

Commit 2f1a2dc does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

Base automatically changed from pr/joe/ipcache-new to master September 1, 2022 18:43
@joestringer
node: Switch NodeManager to async ipcache API
d7301e0 
This is already a little hairy.

A few notes:
* NodeManager expects to complete the IPCache push to then trigger
  updates to datapath routes, encrypt state, etc.; New interface doesn't
  provide this guarantee.
* There's a bit of weirdness in how IPCache just accepts random numeric
  identities as input, meaning that another node could tell us what its
  identity is but we don't yet know what that identity means(!). How
  should we handle this? Seems like something we could route through the
  ipcache to defer until we understand what it means & resolve policy
  etc....
* Should 'hostIP' for a (pod / node) ipcache entry be part of 'aux'?
* We could probably fold Identity source info with encryptKey/tunnel key
  / etc. when generating ipcache entries. Would require making identity
  ipcache updates async though. But maybe that's where we should go
  anyway.

Signed-off-by: Joe Stringer <joe@cilium.io>
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Sep 1, 2022
@brb brb mentioned this pull request Sep 7, 2022
Merged
1 task
@github-actions
Copy link

github-actions bot commented Oct 2, 2022

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Oct 2, 2022
@gandro gandro removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Oct 2, 2022
@github-actions
Copy link

github-actions bot commented Nov 2, 2022

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Nov 2, 2022
@gandro gandro removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Nov 2, 2022
@gandro gandro mentioned this pull request Nov 10, 2022
Closed
@joestringer joestringer removed this from the 1.13 milestone Dec 2, 2022
@github-actions
Copy link

github-actions bot commented Jan 1, 2023

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jan 1, 2023
@gandro gandro removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Jan 9, 2023
@christarazi christarazi self-assigned this Jan 27, 2023
@github-actions
Copy link

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Feb 27, 2023
@gandro
Copy link
Member

gandro commented Feb 27, 2023

Superseeded by #23208

@gandro gandro closed this Feb 27, 2023
@christarazi christarazi deleted the pr/joe/ipcache-new+node branch March 6, 2023 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@gandro gandro

@christarazi christarazi

Labels
release-note/misc This PR makes changes that have no direct user impact. stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@joestringer @gandro @christarazi