fix: kubecf upgrade failure due to can't find multi-az scheduler #1663
Conversation
ShuangMen
changed the title
|
hi, could some one review this ? |
|
add some check on the existence of scheduler statefulset for the case of multi-az and multi-cluster kubecf. |
| @@ -18,8 +18,18 @@ spec: | |||
| - name: cc-deployment-updater-cc-deployment-updater | |||
| readinessProbe: ~ | |||
| ' | |||
| set +o pipefail | |||
| scheduler_list=$(kubectl get statefulsets --namespace "$NAMESPACE" | grep scheduler | cut -d " " -f 1) | |||
| set -o pipefail | |||
There was a problem hiding this comment.
I would prefer to use the kubectl command to specify exactly the output we need instead of post-processing it with grep/awk/sed/cut etc, which can break when the default output format changes. E.g.
scheduler_list=$(kubectl get statefulsets \
--namespace "${NAMESPACE}" \
--selector quarks.cloudfoundry.org/quarks-statefulset-name=scheduler \
--no-headers=true \
--output custom-columns=:metadata.name \
)
There was a problem hiding this comment.
@ShuangMen I just realized that quarks.cloudfoundry.org//quarks-statefulset-name might not be the correct selector; I don't have a multi-az cluster to test.
I think quarks.cloudfoundry.org/instance-group-name is actually the correct label.
Sorry about the confusion!
There was a problem hiding this comment.
@jandubois, thanks. I update the code based on your comments.
| exit 0 | ||
| fi | ||
|
|
||
| for i in ${scheduler_list}; do |
There was a problem hiding this comment.
I don't care too much, but if you already change the PR, why not use a more descriptive name than i? I think scheduler would be an obvious choice.
There was a problem hiding this comment.
@jandubois, thanks. I update the code based on your comments.
| fi | ||
|
|
||
| for i in ${scheduler_list}; do | ||
| kubectl patch statefulset --namespace "$NAMESPACE" $i --patch "$patch" |
There was a problem hiding this comment.
Please always run make lint before submitting a PR (it will not pass CI if it fails the lint step).
In ./chart/hooks/pre-upgrade/remove-deployment-updater-readiness.sh line 31:
kubectl patch statefulset --namespace "$NAMESPACE" $i --patch "$patch"
^-- SC2086: Double quote to prevent globbing and word splitting.
In addition we generally put variables in curly braces as well (although that is not enforced by shellcheck, so we often miss it). The final style would ideally be:
for scheduler in ${scheduler_list}; do
kubectl patch statefulset --namespace "${NAMESPACE}" "${scheduler}" --patch "${patch}"
done
There was a problem hiding this comment.
@jandubois, thanks for your good advice, I've updated the code.
Description
kubecf with multi-az upgrade failed from version v2.6.1 to v2.7.1.
Motivation and Context
#1662
How Has This Been Tested?
run helm upgrade kubecf with the updated code, upgrade process moves on and all schedulers get upgrade successfully.
Screenshots (if appropriate):
Types of changes
Checklist: