Skip to content

K
k3s
Commit 3648eaae authored by Dawn Chen's avatar Dawn Chen Committed by GitHub
Browse files
Options

Revert "controller: unit tests for overlapping and recreate deployments"

parent 4ac5f278
Hide whitespace changes
Inline Side-by-side
Showing with 24 additions and 412 deletions
pkg/controller/deployment/deployment_controller.go
View file @ 3648eaae
......@@ -73,8 +73,6 @@ type DeploymentController struct {
// To allow injection of syncDeployment for testing.
syncHandler func(dKey string) error
// used for unit testing
enqueueDeployment func(deployment *extensions.Deployment)
// A store of deployments, populated by the dController
dLister *cache.StoreToDeploymentLister
......@@ -136,8 +134,6 @@ func NewDeploymentController(dInformer informers.DeploymentInformer, rsInformer
})
dc.syncHandler = dc.syncDeployment
dc.enqueueDeployment = dc.enqueue
dc.dLister = dInformer.Lister()
dc.rsLister = rsInformer.Lister()
dc.podLister = podInformer.Lister()
......@@ -347,7 +343,7 @@ func (dc *DeploymentController) deletePod(obj interface{}) {
}
}
func (dc *DeploymentController) enqueue(deployment *extensions.Deployment) {
func (dc *DeploymentController) enqueueDeployment(deployment *extensions.Deployment) {
key, err := controller.KeyFunc(deployment)
if err != nil {
utilruntime.HandleError(fmt.Errorf("Couldn't get key for object %#v: %v", deployment, err))
......@@ -612,27 +608,14 @@ func (dc *DeploymentController) handleOverlap(d *extensions.Deployment, deployme
// deployments if this one has been marked deleted, we only update its status as long
// as it is not actually deleted.
if foundOverlaps && d.DeletionTimestamp == nil {
overlapping = true
// Look at the overlapping annotation in both deployments. If one of them has it and points
// to the other one then we don't need to compare their timestamps.
otherOverlapsWith := otherD.Annotations[util.OverlapAnnotation]
currentOverlapsWith := d.Annotations[util.OverlapAnnotation]
// The other deployment is already marked as overlapping with the current one.
if otherOverlapsWith == d.Name {
var err error
if d, err = dc.clearDeploymentOverlap(d, otherD.Name); err != nil {
errs = append(errs, err)
}
continue
}
otherCopy, err := util.DeploymentDeepCopy(otherD)
if err != nil {
return false, err
}
overlapping = true
// Skip syncing this one if older overlapping one is found.
if currentOverlapsWith == otherCopy.Name || util.SelectorUpdatedBefore(otherCopy, d) {
if util.SelectorUpdatedBefore(otherCopy, d) {
if _, err = dc.markDeploymentOverlap(d, otherCopy.Name); err != nil {
return false, err
}
......
pkg/controller/deployment/deployment_controller_test.go
View file @ 3648eaae
......@@ -19,7 +19,6 @@ package deployment
import (
"fmt"
"testing"
"time"
"k8s.io/apimachinery/pkg/apimachinery/registered"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
......@@ -31,7 +30,6 @@ import (
"k8s.io/kubernetes/pkg/client/record"
"k8s.io/kubernetes/pkg/client/testing/core"
"k8s.io/kubernetes/pkg/controller"
"k8s.io/kubernetes/pkg/controller/deployment/util"
"k8s.io/kubernetes/pkg/controller/informers"
"k8s.io/kubernetes/pkg/util/intstr"
"k8s.io/kubernetes/pkg/util/uuid"
......@@ -69,10 +67,9 @@ func newDeployment(name string, replicas int, revisionHistoryLimit *int32, maxSu
d := extensions.Deployment{
TypeMeta: metav1.TypeMeta{APIVersion: registered.GroupOrDie(extensions.GroupName).GroupVersion.String()},
ObjectMeta: v1.ObjectMeta{
UID: uuid.NewUUID(),
Name: name,
Namespace: v1.NamespaceDefault,
Annotations: make(map[string]string),
UID: uuid.NewUUID(),
Name: name,
Namespace: v1.NamespaceDefault,
},
Spec: extensions.DeploymentSpec{
Strategy: extensions.DeploymentStrategy{
......@@ -113,10 +110,8 @@ func newReplicaSet(d *extensions.Deployment, name string, replicas int) *extensi
ObjectMeta: v1.ObjectMeta{
Name: name,
Namespace: v1.NamespaceDefault,
Labels: d.Spec.Selector.MatchLabels,
},
Spec: extensions.ReplicaSetSpec{
Selector: d.Spec.Selector,
Replicas: func() *int32 { i := int32(replicas); return &i }(),
Template: d.Spec.Template,
},
......@@ -147,6 +142,10 @@ type fixture struct {
objects []runtime.Object
}
func (f *fixture) expectUpdateDeploymentAction(d *extensions.Deployment) {
f.actions = append(f.actions, core.NewUpdateAction(schema.GroupVersionResource{Resource: "deployments"}, d.Namespace, d))
}
func (f *fixture) expectUpdateDeploymentStatusAction(d *extensions.Deployment) {
action := core.NewUpdateAction(schema.GroupVersionResource{Resource: "deployments"}, d.Namespace, d)
action.Subresource = "status"
......@@ -164,7 +163,7 @@ func newFixture(t *testing.T) *fixture {
return f
}
func (f *fixture) newController() (*DeploymentController, informers.SharedInformerFactory) {
func (f *fixture) run(deploymentName string) {
f.client = fake.NewSimpleClientset(f.objects...)
informers := informers.NewSharedInformerFactory(f.client, nil, controller.NoResyncPeriodFunc())
c := NewDeploymentController(informers.Deployments(), informers.ReplicaSets(), informers.Pods(), f.client)
......@@ -181,11 +180,6 @@ func (f *fixture) newController() (*DeploymentController, informers.SharedInform
for _, pod := range f.podLister {
c.podLister.Indexer.Add(pod)
}
return c, informers
}
func (f *fixture) run(deploymentName string) {
c, informers := f.newController()
stopCh := make(chan struct{})
defer close(stopCh)
informers.Start(stopCh)
......@@ -224,7 +218,7 @@ func TestSyncDeploymentCreatesReplicaSet(t *testing.T) {
rs := newReplicaSet(d, "deploymentrs-4186632231", 1)
f.expectCreateRSAction(rs)
f.expectUpdateDeploymentStatusAction(d)
f.expectUpdateDeploymentAction(d)
f.expectUpdateDeploymentStatusAction(d)
f.run(getKey(d, t))
......@@ -292,260 +286,3 @@ func filterInformerActions(actions []core.Action) []core.Action {
return ret
}
// TestOverlappingDeployment ensures that an overlapping deployment will not be synced by
// the controller.
func TestOverlappingDeployment(t *testing.T) {
f := newFixture(t)
now := metav1.Now()
later := metav1.Time{Time: now.Add(time.Minute)}
foo := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
foo.CreationTimestamp = now
bar := newDeployment("bar", 1, nil, nil, nil, map[string]string{"foo": "bar", "app": "baz"})
bar.CreationTimestamp = later
f.dLister = append(f.dLister, foo, bar)
f.objects = append(f.objects, foo, bar)
f.expectUpdateDeploymentStatusAction(bar)
f.run(getKey(bar, t))
actions := f.client.Actions()
d := actions[0].(core.UpdateAction).GetObject().(*extensions.Deployment)
if len(d.Annotations[util.OverlapAnnotation]) == 0 {
t.Errorf("annotations weren't updated for the overlapping deployment: %v", d.Annotations)
}
}
// TestSyncOverlappedDeployment ensures that from two overlapping deployments, the older
// one will be synced and the newer will be marked as overlapping. Note that in reality it's
// not always the older deployment that is the one that works vs the rest but the one which
// has the selector unchanged for longer time.
func TestSyncOverlappedDeployment(t *testing.T) {
f := newFixture(t)
now := metav1.Now()
later := metav1.Time{Time: now.Add(time.Minute)}
foo := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
foo.CreationTimestamp = now
bar := newDeployment("bar", 1, nil, nil, nil, map[string]string{"foo": "bar", "app": "baz"})
bar.CreationTimestamp = later
f.dLister = append(f.dLister, foo, bar)
f.objects = append(f.objects, foo, bar)
f.expectUpdateDeploymentStatusAction(bar)
f.expectCreateRSAction(newReplicaSet(foo, "foo-rs", 1))
f.expectUpdateDeploymentStatusAction(foo)
f.expectUpdateDeploymentStatusAction(foo)
f.run(getKey(foo, t))
actions := f.client.Actions()
d := actions[0].(core.UpdateAction).GetObject().(*extensions.Deployment)
if d.Annotations[util.OverlapAnnotation] != "foo" {
t.Errorf("annotations weren't updated for the overlapping deployment: %v", d.Annotations)
}
}
// TestSelectorUpdate ensures that from two overlapping deployments, the one that is working won't
// be marked as overlapping if its selector is updated but still overlaps with the other one.
func TestSelectorUpdate(t *testing.T) {
f := newFixture(t)
now := metav1.Now()
later := metav1.Time{Time: now.Add(time.Minute)}
selectorUpdated := metav1.Time{Time: later.Add(time.Minute)}
foo := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
foo.CreationTimestamp = now
foo.Annotations = map[string]string{util.SelectorUpdateAnnotation: selectorUpdated.Format(time.RFC3339)}
bar := newDeployment("bar", 1, nil, nil, nil, map[string]string{"foo": "bar", "app": "baz"})
bar.CreationTimestamp = later
bar.Annotations = map[string]string{util.OverlapAnnotation: "foo"}
f.dLister = append(f.dLister, foo, bar)
f.objects = append(f.objects, foo, bar)
f.expectCreateRSAction(newReplicaSet(foo, "foo-rs", 1))
f.expectUpdateDeploymentStatusAction(foo)
f.expectUpdateDeploymentStatusAction(foo)
f.run(getKey(foo, t))
for _, a := range filterInformerActions(f.client.Actions()) {
action, ok := a.(core.UpdateAction)
if !ok {
continue
}
d, ok := action.GetObject().(*extensions.Deployment)
if !ok {
continue
}
if d.Name == "foo" && len(d.Annotations[util.OverlapAnnotation]) > 0 {
t.Errorf("deployment %q should not have the overlapping annotation", d.Name)
}
if d.Name == "bar" && len(d.Annotations[util.OverlapAnnotation]) == 0 {
t.Errorf("deployment %q should have the overlapping annotation", d.Name)
}
}
}
// TestDeletedDeploymentShouldCleanupOverlaps ensures that the deletion of a deployment
// will cleanup any deployments that overlap with it.
func TestDeletedDeploymentShouldCleanupOverlaps(t *testing.T) {
f := newFixture(t)
now := metav1.Now()
earlier := metav1.Time{Time: now.Add(-time.Minute)}
later := metav1.Time{Time: now.Add(time.Minute)}
foo := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
foo.CreationTimestamp = earlier
foo.DeletionTimestamp = &now
bar := newDeployment("bar", 1, nil, nil, nil, map[string]string{"foo": "bar"})
bar.CreationTimestamp = later
bar.Annotations = map[string]string{util.OverlapAnnotation: "foo"}
f.dLister = append(f.dLister, foo, bar)
f.objects = append(f.objects, foo, bar)
f.expectUpdateDeploymentStatusAction(bar)
f.expectUpdateDeploymentStatusAction(foo)
f.run(getKey(foo, t))
for _, a := range filterInformerActions(f.client.Actions()) {
action, ok := a.(core.UpdateAction)
if !ok {
continue
}
d := action.GetObject().(*extensions.Deployment)
if d.Name != "bar" {
continue
}
if len(d.Annotations[util.OverlapAnnotation]) > 0 {
t.Errorf("annotations weren't cleaned up for the overlapping deployment: %v", d.Annotations)
}
}
}
// TestDeletedDeploymentShouldNotCleanupOtherOverlaps ensures that the deletion of
// a deployment will not cleanup deployments that overlap with another deployment.
func TestDeletedDeploymentShouldNotCleanupOtherOverlaps(t *testing.T) {
f := newFixture(t)
now := metav1.Now()
earlier := metav1.Time{Time: now.Add(-time.Minute)}
later := metav1.Time{Time: now.Add(time.Minute)}
foo := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
foo.CreationTimestamp = earlier
foo.DeletionTimestamp = &now
bar := newDeployment("bar", 1, nil, nil, nil, map[string]string{"bla": "bla"})
bar.CreationTimestamp = later
// Notice this deployment is overlapping with another deployment
bar.Annotations = map[string]string{util.OverlapAnnotation: "baz"}
f.dLister = append(f.dLister, foo, bar)
f.objects = append(f.objects, foo, bar)
f.expectUpdateDeploymentStatusAction(foo)
f.run(getKey(foo, t))
for _, a := range filterInformerActions(f.client.Actions()) {
action, ok := a.(core.UpdateAction)
if !ok {
continue
}
d := action.GetObject().(*extensions.Deployment)
if d.Name != "bar" {
continue
}
if len(d.Annotations[util.OverlapAnnotation]) == 0 {
t.Errorf("overlapping annotation should not be cleaned up for bar: %v", d.Annotations)
}
}
}
// TestPodDeletionEnqueuesRecreateDeployment ensures that the deletion of a pod
// will requeue a Recreate deployment iff there is no other pod returned from the
// client.
func TestPodDeletionEnqueuesRecreateDeployment(t *testing.T) {
f := newFixture(t)
foo := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
foo.Spec.Strategy.Type = extensions.RecreateDeploymentStrategyType
rs := newReplicaSet(foo, "foo-1", 1)
pod := generatePodFromRS(rs)
f.dLister = append(f.dLister, foo)
f.rsLister = append(f.rsLister, rs)
f.objects = append(f.objects, foo, rs)
c, informers := f.newController()
enqueued := false
c.enqueueDeployment = func(d *extensions.Deployment) {
if d.Name == "foo" {
enqueued = true
}
}
stopCh := make(chan struct{})
defer close(stopCh)
informers.Start(stopCh)
c.deletePod(pod)
if !enqueued {
t.Errorf("expected deployment %q to be queued after pod deletion", foo.Name)
}
}
// TestPodDeletionDoesntEnqueueRecreateDeployment ensures that the deletion of a pod
// will not requeue a Recreate deployment iff there are other pods returned from the
// client.
func TestPodDeletionDoesntEnqueueRecreateDeployment(t *testing.T) {
f := newFixture(t)
foo := newDeployment("foo", 1, nil, nil, nil, map[string]string{"foo": "bar"})
foo.Spec.Strategy.Type = extensions.RecreateDeploymentStrategyType
rs := newReplicaSet(foo, "foo-1", 1)
pod := generatePodFromRS(rs)
f.dLister = append(f.dLister, foo)
f.rsLister = append(f.rsLister, rs)
// Let's pretend this is a different pod. The gist is that the pod lister needs to
// return a non-empty list.
f.podLister = append(f.podLister, pod)
c, informers := f.newController()
enqueued := false
c.enqueueDeployment = func(d *extensions.Deployment) {
if d.Name == "foo" {
enqueued = true
}
}
stopCh := make(chan struct{})
defer close(stopCh)
informers.Start(stopCh)
c.deletePod(pod)
if enqueued {
t.Errorf("expected deployment %q not to be queued after pod deletion", foo.Name)
}
}
// generatePodFromRS creates a pod, with the input ReplicaSet's selector and its template
func generatePodFromRS(rs *extensions.ReplicaSet) *v1.Pod {
trueVar := true
return &v1.Pod{
ObjectMeta: v1.ObjectMeta{
Name: rs.Name + "-pod",
Namespace: rs.Namespace,
Labels: rs.Spec.Selector.MatchLabels,
OwnerReferences: []metav1.OwnerReference{
{UID: rs.UID, APIVersion: "v1beta1", Kind: "ReplicaSet", Name: rs.Name, Controller: &trueVar},
},
},
Spec: rs.Spec.Template.Spec,
}
}
pkg/controller/deployment/util/deployment_util.go
View file @ 3648eaae
......@@ -1003,7 +1003,7 @@ func DeploymentDeepCopy(deployment *extensions.Deployment) (*extensions.Deployme
}
// SelectorUpdatedBefore returns true if the former deployment's selector
// is updated before the latter, false otherwise.
// is updated before the latter, false otherwise
func SelectorUpdatedBefore(d1, d2 *extensions.Deployment) bool {
t1, t2 := LastSelectorUpdate(d1), LastSelectorUpdate(d2)
return t1.Before(t2)
......@@ -1013,7 +1013,7 @@ func SelectorUpdatedBefore(d1, d2 *extensions.Deployment) bool {
func LastSelectorUpdate(d *extensions.Deployment) metav1.Time {
t := d.Annotations[SelectorUpdateAnnotation]
if len(t) > 0 {
parsedTime, err := time.Parse(time.RFC3339, t)
parsedTime, err := time.Parse(t, time.RFC3339)
// If failed to parse the time, use creation timestamp instead
if err != nil {
return d.CreationTimestamp
......
pkg/controller/deployment/util/deployment_util_test.go
View file @ 3648eaae
......@@ -177,8 +177,7 @@ func generateDeployment(image string) extensions.Deployment {
terminationSec := int64(30)
return extensions.Deployment{
ObjectMeta: v1.ObjectMeta{
Name: image,
Annotations: make(map[string]string),
Name: image,
},
Spec: extensions.DeploymentSpec{
Replicas: func(i int32) *int32 { return &i }(1),
......@@ -626,6 +625,7 @@ func TestGetReplicaCountForReplicaSets(t *testing.T) {
}
func TestResolveFenceposts(t *testing.T) {
tests := []struct {
maxSurge string
maxUnavailable string
......@@ -1107,111 +1107,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)
}
}
}
test/e2e/deployment.go
View file @ 3648eaae
......@@ -1194,27 +1194,27 @@ func testOverlappingDeployment(f *framework.Framework) {
podLabels = map[string]string{"name": nginxImageName}
By(fmt.Sprintf("Creating deployment %q", deploymentName))
d = newDeployment(deploymentName, replicas, podLabels, nginxImageName, nginxImage, extensions.RollingUpdateDeploymentStrategyType, nil)
thirdDeployment, err := c.Extensions().Deployments(ns).Create(d)
deployLater, err := c.Extensions().Deployments(ns).Create(d)
Expect(err).NotTo(HaveOccurred(), "Failed creating the third deployment")
// Wait for it to be updated to revision 1
err = framework.WaitForDeploymentRevisionAndImage(c, ns, thirdDeployment.Name, "1", nginxImage)
err = framework.WaitForDeploymentRevisionAndImage(c, ns, deployLater.Name, "1", nginxImage)
Expect(err).NotTo(HaveOccurred(), "The third deployment failed to update to revision 1")
// Update the second deployment's selector to make it overlap with the third deployment
By(fmt.Sprintf("Updating deployment %q selector to make it overlap with existing one", deployOverlapping.Name))
deployOverlapping, err = framework.UpdateDeploymentWithRetries(c, ns, deployOverlapping.Name, func(update *extensions.Deployment) {
update.Spec.Selector = thirdDeployment.Spec.Selector
update.Spec.Template.Labels = thirdDeployment.Spec.Template.Labels
update.Spec.Selector = deployLater.Spec.Selector
update.Spec.Template.Labels = deployLater.Spec.Template.Labels
update.Spec.Template.Spec.Containers[0].Image = redisImage
})
Expect(err).NotTo(HaveOccurred())
// Wait for overlapping annotation updated to both deployments
By("Waiting for the second deployment to have the overlapping annotation")
err = framework.WaitForOverlappingAnnotationMatch(c, ns, deployOverlapping.Name, thirdDeployment.Name)
Expect(err).NotTo(HaveOccurred(), "Failed to update the second deployment's overlapping annotation")
err = framework.WaitForOverlappingAnnotationMatch(c, ns, thirdDeployment.Name, "")
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
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment