Merge pull request #34456 from kargakis/revert-29808

Automatic merge from submit-queue

Revert "Error out when any RS has more available pods then its spec r…

Reverts https://github.com/kubernetes/kubernetes/pull/29808

The PR is wrong because we can have more available pods than desired every time we scale down.

@kubernetes/deployment ptal
pull/6/head
Kubernetes Submit Queue 2016-10-13 03:01:47 -07:00 committed by GitHub
commit 5496d22733
2 changed files with 6 additions and 19 deletions

View File

@ -395,7 +395,6 @@ func TestDeploymentController_scaleDownOldReplicaSetsForRollingUpdate(t *testing
oldReplicas int
scaleExpected bool
expectedOldReplicas int
errorExpected bool
}{
{
deploymentReplicas: 10,
@ -404,7 +403,6 @@ func TestDeploymentController_scaleDownOldReplicaSetsForRollingUpdate(t *testing
oldReplicas: 10,
scaleExpected: true,
expectedOldReplicas: 9,
errorExpected: false,
},
{
deploymentReplicas: 10,
@ -413,7 +411,6 @@ func TestDeploymentController_scaleDownOldReplicaSetsForRollingUpdate(t *testing
oldReplicas: 10,
scaleExpected: true,
expectedOldReplicas: 8,
errorExpected: false,
},
{
deploymentReplicas: 10,
@ -421,7 +418,6 @@ func TestDeploymentController_scaleDownOldReplicaSetsForRollingUpdate(t *testing
readyPods: 8,
oldReplicas: 10,
scaleExpected: false,
errorExpected: false,
},
{
deploymentReplicas: 10,
@ -429,7 +425,6 @@ func TestDeploymentController_scaleDownOldReplicaSetsForRollingUpdate(t *testing
readyPods: 10,
oldReplicas: 0,
scaleExpected: false,
errorExpected: true,
},
{
deploymentReplicas: 10,
@ -437,7 +432,6 @@ func TestDeploymentController_scaleDownOldReplicaSetsForRollingUpdate(t *testing
readyPods: 1,
oldReplicas: 10,
scaleExpected: false,
errorExpected: false,
},
}
@ -479,7 +473,7 @@ func TestDeploymentController_scaleDownOldReplicaSetsForRollingUpdate(t *testing
eventRecorder: &record.FakeRecorder{},
}
scaled, err := controller.scaleDownOldReplicaSetsForRollingUpdate(allRSs, oldRSs, deployment)
if !test.errorExpected && err != nil {
if err != nil {
t.Errorf("unexpected error: %v", err)
continue
}

View File

@ -668,7 +668,7 @@ func GetAvailablePodsForReplicaSets(c clientset.Interface, deployment *extension
// CountAvailablePodsForReplicaSets returns the number of available pods corresponding to the given pod list and replica sets.
// Note that the input pod list should be the pods targeted by the deployment of input replica sets.
func CountAvailablePodsForReplicaSets(podList *api.PodList, rss []*extensions.ReplicaSet, minReadySeconds int32) (int32, error) {
rsPods, err := filterPodsMatchingReplicaSets(rss, podList, minReadySeconds)
rsPods, err := filterPodsMatchingReplicaSets(rss, podList)
if err != nil {
return 0, err
}
@ -723,8 +723,8 @@ func IsPodAvailable(pod *api.Pod, minReadySeconds int32, now time.Time) bool {
}
// filterPodsMatchingReplicaSets filters the given pod list and only return the ones targeted by the input replicasets
func filterPodsMatchingReplicaSets(replicaSets []*extensions.ReplicaSet, podList *api.PodList, minReadySeconds int32) ([]api.Pod, error) {
allRSPods := []api.Pod{}
func filterPodsMatchingReplicaSets(replicaSets []*extensions.ReplicaSet, podList *api.PodList) ([]api.Pod, error) {
rsPods := []api.Pod{}
for _, rs := range replicaSets {
matchingFunc, err := rsutil.MatchingPodsFunc(rs)
if err != nil {
@ -733,16 +733,9 @@ func filterPodsMatchingReplicaSets(replicaSets []*extensions.ReplicaSet, podList
if matchingFunc == nil {
continue
}
rsPods := podutil.Filter(podList, matchingFunc)
avaPodsCount := countAvailablePods(rsPods, minReadySeconds)
if avaPodsCount > rs.Spec.Replicas {
msg := fmt.Sprintf("Found %s/%s with %d available pods, more than its spec replicas %d", rs.Namespace, rs.Name, avaPodsCount, rs.Spec.Replicas)
glog.Errorf("ERROR: %s", msg)
return nil, fmt.Errorf(msg)
}
allRSPods = append(allRSPods, podutil.Filter(podList, matchingFunc)...)
rsPods = append(rsPods, podutil.Filter(podList, matchingFunc)...)
}
return allRSPods, nil
return rsPods, nil
}
// IsRollingUpdate returns true if the strategy type is a rolling update.