HIVE-3067: infra_test: match machine pool e2e by Hive labels#2841
HIVE-3067: infra_test: match machine pool e2e by Hive labels#2841huangmingxia wants to merge 1 commit intoopenshift:masterfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
bot
added
the
do-not-merge/hold
|
@huangmingxia: This pull request references HIVE-3067 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: huangmingxia The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2841 +/- ##
==========================================
- Coverage 50.42% 50.31% -0.12%
==========================================
Files 279 280 +1
Lines 34198 34274 +76
==========================================
Hits 17244 17244
- Misses 15593 15669 +76
Partials 1361 1361 🚀 New features to boost your workflow:
|
2uasimojo
left a comment
2uasimojo
left a comment
There was a problem hiding this comment.
This seems sane. One non-blocking suggestion inline.
huangmingxia
force-pushed
the
HIVE-3067
branch
3 times, most recently
from
7fd8b42 to
45f5a2c
Compare
| clusterAutoscaler.Spec.IgnoreDaemonsetsUtilization = ptr.To(true) | ||
| clusterAutoscaler.Spec.SkipNodesWithLocalStorage = ptr.To(true) | ||
| clusterAutoscaler.Spec.BalanceSimilarNodeGroups = ptr.To(true) |
There was a problem hiding this comment.
Are you just experimenting here? This change seems orthogonal to the purpose of the PR.
With respect to the change itself, my question would be: why now? These knobs have been on the autoscaler for years and we've never needed them before.
There was a problem hiding this comment.
@2uasimojo Thanks for your review!
This change seems orthogonal to the purpose of the PR.
Yes，this change itself should ideally live in a separate PR. If I understand correctly, this change addresses the root cause of the scale-down test flakiness.
The original commit included a misunderstanding. The only change needed here is enabling IgnoreDaemonsetsUtilization; BalanceSimilarNodeGroups/SkipNodesWithLocalStorage is not related to the case failure. I have removed.
Could you please review this and let me know if I should move this change into a separate commit? Thanks:)
With respect to the change itself, my question would be: why now? These knobs have been on the autoscaler for years and we've never needed them before.
Since the e2e-weekly CI switched to 4.21, I checked the job history and it has never passed. When I running the e2e tests locally, they continue to fail on 4.21 and 4.22 as well, but passed on 4.20.
So I checked the cluster-autoscaler side configuration, we have been using the default ClusterAutoscaler configuration.
IgnoreDaemonsetsUtilizationis disabled by default.BalanceSimilarNodeGroupswas enabled by default when we set up MachinePool autoscaling.
I checked the IgnoreDaemonsetsUtilization configuration. On the spoke cluster, after removing the BusyBox pod, node utilization exceeded the 50% threshold, which blocked scale-down.
Setting ignoreDaemonsetsUtilization: true excludes DaemonSet pods from the node utilization calculation, allowing the autoscaler to evaluate scale-down based on actual workloads rather than unavoidable system overhead. With this change, tests passed on 4.21 and 4.22.
It's possible that this issue has existed for some time and was only exposed after switching to 4.21. I'm not sure why DaemonSet resource requests increased in 4.21+; this could be due to changes in other OpenShift components or may need further investigation.
Later update: For this case failure on AWS/Azure/GCP, increasing waitForNode / waitForMachines timeout does not resolve this tests failure.
There was a problem hiding this comment.
It's possible that this issue has existed for some time and was
onlyexposed after switching to 4.21. I'm not sure why DaemonSet resource requests increased in 4.21+; this could be due to changes in other OpenShift components or may need further investigation.
Right, this is exactly my point. When a behavior changes without us doing something to cause it, we need to understand why. I'll start a conversation with the autoscaler team.
There was a problem hiding this comment.
@2uasimojo Thank you so much for your help. I have removed this workaround, and the scale-down issue for 4.21+ should be tracked under HIVE-3068.
huangmingxia
force-pushed
the
HIVE-3067
branch
5 times, most recently
from
31275fe to
21a27c6
Compare
huangmingxia
force-pushed
the
HIVE-3067
branch
from
21a27c6 to
380a50a
Compare
|
@huangmingxia: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@huangmingxia: This pull request references HIVE-3067 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
| while [ $i -le ${max_cluster_deployment_status_checks} ]; do | ||
| CD_JSON=$(oc get cd ${CLUSTER_NAME} -n ${CLUSTER_NAMESPACE} -o json) | ||
| if [[ $(jq .spec.installed <<<"${CD_JSON}") == "true" ]] ; then | ||
| if [[ $(jq .spec.installed <<<"${CD_JSON}") == "true" ]] && [[ $(jq -r .status.powerState <<<"${CD_JSON}") == "Running" ]] ; then |
There was a problem hiding this comment.
Why add the powerState=Running check:
When we create a cluster using hiveutil, a worker MachinePool is also created. After the ClusterDeployment completes installation, cd.spec.installed=true, but .status.powerState not have reached Running.
If there is an issue with the MachinePool, the MachineSets/Machines will continue syncing, and the spoke cluster's cluster operators will also be impacted. Until then, the CD will not reach the Running state.
|
@huangmingxia is this ready or are we waiting for the outcome of the autoscaler issue? |
|
@2uasimojo Thanks. |
3 participants
Improve machinepool matching in e2e tests by using Hive labels instead of machine name prefixes
Changes: