Check that LocalQueue CRD exist before policy sync #8352
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gbenhaim The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
🤖 Gemini AI Assistant AvailableHi @gbenhaim! I'm here to help with your pull request. You can interact with me using the following commands: Available Commands
How to Use
PermissionsOnly OWNER, MEMBER, or COLLABORATOR users can trigger my responses. This ensures secure and appropriate usage. This message was automatically added to help you get started with the Gemini AI assistant. Feel free to delete this comment if you don't need assistance. |
|
🤖 Hi @gbenhaim, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
gbenhaim
force-pushed
the
check-for-local-q
branch
from
f11e783 to
2768029
Compare
|
@gemini-cli /review |
gbenhaim
force-pushed
the
check-for-local-q
branch
3 times, most recently
from
19220a8 to
cb8c692
Compare
I was observed the syncing the Kyverno policy for creating LocalQueue can have great delay if it starts before the LocalQueue CRD is installed on the cluster. Add a PreSync hook the will delay the sync of the policies until the LocalQueue CRD is installed on the cluster. Assisted-By: Cursor Signed-off-by: Gal Ben Haim <[email protected]>
gbenhaim
force-pushed
the
check-for-local-q
branch
from
cb8c692 to
609a22e
Compare
| command: | ||
| - /bin/bash | ||
| - -c | ||
| - | | ||
| set -e | ||
|
|
||
| echo "Starting CRD check..." | ||
|
|
||
| # Configuration | ||
| TIMEOUT=300 # 5 minutes timeout | ||
| INTERVAL=10 # Check every 10 seconds | ||
| START_TIME=$(date +%s) | ||
|
|
||
| # Define CRDs to check | ||
| CRDS=( | ||
| "localqueues.kueue.x-k8s.io" | ||
| ) | ||
|
|
||
| echo "Checking if required CRDs exist..." | ||
|
|
||
| # Check all CRDs with global timeout | ||
| while ! oc get crd "${CRDS[@]}"; do | ||
| CURRENT_TIME=$(date +%s) | ||
| ELAPSED=$((CURRENT_TIME - START_TIME)) | ||
|
|
||
| if [ $ELAPSED -gt $TIMEOUT ]; then | ||
| echo "ERROR: Global timeout reached (${TIMEOUT}s). Required CRDs not found." | ||
| echo "Missing CRDs: ${CRDS[*]}" | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "Required CRDs not found yet. Waiting ${INTERVAL}s... (${ELAPSED}s/${TIMEOUT}s elapsed)" | ||
| sleep $INTERVAL | ||
| done | ||
|
|
||
| echo "SUCCESS: All required CRDs found!" | ||
| echo "Pre-sync hook completed successfully." | ||
| exit 0 |
There was a problem hiding this comment.
To simplify this logic, I'd suggest to:
- use
kubectl wait --for=createinstead of polling onget - scope to just a single CustomResourceDefinition
- take
TIMEOUTandCRDfrom configuration
| command: | |
| - /bin/bash | |
| - -c | |
| - | | |
| set -e | |
| echo "Starting CRD check..." | |
| # Configuration | |
| TIMEOUT=300 # 5 minutes timeout | |
| INTERVAL=10 # Check every 10 seconds | |
| START_TIME=$(date +%s) | |
| # Define CRDs to check | |
| CRDS=( | |
| "localqueues.kueue.x-k8s.io" | |
| ) | |
| echo "Checking if required CRDs exist..." | |
| # Check all CRDs with global timeout | |
| while ! oc get crd "${CRDS[@]}"; do | |
| CURRENT_TIME=$(date +%s) | |
| ELAPSED=$((CURRENT_TIME - START_TIME)) | |
| if [ $ELAPSED -gt $TIMEOUT ]; then | |
| echo "ERROR: Global timeout reached (${TIMEOUT}s). Required CRDs not found." | |
| echo "Missing CRDs: ${CRDS[*]}" | |
| exit 1 | |
| fi | |
| echo "Required CRDs not found yet. Waiting ${INTERVAL}s... (${ELAPSED}s/${TIMEOUT}s elapsed)" | |
| sleep $INTERVAL | |
| done | |
| echo "SUCCESS: All required CRDs found!" | |
| echo "Pre-sync hook completed successfully." | |
| exit 0 | |
| env: | |
| - name: CRD | |
| value: localqueues.kueue.x-k8s.io | |
| command: | |
| - /bin/bash | |
| - -c | |
| - | | |
| TIMEOUT=${TIMEOUT:-300} # default to 5 minutes timeout | |
| echo "Checking CRD ${CRD} exists..." | |
| kubectl wait --for=create "${CRD}" --timeout ${TIMEOUT} |
There was a problem hiding this comment.
I don't see the oc (at least the one provided by ocp 4.17) supports wait --for=create
--for='': The condition to wait on: [delete|condition=condition-name[=condition-value]|jsonpath='{JSONPath expression}'=[JSONPath value]]. The default condition-value is true. Condition values are compared after Unicode simple case folding, which is a more general form of case-insensitivity.
There was a problem hiding this comment.
also, I would like to support checking for the existence of multiple CRDs because I expect that more will come (and I don't think it introduce to much complexity).
| name: kueue-crd-checker | ||
| annotations: | ||
| argocd.argoproj.io/hook: PreSync | ||
| argocd.argoproj.io/hook-delete-policy: BeforeHookCreation |
There was a problem hiding this comment.
Should we use HookSucceeded instead? It seems to me that with BeforeHookCreation this resources will always be around, just updated before the next wave. I'd rather prefer them to be gone if everything applied correctly
https://argo-cd.readthedocs.io/en/stable/user-guide/sync-waves/#hook-lifecycle-and-cleanup
There was a problem hiding this comment.
I choose BeforeHookCreation because I use metadata.name, from the docs
BeforeHookCreation Any existing hook resource is deleted before the new one is created (since v1.3). It is meant to be used with /metadata/name.
|
/test konflux-e2e-v418-optional |
Co-authored-by: Francesco Ilario <[email protected]>
|
@gbenhaim: The following test failed, say
Full PR test history. Your PR dashboard. 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
I was observed the syncing the Kyverno policy for creating LocalQueue can have great delay if it starts before the LocalQueue CRD is installed on the cluster.
Add a PreSync hook the will delay the sync of the policies until the LocalQueue CRD is installed on the cluster.
Assisted-By: Cursor