github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #27048 from kargakis/ignore-only-409-in-scaler

Browse Source 
Automatic merge from submit-queue

kubectl: ignore only update conflicts in the scaler

@kubernetes/kubectl is there any reason to retry any other errors?
This commit is contained in:
k8s-merge-robot
2016-07-02 21:28:37 -07:00
committed by GitHub
parent 4e046a8dc5 c7140b3749
commit 2f9feef9a6
2 changed files with 80 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
pkg/kubectl/scale.go
View File

@@ -81,7 +81,7 @@ type ScaleErrorType int
const ( const (
ScaleGetFailure ScaleErrorType = iota ScaleGetFailure ScaleErrorType = iota
ScaleUpdateFailure ScaleUpdateFailure
ScaleUpdateInvalidFailure ScaleUpdateConflictFailure
) )
// A ScaleError is returned when a scale request passes // A ScaleError is returned when a scale request passes
@@ -115,11 +115,8 @@ func ScaleCondition(r Scaler, precondition *ScalePrecondition, namespace, name s
case nil: case nil:
return true, nil return true, nil
case ScaleError: case ScaleError:
// if it's invalid we shouldn't keep waiting // Retry only on update conflicts.
if e.FailureType == ScaleUpdateInvalidFailure { if e.FailureType == ScaleUpdateConflictFailure {
return false, err
}
if e.FailureType == ScaleUpdateFailure {
return false, nil return false, nil
} }
} }
@@ -153,10 +150,9 @@ func (scaler *ReplicationControllerScaler) ScaleSimple(namespace, name string, p
} }
} }
controller.Spec.Replicas = int32(newSize) controller.Spec.Replicas = int32(newSize)
// TODO: do retry on 409 errors here?
if _, err := scaler.c.ReplicationControllers(namespace).Update(controller); err != nil { if _, err := scaler.c.ReplicationControllers(namespace).Update(controller); err != nil {
if errors.IsInvalid(err) { if errors.IsConflict(err) {
return ScaleError{ScaleUpdateInvalidFailure, controller.ResourceVersion, err} return ScaleError{ScaleUpdateConflictFailure, controller.ResourceVersion, err}
} }
return ScaleError{ScaleUpdateFailure, controller.ResourceVersion, err} return ScaleError{ScaleUpdateFailure, controller.ResourceVersion, err}
} }
@@ -183,8 +179,11 @@ func (scaler *ReplicationControllerScaler) Scale(namespace, name string, newSize
if err != nil { if err != nil {
return err return err
} }
return wait.Poll(waitForReplicas.Interval, waitForReplicas.Timeout, err = wait.Poll(waitForReplicas.Interval, waitForReplicas.Timeout, client.ControllerHasDesiredReplicas(scaler.c, rc))
client.ControllerHasDesiredReplicas(scaler.c, rc)) if err == wait.ErrWaitTimeout {
return fmt.Errorf("timed out waiting for %q to be synced", name)
}
return err
} }
return nil return nil
} }
@@ -215,10 +214,9 @@ func (scaler *ReplicaSetScaler) ScaleSimple(namespace, name string, precondition
} }
} }
rs.Spec.Replicas = int32(newSize) rs.Spec.Replicas = int32(newSize)
// TODO: do retry on 409 errors here?
if _, err := scaler.c.ReplicaSets(namespace).Update(rs); err != nil { if _, err := scaler.c.ReplicaSets(namespace).Update(rs); err != nil {
if errors.IsInvalid(err) { if errors.IsConflict(err) {
return ScaleError{ScaleUpdateInvalidFailure, rs.ResourceVersion, err} return ScaleError{ScaleUpdateConflictFailure, rs.ResourceVersion, err}
} }
return ScaleError{ScaleUpdateFailure, rs.ResourceVersion, err} return ScaleError{ScaleUpdateFailure, rs.ResourceVersion, err}
} }
@@ -245,8 +243,11 @@ func (scaler *ReplicaSetScaler) Scale(namespace, name string, newSize uint, prec
if err != nil { if err != nil {
return err return err
} }
return wait.Poll(waitForReplicas.Interval, waitForReplicas.Timeout, err = wait.Poll(waitForReplicas.Interval, waitForReplicas.Timeout, client.ReplicaSetHasDesiredReplicas(scaler.c, rs))
client.ReplicaSetHasDesiredReplicas(scaler.c, rs)) if err == wait.ErrWaitTimeout {
return fmt.Errorf("timed out waiting for %q to be synced", name)
}
return err
} }
return nil return nil
} }
@@ -283,8 +284,8 @@ func (scaler *JobScaler) ScaleSimple(namespace, name string, preconditions *Scal
parallelism := int32(newSize) parallelism := int32(newSize)
job.Spec.Parallelism = &parallelism job.Spec.Parallelism = &parallelism
if _, err := scaler.c.Jobs(namespace).Update(job); err != nil { if _, err := scaler.c.Jobs(namespace).Update(job); err != nil {
if errors.IsInvalid(err) { if errors.IsConflict(err) {
return ScaleError{ScaleUpdateInvalidFailure, job.ResourceVersion, err} return ScaleError{ScaleUpdateConflictFailure, job.ResourceVersion, err}
} }
return ScaleError{ScaleUpdateFailure, job.ResourceVersion, err} return ScaleError{ScaleUpdateFailure, job.ResourceVersion, err}
} }
@@ -311,8 +312,11 @@ func (scaler *JobScaler) Scale(namespace, name string, newSize uint, preconditio
if err != nil { if err != nil {
return err return err
} }
return wait.Poll(waitForReplicas.Interval, waitForReplicas.Timeout, err = wait.Poll(waitForReplicas.Interval, waitForReplicas.Timeout, client.JobHasDesiredParallelism(scaler.c, job))
client.JobHasDesiredParallelism(scaler.c, job)) if err == wait.ErrWaitTimeout {
return fmt.Errorf("timed out waiting for %q to be synced", name)
}
return err
} }
return nil return nil
} }
@@ -348,8 +352,8 @@ func (scaler *DeploymentScaler) ScaleSimple(namespace, name string, precondition
// For now I'm falling back to regular Deployment update operation. // For now I'm falling back to regular Deployment update operation.
deployment.Spec.Replicas = int32(newSize) deployment.Spec.Replicas = int32(newSize)
if _, err := scaler.c.Deployments(namespace).Update(deployment); err != nil { if _, err := scaler.c.Deployments(namespace).Update(deployment); err != nil {
if errors.IsInvalid(err) { if errors.IsConflict(err) {
return ScaleError{ScaleUpdateInvalidFailure, deployment.ResourceVersion, err} return ScaleError{ScaleUpdateConflictFailure, deployment.ResourceVersion, err}
} }
return ScaleError{ScaleUpdateFailure, deployment.ResourceVersion, err} return ScaleError{ScaleUpdateFailure, deployment.ResourceVersion, err}
} }
@@ -375,8 +379,11 @@ func (scaler *DeploymentScaler) Scale(namespace, name string, newSize uint, prec
if err != nil { if err != nil {
return err return err
} }
return wait.Poll(waitForReplicas.Interval, waitForReplicas.Timeout, err = wait.Poll(waitForReplicas.Interval, waitForReplicas.Timeout, client.DeploymentHasDesiredReplicas(scaler.c, deployment))
client.DeploymentHasDesiredReplicas(scaler.c, deployment)) if err == wait.ErrWaitTimeout {
return fmt.Errorf("timed out waiting for %q to be synced", name)
}
return err
} }
return nil return nil
} }

71
pkg/kubectl/scale_test.go
View File

@@ -30,27 +30,36 @@ import (
type ErrorReplicationControllers struct { type ErrorReplicationControllers struct {
testclient.FakeReplicationControllers testclient.FakeReplicationControllers
invalid bool conflict bool
invalid bool
} }
func (c *ErrorReplicationControllers) Update(controller *api.ReplicationController) (*api.ReplicationController, error) { func (c *ErrorReplicationControllers) Update(controller *api.ReplicationController) (*api.ReplicationController, error) {
if c.invalid { switch {
case c.invalid:
return nil, kerrors.NewInvalid(api.Kind(controller.Kind), controller.Name, nil) return nil, kerrors.NewInvalid(api.Kind(controller.Kind), controller.Name, nil)
case c.conflict:
return nil, kerrors.NewConflict(api.Resource(controller.Kind), controller.Name, nil)
} }
return nil, errors.New("Replication controller update failure") return nil, errors.New("Replication controller update failure")
} }
type ErrorReplicationControllerClient struct { type ErrorReplicationControllerClient struct {
testclient.Fake testclient.Fake
invalid bool conflict bool
invalid bool
} }
func (c *ErrorReplicationControllerClient) ReplicationControllers(namespace string) client.ReplicationControllerInterface { func (c *ErrorReplicationControllerClient) ReplicationControllers(namespace string) client.ReplicationControllerInterface {
return &ErrorReplicationControllers{testclient.FakeReplicationControllers{Fake: &c.Fake, Namespace: namespace}, c.invalid} return &ErrorReplicationControllers{
FakeReplicationControllers: testclient.FakeReplicationControllers{Fake: &c.Fake, Namespace: namespace},
conflict: c.conflict,
invalid: c.invalid,
}
} }
func TestReplicationControllerScaleRetry(t *testing.T) { func TestReplicationControllerScaleRetry(t *testing.T) {
fake := &ErrorReplicationControllerClient{Fake: testclient.Fake{}, invalid: false} fake := &ErrorReplicationControllerClient{Fake: testclient.Fake{}, conflict: true}
scaler := ReplicationControllerScaler{fake} scaler := ReplicationControllerScaler{fake}
preconditions := ScalePrecondition{-1, ""} preconditions := ScalePrecondition{-1, ""}
count := uint(3) count := uint(3)
@@ -63,7 +72,7 @@ func TestReplicationControllerScaleRetry(t *testing.T) {
t.Errorf("Expected an update failure to return pass = false, got pass = %v", pass) t.Errorf("Expected an update failure to return pass = false, got pass = %v", pass)
} }
if err != nil { if err != nil {
t.Errorf("Did not expect an error on update failure, got %v", err) t.Errorf("Did not expect an error on update conflict failure, got %v", err)
} }
preconditions = ScalePrecondition{3, ""} preconditions = ScalePrecondition{3, ""}
scaleFunc = ScaleCondition(&scaler, &preconditions, namespace, name, count) scaleFunc = ScaleCondition(&scaler, &preconditions, namespace, name, count)
@@ -87,7 +96,7 @@ func TestReplicationControllerScaleInvalid(t *testing.T) {
t.Errorf("Expected an update failure to return pass = false, got pass = %v", pass) t.Errorf("Expected an update failure to return pass = false, got pass = %v", pass)
} }
e, ok := err.(ScaleError) e, ok := err.(ScaleError)
if err == nil || !ok || e.FailureType != ScaleUpdateInvalidFailure { if err == nil || !ok || e.FailureType != ScaleUpdateFailure {
t.Errorf("Expected error on invalid update failure, got %v", err) t.Errorf("Expected error on invalid update failure, got %v", err)
} }
} }
@@ -250,12 +259,16 @@ func TestValidateReplicationController(t *testing.T) {
type ErrorJobs struct { type ErrorJobs struct {
testclient.FakeJobsV1 testclient.FakeJobsV1
invalid bool conflict bool
invalid bool
} }
func (c *ErrorJobs) Update(job *batch.Job) (*batch.Job, error) { func (c *ErrorJobs) Update(job *batch.Job) (*batch.Job, error) {
if c.invalid { switch {
return nil, kerrors.NewInvalid(batch.Kind(job.Kind), job.Name, nil) case c.invalid:
return nil, kerrors.NewInvalid(api.Kind(job.Kind), job.Name, nil)
case c.conflict:
return nil, kerrors.NewConflict(api.Resource(job.Kind), job.Name, nil)
} }
return nil, errors.New("Job update failure") return nil, errors.New("Job update failure")
} }
@@ -271,15 +284,20 @@ func (c *ErrorJobs) Get(name string) (*batch.Job, error) {
type ErrorJobClient struct { type ErrorJobClient struct {
testclient.FakeBatch testclient.FakeBatch
invalid bool conflict bool
invalid bool
} }
func (c *ErrorJobClient) Jobs(namespace string) client.JobInterface { func (c *ErrorJobClient) Jobs(namespace string) client.JobInterface {
return &ErrorJobs{testclient.FakeJobsV1{Fake: &c.FakeBatch, Namespace: namespace}, c.invalid} return &ErrorJobs{
FakeJobsV1: testclient.FakeJobsV1{Fake: &c.FakeBatch, Namespace: namespace},
conflict: c.conflict,
invalid: c.invalid,
}
} }
func TestJobScaleRetry(t *testing.T) { func TestJobScaleRetry(t *testing.T) {
fake := &ErrorJobClient{FakeBatch: testclient.FakeBatch{}, invalid: false} fake := &ErrorJobClient{FakeBatch: testclient.FakeBatch{}, conflict: true}
scaler := JobScaler{fake} scaler := JobScaler{fake}
preconditions := ScalePrecondition{-1, ""} preconditions := ScalePrecondition{-1, ""}
count := uint(3) count := uint(3)
@@ -336,7 +354,7 @@ func TestJobScaleInvalid(t *testing.T) {
t.Errorf("Expected an update failure to return pass = false, got pass = %v", pass) t.Errorf("Expected an update failure to return pass = false, got pass = %v", pass)
} }
e, ok := err.(ScaleError) e, ok := err.(ScaleError)
if err == nil || !ok || e.FailureType != ScaleUpdateInvalidFailure { if err == nil || !ok || e.FailureType != ScaleUpdateFailure {
t.Errorf("Expected error on invalid update failure, got %v", err) t.Errorf("Expected error on invalid update failure, got %v", err)
} }
} }
@@ -491,12 +509,16 @@ func TestValidateJob(t *testing.T) {
type ErrorDeployments struct { type ErrorDeployments struct {
testclient.FakeDeployments testclient.FakeDeployments
invalid bool conflict bool
invalid bool
} }
func (c *ErrorDeployments) Update(deployment *extensions.Deployment) (*extensions.Deployment, error) { func (c *ErrorDeployments) Update(deployment *extensions.Deployment) (*extensions.Deployment, error) {
if c.invalid { switch {
return nil, kerrors.NewInvalid(extensions.Kind(deployment.Kind), deployment.Name, nil) case c.invalid:
return nil, kerrors.NewInvalid(api.Kind(deployment.Kind), deployment.Name, nil)
case c.conflict:
return nil, kerrors.NewConflict(api.Resource(deployment.Kind), deployment.Name, nil)
} }
return nil, errors.New("deployment update failure") return nil, errors.New("deployment update failure")
} }
@@ -511,15 +533,20 @@ func (c *ErrorDeployments) Get(name string) (*extensions.Deployment, error) {
type ErrorDeploymentClient struct { type ErrorDeploymentClient struct {
testclient.FakeExperimental testclient.FakeExperimental
invalid bool conflict bool
invalid bool
} }
func (c *ErrorDeploymentClient) Deployments(namespace string) client.DeploymentInterface { func (c *ErrorDeploymentClient) Deployments(namespace string) client.DeploymentInterface {
return &ErrorDeployments{testclient.FakeDeployments{Fake: &c.FakeExperimental, Namespace: namespace}, c.invalid} return &ErrorDeployments{
FakeDeployments: testclient.FakeDeployments{Fake: &c.FakeExperimental, Namespace: namespace},
invalid: c.invalid,
conflict: c.conflict,
}
} }
func TestDeploymentScaleRetry(t *testing.T) { func TestDeploymentScaleRetry(t *testing.T) {
fake := &ErrorDeploymentClient{FakeExperimental: testclient.FakeExperimental{Fake: &testclient.Fake{}}, invalid: false} fake := &ErrorDeploymentClient{FakeExperimental: testclient.FakeExperimental{}, conflict: true}
scaler := &DeploymentScaler{fake} scaler := &DeploymentScaler{fake}
preconditions := &ScalePrecondition{-1, ""} preconditions := &ScalePrecondition{-1, ""}
count := uint(3) count := uint(3)
@@ -563,7 +590,7 @@ func TestDeploymentScale(t *testing.T) {
} }
func TestDeploymentScaleInvalid(t *testing.T) { func TestDeploymentScaleInvalid(t *testing.T) {
fake := &ErrorDeploymentClient{FakeExperimental: testclient.FakeExperimental{Fake: &testclient.Fake{}}, invalid: true} fake := &ErrorDeploymentClient{FakeExperimental: testclient.FakeExperimental{}, invalid: true}
scaler := DeploymentScaler{fake} scaler := DeploymentScaler{fake}
preconditions := ScalePrecondition{-1, ""} preconditions := ScalePrecondition{-1, ""}
count := uint(3) count := uint(3)
@@ -576,7 +603,7 @@ func TestDeploymentScaleInvalid(t *testing.T) {
t.Errorf("Expected an update failure to return pass = false, got pass = %v", pass) t.Errorf("Expected an update failure to return pass = false, got pass = %v", pass)
} }
e, ok := err.(ScaleError) e, ok := err.(ScaleError)
if err == nil || !ok || e.FailureType != ScaleUpdateInvalidFailure { if err == nil || !ok || e.FailureType != ScaleUpdateFailure {
t.Errorf("Expected error on invalid update failure, got %v", err) t.Errorf("Expected error on invalid update failure, got %v", err)
} }
} }