Create volumes with one topology label, not both

[wongma7]

/kind bug

What happened? https://github.com/kubernetes-sigs/aws-ebs-csi-driver/blob/master/pkg/driver/controller.go#L658 Details: kubernetes/kubernetes#102248

The expressions are ANDED, meaning a provisioned PV will only land on nodes that have BOTH labels. this was not the intention, we want it to be able to land on nodes with one or the other.

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:

[jsafrane]

we want it to be able to land on nodes with one or the other.

Why? When the CSI driver returns only topology.ebs.csi.aws.com/zone label for a volume, we can be sure that a pod that uses the CSI volume will land on a node that has both the CSI driver (i.e. it has the label) and is in the right zone.

Translation we use for CSI migration will make sure that the label is translated from CSI topology.ebs.csi.aws.com/zone to in-tree topology.kubernetes.io/zone & topology.kubernetes.io/region and the other way around. IMO the driver should not worry about the CSI migration, as long as there is 1:1 mapping between in-tree and csi labels.

[wongma7]

I thought the NodeGetInfo response for topology labels doesn't get translated, so if we only return topology.ebs.csi.aws.com/zone then translated PVs with topology.kubernetes.io/zone won't get scheduled . That's why I thought we need to add both labels in NodeGetInfo response. Let me verify. (BTW we are lacking a migration topology e2e, I'll create issue/add it soon).

I agree with you that driver should not worry about the label. But we weren't originally thinking about topology.ebs.csi.aws.com/zone being a 1:1 mapping to topology.kubernetes.io/zone, rather we were thinking that we should try to use topology.kubernetes.io/zone as a replacement of topology.ebs.csi.aws.com/zone. Since there already exists PVs out there with topology.ebs.csi.aws.com/zone, I don't think we can ever remove it without breaking change, but IIRC we wanted to at least deprecate it and someday remove it.

cc @ayberk (he is out right now but knows more than me,i'm throwing him under the bus.)

[himanshu-kun]

We are facing an issue with v0.10.x of CSI driver where it doesn't prioritize the topology.kubernetes.io/zone label over topology.ebs.csi.aws.com/zone . We thought it would do so according to the following comment from @ayberk #729 (comment) but looking at this issue I think AND of expressions is causing the issue.
So it would be really helpful if you could let us know when can we expect the feature where well-known zone label is prioritized over csi label.
The issue we are facing in detail is gardener/autoscaler#83 .

[wongma7]

@himanshu-kun

The fallback he is referring to in that comment is in the driver controller when it is deciding in which zone to create a volume, but the scheduler doesn't have such a fallback, the scheduler is doing the AND and potentially blocking PVs/Pods from getting scheduled

this issue is fixed by #912 released in 1.1.0, from tha tversion on PVs and CSINode will have topology.ebs.csi.aws.com/zone topology label