test/openshift/e2e: Smoke test for scale down #32
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openshift-ci-robot
added
the
size/L
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
As we're only concerned with delta values when validating the size of a machine set and the resultant number of nodes we only need to consider one additional node. This commit reduces the MaxReplicas from 12 => 2.
frobware
force-pushed
the
smoke-test-for-scale-down
branch
from
e80dd46
to
ef88007
Compare
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
This PR would also benefit from openshift/cluster-autoscaler-operator#37 |
|
/approve |
enxebre
reviewed
Feb 8, 2019
| if err != nil { | ||
| return err | ||
| } | ||
| // As we have just deleted the workload the autoscaler will |
There was a problem hiding this comment.
I'm not sure if deleting the workload (job) will remove the pods as well, if not the autoscaler will still try to allocate new resources instead of scaling down
There was a problem hiding this comment.
I believe it does. That's how I've done 100% of my manual testing over the last few months.
|
/refresh |
|
/retest |
openshift-merge-robot
pushed a commit
that referenced
this pull request
Feb 15, 2019
We were adjusting the replica count when the cluster-autoscaler was still running which meant that the test would occasionally flake. It is likely that this bug was introduced in PR #32. For example, you would occasionally see the following: ```console I0214 12:51:41.034700 1 scale_up.go:584] Scale-up: setting group size to 3 I0214 12:51:51.129943 1 scale_up.go:584] Scale-up: setting group size to 3 ``` Between these two calls we were adjusting the replica count in an attempt to clean up at the end of the test. But occasionally the autoscaler would do its scan-of-the-state-of-the-cluster and would up add new nodes because the replica count was less then desired. When this condition occurred the node count could never drop below the initial node count as we just added a further max-min nodes. The test would eventually timeout trying to assert that the node count matched the initial node count. The fix here is to not reset the replica count but instead rely on the autoscaler to scale down and adjust the replica count naturally; this change further helps to verify that scale down is working properly. There are additional smaller fixes here too: - we set cascading delete in the batch job (i.e., workload) - we assert that the replica count == the initial replica count - we explicitly set the clusterautoscaler's ScaleDown config
openshift-merge-robot
pushed a commit
that referenced
this pull request
Apr 4, 2019
…a count We were adjusting the replica count when the cluster-autoscaler was still running which meant that the test would occasionally flake. It is likely that this bug was introduced in PR #32. For example, you would occasionally see the following: ```console I0214 12:51:41.034700 1 scale_up.go:584] Scale-up: setting group size to 3 I0214 12:51:51.129943 1 scale_up.go:584] Scale-up: setting group size to 3 ``` Between these two calls we were adjusting the replica count in an attempt to clean up at the end of the test. But occasionally the autoscaler would do its scan-of-the-state-of-the-cluster and would up add new nodes because the replica count was less then desired. When this condition occurred the node count could never drop below the initial node count as we just added a further max-min nodes. The test would eventually timeout trying to assert that the node count matched the initial node count. The fix here is to not reset the replica count but instead rely on the autoscaler to scale down and adjust the replica count naturally; this change further helps to verify that scale down is working properly. There are additional smaller fixes here too: - we set cascading delete in the batch job (i.e., workload) - we assert that the replica count == the initial replica count - we explicitly set the clusterautoscaler's ScaleDown config
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reworked the e2e test to delete the workload. This will:
The smoke tests asserts that:
I also reduced the number of replicas we spin up from 12 => 2 as we're only testing for a delta of 1.