Operator fixes #563
Open
Operator fixes #563
Conversation
This file contains hidden or 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
Contributor
WalkthroughThe operator integration test workflow was enhanced with two new test steps to verify ConfigMap update triggers and selected APIs status accuracy, plus a debug step to emit diagnostic logs when failures occur. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Comment on lines
+1142
to
+1170
| - name: Test ConfigMap Update Trigger | ||
| run: | | ||
| echo "=== Testing ConfigMap Update Trigger ===" | ||
| # Get current configHash | ||
| INITIAL_HASH=$(kubectl get gateway test-gateway -o jsonpath='{.status.configHash}') | ||
| echo "Initial configHash: $INITIAL_HASH" | ||
| # Update ConfigMap | ||
| echo "Updating ConfigMap..." | ||
| kubectl patch configmap test-gateway-config --type merge -p '{"data":{"values.yaml":"# Updated config\nnameOverride: \"\""}}' | ||
| # Wait for Operator to reconcile (give it some time to detect change) | ||
| # The operator should detect the hash change and update the status | ||
| echo "Waiting for operator to process ConfigMap change..." | ||
| sleep 60 | ||
| # Get new configHash | ||
| NEW_HASH=$(kubectl get gateway test-gateway -o jsonpath='{.status.configHash}') | ||
| echo "New configHash: $NEW_HASH" | ||
| if [ "$INITIAL_HASH" == "$NEW_HASH" ]; then | ||
| echo "FAILED: configHash did not change after ConfigMap update!" | ||
| echo "Expected different hash than $INITIAL_HASH" | ||
| exit 1 | ||
| fi | ||
| echo "SUCCESS: configHash updated successfully." | ||
Contributor
There was a problem hiding this comment.
The ConfigMap patch replaces the entire configuration, likely breaking the gateway.
Line 1152 replaces values.yaml with minimal content that lacks all the required gateway configuration (controller image, router image, policy engine config, etc.). This would likely cause the gateway to fail, making this test validate a broken state rather than a genuine config update scenario.
Additionally, the fixed 60-second sleep (line 1157) without polling is brittle and slow.
🔎 Recommended fix: Use a non-breaking config update
- # Update ConfigMap
echo "Updating ConfigMap..."
- kubectl patch configmap test-gateway-config --type merge -p '{"data":{"values.yaml":"# Updated config\nnameOverride: \"\""}}'
+ # Make a small, non-breaking change like adding a comment or annotation
+ CURRENT_CONFIG=$(kubectl get configmap test-gateway-config -o jsonpath='{.data.values\.yaml}')
+ UPDATED_CONFIG=$(echo "$CURRENT_CONFIG" | sed '1s/^/# Test update '"$(date +%s)"'\n/')
+ kubectl patch configmap test-gateway-config --type merge -p "{\"data\":{\"values.yaml\":\"$UPDATED_CONFIG\"}}"
- # Wait for Operator to reconcile (give it some time to detect change)
- # The operator should detect the hash change and update the status
- echo "Waiting for operator to process ConfigMap change..."
- sleep 60
+ # Poll for hash change instead of fixed sleep
+ echo "Polling for configHash change..."
+ for i in {1..30}; do
+ NEW_HASH=$(kubectl get gateway test-gateway -o jsonpath='{.status.configHash}')
+ if [ "$INITIAL_HASH" != "$NEW_HASH" ]; then
+ break
+ fi
+ echo "Attempt $i/30: Hash unchanged, waiting..."
+ sleep 2
+ done
Comment on lines
+1171
to
+1241
| - name: Test Status Accuracy (Selected APIs) | ||
| run: | | ||
| echo "=== Testing Status Accuracy (SelectedAPIs) ===" | ||
| # Current state: test-gateway has 'test-api' (1 API) | ||
| # Verify initial count | ||
| COUNT=$(kubectl get gateway test-gateway -o jsonpath='{.status.selectedAPIs}') | ||
| echo "Initial SelectedAPIs count: $COUNT" | ||
| if [ "$COUNT" != "1" ]; then | ||
| echo "FAILED: Expected 1 selected API, got $COUNT" | ||
| exit 1 | ||
| fi | ||
| # Add a second API | ||
| echo "Creating second API (status-test-api)..." | ||
| cat <<EOF | kubectl apply -f - | ||
| apiVersion: gateway.api-platform.wso2.com/v1alpha1 | ||
| kind: RestApi | ||
| metadata: | ||
| name: status-test-api | ||
| namespace: default | ||
| spec: | ||
| displayName: status-test-api | ||
| version: v1.0 | ||
| context: /status-test | ||
| upstream: | ||
| main: | ||
| url: http://httpbin.default.svc.cluster.local:80 | ||
| operations: | ||
| - method: GET | ||
| path: /get | ||
| EOF | ||
| echo "Waiting for status-test-api to be programmed..." | ||
| kubectl wait --for=condition=Programmed restapi/status-test-api --timeout=120s | ||
| # Wait for Gateway status update | ||
| sleep 10 | ||
| # Verify count incremented | ||
| COUNT=$(kubectl get gateway test-gateway -o jsonpath='{.status.selectedAPIs}') | ||
| echo "SelectedAPIs count after adding API: $COUNT" | ||
| if [ "$COUNT" != "2" ]; then | ||
| echo "FAILED: Expected 2 selected APIs, got $COUNT" | ||
| kubectl get restapi | ||
| exit 1 | ||
| fi | ||
| # Delete second API | ||
| echo "Deleting status-test-api..." | ||
| kubectl delete restapi status-test-api | ||
| echo "Waiting for deletion..." | ||
| kubectl wait --for=delete restapi/status-test-api --timeout=60s | ||
| # Wait for Gateway status update | ||
| sleep 10 | ||
| # Verify count decremented | ||
| COUNT=$(kubectl get gateway test-gateway -o jsonpath='{.status.selectedAPIs}') | ||
| echo "SelectedAPIs count after deleting API: $COUNT" | ||
| if [ "$COUNT" != "1" ]; then | ||
| echo "FAILED: Expected 1 selected API, got $COUNT" | ||
| exit 1 | ||
| fi | ||
| echo "SUCCESS: Status SelectedAPIs updated accurately." | ||
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace fixed sleeps with polling to improve test reliability.
Lines 1209 and 1229 use fixed 10-second sleeps to wait for the gateway status to update. This creates a race condition—if the operator takes longer than 10 seconds to reconcile, the test becomes flaky.
🔎 Suggested improvement: Poll for status changes
For line 1209, replace:
- # Wait for Gateway status update
- sleep 10
-
- # Verify count incremented
COUNT=$(kubectl get gateway test-gateway -o jsonpath='{.status.selectedAPIs}')
+
+ # Poll for status update with timeout
+ echo "Polling for selectedAPIs count to update..."
+ for i in {1..20}; do
+ COUNT=$(kubectl get gateway test-gateway -o jsonpath='{.status.selectedAPIs}')
+ if [ "$COUNT" == "2" ]; then
+ break
+ fi
+ echo "Attempt $i/20: Count is $COUNT, waiting..."
+ sleep 2
+ done
+
echo "SelectedAPIs count after adding API: $COUNT"Apply similar polling logic at line 1229 for the deletion verification.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .github/workflows/operator-integration-test.yml around lines 1171 to 1241 the
script uses fixed 10s sleeps at lines ~1209 and ~1229 which makes the test
flaky; replace each fixed sleep with a polling loop that repeatedly queries
kubectl get gateway test-gateway -o jsonpath='{.status.selectedAPIs}' (or uses
kubectl wait with a JSONPath/condition if available), compare against the
expected value (2 after creation, 1 after deletion), retry at short intervals
(e.g., 1s) with an overall timeout (e.g., 60–120s), and fail if the timeout is
reached; ensure you remove the hardcoded sleep lines and add the polling loop
before the verification checks so the script only proceeds when the Gateway
status has actually updated.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Purpose
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.