Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deploy: revert proportional recreate timeout and default to 10m #12259

Merged
merged 1 commit into from
Dec 16, 2016
Merged

deploy: revert proportional recreate timeout and default to 10m #12259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/deploy/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const (
// DefaultRollingTimeoutSeconds is the default TimeoutSeconds for RollingDeploymentStrategyParams.
DefaultRollingTimeoutSeconds int64 = 10 * 60
// DefaultRecreateTimeoutSeconds is the default TimeoutSeconds for RecreateDeploymentStrategyParams.
DefaultRecreateTimeoutSeconds int64 = 2 * 60
DefaultRecreateTimeoutSeconds int64 = 10 * 60
// DefaultRollingIntervalSeconds is the default IntervalSeconds for RollingDeploymentStrategyParams.
DefaultRollingIntervalSeconds int64 = 1
// DefaultRollingUpdatePeriodSeconds is the default PeriodSeconds for RollingDeploymentStrategyParams.
Expand Down
2 changes: 1 addition & 1 deletion pkg/deploy/api/v1/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func SetDefaults_DeploymentStrategy(obj *DeploymentStrategy) {

func SetDefaults_RecreateDeploymentStrategyParams(obj *RecreateDeploymentStrategyParams) {
if obj.TimeoutSeconds == nil {
obj.TimeoutSeconds = mkintp(deployapi.DefaultRollingTimeoutSeconds)
obj.TimeoutSeconds = mkintp(deployapi.DefaultRecreateTimeoutSeconds)
}
}

Expand Down
32 changes: 7 additions & 25 deletions pkg/deploy/strategy/recreate/recreate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ func NewRecreateDeploymentStrategy(oldClient kclient.Interface, tagClient client
decoder: decoder,
hookExecutor: stratsupport.NewHookExecutor(client.Core(), tagClient, client.Core(), os.Stdout, decoder),
retryPeriod: 1 * time.Second,
now: func() time.Time { return time.Now() },
}
}

Expand All @@ -120,41 +119,24 @@ func (s *RecreateDeploymentStrategy) DeployWithAcceptor(from *kapi.ReplicationCo
return fmt.Errorf("couldn't decode config from deployment %s: %v", to.Name, err)
}

var retryTimeout time.Duration
retryTimeout := time.Duration(deployapi.DefaultRecreateTimeoutSeconds) * time.Second
params := config.Spec.Strategy.RecreateParams

// for rolling strategy recreate might be the "initial strategy" and for that we need to
// set the TimeoutSeconds to rolling params.
rollingParams := config.Spec.Strategy.RollingParams

if params != nil {
if params.TimeoutSeconds != nil {
retryTimeout = time.Duration(*params.TimeoutSeconds) * time.Second
} else {
retryTimeout = time.Duration(deployapi.DefaultRecreateTimeoutSeconds) * time.Second
}
}

if retryTimeout == 0 && rollingParams != nil {
if rollingParams.TimeoutSeconds != nil {
retryTimeout = time.Duration(*rollingParams.TimeoutSeconds) * time.Second
} else {
retryTimeout = time.Duration(deployapi.DefaultRollingIntervalSeconds) * time.Second
}
if params != nil && params.TimeoutSeconds != nil {
retryTimeout = time.Duration(*params.TimeoutSeconds) * time.Second
}

deployerPod, err := s.podClient.Pods(to.Namespace).Get(deployutil.DeployerPodNameForDeployment(to.Name))
if err == nil {
deployerRunningSeconds := deployerPod.Status.StartTime.Time.Sub(s.now())
retryTimeout -= deployerRunningSeconds
// When doing the initial rollout for rolling strategy we use recreate and for that we
// have to set the TimeoutSecond based on the rollling strategy parameters.
if rollingParams != nil && rollingParams.TimeoutSeconds != nil {
retryTimeout = time.Duration(*rollingParams.TimeoutSeconds) * time.Second
}
fmt.Fprintf(s.out, "--> Waiting up to %s for rollout to finish\n", retryTimeout)

s.retryParams = kubectl.NewRetryParams(s.retryPeriod, retryTimeout)
waitParams := kubectl.NewRetryParams(s.retryPeriod, retryTimeout)

if updateAcceptor == nil {
// updateAcceptor = s.getUpdateAcceptor(time.Duration(*params.TimeoutSeconds)*time.Second, config.Spec.MinReadySeconds)
updateAcceptor = s.getUpdateAcceptor(retryTimeout, config.Spec.MinReadySeconds)
}

Expand Down
73 changes: 0 additions & 73 deletions pkg/deploy/strategy/recreate/recreate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"time"

kapi "k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/unversioned"
"k8s.io/kubernetes/pkg/apimachinery/registered"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
kcoreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/unversioned"
Expand All @@ -32,24 +31,13 @@ func (c *fakeControllerClient) ReplicationControllers(ns string) kcoreclient.Rep

type fakePodClient struct {
deployerName string
startTime *unversioned.Time
}

var startTime = time.Now()

func (c *fakePodClient) setTime(t time.Time) {
c.startTime = &unversioned.Time{Time: t}
}

func (c *fakePodClient) Pods(ns string) kcoreclient.PodInterface {
deployerPod := &kapi.Pod{}
deployerPod.Name = c.deployerName
deployerPod.Namespace = ns
deployerPod.Status = kapi.PodStatus{}
if c.startTime == nil {
c.setTime(startTime)
}
deployerPod.Status.StartTime = c.startTime
return fake.NewSimpleClientset(deployerPod).Core().Pods(ns)
}

Expand All @@ -72,7 +60,6 @@ func TestRecreate_initialDeployment(t *testing.T) {
getUpdateAcceptor: getUpdateAcceptor,
scaler: scaler,
eventClient: fake.NewSimpleClientset().Core(),
now: func() time.Time { return startTime },
}

config := deploytest.OkDeploymentConfig(1)
Expand All @@ -95,59 +82,6 @@ func TestRecreate_initialDeployment(t *testing.T) {
}
}

func TestRecreate_retryParams(t *testing.T) {
tests := []struct {
desc string
timeoutSeconds int64
expect time.Duration
startTime func() time.Time
}{
{
desc: "timeoutSeconds is defaulted to 2m but deployer took 5s to start",
timeoutSeconds: 120,
expect: 115 * time.Second,
startTime: func() time.Time { t := startTime; return t.Add(5 * time.Second) },
},
{
desc: "timeoutSeconds set to 50s and deployer took 5s to start",
timeoutSeconds: 50,
expect: 45 * time.Second,
startTime: func() time.Time { t := startTime; return t.Add(5 * time.Second) },
},
}

for _, tc := range tests {
scaler := &cmdtest.FakeScaler{}
var deployment *kapi.ReplicationController
strategy := &RecreateDeploymentStrategy{
out: &bytes.Buffer{},
errOut: &bytes.Buffer{},
decoder: kapi.Codecs.UniversalDecoder(),
retryPeriod: 1 * time.Millisecond,
getUpdateAcceptor: getUpdateAcceptor,
scaler: scaler,
eventClient: fake.NewSimpleClientset().Core(),
now: func() time.Time { return startTime },
}
config := deploytest.OkDeploymentConfig(1)
config.Spec.Strategy = recreateParams(tc.timeoutSeconds, "", "", "")
deployment, _ = deployutil.MakeDeployment(config, kapi.Codecs.LegacyCodec(registered.GroupOrDie(kapi.GroupName).GroupVersions[0]))
podClient := &fakePodClient{deployerName: deployutil.DeployerPodNameForDeployment(deployment.Name)}
podClient.setTime(tc.startTime())

strategy.podClient = podClient
strategy.rcClient = &fakeControllerClient{deployment: deployment}

err := strategy.Deploy(nil, deployment, 1)
if err != nil {
t.Fatalf("unexpected deploy error: %#v", err)
}
if strategy.retryParams.Timeout != tc.expect {
t.Errorf("%s: expect:%s timeoutSeconds:%s, got:%s", tc.desc, tc.expect, time.Duration(tc.timeoutSeconds)*time.Second, strategy.retryParams.Timeout)
}
}
}

func TestRecreate_deploymentPreHookSuccess(t *testing.T) {
config := deploytest.OkDeploymentConfig(1)
config.Spec.Strategy = recreateParams(30, deployapi.LifecycleHookFailurePolicyAbort, "", "")
Expand All @@ -163,7 +97,6 @@ func TestRecreate_deploymentPreHookSuccess(t *testing.T) {
getUpdateAcceptor: getUpdateAcceptor,
eventClient: fake.NewSimpleClientset().Core(),
rcClient: &fakeControllerClient{deployment: deployment},
now: func() time.Time { return startTime },
hookExecutor: &hookExecutorImpl{
executeFunc: func(hook *deployapi.LifecycleHook, deployment *kapi.ReplicationController, suffix, label string) error {
hookExecuted = true
Expand Down Expand Up @@ -197,7 +130,6 @@ func TestRecreate_deploymentPreHookFail(t *testing.T) {
getUpdateAcceptor: getUpdateAcceptor,
eventClient: fake.NewSimpleClientset().Core(),
rcClient: &fakeControllerClient{deployment: deployment},
now: func() time.Time { return startTime },
hookExecutor: &hookExecutorImpl{
executeFunc: func(hook *deployapi.LifecycleHook, deployment *kapi.ReplicationController, suffix, label string) error {
return fmt.Errorf("hook execution failure")
Expand Down Expand Up @@ -230,7 +162,6 @@ func TestRecreate_deploymentMidHookSuccess(t *testing.T) {
rcClient: &fakeControllerClient{deployment: deployment},
eventClient: fake.NewSimpleClientset().Core(),
getUpdateAcceptor: getUpdateAcceptor,
now: func() time.Time { return startTime },
hookExecutor: &hookExecutorImpl{
executeFunc: func(hook *deployapi.LifecycleHook, deployment *kapi.ReplicationController, suffix, label string) error {
return fmt.Errorf("hook execution failure")
Expand Down Expand Up @@ -263,7 +194,6 @@ func TestRecreate_deploymentPostHookSuccess(t *testing.T) {
rcClient: &fakeControllerClient{deployment: deployment},
eventClient: fake.NewSimpleClientset().Core(),
getUpdateAcceptor: getUpdateAcceptor,
now: func() time.Time { return startTime },
hookExecutor: &hookExecutorImpl{
executeFunc: func(hook *deployapi.LifecycleHook, deployment *kapi.ReplicationController, suffix, label string) error {
hookExecuted = true
Expand Down Expand Up @@ -298,7 +228,6 @@ func TestRecreate_deploymentPostHookFail(t *testing.T) {
rcClient: &fakeControllerClient{deployment: deployment},
eventClient: fake.NewSimpleClientset().Core(),
getUpdateAcceptor: getUpdateAcceptor,
now: func() time.Time { return startTime },
hookExecutor: &hookExecutorImpl{
executeFunc: func(hook *deployapi.LifecycleHook, deployment *kapi.ReplicationController, suffix, label string) error {
hookExecuted = true
Expand Down Expand Up @@ -329,7 +258,6 @@ func TestRecreate_acceptorSuccess(t *testing.T) {
decoder: kapi.Codecs.UniversalDecoder(),
retryPeriod: 1 * time.Millisecond,
scaler: scaler,
now: func() time.Time { return startTime },
}

acceptorCalled := false
Expand Down Expand Up @@ -376,7 +304,6 @@ func TestRecreate_acceptorFail(t *testing.T) {
retryPeriod: 1 * time.Millisecond,
scaler: scaler,
eventClient: fake.NewSimpleClientset().Core(),
now: func() time.Time { return startTime },
}

acceptor := &testAcceptor{
Expand Down