Skip to content

Infra migration refactor - #169

Open
averevki wants to merge 1 commit into
Kuadrant:mainfrom
averevki:itup-refactor
Open

Infra migration refactor#169
averevki wants to merge 1 commit into
Kuadrant:mainfrom
averevki:itup-refactor

Conversation

@averevki

@averevki averevki commented May 14, 2026

Copy link
Copy Markdown
Member

Changes summary

  • pipelinesascode.tekton.dev/coschedule: workspaces annotation on nightly pipeline, hardcoding tasks coscheduling for nightly pipelines as it is not possible to configure globally on the new tenant
  • onError: continue on nightly testing tasks, ensuring pipeline continue running even if one of the targets fail
  • remove limits resources definitions on all the tasks, instead they will be globally defined with LimitRange CR on the new tenant, and on the existing ocp clusters before the migration happens:
apiVersion: v1
kind: LimitRange
metadata:
  name: kuadrant-pipelines-limits
spec:
  limits:
    - default:
        cpu: "2"
        memory: 2Gi
      defaultRequest:
        cpu: 200m
        memory: 256Mi
      type: Container

Summary by CodeRabbit

  • New Features

    • Pipelines now continue past failing test tasks so nightly runs complete and later steps still run.
    • Nightly image generation now respects Europe/Prague timezone.
  • Bug Fixes

    • Added a timeout for long-running test steps to avoid indefinite runs.
  • Chores

    • Streamlined resource declarations across pipeline tasks.
    • Kubeconfig handling updated to use a dedicated per-cluster kubeconfig directory.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The head commit changed during the review from 125309b to 0f08903.

📝 Walkthrough

Walkthrough

Adds a co-scheduling annotation to the nightly pipeline, marks multiple run-tests* tasks as onError: continue, removes many step-level computeResources.limits, adjusts specific requests/timeouts, adds TZ for nightly-image-date, and reorganises kubeconfig placement in kubectl-login.

Changes

Tekton resource and runtime configuration updates

Layer / File(s) Summary
Pipeline co-scheduling and error handling
pipelines/test/nightly/pipeline.yaml
Added pipelinesascode.tekton.dev/coschedule: workspaces annotation to pipeline metadata and set onError: continue on multiple test task definitions to allow pipeline execution to proceed when test tasks fail.
Kubeconfig directory structure in kubectl-login
tasks/login/kubectl-login.yaml
kubectl-login task creates a dedicated kubeconfigs directory under the shared workspace and sets KUBECONFIG to a cluster-specific file path within that directory instead of at the workspace root; step computeResources removed.
Resource request and timeout adjustments
tasks/infra/provision-ocp-aws.yaml, tasks/test/run-tests.yaml
provision-aws-ocp requests changed CPU 1000m1 and memory 2Gi1Gi; run-tests memory reduced 2000Mi1Gi and timeout: 2h added.
Timezone environment variable in nightly-image-date
tasks/deploy/nightly-image-date.yaml
Added TZ=Europe/Prague to the generate-nightly-url step and removed its computeResources.limits block.
Removal of computeResources limits across task steps
tasks/deploy/*, tasks/infra/*, tasks/login/*, tasks/misc/*, tasks/test/*
Removed explicit CPU and memory computeResources.limits from numerous step definitions so steps now omit hard resource limits and continue to their env/image/script fields as appropriate.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Kuadrant/testsuite-pipelines#162: Related changes touching tasks/test/collect-info.yaml and tasks/test/rptool-upload.yaml which also removed computeResources.limits in earlier edits.

Suggested reviewers

  • emmaaroche

Poem

A rabbit hops through pipelines bright,
Removing limits left and right,
Co-schedules tasks and keeps the time,
Kubeconfigs nested, tidy and prime,
The nightly tests hum on through the night 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is vague and overly broad, referring to 'infra migration refactor' without clearly describing the main substantive changes (resource limit removal, pipeline annotation addition, and error handling updates). Consider a more specific title such as 'Remove resource limits and add pipeline resilience configuration' or 'Migrate resource management to cluster-level LimitRange' to better convey the key changes.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@averevki
averevki requested a review from a team May 14, 2026 15:01
@averevki averevki self-assigned this May 14, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tasks/test/run-tests.yaml (1)

43-43: ⚡ Quick win

Parameterise memory request for heavier test targets.

Hard-coding 1Gi for all runs risks flakiness/OOM on heavier targets; consider a task param (default 1Gi) so nightly can raise memory for multicluster/disruptive runs without editing the Task.

As per coding guidelines, "Make target execution in test tasks must support the following targets: kuadrant (Service Protection tests), authorino-standalone (Standalone Authorino tests), multicluster (Multi-cluster tests), dnstls (DNSPolicy and TLSPolicy tests), and disruptive (Disruptive tests)".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tasks/test/run-tests.yaml` at line 43, The task currently hard-codes "memory:
1Gi"; add a Task param (e.g., name: memory, default: "1Gi") and replace the
literal with the param reference (e.g., use $(params.memory)) so callers can
override memory for heavy targets (multicluster/disruptive) without editing the
Task; update the Task's params list and any documentation/comment to mention
default "1Gi" and that nightly runs should override $(params.memory) for heavier
targets like multicluster and disruptive.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tasks/test/run-tests.yaml`:
- Line 43: The task currently hard-codes "memory: 1Gi"; add a Task param (e.g.,
name: memory, default: "1Gi") and replace the literal with the param reference
(e.g., use $(params.memory)) so callers can override memory for heavy targets
(multicluster/disruptive) without editing the Task; update the Task's params
list and any documentation/comment to mention default "1Gi" and that nightly
runs should override $(params.memory) for heavier targets like multicluster and
disruptive.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 41ebfd27-a4dc-449b-80b0-30155a8417b2

📥 Commits

Reviewing files that changed from the base of the PR and between 36d4819 and 69ba2a0.

📒 Files selected for processing (35)
  • pipelines/test/nightly/pipeline.yaml
  • tasks/deploy/check-image-existence.yaml
  • tasks/deploy/nightly-image-date.yaml
  • tasks/infra/cluster-secret-cleanup.yaml
  • tasks/infra/delete-aro.yaml
  • tasks/infra/delete-ocp-aws.yaml
  • tasks/infra/delete-osd.yaml
  • tasks/infra/delete-rosa.yaml
  • tasks/infra/do-custom-updates.yaml
  • tasks/infra/get-cluster-id.yaml
  • tasks/infra/get-osd-credentials.yaml
  • tasks/infra/ocm-login.yaml
  • tasks/infra/operator-pod-restart.yaml
  • tasks/infra/parameter-sanity-check.yaml
  • tasks/infra/pause-pipeline.yaml
  • tasks/infra/prepare-for-rhcl-rc-install.yaml
  • tasks/infra/provision-aro.yaml
  • tasks/infra/provision-ocp-aws.yaml
  • tasks/infra/provision-osd-aws.yaml
  • tasks/infra/provision-osd-gcp.yaml
  • tasks/infra/provision-rosa.yaml
  • tasks/infra/rosa-login.yaml
  • tasks/infra/upgrade-to-latest.yaml
  • tasks/infra/wait-for-valid-certificates.yaml
  • tasks/infra/wait-till-osd-is-deleted.yaml
  • tasks/infra/wait-till-osd-is-ready.yaml
  • tasks/login/kubectl-login.yaml
  • tasks/misc/clone-rapidast-repo.yaml
  • tasks/misc/compose-config-file.yaml
  • tasks/misc/rapidast-cleanup.yaml
  • tasks/misc/rapidast-scan.yaml
  • tasks/misc/wait-for-job-to-complete.yaml
  • tasks/test/collect-info.yaml
  • tasks/test/rptool-upload.yaml
  • tasks/test/run-tests.yaml
💤 Files with no reviewable changes (8)
  • tasks/infra/pause-pipeline.yaml
  • tasks/misc/wait-for-job-to-complete.yaml
  • tasks/deploy/check-image-existence.yaml
  • tasks/deploy/nightly-image-date.yaml
  • tasks/infra/do-custom-updates.yaml
  • tasks/test/rptool-upload.yaml
  • tasks/test/collect-info.yaml
  • tasks/misc/clone-rapidast-repo.yaml

Signed-off-by: averevki <sandyverevkin@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant