github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-24 20:24:09 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #34952 from kargakis/update-observedgeneration-for-overlapping-deployments

Browse Source 
Automatic merge from submit-queue

Make overlapping deployments deletable

@kubernetes/deployment ptal

Fixes https://github.com/kubernetes/kubernetes/issues/34466 by 1) not adding the overlapping annotation in the working deployment, 2) updates observedGeneration for overlapping deployments, and 3) updates the kubectl deployment reaper to do non-cascading deletion for deployments with the overlapping annotation.
This commit is contained in:
Kubernetes Submit Queue 2016-10-29 14:50:16 -07:00 committed by GitHub
commit 7c9c8cbf28
4 changed files with 57 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
hack/make-rules/test-cmd.sh
View File

@ -2268,6 +2268,13 @@ __EOF__
kubectl get rs nginx-618515232 -o yaml | grep "deployment.kubernetes.io/revision-history: 1,3"
# 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
kubectl delete deployment nginx "${kube_flags[@]}"

74
pkg/controller/deployment/deployment_controller.go
View File

@ -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)
}

5
pkg/kubectl/stop.go
View File

@ -435,6 +435,11 @@ 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 := unversioned.LabelSelectorAsSelector(deployment.Spec.Selector)
if err != nil {

12
test/e2e/deployment.go
View File

@ -1239,11 +1239,11 @@ func testOverlappingDeployment(f *framework.Framework) {
Expect(err).NotTo(HaveOccurred(), "Failed creating the second deployment")
// Wait for overlapping annotation updated to both deployments
By("Waiting for both deployments to have overlapping annotations")
err = framework.WaitForOverlappingAnnotationMatch(c, ns, deploy.Name, deployOverlapping.Name)
Expect(err).NotTo(HaveOccurred(), "Failed to update the first deployment's overlapping annotation")
By("Waiting for the overlapping deployment to have overlapping annotation")
err = framework.WaitForOverlappingAnnotationMatch(c, ns, deployOverlapping.Name, deploy.Name)
Expect(err).NotTo(HaveOccurred(), "Failed to update the second deployment's overlapping annotation")
err = framework.WaitForOverlappingAnnotationMatch(c, ns, deploy.Name, "")
Expect(err).NotTo(HaveOccurred(), "The deployment that holds the oldest selector shouldn't have the overlapping annotation")
// Only the first deployment is synced
By("Checking only the first overlapping deployment is synced")
@ -1295,11 +1295,11 @@ func testOverlappingDeployment(f *framework.Framework) {
Expect(err).NotTo(HaveOccurred())
// Wait for overlapping annotation updated to both deployments
By("Waiting for both deployments to have overlapping annotations")
err = framework.WaitForOverlappingAnnotationMatch(c, ns, deployOverlapping.Name, deployLater.Name)
Expect(err).NotTo(HaveOccurred(), "Failed to update the second deployment's overlapping annotation")
By("Waiting for the third deployment to have the overlapping annotation")
err = framework.WaitForOverlappingAnnotationMatch(c, ns, deployLater.Name, deployOverlapping.Name)
Expect(err).NotTo(HaveOccurred(), "Failed to update the third deployment's overlapping annotation")
err = framework.WaitForOverlappingAnnotationMatch(c, ns, deployOverlapping.Name, "")
Expect(err).NotTo(HaveOccurred(), "The deployment that holds the oldest selector shouldn't have the overlapping annotation")
// The second deployment shouldn't be synced
By("Checking the second deployment is not synced")