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

Topology translation test case fails randomly due to assumed ordering #87163

Open
jiahuif opened this issue Jan 13, 2020 · 1 comment
Open

Topology translation test case fails randomly due to assumed ordering #87163

jiahuif opened this issue Jan 13, 2020 · 1 comment

Comments

@jiahuif
Copy link

@jiahuif jiahuif commented Jan 13, 2020

What happened:
Received the following test failure during unit test execution

--- FAIL: TestTopologyTranslation (0.00s)
    translate_test.go:171: Testing GCE PD with zone labels
    translate_test.go:171: Testing GCE PD with existing topology (beta keys)
    translate_test.go:171: Testing GCE PD with existing topology (CSI keys)
    translate_test.go:171: Testing GCE PD with zone labels and topology
    translate_test.go:171: Testing GCE PD with regional zones
    translate_test.go:181: Expected node affinity {&NodeSelector{NodeSelectorTerms:[]NodeSelectorTerm{NodeSelectorTerm{MatchExpressions:[]NodeSelectorRequirement{NodeSelectorRequirement{Key:topology.gke.io/zone,Operator:In,Values:[europe-west1-b europe-west1-c],},},MatchFields:[]NodeSelectorRequirement{},},},}}, got {&NodeSelector{NodeSelectorTerms:[]NodeSelectorTerm{NodeSelectorTerm{MatchExpressions:[]NodeSelectorRequirement{NodeSelectorRequirement{Key:topology.gke.io/zone,Operator:In,Values:[europe-west1-c europe-west1-b],},},MatchFields:[]NodeSelectorRequirement{},},},}}
    translate_test.go:192: Expected node affinity {&NodeSelector{NodeSelectorTerms:[]NodeSelectorTerm{NodeSelectorTerm{MatchExpressions:[]NodeSelectorRequirement{NodeSelectorRequirement{Key:topology.gke.io/zone,Operator:In,Values:[europe-west1-b europe-west1-c],},},MatchFields:[]NodeSelectorRequirement{},},},}}, got {&NodeSelector{NodeSelectorTerms:[]NodeSelectorTerm{NodeSelectorTerm{MatchExpressions:[]NodeSelectorRequirement{NodeSelectorRequirement{Key:topology.gke.io/zone,Operator:In,Values:[europe-west1-c europe-west1-b],},},MatchFields:[]NodeSelectorRequirement{},},},}}
    translate_test.go:171: Testing GCE PD with regional topology
    translate_test.go:171: Testing GCE PD with regional zone and topology
    translate_test.go:171: Testing GCE PD with multiple node selector terms
    translate_test.go:171: Testing AWS EBS with zone labels
    translate_test.go:171: Testing AWS EBS with zone labels and topology
    translate_test.go:171: Testing OpenStack Cinder with zone labels
    translate_test.go:171: Testing OpenStack Cinder with zone labels and topology
FAIL
FAIL	k8s.io/kubernetes/vendor/k8s.io/csi-translation-lib	0.071s

What you expected to happen:
aforementioned test case passes consistently.
How to reproduce it (as minimally and precisely as possible):
Run the affected test case multiple times locally. I am still reporting this as a bug because it involves functionality of implementation code.
Anything else we need to know?:

root cause is as follows

// addTopology appends the topology to the given PV.
func addTopology(pv *v1.PersistentVolume, topologyKey string, zones []string) error {
// Make sure there are no duplicate or empty strings
filteredZones := sets.String{}
for i := range zones {
zone := strings.TrimSpace(zones[i])
if len(zone) > 0 {
filteredZones.Insert(zone)
}
}
zones = filteredZones.UnsortedList()
if len(zones) < 1 {
return errors.New("there are no valid zones to add to pv")
}
// Make sure the necessary fields exist
pv.Spec.NodeAffinity = new(v1.VolumeNodeAffinity)
pv.Spec.NodeAffinity.Required = new(v1.NodeSelector)
pv.Spec.NodeAffinity.Required.NodeSelectorTerms = make([]v1.NodeSelectorTerm, 1)
topology := v1.NodeSelectorRequirement{
Key: topologyKey,
Operator: v1.NodeSelectorOpIn,
Values: zones,
}
pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions = append(
pv.Spec.NodeAffinity.Required.NodeSelectorTerms[0].MatchExpressions,
topology,
)
return nil
}

at line 122

zones = filteredZones.UnsortedList()

This will cause zones to have undefined ordering. However, in the test, the ordering is assumed when creating the expected value, being "europe-west1-b" before "europe-west1-c"

name: "GCE PD with regional zones",
pv: makeGCEPDPV(regionalPDLabels, nil /*topology*/),
expectedNodeAffinity: makeNodeAffinity(false /*multiTerms*/, plugins.GCEPDTopologyKey, "europe-west1-b", "europe-west1-c"),
},

This case will pass if the translated value has the assumed ordering.

I would like to change

zones = filteredZones.UnsortedList()

to

zones = filteredZones.List()

Let me know if I can proceed with such change. Or, if the original behavior is required for some reason, we can change the test instead.

/kind bug
/sig storage

@riking

This comment has been minimized.

Copy link

@riking riking commented Jan 16, 2020

/kind flake

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.