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: |
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
|
/hold cancel, rehearsed |
|
/hold cancel |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
looks good, holler when you need this to land. Future state, not in this pr will be kubevirt-dm tests only running in kdm-controller and kdm-plugin repos. Running CSI backups CSI/DM tests will not be needed. |
Replaces make TEST_VIRT=true (full virt suite) with TEST_VIRT_KDM=true (kdm-labeled subset) in all 4 KDM e2e-test-aws jobs. Requires openshift/oadp-operator#2350 to merge and promote first -- until then TEST_VIRT_KDM is an unset Make var and these jobs would run zero virt specs. Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
|
Pushed a commit switching all 4 Holding on rehearsal/merge: Note Responses generated with Claude |
|
New changes are detected. LGTM label has been removed. |
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
openshift-merge-bot
Bot
removed
the
rehearsals-ack
…dk-bundle-image Reverts the KDM-specific OO_MIRROR_TO_CLUSTER_REGISTRY/OO_PSA_ENFORCE_PRIVILEGED additions to the shared optional-operators-operator-sdk-non-ci-bundle-image step back to its upstream/main state (zero diff, drops out of this PR's file list), and moves that logic into a new full copy under ci-operator/step-registry/oadp/, owned by the OADP team instead of the shared step's KubeVirt/HCO OWNERS. The 4 KDM configs now reference the new oadp-operator-sdk-bundle-image ref. Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
|
Pushed a commit forking the operator-sdk bundle-install step into an OADP-owned copy: Intent: the KDM-specific mirror-to-cluster-registry/PSA-enforcement logic had been added directly to the shared This commit reverts the shared step back to its exact Trade-off: this is a full duplication, not a thin wrapper, so future upstream fixes to the shared step won't automatically apply here — the new step's README notes its provenance so that's discoverable later. Will rehearse all 4 jobs against the new ref before considering this settled. Note Responses generated with Claude |
|
/pj-rehearse Note Responses generated with Claude |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Joeavaikath, kaovilai The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
do-not-merge/invalid-owners-file
|
@kaovilai: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Merges openshift/oadp-operator's own approvers/reviewers into this dir's OWNERS so oadp-operator team members can approve changes under ci-operator/step-registry/oadp/ (including the new oadp-operator-sdk-bundle-image fork) without needing a separate step-specific OWNERS override. Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
openshift org membership check confirmed tareqalayan is not an org member (404 on orgs/openshift/members), which openshift/release's own OWNERS validation flags as invalid. Drops it from step-registry/oadp/OWNERS. Also replaces the operator-sdk-bundle-image step's OWNERS (registry-metadata requires a physical OWNERS file per step dir, no directory-level inheritance) with only the oadp-operator repo OWNERS actually confirmed as openshift org members. Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
|
/pj-rehearse pull-ci-migtools-kubevirt-datamover-controller-oadp-dev-e2e-test-aws Note Responses generated with Claude |
|
@kaovilai: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/verify-owners Dropped Note Responses generated with Claude |
openshift-ci
Bot
removed
the
do-not-merge/invalid-owners-file
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@kaovilai: all tests passed! 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 scrum feedback, filed a follow-up enhancement issue for the "only one unmerged PR at a time" limitation of this e2e setup: openshift/oadp-operator#2389. Linked under the tracking issue #1832. Uses migtools/kubevirt-datamover-controller#84 (concurrency limiter, split across a kdm-controller PR and an oadp-operator DPA-wiring PR) as the concrete motivating example. Not a blocker for this PR — just tracked as future work. Note Responses generated with Claude |
3 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'se2e-test-awspresubmit installs oadp-operator from its promoted bundle image (promoted by openshift/release#83110, merged), injects the PR's own controller/plugin image into the running operator, and runs oadp-operator'skdm-labeled e2e specs — the KubeVirt VM backup/restore scenarios specific to kubevirt-datamover (e.g. openshift/oadp-operator#2350), not its full virt suite.Design
KDM presubmits (
e2e-test-aws,run_if_changed)For each of the 4 jobs (controller/plugin × oadp-dev/oadp-1.6):
ipi-aws-pre).oadp-operator-sdk-bundle-imagefrom its promoted bundle (quay-proxy.ci.openshift.org/openshift/ci:konveyor_oadp-operator-bundle_{oadp-dev,oadp-1.6}) usingoperator-sdk run bundle, withOO_MIRROR_TO_CLUSTER_REGISTRYandOO_PSA_ENFORCE_PRIVILEGEDenabled (see below).set-related-image: patch the Subscription'sspec.config.envwithRELATED_IMAGE_KUBEVIRT_DATAMOVER_CONTROLLER/_PLUGINpointing at the PR's own built image — OLM's supported override mechanism, which wins over same-named CSV env vars and survives reconciliation, unlike patching the Deployment directly — then wait for the Deployment to roll out the change.make TEST_VIRT_KDM=true test-e2efrom the promotedtest-oadp-operatorimage, against the cluster with the PR's image now in place.TEST_VIRT_KDM=true(added by openshift/oadp-operator#2350) runs only thekdm-labeled specs — a strict subset of the fullvirtsuiteTEST_VIRT=truewould otherwise run — and forces the same community-HCO/KubeVirt setup those specs need even thoughTEST_VIRTitself is left unset.oadp-operator-sdk-bundle-imagestepAn OADP-owned fork of
optional-operators-operator-sdk-non-ci-bundle-image(that shared step's own diff againstmainis empty in this PR — it's untouched), underci-operator/step-registry/oadp/, kept separate so KDM-specific changes don't require approval from that shared step's own OWNERS. Adds two opt-in env vars used only by these 4 KDM jobs:OO_MIRROR_TO_CLUSTER_REGISTRY:operator-sdk run bundle's own containerd-based bundle-pull client can't be given working credentials for some external registries (observed against this CI registry proxy), even when the same credential is independently valid viaoc image mirror/oc image info. When enabled, mirrorOO_BUNDLEinto the test cluster's own internal registry first, mark the registry route insecure cluster-wide (the bundle-unpack Job pulls via kubelet/CRI-O, which does full TLS verification and fails against the route's self-signed certificate), grant anonymous pull on the install namespace sooperator-sdkneeds no credentials to resolve the mirrored copy, and wait for the MachineConfigPool rollout the insecure-registry change triggers before installing.OO_PSA_ENFORCE_PRIVILEGED: forcepod-security.kubernetes.io/enforce=privilegedon anopenshift-*install namespace, extending the treatment already applied unconditionally to non-openshift-*namespaces — PSA derived purely from granted SCCs can lag behind what the operator-sdk-managed registry pod needs under PodSecurityrestricted.The MachineConfigPool wait is deterministic: it captures each pool's rendered-config name immediately before patching, then polls until every pool has moved to a new config and finished applying it to all its machines. A plain
oc wait --for=condition=Updatedcan return instantly if the MCO hasn't started rolling out yet (Updatedcan still readTruefrom before the patch). The wait fails fast, with diagnostics, if any pool reports degraded machines, if no MachineConfigPools are found at all, or if the wait budget is exhausted — rather than proceeding to an install that's already known to fail.Known limitations
oadp-operator-sdk-bundle-imageis a full copy of the shared step, not a thin wrapper. Future upstream fixes tooptional-operators-operator-sdk-non-ci-bundle-imagewon't automatically apply here; its README notes the provenance so that's discoverable later.image.config.openshift.io/cluster'sspec.additionalTrustedCA) disables TLS verification for that host entirely. Narrower and strictly better, but needs its own validation before swapping in.oc image mirror --insecureapplies to both ends of the mirror invocation (no per-registry flag exists), so it also skips TLS verification forOO_BUNDLE's own source registry, not just the internal destination. Splitting into a verified pull followed by an insecure-only push would close this gap.operator-sdk run bundlereturns would risk anImagePullBackOffif that pod is ever rescheduled later in the job. Leaving it granted is acceptable only because the test cluster is ephemeral and torn down at job end, and only the 4 KDM configs opt into any of this.Subscription.spec.config.envis patched with a full-array JSON merge patch, not a read-modify-write. Safe today becauseoperator-sdk run bundle's freshly created Subscription has no pre-existingconfig.enventries to lose; would need a read-modify-write if that ever changes.Testing
All 4 jobs (controller/plugin × oadp-dev/oadp-1.6) were rehearsed end to end against real AWS clusters through the
oadp-operator-sdk-bundle-imagefork andTEST_VIRT_KDM=true, and passed.Test plan
make ci-operator-config/make jobs/make registry-metadatapass cleanly.e2e-test-awsjobs rehearsed successfully against real AWS clusters through thekdm-labeled e2e specs.Note
Responses generated with Claude