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

feat: deleted the pods that are not unmanaged by Cilium #22911

Merged
merged 1 commit into from Jan 10, 2023

Conversation

lvyanru8200
Copy link
Contributor

@lvyanru8200 lvyanru8200 commented Jan 3, 2023

Set operator to remove the label of a pod that existed before the node taint

  1. Delete the specified label pod according to the parameter --pod-restart-selector, default value is k8s-app=kube-dns
  2. --pod-restart-selector="" Remove all pods

Fixes: #21594
Signed-off-by: tigerK yanru.lv@daocloud.io

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Thanks for contributing!

Fixes: #issue-number

Allow Cilium Operator to restart any unmanaged pods via --pod-restart-selector, rather than just kube-dns pods 

Set operator to remove the label of a pod that existed before the node taint

1. Delete the specified label pod according to the parameter --pod-restart-selector, default value is k8s-app=kube-dns
2. --pod-restart-selector="" Remove all pods

Fixes: cilium#21594
Signed-off-by: tigerK <yanru.lv@daocloud.io>
@lvyanru8200 lvyanru8200 requested a review from a team as a code owner January 3, 2023 03:46
@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 Jan 3, 2023
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Jan 3, 2023
@lvyanru8200
Copy link
Contributor Author

@joestringer @christarazi @aanm @nebril Sorry, please see this,My last pr time was too long there were a lot of other commissions between the two commissions

@christarazi christarazi removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 3, 2023
@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 Jan 3, 2023
@christarazi christarazi added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. kind/feature This introduces new functionality. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. area/operator Impacts the cilium-operator component and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jan 3, 2023
@christarazi
Copy link
Member

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 9, 2023
@aanm aanm merged commit 5af4a0e into cilium:master Jan 10, 2023
@owais
Copy link

owais commented Mar 10, 2023

Will this make it into 1.13.1? If so, do we know approximately when 1.13.1 will be released? Thanks!

@christarazi
Copy link
Member

The feature was merged for 1.14 release and I don't see any backport labels for 1.13.

@owais
Copy link

owais commented Mar 15, 2023

Thanks @christarazi. I see 1.13.1 is being prepped already. We are running into similar issues with cilium adoption and this feature would help a lot. Any chance this can make it to 1.13.2? 😬

@williamclot
Copy link

I'm trying this feature with the 1.14.0-snapshot.1 release. I'm deploying Cilium using helm and have enabled this option using the extraArgs helm value (as I don't see a specific helm value for this option):

operator:
  extraArgs: ["--pod-restart-selector="]

This seems to be picked up properly as I see in the logs at the operator startup:

level=info msg=" --pod-restart-selector=''" subsys=cilium-operator-generic
level=info msg=" --unmanaged-pod-watcher-interval='15'" subsys=cilium-operator-generic

And yet unmanaged pods aren't being restarted from what I can see. When I run cilium status I still have only 40/43 pods managed by Cilium. Am I missing something to make this work?

@christarazi
Copy link
Member

@williamclot If you believe it's a bug, please file an issue and provide reproduction steps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/operator Impacts the cilium-operator component kind/community-contribution This was a contribution made by a community member. kind/feature This introduces new functionality. 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. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unmanaged pods with preemptible nodes on GKE
6 participants