Skip to content

Commit

Permalink
[release-1.6] Fix BZ-2080547: mis-read the priority class after update (
Browse files Browse the repository at this point in the history
#2003)

* Fix a bug when updating the priority class

Since most of the priority class fiels are emutable, when updating it,
HCO remove and re-create it. Then HCO reads the new created priority
class in order to set its refertence in the HyperConverged
relatedObjects list.

The re-reading often fails with "no found" just because some timing and
cache issues.

This PR is fixing the issue by droping the re-read.

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>

* fix the functional test in kubevirtci

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>

Co-authored-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>
  • Loading branch information
kubevirt-bot and nunnatsa committed Jun 9, 2022
1 parent bd29043 commit 75aa9b7
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 7 deletions.
32 changes: 32 additions & 0 deletions hack/check_update_priority_class.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#!/usr/bin/env bash
set -ex

# edit priority class labels, causing HCO to recreate it, making sure that the relatedObjects list contains the new priorityClass

# check that the new priority class was created and that it does not include the new label
function check_priority_class() {
set -ex
OLD_PC_UID=$1

# make sure the object was actually changed
NEW_PC_UID=$(${KUBECTL_BINARY} get priorityclass -n ${INSTALLED_NAMESPACE} kubevirt-cluster-critical -o jsonpath='{.metadata.uid}')
[[ "${OLD_PC_UID}" != "${NEW_PC_UID}" ]]

# read the UID from the HyperConverged relatedObject list
NEW_PC_REF_UID=$(${KUBECTL_BINARY} get hco -n ${INSTALLED_NAMESPACE} kubevirt-hyperconverged -o json | jq -r '.status.relatedObjects[] | select(.kind == "PriorityClass" and .name == "kubevirt-cluster-critical") | .uid')

[[ "${NEW_PC_UID}" == "${NEW_PC_REF_UID}" ]]

# make sure the new label was removed:
TEST_LABEL=$(${KUBECTL_BINARY} get priorityclass -n ${INSTALLED_NAMESPACE} kubevirt-cluster-critical -o jsonpath='{.metadata.labels.test}')
[[ -z "${TEST_LABEL}" ]]
}

export -f check_priority_class

OLD_PC_REF_UID=$(${KUBECTL_BINARY} get hco -n ${INSTALLED_NAMESPACE} kubevirt-hyperconverged -o json | jq -r '.status.relatedObjects[] | select(.kind == "PriorityClass" and .name == "kubevirt-cluster-critical") | .uid')
OLD_PC_UID=$(${KUBECTL_BINARY} get priorityclass -n ${INSTALLED_NAMESPACE} kubevirt-cluster-critical -o jsonpath='{.metadata.uid}')
[[ "${OLD_PC_REF_UID}" == "${OLD_PC_UID}" ]]

${KUBECTL_BINARY} patch priorityclass kubevirt-cluster-critical -n ${INSTALLED_NAMESPACE} --type=json -p='[{"op": "add", "path": "/metadata/labels/test", "value": "test"}]'
./hack/retry.sh 3 10 "check_priority_class ${OLD_PC_UID}"
4 changes: 3 additions & 1 deletion hack/run-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

set -euo pipefail

INSTALLED_NAMESPACE=${INSTALLED_NAMESPACE:-"kubevirt-hyperconverged"}
export INSTALLED_NAMESPACE=${INSTALLED_NAMESPACE:-"kubevirt-hyperconverged"}

source hack/common.sh
source cluster/kubevirtci.sh
Expand Down Expand Up @@ -55,5 +55,7 @@ ${KUBECTL_BINARY} label priorityclass kubevirt-cluster-critical app-
sleep 10
[[ $(${KUBECTL_BINARY} get priorityclass kubevirt-cluster-critical -o=jsonpath='{.metadata.labels.app}') == 'kubevirt-hyperconverged' ]]

./hack/check_update_priority_class.sh

# Check the webhook, to see if it allow deleteing of the HyperConverged CR
./hack/retry.sh 10 30 "${KUBECTL_BINARY} delete hco -n ${INSTALLED_NAMESPACE} kubevirt-hyperconverged"
7 changes: 1 addition & 6 deletions pkg/controller/operands/kubevirt.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/yaml"
"sigs.k8s.io/controller-runtime/pkg/client"

Expand Down Expand Up @@ -580,11 +579,7 @@ func (h *kvPriorityClassHooks) updateCr(req *common.HcoRequest, Client client.Cl
return false, false, err
}

// update found object for object references
err = Client.Get(req.Ctx, types.NamespacedName{Name: found.Name, Namespace: found.Namespace}, found)
if err != nil {
return true, !req.HCOTriggered, err
}
pc.DeepCopyInto(found)

return true, !req.HCOTriggered, nil
}
Expand Down

0 comments on commit 75aa9b7

Please sign in to comment.