diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh index 882c2976a0c..59b86209d65 100644 --- a/hack/make-rules/test-cmd-util.sh +++ b/hack/make-rules/test-cmd-util.sh @@ -2382,10 +2382,6 @@ run_deployment_tests() { # Check that trying to watch the status of a superseded revision returns an error ! kubectl rollout status deployment/nginx --revision=3 cat hack/testdata/deployment-revision1.yaml | $SED "s/name: nginx$/name: nginx2/" | kubectl create -f - "${kube_flags[@]}" - # Newest deployment should be marked as overlapping - kubectl get deployment nginx2 -o yaml "${kube_flags[@]}" | grep "deployment.kubernetes.io/error-selector-overlapping-with" - # Oldest deployment should not be marked as overlapping - ! kubectl get deployment nginx -o yaml "${kube_flags[@]}" | grep "deployment.kubernetes.io/error-selector-overlapping-with" # Deletion of both deployments should not be blocked kubectl delete deployment nginx2 "${kube_flags[@]}" # Clean up diff --git a/pkg/controller/deployment/util/deployment_util.go b/pkg/controller/deployment/util/deployment_util.go index b0fcfcb0d37..296dba29acf 100644 --- a/pkg/controller/deployment/util/deployment_util.go +++ b/pkg/controller/deployment/util/deployment_util.go @@ -67,14 +67,6 @@ const ( RollbackTemplateUnchanged = "DeploymentRollbackTemplateUnchanged" // RollbackDone is the done rollback event reason RollbackDone = "DeploymentRollback" - // OverlapAnnotation marks deployments with overlapping selector with other deployments - // TODO: Delete this annotation when we gracefully handle overlapping selectors. - // See https://github.com/kubernetes/kubernetes/issues/2210 - OverlapAnnotation = "deployment.kubernetes.io/error-selector-overlapping-with" - // SelectorUpdateAnnotation marks the last time deployment selector update - // TODO: Delete this annotation when we gracefully handle overlapping selectors. - // See https://github.com/kubernetes/kubernetes/issues/2210 - SelectorUpdateAnnotation = "deployment.kubernetes.io/selector-updated-at" // Reasons for deployment conditions // @@ -295,8 +287,6 @@ var annotationsToSkip = map[string]bool{ RevisionHistoryAnnotation: true, DesiredReplicasAnnotation: true, MaxReplicasAnnotation: true, - OverlapAnnotation: true, - SelectorUpdateAnnotation: true, } // skipCopyAnnotation returns true if we should skip copying the annotation with the given annotation key @@ -1020,59 +1010,3 @@ func DeploymentDeepCopy(deployment *extensions.Deployment) (*extensions.Deployme } return copied, nil } - -// SelectorUpdatedBefore returns true if the former deployment's selector -// is updated before the latter, false otherwise. -func SelectorUpdatedBefore(d1, d2 *extensions.Deployment) bool { - t1, t2 := LastSelectorUpdate(d1), LastSelectorUpdate(d2) - return t1.Before(t2) -} - -// LastSelectorUpdate returns the last time given deployment's selector is updated -func LastSelectorUpdate(d *extensions.Deployment) metav1.Time { - t := d.Annotations[SelectorUpdateAnnotation] - if len(t) > 0 { - parsedTime, err := time.Parse(time.RFC3339, t) - // If failed to parse the time, use creation timestamp instead - if err != nil { - return d.CreationTimestamp - } - return metav1.Time{Time: parsedTime} - } - // If it's never updated, use creation timestamp instead - return d.CreationTimestamp -} - -// BySelectorLastUpdateTime sorts a list of deployments by the last update time of their selector, -// first using their creation timestamp and then their names as a tie breaker. -type BySelectorLastUpdateTime []*extensions.Deployment - -func (o BySelectorLastUpdateTime) Len() int { return len(o) } -func (o BySelectorLastUpdateTime) Swap(i, j int) { o[i], o[j] = o[j], o[i] } -func (o BySelectorLastUpdateTime) Less(i, j int) bool { - ti, tj := LastSelectorUpdate(o[i]), LastSelectorUpdate(o[j]) - if ti.Equal(tj) { - if o[i].CreationTimestamp.Equal(o[j].CreationTimestamp) { - return o[i].Name < o[j].Name - } - return o[i].CreationTimestamp.Before(o[j].CreationTimestamp) - } - return ti.Before(tj) -} - -// OverlapsWith returns true when two given deployments are different and overlap with each other -func OverlapsWith(current, other *extensions.Deployment) (bool, error) { - if current.UID == other.UID { - return false, nil - } - currentSelector, err := metav1.LabelSelectorAsSelector(current.Spec.Selector) - if err != nil { - return false, fmt.Errorf("deployment %s/%s has invalid label selector: %v", current.Namespace, current.Name, err) - } - otherSelector, err := metav1.LabelSelectorAsSelector(other.Spec.Selector) - if err != nil { - return false, fmt.Errorf("deployment %s/%s has invalid label selector: %v", other.Namespace, other.Name, err) - } - return (!currentSelector.Empty() && currentSelector.Matches(labels.Set(other.Spec.Template.Labels))) || - (!otherSelector.Empty() && otherSelector.Matches(labels.Set(current.Spec.Template.Labels))), nil -} diff --git a/pkg/controller/deployment/util/deployment_util_test.go b/pkg/controller/deployment/util/deployment_util_test.go index add606ce043..18abfeb04d3 100644 --- a/pkg/controller/deployment/util/deployment_util_test.go +++ b/pkg/controller/deployment/util/deployment_util_test.go @@ -1189,111 +1189,3 @@ func TestDeploymentTimedOut(t *testing.T) { } } } - -func TestSelectorUpdatedBefore(t *testing.T) { - now := metav1.Now() - later := metav1.Time{Time: now.Add(time.Minute)} - selectorUpdated := metav1.Time{Time: later.Add(time.Minute)} - selectorUpdatedLater := metav1.Time{Time: selectorUpdated.Add(time.Minute)} - - tests := []struct { - name string - - d1 extensions.Deployment - creationTimestamp1 *metav1.Time - selectorUpdated1 *metav1.Time - - d2 extensions.Deployment - creationTimestamp2 *metav1.Time - selectorUpdated2 *metav1.Time - - expected bool - }{ - { - name: "d1 created before d2", - - d1: generateDeployment("foo"), - creationTimestamp1: &now, - - d2: generateDeployment("bar"), - creationTimestamp2: &later, - - expected: true, - }, - { - name: "d1 created after d2", - - d1: generateDeployment("foo"), - creationTimestamp1: &later, - - d2: generateDeployment("bar"), - creationTimestamp2: &now, - - expected: false, - }, - { - // Think of the following scenario: - // d1 is created first, d2 is created after and its selector overlaps - // with d1. d2 is marked as overlapping correctly. If d1's selector is - // updated and continues to overlap with the selector of d2 then d1 is - // now marked overlapping and d2 is cleaned up. Proved by the following - // test case. Callers of SelectorUpdatedBefore should first check for - // the existence of the overlapping annotation in any of the two deployments - // prior to comparing their timestamps and as a matter of fact this is - // now handled in `(dc *DeploymentController) handleOverlap`. - name: "d1 created before d2 but updated its selector afterwards", - - d1: generateDeployment("foo"), - creationTimestamp1: &now, - selectorUpdated1: &selectorUpdated, - - d2: generateDeployment("bar"), - creationTimestamp2: &later, - - expected: false, - }, - { - name: "d1 selector is older than d2", - - d1: generateDeployment("foo"), - selectorUpdated1: &selectorUpdated, - - d2: generateDeployment("bar"), - selectorUpdated2: &selectorUpdatedLater, - - expected: true, - }, - { - name: "d1 selector is younger than d2", - - d1: generateDeployment("foo"), - selectorUpdated1: &selectorUpdatedLater, - - d2: generateDeployment("bar"), - selectorUpdated2: &selectorUpdated, - - expected: false, - }, - } - - for _, test := range tests { - t.Logf("running scenario %q", test.name) - - if test.creationTimestamp1 != nil { - test.d1.CreationTimestamp = *test.creationTimestamp1 - } - if test.creationTimestamp2 != nil { - test.d2.CreationTimestamp = *test.creationTimestamp2 - } - if test.selectorUpdated1 != nil { - test.d1.Annotations[SelectorUpdateAnnotation] = test.selectorUpdated1.Format(time.RFC3339) - } - if test.selectorUpdated2 != nil { - test.d2.Annotations[SelectorUpdateAnnotation] = test.selectorUpdated2.Format(time.RFC3339) - } - - if got := SelectorUpdatedBefore(&test.d1, &test.d2); got != test.expected { - t.Errorf("expected d1 selector to be updated before d2: %t, got: %t", test.expected, got) - } - } -} diff --git a/pkg/kubectl/stop.go b/pkg/kubectl/stop.go index 5a70a3958e5..c46d1039f50 100644 --- a/pkg/kubectl/stop.go +++ b/pkg/kubectl/stop.go @@ -441,11 +441,6 @@ func (reaper *DeploymentReaper) Stop(namespace, name string, timeout time.Durati return err } - // Do not cascade deletion for overlapping deployments. - if len(deployment.Annotations[deploymentutil.OverlapAnnotation]) > 0 { - return deployments.Delete(name, nil) - } - // Stop all replica sets. selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector) if err != nil { diff --git a/pkg/printers/internalversion/describe.go b/pkg/printers/internalversion/describe.go index ad19bc4d55f..0a6d6606c3f 100644 --- a/pkg/printers/internalversion/describe.go +++ b/pkg/printers/internalversion/describe.go @@ -2439,10 +2439,6 @@ func (dd *DeploymentDescriber) Describe(namespace, name string, describerSetting } w.Write(LEVEL_0, "NewReplicaSet:\t%s\n", printReplicaSetsByLabels(newRSs)) } - overlapWith := d.Annotations[deploymentutil.OverlapAnnotation] - if len(overlapWith) > 0 { - w.Write(LEVEL_0, "!!!WARNING!!! This deployment has overlapping label selector with deployment %q and won't behave as expected. Please fix it before continue.\n", overlapWith) - } if describerSettings.ShowEvents { events, err := dd.Core().Events(namespace).Search(api.Scheme, d) if err == nil && events != nil { diff --git a/pkg/registry/extensions/deployment/BUILD b/pkg/registry/extensions/deployment/BUILD index 752a9b1e0c7..a7c8a9a66be 100644 --- a/pkg/registry/extensions/deployment/BUILD +++ b/pkg/registry/extensions/deployment/BUILD @@ -20,7 +20,6 @@ go_library( "//pkg/api:go_default_library", "//pkg/apis/extensions:go_default_library", "//pkg/apis/extensions/validation:go_default_library", - "//pkg/controller/deployment/util:go_default_library", "//vendor:k8s.io/apimachinery/pkg/apis/meta/internalversion", "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1", "//vendor:k8s.io/apimachinery/pkg/fields", diff --git a/pkg/registry/extensions/deployment/strategy.go b/pkg/registry/extensions/deployment/strategy.go index 831082b24ec..059e1b20998 100644 --- a/pkg/registry/extensions/deployment/strategy.go +++ b/pkg/registry/extensions/deployment/strategy.go @@ -19,9 +19,7 @@ package deployment import ( "fmt" "reflect" - "time" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -34,7 +32,6 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/kubernetes/pkg/apis/extensions/validation" - "k8s.io/kubernetes/pkg/controller/deployment/util" ) // deploymentStrategy implements behavior for Deployments. @@ -93,15 +90,6 @@ func (deploymentStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj, o !reflect.DeepEqual(newDeployment.Annotations, oldDeployment.Annotations) { newDeployment.Generation = oldDeployment.Generation + 1 } - - // Records timestamp on selector updates in annotation - if !reflect.DeepEqual(newDeployment.Spec.Selector, oldDeployment.Spec.Selector) { - if newDeployment.Annotations == nil { - newDeployment.Annotations = make(map[string]string) - } - now := metav1.Now() - newDeployment.Annotations[util.SelectorUpdateAnnotation] = now.Format(time.RFC3339) - } } // ValidateUpdate is the default update validation for an end user.