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

fix: system-upgrade-controller-drainer: add missing delete permission for pods in clusterrole #320

Merged
merged 1 commit into from
Aug 29, 2024
Merged

fix: system-upgrade-controller-drainer: add missing delete permission for pods in clusterrole #320

merged 1 commit into from
Aug 29, 2024

Conversation

damdo
Copy link
Contributor

@damdo damdo commented Jul 21, 2024 •
edited by zube bot
Loading

fix: system-upgrade-controller-drainer: add missing delete permission for pods in clusterrole

Fixes #319

@damdo damdo mentioned this pull request Jul 21, 2024
Open
@damdo
Copy link
Contributor Author

damdo commented Jul 21, 2024

cc. @brandond @SISheogorath

@damdo
Copy link
Contributor Author

damdo commented Jul 24, 2024

cc. @jiaqiluo @bfbachmann

jiaqiluo
jiaqiluo approved these changes Jul 24, 2024
View reviewed changes
Copy link
Member

@jiaqiluo jiaqiluo left a comment

Choose a reason for hiding this comment

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

LGTM.

Hi @damdo thank you for raising the PR. I am wondering how you use the system-uppgrade-controller, do you apply the manifest directly into the cluster, or bundle them into a Helm chart, or some way else?

@damdo
Copy link
Contributor Author

damdo commented Jul 26, 2024

I install the system-upgrade-controller via a Helm chart. Then apply the plan via ArgoCD which triggers the upgrade rollout.

@jiaqiluo jiaqiluo requested a review from a team July 26, 2024 16:05
@damdo damdo mentioned this pull request Aug 7, 2024
Closed
@marcinbojko
Copy link

Hey @damdo - thanks for the PR, I was doing that manually though.But adding delete permissions has a (I think) unwanted effect - pod with a job itself is being killed during/after drain phase.

@brandond
Copy link
Member

Ahh.. I see. The controller itself does not delete pods, but the kubectl drain command does - and it runs with the controller's serviceaccount rbac.

@brandond brandond merged commit 256e42b into rancher:master Aug 29, 2024
2 checks passed
lumiere-bot bot referenced this pull request in coolguy1771/home-ops Sep 26, 2024
@lumiere-bot
feat(container): update system upgrade controller group ( v0.13.4 → v…
4930bb7 
…0.14.0 ) (minor) (#5496)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| docker.io/rancher/system-upgrade-controller | minor | `v0.13.4` ->
`v0.14.0` |
|
[rancher/system-upgrade-controller](https://redirect.github.com/rancher/system-upgrade-controller)
| minor | `v0.13.4` -> `v0.14.0` |

---

### Release Notes

<details>
<summary>rancher/system-upgrade-controller
(rancher/system-upgrade-controller)</summary>

###
[`v0.14.0`](https://redirect.github.com/rancher/system-upgrade-controller/releases/tag/v0.14.0)

[Compare
Source](https://redirect.github.com/rancher/system-upgrade-controller/compare/v0.13.4...v0.14.0)

##### What's Changed

- fix: Drop unneeded reorder surpression by
[@&#8203;SISheogorath](https://redirect.github.com/SISheogorath) in
[https://github.com/rancher/system-upgrade-controller/pull/296](https://redirect.github.com/rancher/system-upgrade-controller/pull/296)
- Update/fix README kustomize command by
[@&#8203;SISheogorath](https://redirect.github.com/SISheogorath) in
[https://github.com/rancher/system-upgrade-controller/pull/297](https://redirect.github.com/rancher/system-upgrade-controller/pull/297)
- Add GHA workflows by
[@&#8203;bfbachmann](https://redirect.github.com/bfbachmann) in
[https://github.com/rancher/system-upgrade-controller/pull/311](https://redirect.github.com/rancher/system-upgrade-controller/pull/311)
- Upgrade outdated golang dependencies by
[@&#8203;harsimranmaan](https://redirect.github.com/harsimranmaan) in
[https://github.com/rancher/system-upgrade-controller/pull/326](https://redirect.github.com/rancher/system-upgrade-controller/pull/326)
- feat: allow pod replacement policy override by
[@&#8203;buroa](https://redirect.github.com/buroa) in
[https://github.com/rancher/system-upgrade-controller/pull/328](https://redirect.github.com/rancher/system-upgrade-controller/pull/328)
- fix: system-upgrade-controller-drainer: add missing delete permission
for pods in clusterrole by
[@&#8203;damdo](https://redirect.github.com/damdo) in
[https://github.com/rancher/system-upgrade-controller/pull/320](https://redirect.github.com/rancher/system-upgrade-controller/pull/320)
- Bump golang.org/x/net from 0.17.0 to 0.23.0 in /pkg/apis by
[@&#8203;dependabot](https://redirect.github.com/dependabot) in
[https://github.com/rancher/system-upgrade-controller/pull/307](https://redirect.github.com/rancher/system-upgrade-controller/pull/307)
- Fix GHA release workflow by
[@&#8203;brandond](https://redirect.github.com/brandond) in
[https://github.com/rancher/system-upgrade-controller/pull/330](https://redirect.github.com/rancher/system-upgrade-controller/pull/330)
- Fix image tag by
[@&#8203;brandond](https://redirect.github.com/brandond) in
[https://github.com/rancher/system-upgrade-controller/pull/331](https://redirect.github.com/rancher/system-upgrade-controller/pull/331)
- Fix artifact permissions by
[@&#8203;brandond](https://redirect.github.com/brandond) in
[https://github.com/rancher/system-upgrade-controller/pull/332](https://redirect.github.com/rancher/system-upgrade-controller/pull/332)

##### New Contributors

- [@&#8203;bfbachmann](https://redirect.github.com/bfbachmann) made
their first contribution in
[https://github.com/rancher/system-upgrade-controller/pull/311](https://redirect.github.com/rancher/system-upgrade-controller/pull/311)
- [@&#8203;harsimranmaan](https://redirect.github.com/harsimranmaan)
made their first contribution in
[https://github.com/rancher/system-upgrade-controller/pull/326](https://redirect.github.com/rancher/system-upgrade-controller/pull/326)
- [@&#8203;damdo](https://redirect.github.com/damdo) made their first
contribution in
[https://github.com/rancher/system-upgrade-controller/pull/320](https://redirect.github.com/rancher/system-upgrade-controller/pull/320)

**Full Changelog**:
rancher/system-upgrade-controller@v0.13.4...v0.14.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://redirect.github.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC45NS40IiwidXBkYXRlZEluVmVyIjoiMzguOTUuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsicmVub3ZhdGUvY29udGFpbmVyIiwicmVub3ZhdGUvZ2l0aHViLXJlbGVhc2UiLCJ0eXBlL21pbm9yIl19-->

Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jiaqiluo jiaqiluo jiaqiluo approved these changes

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

Error during upgrade: User "system:serviceaccount:system-upgrade:system-upgrade" cannot delete resource "pods"
5 participants
@damdo @marcinbojko @brandond @jiaqiluo @SISheogorath