Bugfix: pvs not deleting was: Remove the assumption that a node's name == its hostname #414
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As per https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#built-in-node-labels [[[ Note: The value of these labels is cloud provider specific and is not guaranteed to be reliable. For example, the value of kubernetes.io/hostname may be the same as the node name in some environments and a different value in other environments. ]]]
|
I'm not a kustomize expert - I think it'd be useful to demonstrate this working both "from scratch" and also with a granfathered-in PV with the legacy hostname affinity requirement on it - could do with a hand there! (Hence the draft status) |
|
This could really do with #416 to close down the raciness of the e2e tests. |
This applies a small refactor to the e2e tests to ensure that the newer provisioner is capable of siting helper pods correctly to clean up PVs with "legacy" affinity constraints. The kind cluster itself is reconfigured to ensure that all nodes have `metadata.name` != `metadata.labels["kubernetes.io/hostname"]`, which is an assumption that does not hold for many cloud providers.
jan-g
force-pushed
the
change-node-affinity-annotation
branch
from
97b39e1
to
199ec0a
Compare
|
Fixed up the new e2e test (it creates a PV that should be cleaned up & waits on the provisioner; therefore we don't want deleteKustomizeDeployment to target it) |
|
Commenting ro remove the stale tag |
|
Hi @jan-g thanks for the great fix, when can we get this merged to master? @derekbit @RafalKorepta |
|
Hi @liangjuf I'm not maintainer, so I don't have power to merge. |
|
There's this PR and the one next to it that fixes up some issues with occasional e2e test failures (that means this one's tests should work reliably). This'll need rebasing atop the e2e one once that lands. Maintainers are a little thin on the ground (and derekbit is in a different tz) so I'd ask for a little patience. |
jan-g
changed the title
lumiere-bot bot
added a commit
to coolguy1771/home-ops
that referenced
this pull request
Sep 3, 2024
…9 ) (#5335) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [local-path-provisioner](https://redirect.github.com/rancher/local-path-provisioner) | patch | `v0.0.28` -> `v0.0.29` | --- ### Release Notes <details> <summary>rancher/local-path-provisioner (local-path-provisioner)</summary> ### [`v0.0.29`](https://redirect.github.com/rancher/local-path-provisioner/releases/tag/v0.0.29): Local Path Provisioner v0.0.29 [Compare Source](https://redirect.github.com/rancher/local-path-provisioner/compare/v0.0.28...v0.0.29) #### What's Changed - Update Go to 1.22.5 by [@​unguiculus](https://redirect.github.com/unguiculus) in [rancher/local-path-provisioner#435 - vendor: update dependencies by [@​derekbit](https://redirect.github.com/derekbit) in [rancher/local-path-provisioner#446 - Upgrade sig storage lib external provisioner by [@​rorosen](https://redirect.github.com/rorosen) in [rancher/local-path-provisioner#445 - chore(workflow): add "Scan With Trivy and Upload Results to GitHub Security Tab" by [@​derekbit](https://redirect.github.com/derekbit) in [rancher/local-path-provisioner#448 - Bugfix: pvs not deleting was: Remove the assumption that a node's name == its hostname by [@​jan-g](https://redirect.github.com/jan-g) in [rancher/local-path-provisioner#414 - Bump to v0.0.29 by [@​derekbit](https://redirect.github.com/derekbit) in [rancher/local-path-provisioner#450 #### Contributors - [@​unguiculus](https://redirect.github.com/unguiculus) - [@​galal-hussein](https://redirect.github.com/galal-hussein) - [@​jamshidi799](https://redirect.github.com/jamshidi799) - [@​rorosen](https://redirect.github.com/rorosen) - [@​jan-g](https://redirect.github.com/jan-g) </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 this update 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:eyJjcmVhdGVkSW5WZXIiOiIzOC42NC4yIiwidXBkYXRlZEluVmVyIjoiMzguNjQuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsicmVub3ZhdGUvZ2l0aHViLXJlbGVhc2UiLCJ0eXBlL3BhdGNoIl19--> Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
lumiere-bot bot
added a commit
to coolguy1771/home-ops
that referenced
this pull request
Sep 3, 2024
…9 ) (#5334) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [local-path-provisioner](https://redirect.github.com/rancher/local-path-provisioner) | patch | `v0.0.28` -> `v0.0.29` | --- ### Release Notes <details> <summary>rancher/local-path-provisioner (local-path-provisioner)</summary> ### [`v0.0.29`](https://redirect.github.com/rancher/local-path-provisioner/releases/tag/v0.0.29): Local Path Provisioner v0.0.29 [Compare Source](https://redirect.github.com/rancher/local-path-provisioner/compare/v0.0.28...v0.0.29) #### What's Changed - Update Go to 1.22.5 by [@​unguiculus](https://redirect.github.com/unguiculus) in [rancher/local-path-provisioner#435 - vendor: update dependencies by [@​derekbit](https://redirect.github.com/derekbit) in [rancher/local-path-provisioner#446 - Upgrade sig storage lib external provisioner by [@​rorosen](https://redirect.github.com/rorosen) in [rancher/local-path-provisioner#445 - chore(workflow): add "Scan With Trivy and Upload Results to GitHub Security Tab" by [@​derekbit](https://redirect.github.com/derekbit) in [rancher/local-path-provisioner#448 - Bugfix: pvs not deleting was: Remove the assumption that a node's name == its hostname by [@​jan-g](https://redirect.github.com/jan-g) in [rancher/local-path-provisioner#414 - Bump to v0.0.29 by [@​derekbit](https://redirect.github.com/derekbit) in [rancher/local-path-provisioner#450 #### Contributors - [@​unguiculus](https://redirect.github.com/unguiculus) - [@​galal-hussein](https://redirect.github.com/galal-hussein) - [@​jamshidi799](https://redirect.github.com/jamshidi799) - [@​rorosen](https://redirect.github.com/rorosen) - [@​jan-g](https://redirect.github.com/jan-g) </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 this update 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:eyJjcmVhdGVkSW5WZXIiOiIzOC42NC4yIiwidXBkYXRlZEluVmVyIjoiMzguNjQuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsicmVub3ZhdGUvZ2l0aHViLXJlbGVhc2UiLCJ0eXBlL3BhdGNoIl19--> 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As per https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#built-in-node-labels
[[[
Note:
The value of these labels is cloud provider specific and is not guaranteed to be reliable. For example, the value of kubernetes.io/hostname may be the same as the node name in some environments and a different value in other environments.
]]]
(This is a quick handwave in the direction of #413)