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.

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

docs: Deprecate CNPStatusUpdates/K8sEventHandover #24464

Merged
merged 1 commit into from Jun 6, 2023

Conversation

marseel
Copy link
Contributor

@marseel marseel commented Mar 20, 2023

CNP Node status updates are known to cause scalability-related issues. Deprecate it in preparation to remove support in a future release, to minimize the maintenance burden of this code.

Related #24503

Deprecate CNP Node status updates. 

@marseel marseel requested review from a team as code owners March 20, 2023 12:20
@marseel marseel requested a review from squeed March 20, 2023 12:20
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 20, 2023
@marseel marseel added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Mar 20, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 20, 2023
@squeed
Copy link
Contributor

squeed commented Mar 21, 2023

Is there a larger issue or roadmap item for this deprecation? Anything with more context you can link to?

@marseel marseel force-pushed the deprecate_cnp_status_updates branch from 762d5a5 to 6c98b7f Compare March 22, 2023 09:26
Copy link
Contributor

@squeed squeed left a comment

Choose a reason for hiding this comment

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

One last request (sorry) -- can you add something in upgrade.rst about the deprecation?

@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 May 19, 2023
@aanm
Copy link
Member

aanm commented May 30, 2023

@marseel any updates on this?

@github-actions github-actions bot removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label May 31, 2023
@marseel marseel force-pushed the deprecate_cnp_status_updates branch from 6c98b7f to 62ab471 Compare May 31, 2023 11:23
@marseel marseel requested a review from a team as a code owner May 31, 2023 11:23
@marseel marseel requested a review from qmonnet May 31, 2023 11:23
Documentation/operations/upgrade.rst Outdated Show resolved Hide resolved
@marseel marseel force-pushed the deprecate_cnp_status_updates branch 2 times, most recently from c90907f to 3034500 Compare May 31, 2023 12:58
@marseel marseel requested review from qmonnet and squeed May 31, 2023 13:19
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Thank you!

@marseel
Copy link
Contributor Author

marseel commented Jun 5, 2023

/test

2 similar comments
@marseel
Copy link
Contributor Author

marseel commented Jun 5, 2023

/test

@marseel
Copy link
Contributor Author

marseel commented Jun 6, 2023

/test

@squeed
Copy link
Contributor

squeed commented Jun 6, 2023

I would argue this change is trivial enough as to not require a full CI run.

CNP Status Updates are known to cause scalability-related issues.
Deprecate it in preparation to remove support in a future release,
to minimize the maintenance burden of this code.

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
@marseel marseel force-pushed the deprecate_cnp_status_updates branch from 3034500 to dff1622 Compare June 6, 2023 11:35
@marseel
Copy link
Contributor Author

marseel commented Jun 6, 2023

I would argue this change is trivial enough as to not require a full CI run.

Marking as ready-to-merge as all basic CI tests passed.

@marseel marseel added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 6, 2023
@dylandreimerink dylandreimerink merged commit 8582a62 into cilium:main Jun 6, 2023
48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants