Skip to content

Commit

Permalink
Removes alpha feature gate for affinity annotations. Beta fields shou…
Browse files Browse the repository at this point in the history
…ld be used.
  • Loading branch information
timothysc committed Jun 23, 2017
1 parent 8679677 commit 4aea626
Show file tree
Hide file tree
Showing 23 changed files with 42 additions and 2,348 deletions.
5 changes: 0 additions & 5 deletions pkg/api/annotation_key_constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,6 @@ const (
// This annotation can be attached to node.
ObjectTTLAnnotationKey string = "node.alpha.kubernetes.io/ttl"

// AffinityAnnotationKey represents the key of affinity data (json serialized)
// in the Annotations of a Pod.
// TODO: remove when alpha support for affinity is removed
AffinityAnnotationKey string = "scheduler.alpha.kubernetes.io/affinity"

// annotation key prefix used to identify non-convertible json paths.
NonConvertibleAnnotationPrefix = "non-convertible.kubernetes.io"

Expand Down
15 changes: 0 additions & 15 deletions pkg/api/helper/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -519,21 +519,6 @@ func PodAnnotationsFromSysctls(sysctls []api.Sysctl) string {
return strings.Join(kvs, ",")
}

// GetAffinityFromPodAnnotations gets the json serialized affinity data from Pod.Annotations
// and converts it to the Affinity type in api.
// TODO: remove when alpha support for affinity is removed
func GetAffinityFromPodAnnotations(annotations map[string]string) (*api.Affinity, error) {
if len(annotations) > 0 && annotations[api.AffinityAnnotationKey] != "" {
var affinity api.Affinity
err := json.Unmarshal([]byte(annotations[api.AffinityAnnotationKey]), &affinity)
if err != nil {
return nil, err
}
return &affinity, nil
}
return nil, nil
}

// GetPersistentVolumeClass returns StorageClassName.
func GetPersistentVolumeClass(volume *api.PersistentVolume) string {
// Use beta annotation first
Expand Down
15 changes: 0 additions & 15 deletions pkg/api/v1/helper/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -445,21 +445,6 @@ func RemoveTaint(node *v1.Node, taint *v1.Taint) (*v1.Node, bool, error) {
return newNode, true, nil
}

// GetAffinityFromPodAnnotations gets the json serialized affinity data from Pod.Annotations
// and converts it to the Affinity type in api.
// TODO: remove when alpha support for affinity is removed
func GetAffinityFromPodAnnotations(annotations map[string]string) (*v1.Affinity, error) {
if len(annotations) > 0 && annotations[v1.AffinityAnnotationKey] != "" {
var affinity v1.Affinity
err := json.Unmarshal([]byte(annotations[v1.AffinityAnnotationKey]), &affinity)
if err != nil {
return nil, err
}
return &affinity, nil
}
return nil, nil
}

// GetPersistentVolumeClass returns StorageClassName.
func GetPersistentVolumeClass(volume *v1.PersistentVolume) string {
// Use beta annotation first
Expand Down
57 changes: 0 additions & 57 deletions pkg/api/v1/helper/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,63 +443,6 @@ func TestSysctlsFromPodAnnotation(t *testing.T) {
}
}

// TODO: remove when alpha support for affinity is removed
func TestGetAffinityFromPodAnnotations(t *testing.T) {
testCases := []struct {
pod *v1.Pod
expectErr bool
}{
{
pod: &v1.Pod{},
expectErr: false,
},
{
pod: &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
v1.AffinityAnnotationKey: `
{"nodeAffinity": { "requiredDuringSchedulingIgnoredDuringExecution": {
"nodeSelectorTerms": [{
"matchExpressions": [{
"key": "foo",
"operator": "In",
"values": ["value1", "value2"]
}]
}]
}}}`,
},
},
},
expectErr: false,
},
{
pod: &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
v1.AffinityAnnotationKey: `
{"nodeAffinity": { "requiredDuringSchedulingIgnoredDuringExecution": {
"nodeSelectorTerms": [{
"matchExpressions": [{
"key": "foo",
`,
},
},
},
expectErr: true,
},
}

for i, tc := range testCases {
_, err := GetAffinityFromPodAnnotations(tc.pod.Annotations)
if err == nil && tc.expectErr {
t.Errorf("[%v]expected error but got none.", i)
}
if err != nil && !tc.expectErr {
t.Errorf("[%v]did not expect error but got: %v", i, err)
}
}
}

// TODO: remove when alpha support for topology constraints is removed
func TestGetNodeAffinityFromAnnotations(t *testing.T) {
testCases := []struct {
Expand Down
25 changes: 1 addition & 24 deletions pkg/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,6 @@ func ValidateDNS1123Subdomain(value string, fldPath *field.Path) field.ErrorList
func ValidatePodSpecificAnnotations(annotations map[string]string, spec *api.PodSpec, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if annotations[api.AffinityAnnotationKey] != "" {
allErrs = append(allErrs, ValidateAffinityInPodAnnotations(annotations, fldPath)...)
}

if value, isMirror := annotations[api.MirrorPodAnnotationKey]; isMirror {
if len(spec.NodeName) == 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Key(api.MirrorPodAnnotationKey), value, "must set spec.nodeName if mirror pod annotation is set"))
Expand Down Expand Up @@ -164,23 +160,6 @@ func ValidateTolerationsInPodAnnotations(annotations map[string]string, fldPath
return allErrs
}

// ValidateAffinityInPodAnnotations tests that the serialized Affinity in Pod.Annotations has valid data
func ValidateAffinityInPodAnnotations(annotations map[string]string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

affinity, err := helper.GetAffinityFromPodAnnotations(annotations)
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath, api.AffinityAnnotationKey, err.Error()))
return allErrs
}
if affinity == nil {
return allErrs
}

allErrs = append(allErrs, validateAffinity(affinity, fldPath.Child("affinity"))...)
return allErrs
}

func ValidatePodSpecificAnnotationUpdates(newPod, oldPod *api.Pod, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
newAnnotations := newPod.Annotations
Expand Down Expand Up @@ -2430,9 +2409,7 @@ func ValidatePreferredSchedulingTerms(terms []api.PreferredSchedulingTerm, fldPa
// validatePodAffinityTerm tests that the specified podAffinityTerm fields have valid data
func validatePodAffinityTerm(podAffinityTerm api.PodAffinityTerm, allowEmptyTopologyKey bool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if !utilfeature.DefaultFeatureGate.Enabled(features.AffinityInAnnotations) && len(podAffinityTerm.TopologyKey) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("topologyKey"), "can not be empty"))
}

allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(podAffinityTerm.LabelSelector, fldPath.Child("matchExpressions"))...)
for _, name := range podAffinityTerm.Namespaces {
for _, msg := range ValidateNamespaceName(name, false) {
Expand Down
38 changes: 19 additions & 19 deletions pkg/api/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5044,7 +5044,7 @@ func TestValidatePod(t *testing.T) {
}
}

func TestValidatePodWithDisabledAffinityInAnnotations(t *testing.T) {
func TestValidatePodWithAffinity(t *testing.T) {
validPodSpec := func(affinity *api.Affinity) api.PodSpec {
spec := api.PodSpec{
Containers: []api.Container{{Name: "ctr", Image: "image", ImagePullPolicy: "IfNotPresent", TerminationMessagePolicy: "File"}},
Expand All @@ -5057,7 +5057,6 @@ func TestValidatePodWithDisabledAffinityInAnnotations(t *testing.T) {
return spec
}

utilfeature.DefaultFeatureGate.Set("AffinityInAnnotations=False")
errorCases := []api.Pod{
{
ObjectMeta: metav1.ObjectMeta{Name: "123", Namespace: "ns"},
Expand Down Expand Up @@ -5129,29 +5128,30 @@ func TestValidatePodWithDisabledAffinityInAnnotations(t *testing.T) {
}),
},
{
ObjectMeta: metav1.ObjectMeta{Name: "123", Namespace: "ns"},
Spec: validPodSpec(&api.Affinity{
PodAntiAffinity: &api.PodAntiAffinity{
PreferredDuringSchedulingIgnoredDuringExecution: []api.WeightedPodAffinityTerm{
{
Weight: 10,
PodAffinityTerm: api.PodAffinityTerm{
LabelSelector: &metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "key2",
Operator: metav1.LabelSelectorOpNotIn,
Values: []string{"value1", "value2"},
},
/* TODO: Re-enable if/when topologykey is required.
ObjectMeta: metav1.ObjectMeta{Name: "123", Namespace: "ns"},
Spec: validPodSpec(&api.Affinity{
PodAntiAffinity: &api.PodAntiAffinity{
PreferredDuringSchedulingIgnoredDuringExecution: []api.WeightedPodAffinityTerm{
{
Weight: 10,
PodAffinityTerm: api.PodAffinityTerm{
LabelSelector: &metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "key2",
Operator: metav1.LabelSelectorOpNotIn,
Values: []string{"value1", "value2"},
},
},
Namespaces: []string{"ns"},
TopologyKey: "",
},
Namespaces: []string{"ns"},
TopologyKey: "",
},
},
},
}),
},
}),*/
},
}

Expand Down
8 changes: 0 additions & 8 deletions pkg/features/kube_features.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,6 @@ const (
// Note: This feature is not supported for `BestEffort` pods.
ExperimentalCriticalPodAnnotation utilfeature.Feature = "ExperimentalCriticalPodAnnotation"

// owner: @davidopp
// alpha: v1.6
//
// Determines if affinity defined in annotations should be processed
// TODO: remove when alpha support for affinity is removed
AffinityInAnnotations utilfeature.Feature = "AffinityInAnnotations"

// owner: @vishh
// alpha: v1.6
//
Expand Down Expand Up @@ -137,7 +130,6 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS
DynamicVolumeProvisioning: {Default: true, PreRelease: utilfeature.Alpha},
ExperimentalHostUserNamespaceDefaultingGate: {Default: false, PreRelease: utilfeature.Beta},
ExperimentalCriticalPodAnnotation: {Default: false, PreRelease: utilfeature.Alpha},
AffinityInAnnotations: {Default: false, PreRelease: utilfeature.Alpha},
Accelerators: {Default: false, PreRelease: utilfeature.Alpha},
TaintBasedEvictions: {Default: false, PreRelease: utilfeature.Alpha},
RotateKubeletServerCertificate: {Default: false, PreRelease: utilfeature.Alpha},
Expand Down
1 change: 0 additions & 1 deletion plugin/pkg/scheduler/algorithm/predicates/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ go_test(
"//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/labels:go_default_library",
"//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library",
],
)

Expand Down
8 changes: 4 additions & 4 deletions plugin/pkg/scheduler/algorithm/predicates/predicates.go
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ func podMatchesNodeLabels(pod *v1.Pod, node *v1.Node) bool {
// 5. zero-length non-nil []NodeSelectorRequirement matches no nodes also, just for simplicity
// 6. non-nil empty NodeSelectorRequirement is not allowed
nodeAffinityMatches := true
affinity := schedulercache.ReconcileAffinity(pod)
affinity := pod.Spec.Affinity
if affinity != nil && affinity.NodeAffinity != nil {
nodeAffinity := affinity.NodeAffinity
// if no required NodeAffinity requirements, will do no-op, means select all nodes.
Expand Down Expand Up @@ -985,7 +985,7 @@ func (c *PodAffinityChecker) InterPodAffinityMatches(pod *v1.Pod, meta interface
}

// Now check if <pod> requirements will be satisfied on this node.
affinity := schedulercache.ReconcileAffinity(pod)
affinity := pod.Spec.Affinity
if affinity == nil || (affinity.PodAffinity == nil && affinity.PodAntiAffinity == nil) {
return true, nil, nil
}
Expand Down Expand Up @@ -1089,7 +1089,7 @@ func getMatchingAntiAffinityTerms(pod *v1.Pod, nodeInfoMap map[string]*scheduler
}
var nodeResult []matchingPodAntiAffinityTerm
for _, existingPod := range nodeInfo.PodsWithAffinity() {
affinity := schedulercache.ReconcileAffinity(existingPod)
affinity := existingPod.Spec.Affinity
if affinity == nil {
continue
}
Expand All @@ -1116,7 +1116,7 @@ func getMatchingAntiAffinityTerms(pod *v1.Pod, nodeInfoMap map[string]*scheduler
func (c *PodAffinityChecker) getMatchingAntiAffinityTerms(pod *v1.Pod, allPods []*v1.Pod) ([]matchingPodAntiAffinityTerm, error) {
var result []matchingPodAntiAffinityTerm
for _, existingPod := range allPods {
affinity := schedulercache.ReconcileAffinity(existingPod)
affinity := existingPod.Spec.Affinity
if affinity != nil && affinity.PodAntiAffinity != nil {
existingPodNode, err := c.info.GetNodeInfo(existingPod.Spec.NodeName)
if err != nil {
Expand Down
Loading

0 comments on commit 4aea626

Please sign in to comment.