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

fix(cluster-autoscaler/hetzner): pre-existing volumes break scheduling #5322

Merged

Conversation

apricote
Copy link
Member

@apricote apricote commented Nov 21, 2022

Which component this PR applies to?

cluster-autoscaler

What type of PR is this?

/kind bug

What this PR does / why we need it:

The hcloud-csi-driver uses the label csi.hetzner.cloud/location for topology. This label was not added in the response to n.TemplateNodeInfo(), causing cluster-autoscaler to not consider any node group for scaling when a pre-existing volume was attached to the pending pod.

This is fixed by adding the appropriatly named label to the NodeInfo. In practice this label is added by the hcloud-csi-driver.

Which issue(s) this PR fixes:

Further details on the bug are available in the original issue: hetznercloud/csi-driver#302

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


The `hcloud-csi-driver` v1.x uses the label `csi.hetzner.cloud/location`
for topology. This label was not added in the response to
`n.TemplateNodeInfo()`, causing cluster-autoscaler to not consider any
node group for scaling when a pre-existing volume was attached to the
pending pod.

This is fixed by adding the appropriatly named label to the `NodeInfo`.
In practice this label is added by the `hcloud-csi-driver`.

In the upcoming v2 of the driver we migrated to using
`apiv1.LabelZoneRegionStable` for topology constraints, but this fix is
still required so customers do not have to re-create all `PersistentVolumes`.

Further details on the bug are available in the original issue:
hetznercloud/csi-driver#302
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Nov 21, 2022
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 21, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: apricote / name: Julian Tölle (09683d6)

@k8s-ci-robot k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 21, 2022
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Nov 21, 2022
@apricote
Copy link
Member Author

apricote commented Dec 1, 2022

Adding lgtm as @LKaemmerling reviewed and approved (through GitHub) but can not use the command as he is not a member of the org.

/lgtm

@k8s-ci-robot
Copy link
Contributor

@apricote: you cannot LGTM your own PR.

In response to this:

Adding lgtm as @LKaemmerling reviewed and approved (through GitHub) but can not use the command as he is not a member of the org.

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link
Contributor

@mwielgus mwielgus left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 5, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: apricote, LKaemmerling, mwielgus

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 5, 2022
@k8s-ci-robot k8s-ci-robot merged commit e7146e4 into kubernetes:master Dec 5, 2022
@apricote apricote deleted the fix-volume-topology-v1.x branch December 5, 2022 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/cluster-autoscaler cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants