Run oadp-operator e2e against kdm-controller/kdm-plugin PRs (oadp-dev, oadp-1.6) - #83049
Run oadp-operator e2e against kdm-controller/kdm-plugin PRs (oadp-dev, oadp-1.6)#83049kaovilai wants to merge 7 commits into
Conversation
openshift-ci
Bot
added
the
approved
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe CI configurations build and promote OADP images. The datamover controller and plugin configurations add optional AWS virtualized E2E workflows with OADP installation, image injection, rollout checks, credentials, and resource requests. ChangesOADP development CI and E2E integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CI
participant OADPOperator
participant OLMSubscription
participant DatamoverComponent
participant AWSE2ETests
CI->>OADPOperator: Install the dev-channel OADP operator
CI->>OLMSubscription: Set the datamover related image
OLMSubscription->>DatamoverComponent: Apply the controller or plugin image
DatamoverComponent-->>CI: Report deployment rollout completion
CI->>AWSE2ETests: Mount credentials and run TEST_VIRT=true test-e2e
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In
`@ci-operator/config/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-dev.yaml`:
- Around line 30-31: Replace the `commands: make test` entry in the `oadp-dev`
test configuration with a direct `go test ./kubevirt-datamover-plugin/...`
invocation, preserving `from: src` and avoiding the Makefile’s container-runtime
prerequisite.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 1448f45b-6cd1-41b8-88b9-4341aa39dd33
⛔ Files ignored due to path filters (4)
ci-operator/jobs/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev-postsubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-dev-postsubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-dev-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (2)
ci-operator/config/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev.yamlci-operator/config/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-dev.yaml
| commands: make test | ||
| from: src |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read its current content
cat -n ci-operator/config/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-dev.yaml | head -50Repository: openshift/release
Length of output: 1115
🏁 Script executed:
# Also search for any related files in the ci-operator config
find ci-operator/config -name "*kubevirt-datamover*" -type fRepository: openshift/release
Length of output: 1131
🌐 Web query:
kubevirt-datamover-plugin Makefile oadp-dev docker podman check
💡 Result:
The kubevirt-datamover-plugin and its associated kubevirt-datamover-controller are components within the OpenShift API for Data Protection (OADP) ecosystem, primarily developed under the migtools organization with development branches typically tracked under oadp-dev [1][2][3]. Development workflows for these components involve standard Makefile-driven processes that support both Docker and Podman as container runtimes for building and pushing images [4]. Key points regarding the development workflow include: 1. Build and Deployment: The repositories (e.g., kubevirt-datamover-controller) utilize Makefile targets such as make docker-build and make docker-push [4]. These commands facilitate the creation and distribution of container images, often using environment variables like IMG to specify the target registry [4]. 2. Container Runtime Flexibility: While Makefile commands are often documented as docker-build or docker-push, they generally interface with the system's container runtime, which can be Docker or Podman [5][4]. Developers are encouraged to ensure their environment is configured correctly to handle cross-platform builds if necessary (e.g., building amd64 images on arm64 hardware) using --platform flags [4]. 3. Development Environment: Projects are commonly generated using tools like Kubebuilder (e.g., v4.6.0) [2]. The development lifecycle often involves validating changes against the oadp-dev branch of the main oadp-operator repository to ensure compatibility with Velero and other OADP plugins [1][2][3][5]. 4. Checking/Validation: Development validation often involves deploying to a temporary registry like ttl.sh for rapid testing [4], or verifying the generated bundle manifests using the operator-sdk as defined in the operator's main Makefile [6]. For specific implementation details, refer to the Makefile and README.md files located in the respective migtools/kubevirt-datamover-plugin and migtools/kubevirt-datamover-controller repositories [1][2][7].
Citations:
- 1: https://github.com/migtools/kubevirt-datamover-plugin
- 2: https://github.com/migtools/kubevirt-datamover-controller
- 3: https://github.com/openshift/oadp-operator
- 4: https://pkg.go.dev/github.com/migtools/kubevirt-datamover-controller
- 5: https://github.com/oadp-rebasebot/oadp-rebase
- 6: https://github.com/openshift/oadp-operator/blob/oadp-dev/Makefile
- 7: https://github.com/migtools/kubevirt-datamover-controller/blob/oadp-dev/README.md
🏁 Script executed:
# Fetch the Makefile from the specified GitHub link to check its content
curl -s https://github.com/migtools/kubevirt-datamover-plugin/raw/refs/heads/oadp-dev/Makefile | head -100Repository: openshift/release
Length of output: 155
🏁 Script executed:
# Also check if there are any other test-related files in the config
cat -n ci-operator/config/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-1.6.yaml | grep -A 20 "unit-test"Repository: openshift/release
Length of output: 155
🏁 Script executed:
# Try to get the Makefile using git clone or check documentation
# First, let's understand what the ci-operator 'from: src' means by searching for documentation
rg "from: src" ci-operator/config -A 5 -B 5 | head -50Repository: openshift/release
Length of output: 5670
🏁 Script executed:
# Check if there's documentation about ci-operator test steps
find . -name "*.md" -type f | xargs grep -l "from: src" 2>/dev/null | head -5Repository: openshift/release
Length of output: 180
🏁 Script executed:
# Look at the oadp-1.6 version to see if it has the same pattern
cat -n ci-operator/config/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-1.6.yaml | sed -n '20,50p'Repository: openshift/release
Length of output: 346
🌐 Web query:
ci-operator test step from: src container image
💡 Result:
In OpenShift CI, the src container image is a specialized pipeline image that contains the source code of the repository being tested, along with its build dependencies [1]. It is generated automatically by the ci-operator as part of the pipeline initialization process [2][1]. To use the src image in a test step, you reference it via the from field within your test configuration [3][4]. Usage in a Container Test: For a simple container test, you specify the image in the container.from field [3][4]: tests: - as: unit-test commands: make test container: from: src Usage in a Multi-Stage Test: For multi-stage tests, you specify from: src within the individual steps of the test definition [5]: tests: - as: e2e-test steps: test: - as: test-step commands: make test from: src Key Characteristics of the src Image: - Automatic Creation: The ci-operator uses a cloning tool (clonerefs) to inject the source code of the repository into a base image (defined by build_root) and tags the resulting image as src in the pipeline ImageStream [2][1]. - Implicit Source: Unlike standard images, when you use the src image, the source code is already present, so you do not need to perform additional cloning steps within the test container [6]. - Usage: It is primarily intended for running tests, binary builds, or other operations that require access to the source code under test [1].
Citations:
- 1: https://github.com/openshift/ci-operator/blob/master/ONBOARD.md
- 2: https://docs.ci.openshift.org/internals/steps/
- 3: https://github.com/openshift/ci-operator/blob/master/CONFIGURATION.md
- 4: https://docs.ci.openshift.org/architecture/ci-operator/
- 5: https://docs.ci.openshift.org/how-tos/migrating-template-jobs-to-multistage/
- 6: https://docs.ci.openshift.org/getting-started/examples/
🏁 Script executed:
# Check if the kubevirt-datamover-plugin repository is in this repo or if we can look at the Makefile differently
# Let's search for any Makefile references or documentation
find . -type f -name "Makefile" | grep -i "datamover" 2>/dev/nullRepository: openshift/release
Length of output: 155
🏁 Script executed:
# Let's check if there's a reference to the repository content in the release repo
# Look for any OWNER or build info that might hint at the Makefile
rg "oadp-dev" ci-operator/config/migtools/kubevirt-datamover-plugin/ -A 10 -B 10Repository: openshift/release
Length of output: 4257
🏁 Script executed:
# Let's examine what "src" image is in the build_root context for this repo
cat -n ci-operator/config/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-dev.yamlRepository: openshift/release
Length of output: 1115
🌐 Web query:
kubevirt-datamover-plugin oadp-dev Makefile test target
💡 Result:
In the migtools/kubevirt-datamover-plugin project, the make test target is the standard command used to execute unit tests [1]. This command is typically defined in the repository's Makefile to run the project's Go-based test suite, often utilizing standard Go testing tools. While specific implementation details can evolve, typical development commands in related OADP (OpenShift APIs for Data Protection) projects—such as the kubevirt-datamover-controller—include make test for unit testing and additional targets like make fmt, make vet, and make lint for code quality checks [2][3]. For developers working within the oadp-dev branch or related OADP development environments, make test is a foundational command for validating changes before submission [1][2]. If you are working on a specific fork or a version-controlled branch, it is recommended to inspect the local Makefile directly, as it contains the precise shell commands—such as go test configurations—that are executed when the target is invoked [1][4].
Citations:
- 1: https://github.com/migtools/kubevirt-datamover-plugin
- 2: https://github.com/migtools/kubevirt-datamover-controller
- 3: https://pkg.go.dev/github.com/migtools/kubevirt-datamover-controller
- 4: https://github.com/openshift/oadp-operator/blob/oadp-dev/Makefile
🏁 Script executed:
# Let me check the builder image details more carefully and see if there are any other test configs
cat -n ci-operator/config/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-1.6.yamlRepository: openshift/release
Length of output: 832
🌐 Web query:
ubi9-v1.23 konveyor builder image docker podman
💡 Result:
The konveyor/builder is a container image maintained by the Konveyor community, primarily used for building projects and plugins related to Konveyor and OpenShift, such as OADP (OpenShift API for Data Protection) [1][2][3]. The image is based on the Red Hat Universal Base Image 9 (UBI 9) [1][4]. While you may see references to specific UBI9-based tags like ubi9-v1.23, ubi9-v1.24, or ubi9-v1.25 in CI configurations and pull requests (often linked to underlying toolset versions or specific project releases) [2][5], the most common production tag used in Dockerfiles is typically quay.io/konveyor/builder:ubi9-latest [6][3][7]. Usage with Docker or Podman You can use this builder image in your container builds by referencing it in a multi-stage Dockerfile. Both Docker and Podman support this workflow seamlessly as the image follows the standard OCI (Open Container Initiative) format [8]. Example multi-stage Dockerfile pattern: FROM quay.io/konveyor/builder:ubi9-latest AS builder WORKDIR /app COPY.. RUN go build -o my-app. FROM registry.access.redhat.com/ubi9-minimal COPY --from=builder /app/my-app /usr/bin/my-app ENTRYPOINT ["/usr/bin/my-app"] Technical Notes: 1. Compatibility: Because the builder is based on UBI 9, it is designed to be compatible with Red Hat Enterprise Linux 9-based environments [4][9]. 2. Podman/Docker Environments: When building UBI 9 images on non-x86_64 host architectures (like Apple Silicon/macOS ARM64), ensure your container runtime (Podman/Docker) environment is configured correctly for emulation or cross-platform builds, as some older configurations experienced glibc errors related to x86-64-v2 compatibility [10]. 3. Versioning: If you encounter specific versioned tags like ubi9-v1.23, they typically correspond to the specific build environment configuration for a project's CI pipeline [2][5]. For general use, quay.io/konveyor/builder:ubi9-latest is the recommended tag to ensure you are using an updated, UBI 9-based environment [6][3].
Citations:
- 1: https://github.com/konveyor/builder
- 2: Add OADP 1.5 image build and mirroring for hypershift-oadp-plugin #80356
- 3: https://github.com/openshift/oadp-operator/blob/7bfc0307/Dockerfile
- 4: https://github.com/konveyor/builder/blob/main/Dockerfile.ubi9
- 5: openshift/assisted-service@c118a26
- 6: openshift/velero-plugin-for-microsoft-azure@065f7ed
- 7: openshift/velero@b8f68fe
- 8: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/9/html/building_running_and_managing_containers/assembly_working-with-container-images_building-running-and-managing-containers
- 9: https://hub.docker.com/r/redhat/ubi9-micro
- 10: MacOS (arm64) + podman + ubi9: 'Fatal glibc error: CPU does not support x86-64-v2' konveyor/tackle2-hub#180
🏁 Script executed:
# Let me search for any documentation about what the builder image contains
rg "konveyor.*builder" . -A 3 -B 3 | head -50Repository: openshift/release
Length of output: 3227
🏁 Script executed:
# Let me search for any GitHub references or PRs that mention this issue
rg "make test" . -B 5 -A 5 | grep -i "datamover\|podman\|docker" | head -30Repository: openshift/release
Length of output: 5675
🏁 Script executed:
# Check if the konveyor builder Dockerfile is available in the release repo
find . -name "Dockerfile*" -type f | xargs grep -l "konveyor" 2>/dev/null | head -5Repository: openshift/release
Length of output: 155
🌐 Web query:
konveyor builder ubi9 includes podman docker tools
💡 Result:
The Konveyor builder image based on UBI 9 (quay.io/konveyor/builder:ubi9-latest) does not include Podman or Docker tools by default [1]. The Dockerfile.ubi9 for the Konveyor builder image explicitly installs a specific set of development tools: go, git, make, jq, and unzip [1]. It is built upon the standard Red Hat Universal Base Image 9 (ubi9:latest) [1]. While Red Hat Enterprise Linux 9 provides container management tools like Podman, Buildah, and Skopeo as part of the container-tools meta-package [2][3], these are not included in the default configuration of the Konveyor builder image [1]. In many Konveyor-related projects, build processes (such as those defined in Makefiles) are designed to dynamically detect and use whichever container runtime (Podman or Docker) is available on the host system where the build command is executed, rather than relying on tools packaged inside the builder container itself [4].
Citations:
- 1: https://github.com/konveyor/builder/blob/main/Dockerfile.ubi9
- 2: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/9/htmlsingle/building_running_and_managing_containers/assembly_adding-software-to-a-ubi-container_building-running-and-managing-containers
- 3: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/9/html/building_running_and_managing_containers/assembly_adding-software-to-a-ubi-container_building-running-and-managing-containers
- 4: https://github.com/kubevirt/kubevirt-velero-plugin/blob/main/Makefile
🏁 Script executed:
# Let me check if there are any comments or notes in the release repo about this specific issue
rg "container.runtime\|podman\|docker" ci-operator/config/migtools/ -A 2 -B 2Repository: openshift/release
Length of output: 155
🏁 Script executed:
# Let me search for patterns in the release repo about NO_DOCKER or similar workarounds
rg "NO_DOCKER\|NO_PODMAN" ci-operator/config/ | head -20Repository: openshift/release
Length of output: 155
🏁 Script executed:
# Check if there are any other kubevirt or datamover related configs with different commands
rg "datamover\|kubevirt" ci-operator/config/ -A 5 | grep "commands:" | head -10Repository: openshift/release
Length of output: 155
Avoid the Makefile container-runtime prerequisite by calling go test directly.
The oadp-dev Makefile checks for docker or podman at parse time. The src test image (konveyor/builder:ubi9-v1.23) does not include either tool. When make test runs, the Makefile exits before executing go test ./kubevirt-datamover-plugin/.... Call the Go test command directly instead.
Proposed fix
- commands: make test
+ commands: go test ./kubevirt-datamover-plugin/...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| commands: make test | |
| from: src | |
| commands: go test ./kubevirt-datamover-plugin/... | |
| from: src |
🤖 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
`@ci-operator/config/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-dev.yaml`
around lines 30 - 31, Replace the `commands: make test` entry in the `oadp-dev`
test configuration with a direct `go test ./kubevirt-datamover-plugin/...`
invocation, preserving `from: src` and avoiding the Makefile’s container-runtime
prerequisite.
Source: MCP tools
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
kaovilai
changed the title
openshift-ci
Bot
added
the
needs-rebase
|
@kaovilai, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kaovilai 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In
`@ci-operator/config/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev.yaml`:
- Around line 41-79: Run make update to regenerate and sanitize the CI metadata
and Prow job definitions for the new e2e-test-aws job. Apply the generated
changes for
ci-operator/config/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev.yaml
lines 41-79 and
ci-operator/config/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-dev.yaml
lines 41-79, then commit all resulting generated files so Prow registers both
presubmits.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 57b2a657-6601-4add-84b2-27a8de580657
⛔ Files ignored due to path filters (7)
ci-operator/jobs/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev-postsubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-dev-postsubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-dev-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/oadp-operator/openshift-oadp-operator-oadp-dev-periodics.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/oadp-operator/openshift-oadp-operator-oadp-dev-postsubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift/oadp-operator/openshift-oadp-operator-oadp-dev-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (3)
ci-operator/config/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev.yamlci-operator/config/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-dev.yamlci-operator/config/openshift/oadp-operator/openshift-oadp-operator-oadp-dev.yaml
|
/test all Note Responses generated with Claude |
48f6123 to
0b8ef64
Compare
|
/test all Note Responses generated with Claude |
openshift-ci
Bot
removed
the
needs-rebase
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/pj-rehearse pull-ci-migtools-kubevirt-datamover-controller-oadp-dev-e2e-test-aws pull-ci-migtools-kubevirt-datamover-plugin-oadp-dev-e2e-test-aws pull-ci-openshift-oadp-operator-oadp-dev-ci-index Note Responses generated with Claude |
|
@kaovilai: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In
`@ci-operator/config/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev.yaml`:
- Around line 62-66: Add --request-timeout=10s to the oc get invocation in the
retry loop so each request is bounded and the existing 60-iteration deadline
remains enforceable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b4ec95dc-6677-49a4-9dba-ad5a10e72fc1
⛔ Files ignored due to path filters (4)
ci-operator/jobs/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev-postsubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev-presubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-dev-postsubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-dev-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (2)
ci-operator/config/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev.yamlci-operator/config/migtools/kubevirt-datamover-plugin/migtools-kubevirt-datamover-plugin-oadp-dev.yaml
| for i in $(seq 1 60); do | ||
| CURRENT=$(oc get deployment/openshift-adp-controller-manager -n "${OO_INSTALL_NAMESPACE}" -o jsonpath="{.spec.template.spec.containers[?(@.name=='manager')].env[?(@.name=='RELATED_IMAGE_KUBEVIRT_DATAMOVER_CONTROLLER')].value}" 2>/dev/null || true) | ||
| [ "${CURRENT}" = "${KDM_CONTROLLER_IMAGE}" ] && break | ||
| sleep 5 | ||
| done |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
oc options 2>&1 | grep -F -- '--request-timeout'Repository: openshift/release
Length of output: 155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='ci-operator/config/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev.yaml'
printf '%s\n' '--- target lines ---'
sed -n '45,80p' "$file"
printf '%s\n' '--- related timeout and oc usage ---'
rg -n -C 3 --fixed-strings 'openshift-adp-controller-manager' .
rg -n -C 2 'oc (get|wait)|request-timeout|timeout [0-9].*oc|timeout --' "$file" .github ci-operator 2>/dev/null | head -n 240Repository: openshift/release
Length of output: 50375
🌐 Web query:
OpenShift oc CLI documentation for --request-timeout, including whether oc supports the kubectl request-timeout option and its default behavior
💡 Result:
The OpenShift CLI (oc) supports the --request-timeout option, which functions similarly to the equivalent option in kubectl [1][2][3]. Documentation and Behavior: The --request-timeout flag specifies the length of time the client waits before giving up on a single server request [1][4]. It requires a value with a corresponding time unit (e.g., 1s, 2m, 3h) if a non-zero value is provided [1][3]. Default Behavior: The default value for --request-timeout is "0", which signifies that the client will not timeout requests [1][3]. Compatibility with kubectl: Because oc is built upon the Kubernetes client libraries, it supports the --request-timeout option in the same manner as kubectl [3]. Both tools interpret the flag to control the timeout duration for API server interactions [1][3]. While most commands consistently respect this flag, historical issues have been noted where specific sub-commands (such as oc login in older versions) did not always honor the timeout correctly, necessitating patches to ensure the configuration is properly passed to the underlying REST client [5][6][7].
Citations:
- 1: https://manpages.opensuse.org/Tumbleweed/oc/oc.1.en.html
- 2: https://manpages.opensuse.org/Tumbleweed/oc/oc-config.1.en.html
- 3: https://kubernetes.io/docs/reference/kubectl/generated/kubectl_options/
- 4: https://manpages.opensuse.org/Tumbleweed/oc/oc-create-job.1.en.html
- 5: oc login does not honor --request-timeout option origin#18159
- 6: oc login does not honor
--request-timeoutoption origin#12059 - 7: https://github.com/ardaguclu/oc/blob/v4.2.0-alpha.0/pkg/cli/login/loginoptions.go
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- oc references and installation/version evidence ---'
rg -n -C 3 '\boc\b|openshift-client|ocp|OPENSHIFT_VERSION|CLUSTER_VERSION' \
ci-operator/config/migtools/kubevirt-datamover-controller \
ci-operator/jobs/migtools/kubevirt-datamover-controller \
.github 2>/dev/null | head -n 300
printf '%s\n' '--- file metadata and nearby job structure ---'
wc -l ci-operator/config/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev.yaml
sed -n '1,120p' ci-operator/config/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev.yamlRepository: openshift/release
Length of output: 4329
🌐 Web query:
site:docs.openshift.com oc request-timeout oc get
💡 Result:
There is no global --request-timeout flag for the oc get command [1][2]. The --timeout option available with oc get is specifically used to set the length of time to wait for a specific operation to complete, such as waiting for a node or a resource to reach a specific state [1]. If you are looking to manage timeouts for network traffic or application routes, you must use different methods depending on the context: 1. Route Timeouts: You can configure timeouts for specific routes using the oc annotate command to add a timeout value to the route's configuration: oc annotate route <route_name> --overwrite haproxy.router.openshift.io/timeout=<time_unit> Supported time units include ms, s, m, h, and d [3][4]. 2. Ingress Controller Timeouts: For global ingress settings, you can configure timeout values within the Ingress Controller's configuration, such as clientTimeout or serverTimeout [5]. If you were referring to a different CLI tool, such as roxctl, that tool does feature a --timeout flag for API requests [6], but this does not apply to the standard oc CLI.
Citations:
- 1: https://docs.openshift.com/container-platform/4.18/nodes/nodes/nodes-sno-worker-nodes.html
- 2: https://docs.openshift.com/container-platform/4.18/getting_started/openshift-cli.html
- 3: https://docs.openshift.com/container-platform/4.16/networking/routes/route-configuration.html
- 4: https://docs.openshift.com/container-platform/3.5/install_config/configuring_routing.html
- 5: https://docs.openshift.com/container-platform/4.9/networking/ingress-operator.html
- 6: https://docs.openshift.com/acs/4.3/cli/debugging-issues.html
Bound each oc request.
Line 63 can block indefinitely because oc get has no request timeout. Add --request-timeout=10s so the loop can enforce its retry deadline.
🤖 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
`@ci-operator/config/migtools/kubevirt-datamover-controller/migtools-kubevirt-datamover-controller-oadp-dev.yaml`
around lines 62 - 66, Add --request-timeout=10s to the oc get invocation in the
retry loop so each request is bounded and the existing 60-iteration deadline
remains enforceable.
Source: MCP tools
kaovilai
changed the title
|
/test all Note Responses generated with Claude |
|
/pj-rehearse ack Note Responses generated with Claude |
|
@kaovilai: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
openshift-merge-bot
Bot
added
rehearsals-ack
|
/pj-rehearse ack Jobs cannot rehearse yet need images which will come post merge |
|
@kaovilai: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-merge-bot
Bot
added
the
rehearsals-ack
|
Related: #83078 adds Note Responses generated with Claude |
|
/hold management feedback wanna see rehearse b4 this go in |
openshift-ci
Bot
added
the
do-not-merge/hold
| memory: 100Mi | ||
| - as: e2e | ||
| cli: latest | ||
| commands: make TEST_VIRT=true test-e2e |
There was a problem hiding this comment.
since you are NOW running oadp-operator e2e tests against KDM repos and not out of the operator only. It would make sense to create a new VIRT test suite for KDM. You'd only want to run the KDM virt suite against kdm-controller/plugin imho. In the oadp-operator that would mean a new job most likely due to time constraints. Anyhoo this is very cool and let's roll w/ what you have for now.
|
Split the image-promotion piece out into #83110 (oadp-operator bundle/index/e2e-test-binary promotion) so it can merge first and this PR can rehearse against real images instead of hitting the bootstrapping gap. Note Responses generated with Claude |
migtools/kubevirt-datamover-controller and migtools/kubevirt-datamover-plugin (oadp-dev branch) had zero CI test coverage — only image builds. Both ship a self-contained `make test` (envtest for the controller, plain go test for the plugin), so wire that up as a standard unit-test presubmit. disable_sparse_checkout is required: without it prowgen defaults these repos to a Dockerfile-only sparse checkout (fine for the image-build job, but it starves `make test` of the Makefile/go source it needs). Partial step toward openshift/oadp-operator#1832 (Group A2: controllers reconciled by the operator, gated on a DPA feature flag). The harder half — installing oadp-operator via OLM against the PR-built kdm image and running its e2e suite — is blocked on there being no reusable oadp-dev catalog/index image (oadp-operator's ci-index is built ephemeral, per-PR, and never promoted), so that part isn't in this PR. Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
The unit-test presubmit added in the previous commit was redundant with kubevirt-datamover-controller's existing GitHub Actions test.yml, and missed the actual goal: gating these repos' PRs on oadp-operator's real e2e suite (openshift/oadp-operator#1832, Group A2), not local unit tests. Adds the real integration: - oadp-operator's own oadp-dev config now builds an operator bundle/index (operator.bundles) and the ci-Dockerfile e2e test binary, promoting both (as oadp-operator-index / oadp-operator-e2e-tests) on every merge. This is what kdm-controller/plugin need to install oadp-operator via OLM and run its e2e suite — previously only the operator binary was promoted; the bundle/index/test-binary were built ephemeral, per-PR, and only inside oadp-operator's own job. - kdm-controller/plugin presubmits: install oadp-operator via optional-operators-ci-aws from that promoted index, patch the running operator deployment's RELATED_IMAGE_KUBEVIRT_DATAMOVER_CONTROLLER / RELATED_IMAGE_KUBEVIRT_DATAMOVER_PLUGIN env to the PR-built image (oc set env, not the subscribe ref's OO_CONFIG_ENVVARS -- that can only override dependencies a step already declares, not inject a new one), then run `make TEST_VIRT=true test-e2e` from the promoted test binary. TEST_VIRT=true makes the suite install community KubeVirt/HCO itself and run the kubevirt-datamover-labeled specs (incl. VM backup/restore), so no separate CNV step-registry chain is needed. Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
Patching the Deployment directly (oc set env deployment/...) doesn't survive OLM's own reconciliation: CSV.spec is what OLM continuously reconciles the Deployment from, so a direct Deployment edit would very plausibly get reverted mid-test. Subscription.spec.config.env is OLM's documented, supported override mechanism instead -- it wins over a same-named CSV env var and is designed to persist across reconciliation and upgrades. Patch that instead of hand-editing the CSV's deployment array. Also point releases.latest at the 5.0 nightly candidate instead of 4.22, matching the actual current oadp-dev target. (Resource sizing for the e2e step was cross-checked against oadp-operator's own existing e2e-test-kubevirt-aws-periodic job -- already an exact match, no change needed there.) Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
oc patch subscription ... followed immediately by oc rollout status was racy: rollout status can report the existing rollout as already-complete before OLM has even noticed the Subscription change and pushed a new Deployment generation, letting the e2e suite start against the original promoted image instead of the PR's. Poll the Deployment's own env value until it reflects the override before checking rollout status. Also drop prowgen.disable_sparse_checkout: true -- it was only needed for the unit-test job's `from: src` step, which no longer exists (the e2e steps run from `cli` and `test-oadp-operator`, not `src`). Removing it reverts the images/postsubmit jobs to their original Dockerfile-only sparse checkout, avoiding unrelated churn and CI runtime. Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
additional_images maps {destination-name: pipeline-source-tag} -- I had
it backwards (ci-index: oadp-operator-index instead of
oadp-operator-index: ci-index), and had it on the wrong promotion.to[]
entry. Confirmed via ci-tools source (ImageTargets() inserts the *value*
as a required build target) and two working examples elsewhere in this
repo (quay-operator, vertical-pod-autoscaler-operator): the value must be
a real pipeline tag. With the original ordering, the postsubmit's
generated promote targets were `oadp-operator-e2e-tests`/
`oadp-operator-index` -- neither is a real pipeline image in this config,
so the actual postsubmit run would have failed outright once merged.
Also: the original single promotion.to[] entry has an explicit `name`
(shared-imagestream mode), where each promoted image becomes a *tag*
under that one name rather than its own imagestream -- fine for the
existing lone "oadp-operator" image, but wrong for what I actually want
(oadp-operator-index and oadp-operator-e2e-tests as their own separate
imagestreams, matching the kdm repos' base_images references). Moved
additional_images to a second to[] entry with no `name` and an explicit
`tag: latest`, which is the tag-mode that produces per-key imagestreams
instead. Added excluded_images to the original entry so it doesn't also
try to sweep the newly-added test-oadp-operator image into the shared
"oadp-operator" stream.
Verified: generated postsubmit promote targets are now `ci-index` and
`test-oadp-operator` -- real pipeline tags -- instead of the bogus names.
Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
oadp-dev and oadp-1.6 both promoting oadp-operator-index/ oadp-operator-e2e-tests to the same konveyor namespace under tag "latest" would collide -- whichever branch's postsubmit ran last would clobber the other's promoted image. Give oadp-dev's entry an explicit tag: oadp-dev instead, matching the branch-specific-tag/shared-name convention the kdm repos' own promotion already uses. Updated their base_images tag references to match. Replicates the same real oadp-operator e2e wiring (see prior commits on this branch) for the oadp-1.6 branch: - oadp-operator's oadp-1.6 base config: same two-entry promotion split (existing name-mode entry + excluded_images, new tag-mode entry for oadp-operator-index/oadp-operator-e2e-tests tagged oadp-1.6). - kdm-controller/kdm-plugin oadp-1.6 configs: same e2e-test-aws presubmit, adjusted for this branch: OO_CHANNEL: stable (matching oadp-operator's own oadp-1.6 e2e jobs, vs "dev" for oadp-dev), and dependency names pointing at this branch's own local build names (kubevirt-datamover-controller-oadp-1.6 / -plugin-oadp-1.6). Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
CodeRabbit flagged the single-quoted @.name=='manager' style filter comparisons as risky. Kubernetes' JSONPath implementation documents double-quoted string literals in filter expressions; switch to that form (escaped, since the outer jsonpath argument is itself double-quoted) to remove any doubt about whether the polling loop's match would silently fail to parse. Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
1fb6547 to
618a850
Compare
oadp-1.5 and oadp-1.6 already run make TEST_VIRT=true test-e2e as a gating presubmit (e2e-test-kubevirt-aws); oadp-dev only had it as a daily periodic, with no way to get presubmit signal on a PR before merge. Adds the same job to the oadp-dev 5.0/5.1 variants, but as optional (manually triggered via /test, doesn't block merge) rather than gating like 1.5/1.6 -- oadp-dev is the active development branch and this KubeVirt path is newer/less proven there. This is needed for openshift/oadp-operator#2350 (kubevirt-datamover VM backup/restore e2e coverage) to actually be runnable against a PR before merge -- without this, those new specs only get exercised by the existing daily periodic, well after the PR that added them has merged. Related: openshift#83049 (kdm-controller/kdm-plugin presubmit e2e using this same TEST_VIRT=true path). Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
openshift-merge-bot
Bot
removed
the
rehearsals-ack
|
[REHEARSALNOTIFIER]
A total of 98 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@kaovilai: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
Per openshift/release#83049 review feedback (weshayutin): since oadp-operator e2e now runs against the kubevirt-datamover-controller and -plugin repos specifically (not just from the operator repo), a dedicated KDM-only virt test suite is more appropriate than requiring the full TEST_VIRT suite's runtime for CI jobs that only need to validate a KDM change. Adds TEST_VIRT_KDM as a new elif branch alongside the existing TEST_VIRT/TEST_VIRT_GA in the Makefile's TEST_FILTER logic: TEST_VIRT_KDM=true filters to just the "kdm" ginkgo label, a subset of "virt". TEST_VIRT=true is unaffected -- it still filters to the full "virt" label, which includes kdm-labeled specs, so existing jobs using plain TEST_VIRT continue to exercise these specs unchanged. Also fixes -hco_community, which previously read $(TEST_VIRT) directly -- without this, TEST_VIRT_KDM=true alone (without TEST_VIRT=true) would leave HCO uninstalled and break every kdm spec before any of them could even get a VM to test against. Labels the ~8 kubevirt-datamover-CBT-specific specs (incremental backup sequence, the two phase-4-pending scaffolds, and the three live Alpine/Fedora restore Its) with Label("virt", "kdm") instead of just "virt". The plain CSI/VolSync-datamover specs in the generic "Backup and restore virtual machines" table are deliberately left unlabeled with "kdm" -- they aren't kubevirt-datamover-specific. Per sync with the openshift-release side of this work: bundled into this PR directly rather than a follow-up, since it's the same files. oadp-1.6 doesn't have any of these specs yet, so it needs a full backport as its own PR once this lands, not just a label change. Validated via `make -n test-e2e` for TEST_VIRT_KDM=true / TEST_VIRT=true / neither (correct label-filter and -hco_community for all three), and a live `--dry-run` confirming TEST_VIRT_KDM=true selects exactly the 4 live kdm-labeled specs and correctly skips all 45 non-kdm specs. Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
2 participants
Summary
Adds real oadp-operator e2e coverage for
migtools/kubevirt-datamover-controllerandmigtools/kubevirt-datamover-plugin, on bothoadp-devandoadp-1.6, per openshift/oadp-operator#1832 (Group A2: operator-reconciled controllers gated on a DPA feature flag). Each repo's presubmit now installs oadp-operator via OLM, substitutes the PR-built controller/plugin image into the running operator, and runs oadp-operator's own e2e suite — including the KubeVirt VM backup/restore specs already in that suite (e.g. openshift/oadp-operator#2350).Design
openshift/oadp-operator(base config for each branch)Builds the operator bundle (
operator.bundles) and thebuild/ci-Dockerfilee2e test binary (test-oadp-operator), and promotes both on every merge. Previously these existed only ephemerally, built fresh inside oadp-operator's own per-PR jobs — nothing outside that repo could consume them. Promoting them is what lets the two kdm repos install oadp-operator and run its e2e suite from their own presubmits.Promotion is split across two
to[]entries per branch:name: oadp-operator, shared-imagestream mode — each promoted image becomes a tag under that one name) getsexcluded_images: [test-oadp-operator]so the newly-added test-binary image doesn't also get swept into it.nameand an explicit branch tag (oadp-dev/oadp-1.6) — this is tag-mode, which gives eachadditional_imageskey its own separate destination imagestream:oadp-operator-index(the OLM catalog/index) andoadp-operator-e2e-tests(the e2e test binary), each tagged with the branch name sooadp-devandoadp-1.6don't clobber each other's promoted images in the sharedkonveyornamespace.additional_imagesmaps{destination-name: pipeline-source-tag}— confirmed viaci-toolssource (ImageTargets()inserts the value as a required build target) and two other configs in this repo already using it (quay-operator,vertical-pod-autoscaler-operator). Getting this backwards would have generated promote targets that don't correspond to any real pipeline image, so the postsubmit would fail outright on first run — caught before merge by checking the actual generated job diff.One new lightweight
ci-indexpresubmit per branch confirms the bundle still builds on oadp-operator PRs. No existing oadp-operator job behavior changes — every pre-existingoperator.substitutionsblock, and theimages/promotionblocks the kdm repos' own configs feed into those substitutions, is unchanged.migtools/kubevirt-datamover-controller+kubevirt-datamover-plugin(oadp-devandoadp-1.6)New
e2e-test-awspresubmit per branch (run_if_changed, not always-on):optional-operators-ci-awsfrom the branch's newly-promoted index (OO_CHANNEL: devonoadp-dev,stableonoadp-1.6— matching oadp-operator's own existing e2e jobs per branch).oc patch subscription ... RELATED_IMAGE_KUBEVIRT_DATAMOVER_CONTROLLER=<PR-built image>(_PLUGINfor the plugin repo) — substitutes the PR's image into the running operator viaSubscription.spec.config.env, OLM's supported override mechanism: per OLM's own design doc, values set here overwrite same-named env vars and are designed to survive reconciliation, unlike patching the Deployment directly (OLM continuously reconciles the Deployment from the CSV and would just revert that edit). The step then polls the Deployment's own env value until it reflects the override before callingoc rollout status— checking rollout status immediately after the patch is racy, since it can report the pre-existing rollout as already complete before OLM has even noticed the Subscription change.make TEST_VIRT=true test-e2efrom the promotedtest-oadp-operatorimage.TEST_VIRT=truemakes oadp-operator's own e2e suite install community KubeVirt/HCO itself and run thevirt-labeled specs, so no separate CNV step-registry chain is needed.Resource sizing for the e2e step (
1000m/512Mi) matches oadp-operator's own existinge2e-test-kubevirt-aws-periodicjob exactly, so there should be no surprises there.OLMv1 compatibility
The
oadp-operator-indexthis PR promotes is built via ci-operator'soperator.bundlesfield, which generates an index throughIndexGeneratorStepConfiguration— an index database (the legacy sqlite/opm registry serveformatCatalogSource+Subscriptionconsume).ci-toolshas no File-Based Catalog (FBC) generation path today. OLMv1'sClusterExtension/catalogd only reads FBC, so this index isn't installable via OLMv1 as-is — that's a separate, more fundamental gap than oadp-operator's CSV not yet declaringAllNamespacesinstall mode (tracked separately for OLMv1 adoption). Even once the CSV supportsAllNamespaces, this particular catalog would additionally need to be rendered to FBC before aClusterExtensioncould consume it.Known limitations
Rehearsing the new
e2e-test-awsjobs against this PR currently fails at cluster setup: the promotedoadp-operator-index/oadp-operator-e2e-testsimages don't exist in the registry yet, because that promotion is defined in this same PR and only runs on oadp-operator's postsubmit once it actually merges. This is a one-time bootstrapping order issue, not a defect in the job definitions — once this merges and oadp-operator's postsubmit runs once for real on each branch, the images will exist and the kdm jobs become testable end-to-end.Testing
make updateand ci-operator's own config+registry validation pass for both branches. Generated job diffs confirm the new promotion targets appear on oadp-operator's postsubmit for bothoadp-devandoadp-1.6, resolve to real pipeline images (ci-index,test-oadp-operator— not the destination names), and the newe2e-test-awspresubmit is correctly generated for all four kdm configs.Test plan
make update/ ci-operator's own config+registry validation passes for both branches.e2e-test-awspresubmit is correctly generated for both repos on both branches./pj-rehearsetriggered for theoadp-devjobs — confirmed the only failure is the expected bootstrapping gap above, not a job-definition problem.Note
Responses generated with Claude