Skip to content

Commit

Permalink
Merge pull request #16998 from tnozicka/fix-multiple-deployer-pods-du…
Browse files Browse the repository at this point in the history
…e-to-cancellation

Automatic merge from submit-queue (batch tested with PRs 17350, 16998).

apps: extend extended tests to better check for deployer invariants and enable back the old check

enabling back #16956
openshift-merge-robot authored Nov 30, 2017
2 parents 57cbd66 + ba495e5 commit 6f2ba91
Showing 4 changed files with 438 additions and 34 deletions.
3 changes: 2 additions & 1 deletion pkg/apps/controller/deployer/deployer_controller.go
Original file line number Diff line number Diff line change
@@ -379,7 +379,8 @@ func (c *DeploymentController) makeDeployerPod(deployment *v1.ReplicationControl
ObjectMeta: metav1.ObjectMeta{
Name: deployutil.DeployerPodNameForDeployment(deployment.Name),
Annotations: map[string]string{
deployapi.DeploymentAnnotation: deployment.Name,
deployapi.DeploymentAnnotation: deployment.Name,
deployapi.DeploymentConfigAnnotation: deployutil.DeploymentConfigNameFor(deployment),
},
Labels: map[string]string{
deployapi.DeployerPodForDeploymentLabel: deployment.Name,
Original file line number Diff line number Diff line change
@@ -121,16 +121,16 @@ func (c *DeploymentConfigController) Handle(config *deployapi.DeploymentConfig)
return c.updateStatus(config, existingDeployments)
}

latestIsDeployed, latestDeployment := deployutil.LatestDeploymentInfo(config, existingDeployments)
latestExists, latestDeployment := deployutil.LatestDeploymentInfo(config, existingDeployments)

if !latestIsDeployed {
if !latestExists {
if err := c.cancelRunningRollouts(config, existingDeployments, cm); err != nil {
return err
}
}
configCopy := config.DeepCopy()
// Process triggers and start an initial rollouts
shouldTrigger, shouldSkip := triggerActivated(configCopy, latestIsDeployed, latestDeployment, c.codec)
shouldTrigger, shouldSkip := triggerActivated(configCopy, latestExists, latestDeployment, c.codec)
if !shouldSkip && shouldTrigger {
configCopy.Status.LatestVersion++
return c.updateStatus(configCopy, existingDeployments)
@@ -141,18 +141,7 @@ func (c *DeploymentConfigController) Handle(config *deployapi.DeploymentConfig)
}
// If the latest deployment already exists, reconcile existing deployments
// and return early.
if latestIsDeployed {
// If the latest deployment is still running, try again later. We don't
// want to compete with the deployer.
if !deployutil.IsTerminatedDeployment(latestDeployment) {
return c.updateStatus(config, existingDeployments)
}

return c.reconcileDeployments(existingDeployments, config, cm)
}
// If the latest deployment already exists, reconcile existing deployments
// and return early.
if latestIsDeployed {
if latestExists {
// If the latest deployment is still running, try again later. We don't
// want to compete with the deployer.
if !deployutil.IsTerminatedDeployment(latestDeployment) {
@@ -530,7 +519,7 @@ func (c *DeploymentConfigController) cleanupOldDeployments(existingDeployments [
// triggers were activated (config change or image change). The first bool indicates that
// the triggers are active and second indicates if we should skip the rollout because we
// are waiting for the trigger to complete update (waiting for image for example).
func triggerActivated(config *deployapi.DeploymentConfig, latestIsDeployed bool, latestDeployment *v1.ReplicationController, codec runtime.Codec) (bool, bool) {
func triggerActivated(config *deployapi.DeploymentConfig, latestExists bool, latestDeployment *v1.ReplicationController, codec runtime.Codec) (bool, bool) {
if config.Spec.Paused {
return false, false
}
@@ -569,7 +558,7 @@ func triggerActivated(config *deployapi.DeploymentConfig, latestIsDeployed bool,
}

// Wait for the RC to be created
if !latestIsDeployed {
if !latestExists {
return false, true
}

274 changes: 269 additions & 5 deletions test/extended/deployments/deployments.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package deployments

import (
"context"
"errors"
"fmt"
"math/rand"
@@ -27,10 +28,55 @@ import (
const deploymentRunTimeout = 5 * time.Minute
const deploymentChangeTimeout = 30 * time.Second

type dicEntry struct {
dic *deployerPodInvariantChecker
ctx context.Context
cancel func()
}

var _ = g.Describe("[Feature:DeploymentConfig] deploymentconfigs", func() {
defer g.GinkgoRecover()

dicMap := make(map[string]dicEntry)
var oc *exutil.CLI

g.JustBeforeEach(func() {
namespace := oc.Namespace()
o.Expect(namespace).NotTo(o.BeEmpty())
o.Expect(dicMap).NotTo(o.HaveKey(namespace))

dic := NewDeployerPodInvariantChecker(namespace, oc.AdminKubeClient())
ctx, cancel := context.WithCancel(context.Background())
dic.Start(ctx)

dicMap[namespace] = dicEntry{
dic: dic,
ctx: ctx,
cancel: cancel,
}
})

// This have to be registered before we create kube framework (NewCLI).
// It is probably a bug with Ginkgo because AfterEach description say innermost will be run first
// but it runs outermost first.
g.AfterEach(func() {
namespace := oc.Namespace()
o.Expect(namespace).NotTo(o.BeEmpty(), "There is something wrong with testing framework or the AfterEach functions have been registered in wrong order")
o.Expect(dicMap).To(o.HaveKey(namespace))

// Give some time to the checker to catch up
time.Sleep(2 * time.Second)

entry := dicMap[namespace]
delete(dicMap, namespace)

entry.cancel()
entry.dic.Wait()
})

oc = exutil.NewCLI("cli-deployment", exutil.KubeConfigPath())

var (
oc = exutil.NewCLI("cli-deployment", exutil.KubeConfigPath())
deploymentFixture = exutil.FixturePath("testdata", "deployments", "test-deployment-test.yaml")
simpleDeploymentFixture = exutil.FixturePath("testdata", "deployments", "deployment-simple.yaml")
customDeploymentFixture = exutil.FixturePath("testdata", "deployments", "custom-deployment.yaml")
@@ -59,12 +105,13 @@ var _ = g.Describe("[Feature:DeploymentConfig] deploymentconfigs", func() {
_, err := oc.Run("create").Args("-f", simpleDeploymentFixture).Output()
o.Expect(err).NotTo(o.HaveOccurred())

r := rand.New(rand.NewSource(g.GinkgoRandomSeed()))
iterations := 15
for i := 0; i < iterations; i++ {
if rand.Float32() < 0.2 {
time.Sleep(time.Duration(rand.Float32() * rand.Float32() * float32(time.Second)))
if r.Float32() < 0.2 {
time.Sleep(time.Duration(r.Float32() * r.Float32() * float32(time.Second)))
}
switch n := rand.Float32(); {
switch n := r.Float32(); {

case n < 0.4:
// trigger a new deployment
@@ -111,7 +158,7 @@ var _ = g.Describe("[Feature:DeploymentConfig] deploymentconfigs", func() {
for _, pod := range pods {
e2e.Logf("%02d: deleting deployer pod %s", i, pod.Name)
options := metav1.NewDeleteOptions(0)
if rand.Float32() < 0.5 {
if r.Float32() < 0.5 {
options = nil
}
if err := oc.KubeClient().CoreV1().Pods(oc.Namespace()).Delete(pod.Name, options); err != nil {
@@ -1113,4 +1160,221 @@ var _ = g.Describe("[Feature:DeploymentConfig] deploymentconfigs", func() {
})
})
})

g.Describe("keep the deployer pod invariant valid [Conformance]", func() {
dcName := "deployment-simple"

g.AfterEach(func() {
failureTrap(oc, dcName, g.CurrentGinkgoTestDescription().Failed)
})

g.It("should deal with cancellation of running deployment", func() {
namespace := oc.Namespace()

g.By("creating DC")
dc, err := readDCFixture(simpleDeploymentFixture)
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(dc.Name).To(o.Equal(dcName))

dc.Spec.Replicas = 1
// Make sure the deployer pod doesn't end too soon
dc.Spec.MinReadySeconds = 60
dc, err = oc.AppsClient().Apps().DeploymentConfigs(namespace).Create(dc)
o.Expect(err).NotTo(o.HaveOccurred())

g.By("waiting for RC to be created")
dc, err = waitForDCModification(oc, namespace, dcName, deploymentRunTimeout,
dc.GetResourceVersion(), func(config *deployapi.DeploymentConfig) (bool, error) {
cond := deployutil.GetDeploymentCondition(config.Status, deployapi.DeploymentProgressing)
if cond != nil && cond.Reason == deployapi.NewReplicationControllerReason {
return true, nil
}
return false, nil
})
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(dc.Status.LatestVersion).To(o.BeEquivalentTo(1))

g.By("waiting for deployer pod to be running")
rc, err := waitForRCModification(oc, namespace, deployutil.LatestDeploymentNameForConfig(dc), deploymentRunTimeout,
"", func(currentRC *kapiv1.ReplicationController) (bool, error) {
if deployutil.DeploymentStatusFor(currentRC) == deployapi.DeploymentStatusRunning {
return true, nil
}
return false, nil
})

g.By("canceling the deployment")
rc, err = oc.KubeClient().CoreV1().ReplicationControllers(namespace).Patch(
deployutil.LatestDeploymentNameForConfig(dc), types.StrategicMergePatchType,
[]byte(fmt.Sprintf(`{"metadata":{"annotations":{%q: %q, %q: %q}}}`,
deployapi.DeploymentCancelledAnnotation, deployapi.DeploymentCancelledAnnotationValue,
deployapi.DeploymentStatusReasonAnnotation, deployapi.DeploymentCancelledByUser,
)))
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(deployutil.DeploymentVersionFor(rc)).To(o.Equal(dc.Status.LatestVersion))

g.By("redeploying immediately by config change")
o.Expect(dc.Spec.Template.Annotations["foo"]).NotTo(o.Equal("bar"))
dc, err = oc.AppsClient().Apps().DeploymentConfigs(dc.Namespace).Patch(dc.Name, types.StrategicMergePatchType,
[]byte(`{"spec":{"template":{"metadata":{"annotations":{"foo": "bar"}}}}}`))
o.Expect(err).NotTo(o.HaveOccurred())
dc, err = waitForDCModification(oc, namespace, dcName, deploymentRunTimeout,
dc.GetResourceVersion(), func(config *deployapi.DeploymentConfig) (bool, error) {
if config.Status.LatestVersion == 2 {
return true, nil
}
return false, nil
})
o.Expect(err).NotTo(o.HaveOccurred())

// Wait for deployment pod to be running
rc, err = waitForRCModification(oc, namespace, deployutil.LatestDeploymentNameForConfig(dc), deploymentRunTimeout,
"", func(currentRC *kapiv1.ReplicationController) (bool, error) {
if deployutil.DeploymentStatusFor(currentRC) == deployapi.DeploymentStatusRunning {
return true, nil
}
return false, nil
})
o.Expect(err).NotTo(o.HaveOccurred())
})

g.It("should deal with config change in case the deployment is still running", func() {
namespace := oc.Namespace()

g.By("creating DC")
dc, err := readDCFixture(simpleDeploymentFixture)
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(dc.Name).To(o.Equal(dcName))

dc.Spec.Replicas = 1
// Make sure the deployer pod doesn't end too soon
dc.Spec.MinReadySeconds = 60
dc, err = oc.AppsClient().Apps().DeploymentConfigs(namespace).Create(dc)
o.Expect(err).NotTo(o.HaveOccurred())

g.By("waiting for RC to be created")
dc, err = waitForDCModification(oc, namespace, dc.Name, deploymentRunTimeout,
dc.GetResourceVersion(), func(config *deployapi.DeploymentConfig) (bool, error) {
cond := deployutil.GetDeploymentCondition(config.Status, deployapi.DeploymentProgressing)
if cond != nil && cond.Reason == deployapi.NewReplicationControllerReason {
return true, nil
}
return false, nil
})
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(dc.Status.LatestVersion).To(o.BeEquivalentTo(1))

g.By("waiting for deployer pod to be running")
_, err = waitForRCModification(oc, namespace, deployutil.LatestDeploymentNameForConfig(dc), deploymentRunTimeout,
"", func(currentRC *kapiv1.ReplicationController) (bool, error) {
if deployutil.DeploymentStatusFor(currentRC) == deployapi.DeploymentStatusRunning {
return true, nil
}
return false, nil
})

g.By("redeploying immediately by config change")
o.Expect(dc.Spec.Template.Annotations["foo"]).NotTo(o.Equal("bar"))
dc, err = oc.AppsClient().Apps().DeploymentConfigs(dc.Namespace).Patch(dc.Name, types.StrategicMergePatchType,
[]byte(`{"spec":{"template":{"metadata":{"annotations":{"foo": "bar"}}}}}`))
o.Expect(err).NotTo(o.HaveOccurred())
dc, err = waitForDCModification(oc, namespace, dcName, deploymentRunTimeout,
dc.GetResourceVersion(), func(config *deployapi.DeploymentConfig) (bool, error) {
if config.Status.LatestVersion == 2 {
return true, nil
}
return false, nil
})
o.Expect(err).NotTo(o.HaveOccurred())

// Wait for deployment pod to be running
_, err = waitForRCModification(oc, namespace, deployutil.LatestDeploymentNameForConfig(dc), deploymentRunTimeout,
"", func(currentRC *kapiv1.ReplicationController) (bool, error) {
if deployutil.DeploymentStatusFor(currentRC) == deployapi.DeploymentStatusRunning {
return true, nil
}
return false, nil
})
o.Expect(err).NotTo(o.HaveOccurred())
})

g.It("should deal with cancellation after deployer pod succeeded", func() {
namespace := oc.Namespace()

g.By("creating DC")
dc, err := readDCFixture(simpleDeploymentFixture)
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(dc.Name).To(o.Equal(dcName))

dc.Spec.Replicas = 1
// Make sure the deployer pod doesn't immediately
dc.Spec.MinReadySeconds = 3
dc, err = oc.AppsClient().Apps().DeploymentConfigs(namespace).Create(dc)
o.Expect(err).NotTo(o.HaveOccurred())

g.By("waiting for RC to be created")
dc, err = waitForDCModification(oc, namespace, dc.Name, deploymentRunTimeout,
dc.GetResourceVersion(), func(config *deployapi.DeploymentConfig) (bool, error) {
cond := deployutil.GetDeploymentCondition(config.Status, deployapi.DeploymentProgressing)
if cond != nil && cond.Reason == deployapi.NewReplicationControllerReason {
return true, nil
}
return false, nil
})
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(dc.Status.LatestVersion).To(o.BeEquivalentTo(1))

rcName := deployutil.LatestDeploymentNameForConfig(dc)

g.By("waiting for deployer to be completed")
_, err = waitForPodModification(oc, namespace,
deployutil.DeployerPodNameForDeployment(rcName),
deploymentRunTimeout, "",
func(pod *kapiv1.Pod) (bool, error) {
switch pod.Status.Phase {
case kapiv1.PodSucceeded:
return true, nil
case kapiv1.PodFailed:
return true, errors.New("pod failed")
default:
return false, nil
}
})
o.Expect(err).NotTo(o.HaveOccurred())

g.By("canceling the deployment")
rc, err := oc.KubeClient().CoreV1().ReplicationControllers(namespace).Patch(
rcName, types.StrategicMergePatchType,
[]byte(fmt.Sprintf(`{"metadata":{"annotations":{%q: %q, %q: %q}}}`,
deployapi.DeploymentCancelledAnnotation, deployapi.DeploymentCancelledAnnotationValue,
deployapi.DeploymentStatusReasonAnnotation, deployapi.DeploymentCancelledByUser,
)))
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(deployutil.DeploymentVersionFor(rc)).To(o.BeEquivalentTo(1))

g.By("redeploying immediately by config change")
o.Expect(dc.Spec.Template.Annotations["foo"]).NotTo(o.Equal("bar"))
dc, err = oc.AppsClient().Apps().DeploymentConfigs(dc.Namespace).Patch(dc.Name, types.StrategicMergePatchType,
[]byte(`{"spec":{"template":{"metadata":{"annotations":{"foo": "bar"}}}}}`))
o.Expect(err).NotTo(o.HaveOccurred())
dc, err = waitForDCModification(oc, namespace, dcName, deploymentRunTimeout,
dc.GetResourceVersion(), func(config *deployapi.DeploymentConfig) (bool, error) {
if config.Status.LatestVersion == 2 {
return true, nil
}
return false, nil
})
o.Expect(err).NotTo(o.HaveOccurred())

// Wait for deployment pod to be running
_, err = waitForRCModification(oc, namespace, deployutil.LatestDeploymentNameForConfig(dc), deploymentRunTimeout,
"", func(currentRC *kapiv1.ReplicationController) (bool, error) {
if deployutil.DeploymentStatusFor(currentRC) == deployapi.DeploymentStatusRunning {
return true, nil
}
return false, nil
})
o.Expect(err).NotTo(o.HaveOccurred())
})
})
})
Loading

0 comments on commit 6f2ba91

Please sign in to comment.