api defination for MatchLabelKeys in TopologySpreadConstraint

Signed-off-by: Alex Wang <wangqingcan1990@gmail.com>
This commit is contained in:
Alex Wang 2022-07-30 13:21:16 +08:00
parent 5e14c5dfa9
commit e6c2bf8516
6 changed files with 241 additions and 0 deletions

View File

@ -553,6 +553,7 @@ func dropDisabledFields(
dropDisabledTopologySpreadConstraintsFields(podSpec, oldPodSpec)
dropDisabledNodeInclusionPolicyFields(podSpec, oldPodSpec)
dropDisabledMatchLabelKeysField(podSpec, oldPodSpec)
}
// dropDisabledTopologySpreadConstraintsFields removes disabled fields from PodSpec related
@ -626,6 +627,31 @@ func dropDisabledNodeInclusionPolicyFields(podSpec, oldPodSpec *api.PodSpec) {
}
}
// dropDisabledMatchLabelKeysField removes disabled fields from PodSpec related
// to MatchLabelKeys only if it is not already used by the old spec.
func dropDisabledMatchLabelKeysField(podSpec, oldPodSpec *api.PodSpec) {
if !utilfeature.DefaultFeatureGate.Enabled(features.MatchLabelKeysInPodTopologySpread) && !matchLabelKeysInUse(oldPodSpec) {
for i := range podSpec.TopologySpreadConstraints {
podSpec.TopologySpreadConstraints[i].MatchLabelKeys = nil
}
}
}
// matchLabelKeysInUse returns true if the pod spec is non-nil
// and has MatchLabelKeys field set in TopologySpreadConstraints.
func matchLabelKeysInUse(podSpec *api.PodSpec) bool {
if podSpec == nil {
return false
}
for _, c := range podSpec.TopologySpreadConstraints {
if len(c.MatchLabelKeys) > 0 {
return true
}
}
return false
}
// nodeAffinityPolicyInUse returns true if the pod spec is non-nil and has NodeAffinityPolicy field set
// in TopologySpreadConstraints
func nodeAffinityPolicyInUse(podSpec *api.PodSpec) bool {

View File

@ -1930,3 +1930,138 @@ func TestDropNodeInclusionPolicyFields(t *testing.T) {
})
}
}
func TestDropDisabledMatchLabelKeysField(t *testing.T) {
tests := []struct {
name string
enabled bool
podSpec *api.PodSpec
oldPodSpec *api.PodSpec
wantPodSpec *api.PodSpec
}{
{
name: "feature disabled, both pods don't use MatchLabelKeys fields",
enabled: false,
oldPodSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{},
},
podSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{},
},
wantPodSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{},
},
},
{
name: "feature disabled, only old pod uses MatchLabelKeys field",
enabled: false,
oldPodSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{
{MatchLabelKeys: []string{"foo"}},
},
},
podSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{},
},
wantPodSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{},
},
},
{
name: "feature disabled, only current pod uses MatchLabelKeys field",
enabled: false,
oldPodSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{},
},
podSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{
{MatchLabelKeys: []string{"foo"}},
},
},
wantPodSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{{}},
},
},
{
name: "feature disabled, both pods use MatchLabelKeys fields",
enabled: false,
oldPodSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{
{MatchLabelKeys: []string{"foo"}},
},
},
podSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{
{MatchLabelKeys: []string{"foo"}},
},
},
wantPodSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{
{MatchLabelKeys: []string{"foo"}},
},
},
},
{
name: "feature enabled, only old pod uses MatchLabelKeys field",
enabled: true,
oldPodSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{
{MatchLabelKeys: []string{"foo"}},
},
},
podSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{},
},
wantPodSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{},
},
},
{
name: "feature enabled, only current pod uses MatchLabelKeys field",
enabled: true,
oldPodSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{},
},
podSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{
{MatchLabelKeys: []string{"foo"}},
},
},
wantPodSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{
{MatchLabelKeys: []string{"foo"}},
},
},
},
{
name: "feature enabled, both pods use MatchLabelKeys fields",
enabled: false,
oldPodSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{
{MatchLabelKeys: []string{"foo"}},
},
},
podSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{
{MatchLabelKeys: []string{"foo"}},
},
},
wantPodSpec: &api.PodSpec{
TopologySpreadConstraints: []api.TopologySpreadConstraint{
{MatchLabelKeys: []string{"foo"}},
},
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.MatchLabelKeysInPodTopologySpread, test.enabled)()
dropDisabledFields(test.podSpec, nil, test.oldPodSpec, nil)
if diff := cmp.Diff(test.wantPodSpec, test.podSpec); diff != "" {
t.Errorf("unexpected pod spec (-want, +got):\n%s", diff)
}
})
}
}

View File

@ -5732,6 +5732,15 @@ type TopologySpreadConstraint struct {
// This is a alpha-level feature enabled by the NodeInclusionPolicyInPodTopologySpread feature flag.
// +optional
NodeTaintsPolicy *NodeInclusionPolicy
// MatchLabelKeys is a set of pod label keys to select the pods over which
// spreading will be calculated. The keys are used to lookup values from the
// incoming pod labels, those key-value labels are ANDed with labelSelector
// to select the group of existing pods over which spreading will be calculated
// for the incoming pod. Keys that don't exist in the incoming pod labels will
// be ignored. A null or empty list means only match against labelSelector.
// +listType=atomic
// +optional
MatchLabelKeys []string
}
// These are the built-in errors for PortStatus.

View File

@ -6490,6 +6490,7 @@ func validateTopologySpreadConstraints(constraints []core.TopologySpreadConstrai
if err := validateNodeInclusionPolicy(subFldPath.Child("nodeTaintsPolicy"), constraint.NodeTaintsPolicy); err != nil {
allErrs = append(allErrs, err)
}
allErrs = append(allErrs, validateMatchLabelKeys(subFldPath.Child("matchLabelKeys"), constraint.MatchLabelKeys, constraint.LabelSelector)...)
}
return allErrs
@ -6563,6 +6564,33 @@ func validateNodeInclusionPolicy(fldPath *field.Path, policy *core.NodeInclusion
return nil
}
// validateMatchLabelKeys tests that the elements are a valid label name and are not already included in labelSelector.
func validateMatchLabelKeys(fldPath *field.Path, matchLabelKeys []string, labelSelector *metav1.LabelSelector) field.ErrorList {
if len(matchLabelKeys) == 0 {
return nil
}
labelSelectorKeys := sets.String{}
if labelSelector != nil {
for key := range labelSelector.MatchLabels {
labelSelectorKeys.Insert(key)
}
for _, matchExpression := range labelSelector.MatchExpressions {
labelSelectorKeys.Insert(matchExpression.Key)
}
}
allErrs := field.ErrorList{}
for i, key := range matchLabelKeys {
allErrs = append(allErrs, unversionedvalidation.ValidateLabelName(key, fldPath.Index(i))...)
if labelSelectorKeys.Has(key) {
allErrs = append(allErrs, field.Invalid(fldPath.Index(i), key, "exists in both matchLabelKeys and labelSelector"))
}
}
return allErrs
}
// ValidateServiceClusterIPsRelatedFields validates .spec.ClusterIPs,,
// .spec.IPFamilies, .spec.ipFamilyPolicy. This is exported because it is used
// during IP init and allocation.

View File

@ -18855,6 +18855,7 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
fieldPathTopologyKey := subFldPath0.Child("topologyKey")
fieldPathWhenUnsatisfiable := subFldPath0.Child("whenUnsatisfiable")
fieldPathTopologyKeyAndWhenUnsatisfiable := subFldPath0.Child("{topologyKey, whenUnsatisfiable}")
fieldPathMatchLabelKeys := subFldPath0.Child("matchLabelKeys")
nodeAffinityField := subFldPath0.Child("nodeAffinityPolicy")
nodeTaintsField := subFldPath0.Child("nodeTaintsPolicy")
unknown := core.NodeInclusionPolicy("Unknown")
@ -19033,6 +19034,39 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
field.NotSupported(nodeTaintsField, &unknown, supportedPodTopologySpreadNodePolicies.List()),
},
},
{
name: "key in MatchLabelKeys isn't correctly defined",
constraints: []core.TopologySpreadConstraint{
{
MaxSkew: 1,
TopologyKey: "k8s.io/zone",
WhenUnsatisfiable: core.DoNotSchedule,
MatchLabelKeys: []string{"/simple"},
},
},
wantFieldErrors: []*field.Error{field.Invalid(fieldPathMatchLabelKeys.Index(0), "/simple", "prefix part must be non-empty")},
},
{
name: "key exists in both matchLabelKeys and labelSelector",
constraints: []core.TopologySpreadConstraint{
{
MaxSkew: 1,
TopologyKey: "k8s.io/zone",
WhenUnsatisfiable: core.DoNotSchedule,
MatchLabelKeys: []string{"foo"},
LabelSelector: &metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "foo",
Operator: metav1.LabelSelectorOpNotIn,
Values: []string{"value1", "value2"},
},
},
},
},
},
wantFieldErrors: []*field.Error{field.Invalid(fieldPathMatchLabelKeys.Index(0), "foo", "exists in both matchLabelKeys and labelSelector")},
},
}
for _, tc := range testCases {

View File

@ -3455,6 +3455,15 @@ type TopologySpreadConstraint struct {
// This is a alpha-level feature enabled by the NodeInclusionPolicyInPodTopologySpread feature flag.
// +optional
NodeTaintsPolicy *NodeInclusionPolicy `json:"nodeTaintsPolicy,omitempty" protobuf:"bytes,7,opt,name=nodeTaintsPolicy"`
// MatchLabelKeys is a set of pod label keys to select the pods over which
// spreading will be calculated. The keys are used to lookup values from the
// incoming pod labels, those key-value labels are ANDed with labelSelector
// to select the group of existing pods over which spreading will be calculated
// for the incoming pod. Keys that don't exist in the incoming pod labels will
// be ignored. A null or empty list means only match against labelSelector.
// +listType=atomic
// +optional
MatchLabelKeys []string `json:"matchLabelKeys,omitempty" protobuf:"bytes,8,opt,name=matchLabelKeys"`
}
const (