Skip to content

K
k3s
Commit 93528bb8 authored by Mike Danese's avatar Mike Danese Committed by GitHub
Browse files
Options

Merge pull request #39778 from kubernetes/revert-39088-unit-tests-for-the-d-controller

Revert "controller: unit tests for overlapping and recreate deployments"
parents 4ac5f278 3648eaae
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 15 additions and 140 deletions
pkg/controller/deployment/deployment_controller.go
View file @ 93528bb8
......@@ -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/util/deployment_util.go
View file @ 93528bb8
......@@ -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 @ 93528bb8
......@@ -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 @ 93528bb8
......@@ -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