From f2ae192e3a3e7f6f8d1b6ceeddf8e73fccbc38f2 Mon Sep 17 00:00:00 2001 From: Michail Kargakis Date: Fri, 14 Oct 2016 14:40:37 +0200 Subject: [PATCH] controller: update overlapping deployments to be deletable --- .../deployment/deployment_controller.go | 74 ++++++++++--------- 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index 66226677ea9..b2dc7b97f17 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/pkg/controller/deployment/deployment_controller.go @@ -265,25 +265,6 @@ func (dc *DeploymentController) enqueueDeployment(deployment *extensions.Deploym dc.queue.Add(key) } -func (dc *DeploymentController) markDeploymentOverlap(deployment *extensions.Deployment, withDeployment string) (*extensions.Deployment, error) { - if deployment.Annotations[util.OverlapAnnotation] == withDeployment { - return deployment, nil - } - if deployment.Annotations == nil { - deployment.Annotations = make(map[string]string) - } - deployment.Annotations[util.OverlapAnnotation] = withDeployment - return dc.client.Extensions().Deployments(deployment.Namespace).Update(deployment) -} - -func (dc *DeploymentController) clearDeploymentOverlap(deployment *extensions.Deployment) (*extensions.Deployment, error) { - if len(deployment.Annotations[util.OverlapAnnotation]) == 0 { - return deployment, nil - } - delete(deployment.Annotations, util.OverlapAnnotation) - return dc.client.Extensions().Deployments(deployment.Namespace).Update(deployment) -} - // worker runs a worker thread that just dequeues items, processes them, and marks them done. // It enforces that the syncHandler is never invoked concurrently with the same key. func (dc *DeploymentController) worker() { @@ -342,12 +323,6 @@ func (dc *DeploymentController) syncDeployment(key string) error { } deployment := obj.(*extensions.Deployment) - everything := unversioned.LabelSelector{} - if reflect.DeepEqual(deployment.Spec.Selector, &everything) { - dc.eventRecorder.Eventf(deployment, api.EventTypeWarning, "SelectingAll", "This deployment is selecting all pods. A non-empty selector is required.") - return nil - } - // Deep-copy otherwise we are mutating our cache. // TODO: Deep-copy only when needed. d, err := util.DeploymentDeepCopy(deployment) @@ -355,13 +330,23 @@ func (dc *DeploymentController) syncDeployment(key string) error { return err } + everything := unversioned.LabelSelector{} + if reflect.DeepEqual(d.Spec.Selector, &everything) { + dc.eventRecorder.Eventf(d, api.EventTypeWarning, "SelectingAll", "This deployment is selecting all pods. A non-empty selector is required.") + if d.Status.ObservedGeneration < d.Generation { + d.Status.ObservedGeneration = d.Generation + dc.client.Extensions().Deployments(d.Namespace).UpdateStatus(d) + } + return nil + } + if d.DeletionTimestamp != nil { return dc.syncStatusOnly(d) } // Handle overlapping deployments by deterministically avoid syncing deployments that fight over ReplicaSets. if err = dc.handleOverlap(d); err != nil { - dc.eventRecorder.Eventf(deployment, api.EventTypeWarning, "SelectorOverlap", err.Error()) + dc.eventRecorder.Eventf(d, api.EventTypeWarning, "SelectorOverlap", err.Error()) return nil } @@ -413,17 +398,15 @@ func (dc *DeploymentController) handleOverlap(d *extensions.Deployment) error { return err } overlapping = true - // We don't care if the overlapping annotation update failed or not (we don't make decision on it) - d, _ = dc.markDeploymentOverlap(d, other.Name) - deploymentCopy, _ = dc.markDeploymentOverlap(deploymentCopy, d.Name) - // Skip syncing this one if older overlapping one is found - // TODO: figure out a better way to determine which deployment to skip, - // either with controller reference, or with validation. - // Using oldest active replica set to determine which deployment to skip wouldn't make much difference, - // since new replica set hasn't been created after selector update + // Skip syncing this one if older overlapping one is found. if util.SelectorUpdatedBefore(deploymentCopy, d) { - return fmt.Errorf("found deployment %s/%s has overlapping selector with an older deployment %s/%s, skip syncing it", d.Namespace, d.Name, other.Namespace, other.Name) + // We don't care if the overlapping annotation update failed or not (we don't make decision on it) + dc.markDeploymentOverlap(d, deploymentCopy.Name) + dc.clearDeploymentOverlap(deploymentCopy) + return fmt.Errorf("found deployment %s/%s has overlapping selector with an older deployment %s/%s, skip syncing it", d.Namespace, d.Name, deploymentCopy.Namespace, deploymentCopy.Name) } + dc.markDeploymentOverlap(deploymentCopy, d.Name) + d, _ = dc.clearDeploymentOverlap(d) } } if !overlapping { @@ -432,3 +415,24 @@ func (dc *DeploymentController) handleOverlap(d *extensions.Deployment) error { } return nil } + +func (dc *DeploymentController) markDeploymentOverlap(deployment *extensions.Deployment, withDeployment string) (*extensions.Deployment, error) { + if deployment.Annotations[util.OverlapAnnotation] == withDeployment && deployment.Status.ObservedGeneration >= deployment.Generation { + return deployment, nil + } + if deployment.Annotations == nil { + deployment.Annotations = make(map[string]string) + } + // Update observedGeneration for overlapping deployments so that their deletion won't be blocked. + deployment.Status.ObservedGeneration = deployment.Generation + deployment.Annotations[util.OverlapAnnotation] = withDeployment + return dc.client.Extensions().Deployments(deployment.Namespace).UpdateStatus(deployment) +} + +func (dc *DeploymentController) clearDeploymentOverlap(deployment *extensions.Deployment) (*extensions.Deployment, error) { + if len(deployment.Annotations[util.OverlapAnnotation]) == 0 { + return deployment, nil + } + delete(deployment.Annotations, util.OverlapAnnotation) + return dc.client.Extensions().Deployments(deployment.Namespace).UpdateStatus(deployment) +}