kubernetes-sigs / aws-ebs-csi-driver

CSI driver for Amazon EBS https://aws.amazon.com/ebs/
Apache License 2.0
974 stars 787 forks source link

Configurable topology labels #962

Closed wongma7 closed 2 months ago

wongma7 commented 3 years ago

/kind bug

What happened?

https://github.com/kubernetes-sigs/aws-ebs-csi-driver/issues/729 <- autoscaler reads the in-tree topology labels https://github.com/kubernetes-sigs/aws-ebs-csi-driver/issues/899 <- csi migration translation reads in-tree topology labels and our ebs-specific topology label and translates both to normal topology label which results in PV having duplicate labels

to solve 899 we started to only put the ebs-specific topology label on csinode and on PVs. but that breaks 729 in case migration is off which expects to read in-tree topology labels.

Moreover, if there exist PVs with both labels and users upgrade to version with only one label, then scheduler won't be able to schedule PVs with both labels to nodes with just one label, we may already have users who ran into this bug

To fix, we might need to default to having both labels on CSINode again, but still just have one label on PV. The csi translation duplicate label issue will be fixed ,but it'ps unclear at this time whether we should be using the in-tree topology label in CSINode or not, or if that label being kubernetes-specific should be reserved for in-tree kubernetes usage, etc, or if topology labels should be configurable so that by default we include the kubernetes-specific label but (totally hypothetical) non-kubernetes users can omit it.

**What you expected to happen?** **How to reproduce it (as minimally and precisely as possible)?** **Anything else we need to know?**: **Environment** - Kubernetes version (use `kubectl version`): - Driver version:
k8s-triage-robot commented 3 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k1rk commented 3 years ago

/remove-lifecycle stale

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k1rk commented 2 years ago

/remove-lifecycle stale

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k1rk commented 2 years ago

/remove-lifecycle stale

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

olfway commented 2 years ago

/remove-lifecycle stale

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

k8s-ci-robot commented 1 year ago

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to [this](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/issues/962#issuecomment-1363434131): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages issues according to the following rules: >- After 90d of inactivity, `lifecycle/stale` is applied >- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied >- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed > >You can: >- Reopen this issue with `/reopen` >- Mark this issue as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close not-planned > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
jbg commented 1 year ago

The current status is that nodes have both labels but PVs select only on the old non-standardised label (topology.ebs.csi.aws.com/zone). This means scale-up-from-zero with cluster-autoscaler requires manually tagging ASGs if EBS volumes are used. Switching to the new label would make this work without the manual tagging.

I have written a patch to provide a command-line option, disabled by default (as I'm not sure if enabling it by default may break some setups), to make PVs select on the standardised label (topology.kubernetes.io/zone) instead. I'm testing it at the moment and am happy to send a PR if this approach is acceptable.

Can this issue be re-opened?

AndrewSirenko commented 1 year ago

/reopen

k8s-ci-robot commented 1 year ago

@AndrewSirenko: Reopened this issue.

In response to [this](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/issues/962#issuecomment-1730257359): >/reopen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
AndrewSirenko commented 1 year ago

/remove-lifecycle rotten

k8s-triage-robot commented 8 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

mbruner commented 8 months ago

/remove-lifecycle stale

k8s-triage-robot commented 5 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

mbruner commented 4 months ago

/remove-lifecycle stale

ConnorJC3 commented 2 months ago

/close

The original reason for customization of the topology labels (using the k8s standardized default) is no longer necessary as of v1.33.0 - the driver now uses the standard label for all new volumes (see #729 for more info). We have decided not to allow other customization at this time due to the significant footgun it presents.

If you have a specific need for customization other than using the standardized label, please open a new issue so that it can be evaluated.

k8s-ci-robot commented 2 months ago

@ConnorJC3: Closing this issue.

In response to [this](https://github.com/kubernetes-sigs/aws-ebs-csi-driver/issues/962#issuecomment-2271349911): >/close > >The original reason for customization of the topology labels (using the k8s standardized default) is no longer necessary as of `v1.33.0` - the driver now uses the standard label for all new volumes (see #729 for more info). We have decided not to allow other customization at this time due to the significant footgun it presents. > >If you have a specific need for customization other than using the standardized label, please open a new issue so that it can be evaluated. Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.