KFLUXINFRA-3694 Add CI to validate tekton-kueue CEL expressions#343
KFLUXINFRA-3694 Add CI to validate tekton-kueue CEL expressions#343manish-jangra wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: manish-jangra The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Review Summary by QodoAdd CI workflow and tests for tekton-kueue CEL expressions
WalkthroughsDescription• Add GitHub Actions workflow to validate tekton-kueue CEL expressions • Create comprehensive Python test script with 5 test scenarios • Test mutations using actual tekton-kueue container via Podman • Validate queue-name labels, priority classes, and resource annotations • Support both internal-staging and internal-production configurations Diagramflowchart LR
PR["PR triggers on kueue changes"]
WF["GitHub Actions Workflow"]
PY["Python Test Script"]
PREREQ["Check Prerequisites"]
PODMAN["Run Podman Container"]
VALIDATE["Validate Mutations"]
REPORT["Report Results"]
PR --> WF
WF --> PREREQ
PREREQ --> PY
PY --> PODMAN
PODMAN --> VALIDATE
VALIDATE --> REPORT
File Changes1. .github/workflows/test-tekton-kueue-config.yaml
|
Code Review by Qodo
1. Missing prod config blocks CI
|
| "internal-production": { | ||
| "name": "Internal Production", | ||
| "config_file": "components/kueue/internal-production/tekton-kueue/config.yaml", | ||
| "kustomization_file": "components/kueue/internal-production/tekton-kueue/kustomization.yaml" | ||
| } |
There was a problem hiding this comment.
It will skip now, it doesn't find it
| CONFIG_COMBINATIONS: Dict[str, ConfigCombination] = { | ||
| "internal-staging": { | ||
| "name": "Internal Staging", | ||
| "config_file": "components/kueue/internal-staging/tekton-kueue/config.yaml", | ||
| "kustomization_file": "components/kueue/internal-staging/tekton-kueue/kustomization.yaml" | ||
| }, | ||
| "internal-production": { | ||
| "name": "Internal Production", | ||
| "config_file": "components/kueue/internal-production/tekton-kueue/config.yaml", | ||
| "kustomization_file": "components/kueue/internal-production/tekton-kueue/kustomization.yaml" | ||
| } | ||
| } |
There was a problem hiding this comment.
1. Missing prod config blocks ci 🐞 Bug ☼ Reliability
The workflow runs --check-setup, and check_prerequisites() hard-fails if *any* entry in CONFIG_COMBINATIONS points to a missing config/kustomization file. Because CONFIG_COMBINATIONS includes internal-production paths, this CI job will fail until those files land, preventing the new check from ever going green.
Agent Prompt
### Issue description
The CI workflow calls `python hack/test-tekton-kueue-config.py --check-setup`, but `check_prerequisites()` currently fails the entire run if any configured environment is missing on disk. Since `CONFIG_COMBINATIONS` includes `internal-production`, the workflow will fail until production files exist.
### Issue Context
This script is intended to validate CEL expressions for the environments present in-repo. Right now it treats `internal-production` as mandatory.
### Fix Focus Areas
- hack/test-tekton-kueue-config.py[110-159]
- hack/test-tekton-kueue-config.py[302-365]
- .github/workflows/test-tekton-kueue-config.yaml[34-40]
### Suggested fix
- Make `check_prerequisites()` tolerate missing combinations:
- If `validate_config_combination()` fails with `FileNotFoundError`, record a warning message and **continue**, rather than raising.
- If **no** configs are available after filtering, fail with a clear error.
- Ensure tests only run for available configs:
- Either generate `TEST_COMBINATIONS` dynamically from `processed_configs`, or skip combinations whose `config_key` wasn’t processed.
- (Optional) Keep `--check-setup` as a non-blocking informational step if you still want to report missing future environments, but do not fail the job until the files are present.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Add a GitHub Actions workflow and Python test script that validates the tekton-kueue CEL configuration for internal-staging and internal-production. Tests run the actual tekton-kueue container via Podman to verify mutations produce correct queue-name labels, priority classes, and resource annotations for signing pipeline rate-limiting. Addresses review feedback: - Use actions/checkout@v6 (filariow) - Tolerate missing config environments gracefully (qodo) - Add 300s timeout to podman subprocess (qodo) - Validate YAML output type before accessing fields (qodo) - Add job-level timeout-minutes: 15 as safety net Co-authored-by: Cursor <cursoragent@cursor.com>
549ecab to
b67578a
Compare
|
/review |
PR Reviewer Guide 🔍Warning
Here are some key observations to aid the review process:
|
|
/agentic_review |
|
Persistent review updated to latest commit b67578a |
|
PR needs rebase. 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. |
Summary
.github/workflows/test-tekton-kueue-config.yaml) that runs on PRs touchingcomponents/kueue/**hack/test-tekton-kueue-config.py) that validates tekton-kueue CEL expressions using the actual container via PodmanTest scenarios
The test covers all boundary conditions of the CEL configuration:
signing_pipelinenon_signing_pipelineno_labels_pipelinehas(labels)check fails gracefullyrate_limited_falsepartial_labels_pipelineAll tests also validate that
pipelines-queueandkonflux-defaultpriority are unconditionally assigned.Test plan
components/kueue/**changes)Note
The
internal-productionconfig path will be available once #341 merges. Until then, the CI prerequisite check will correctly flag its absence.Made with Cursor