Add ConsoleCLIDownload for oadp-cli

[Joeavaikath]

Why the changes were made

Tackles migtools/oadp-cli#76

Two main changes:

• Add config for deployment and service pointing to image containing oadp-cli binaries hosted on a Go server
• Add a way to start the route and ConsoleCLIDownload resource on operator startup
  • This is done here because the hostname for the route is dynamic: we need to grab that once the route is up, and then create the ConsoleCLIDownload

Flow:

1. Operator deployed, new deployment and service created
2. oadp-cli controller sees deployment, creates route, backs off for hostname to be assigned
3. oadp-cli controller reconciles again, sees hostname and creates a ConsoleCLIDownload resource with this hostname as the download link

How to test the changes made

• Run make deploy-olm
• Click the "?" symbol at the top right of the OpenShift console, and then click "Command Line Tools"

[weshayutin]

do we want s/Download oadp/Download OADP CLI/ ?

[weshayutin]

FWIW... OADP should always be in caps

[Joeavaikath]

So from the other cli tools on that page, I gathered that lowercase is for the actual command (oc, odo)
So lowercase is used to emphasize that it's the CLI tool which gives you access to that command on the CLI
Basically, matches the other CLI tools on that page

[weshayutin]

This is just a step to making the oadp-cli available to customers. Additional requiements

1. build the client for mac, windows, linux in the https://github.com/migtools/oadp-cli repo
2. push container to quay
3. from the oadp-operator, host a container that serves the built artifacts e.g. https://downloads-openshift-console.apps.wdharm420.migration.redhat.com/
4. test. :)
5. Notify @rayfordj of the new container for OADP-1.6 and point at the dockerfile

Issue tracker: migtools/oadp-cli#71

[kaovilai]

note that the yaml here will need to also be in config/ dir and run make bundle. can walk though that later if needed.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Joeavaikath
Once this PR has been reviewed and has the lgtm label, please assign shawn-hurley for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Joeavaikath
Once this PR has been reviewed and has the lgtm label, please assign shawn-hurley for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Joeavaikath
Once this PR has been reviewed and has the lgtm label, please assign shawn-hurley for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Joeavaikath
Once this PR has been reviewed and has the lgtm label, please assign shawn-hurley for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[kaovilai]

ai-explains-test-failure: The unit test build succeeded, but the final lint step failed:

• cmd/main.go:43:1 – the file isn’t properly formatted according to gci rules.
• internal/controller/oadp_cli_controller.go:39:2 – the variable log shadows the imported log package, flagged by revive.

Because golangci‑lint reported these two errors, the job exited with status 2 and the overall step failed.

[Joeavaikath]

/retest

[Joeavaikath]

/retest-required

[weshayutin]

The image needs to be built on both AMD64 and ARM64 for dev testing purposes.

Node-Selectors:              <none>
Tolerations:                 node.kubernetes.io/memory-pressure:NoSchedule op=Exists
                             node.kubernetes.io/not-ready:NoExecute op=Exists for 300s
                             node.kubernetes.io/unreachable:NoExecute op=Exists for 300s
Events:
  Type     Reason          Age                    From               Message
  ----     ------          ----                   ----               -------
  Normal   Scheduled       5m45s                  default-scheduler  Successfully assigned openshift-adp/openshift-adp-oadp-cli-server-7689b54d59-dkzcv to ip-10-0-116-201.us-west-2.compute.internal
  Normal   AddedInterface  5m45s                  multus             Add eth0 [10.131.1.162/23] from ovn-kubernetes
  Normal   Pulled          5m44s                  kubelet            Successfully pulled image "quay.io/konveyor/oadp-cli-binaries" in 996ms (996ms including waiting). Image size: 353031244 bytes.
  Normal   Pulled          5m42s                  kubelet            Successfully pulled image "quay.io/konveyor/oadp-cli-binaries" in 883ms (883ms including waiting). Image size: 353031244 bytes.
  Normal   Pulled          5m28s                  kubelet            Successfully pulled image "quay.io/konveyor/oadp-cli-binaries" in 1.296s (1.296s including waiting). Image size: 353031244 bytes.
  Normal   Pulled          5m2s                   kubelet            Successfully pulled image "quay.io/konveyor/oadp-cli-binaries" in 1.302s (1.302s including waiting). Image size: 353031244 bytes.
  Normal   Pulled          4m8s                   kubelet            Successfully pulled image "quay.io/konveyor/oadp-cli-binaries" in 1.09s (1.09s including waiting). Image size: 353031244 bytes.
  Normal   Created         2m43s (x6 over 5m44s)  kubelet            Created container: oadp-cli-server
  Normal   Started         2m43s (x6 over 5m44s)  kubelet            Started container oadp-cli-server
  Normal   Pulled          2m43s                  kubelet            Successfully pulled image "quay.io/konveyor/oadp-cli-binaries" in 1.157s (1.157s including waiting). Image size: 353031244 bytes.
  Warning  BackOff         16s (x26 over 5m41s)   kubelet            Back-off restarting failed container oadp-cli-server in pod openshift-adp-oadp-cli-server-7689b54d59-dkzcv_openshift-adp(9835f1c8-309f-4bd1-bc9a-f3e0aabbc2fc)
  Normal   Pulling         2s (x7 over 5m45s)     kubelet            Pulling image "quay.io/konveyor/oadp-cli-binaries"
  Normal   Pulled          1s                     kubelet            Successfully pulled image "quay.io/konveyor/oadp-cli-binaries" in 882ms (882ms including waiting). Image size: 353031244 bytes.
whayutin@fedora:~/OPENSHIFT/git/OADP/oadp-operator$ oc get all -n openshift-adp
Warning: apps.openshift.io/v1 DeploymentConfig is deprecated in v4.14+, unavailable in v4.10000+
NAME                                                                  READY   STATUS             RESTARTS      AGE
pod/f31243f3480a6431d31a9ab844198a2fac80d19d92455907166296678bh26sq   0/1     Completed          0             6m13s
pod/openshift-adp-controller-manager-64d4c876f8-svg5g                 1/1     Running            0             6m2s
pod/openshift-adp-oadp-cli-server-7689b54d59-dkzcv                    0/1     CrashLoopBackOff   6 (17s ago)   6m2s
pod/ttl-sh-oadp-operator-bundle-8046d1af-1h                           1/1     Running            0             6m17s

NAME                                                       TYPE        CLUSTER-IP      EXTERNAL-IP   PORT(S)    AGE
service/openshift-adp-cli-server                           ClusterIP   172.30.125.71   <none>        80/TCP     6m6s
service/openshift-adp-controller-manager-metrics-service   ClusterIP   172.30.11.45    <none>        8443/TCP   6m6s

NAME                                               READY   UP-TO-DATE   AVAILABLE   AGE
deployment.apps/openshift-adp-controller-manager   1/1     1            1           6m3s
deployment.apps/openshift-adp-oadp-cli-server      0/1     1            0           6m3s

NAME                                                          DESIRED   CURRENT   READY   AGE
replicaset.apps/openshift-adp-controller-manager-64d4c876f8   1         1         1       6m3s
replicaset.apps/openshift-adp-oadp-cli-server-7689b54d59      1         1         0       6m3s

NAME                                                                        STATUS     COMPLETIONS   DURATION   AGE
job.batch/f31243f3480a6431d31a9ab844198a2fac80d19d92455907166296678b5f237   Complete   1/1           6s         6m14s

NAME                                             HOST/PORT                                                                 PATH   SERVICES                   PORT   TERMINATION     WILDCARD
route.route.openshift.io/oadp-cli-server-route   oadp-cli-server-route-openshift-adp.apps.wdharm420.migration.redhat.com          openshift-adp-cli-server   http   edge/Redirect   None
whayutin@fedora:~/OPENSHIFT/git/OADP/oadp-operator$ oc logs -f deployment.apps/openshift-adp-oadp-cli-server
exec container process `/usr/local/bin/download-server`: Exec format error

[weshayutin]

listed twice

[weshayutin]

Installation should be tightened up a little:

Installation:

tar -xzf kubectl-oadp_*.tar.gz
chmod +x kubectl-oadp
sudo mv kubectl-oadp /usr/local/bin/

TO

tar -xzf kubectl-oadp_*.tar.gz
either 
  - Fix the unpack to be the binary name kubectl-oadp
  - OR
  - mv kubectl-oadp_v*_linux_$arch kubectl-oadp
 sudo mv kubectl-oadp /usr/local/bin/

[Joeavaikath]

ConsoleCLIDownload is a cluster scoped resource
We can't set an Owner reference so that it gets deleted when the operator is uninstalled
So this makes it idempotent, we won't have multiples just updates the existing ConsoleCLIDownload on another install
To clean it up we would need a manual delete

[kaovilai]

To clean it up we would need a manual delete

As long as each time this creates, if one already exists, it gets updated to the newest version without failing I think it's safe to have this be leftover.

[Joeavaikath]

On a fresh operator install, the deployment is reset and so we get the latest image for oadp-cli-binaries regardless of ConsoleCLIDownload resource being there, cause it just points to it
Now, do we want to update it continuously somehow is another question: i.e. actively look for a new version
edit: maybe its best not to disturb versions like that, not reproducable
we could settle on a particular version and update it on our end

[kaovilai]

/retest

ai-retester: The e2e-test-aws step failed because the Mongo application CSI test timed out while waiting for the todolist pod to become ready. The container in the pod is waiting to start/initialize.

[kaovilai]

/retest

ai-retester: The "Mongo application CSI" test timed out because the todolist pod failed to initialize, preventing successful backup and restore.

[kaovilai]

/retest

ai-retester: The end-to-end test e2e-test-aws failed because the Mongo application CSI test timed out after 540 seconds while waiting for a Pod to succeed, and the test suite thus failed.

[kaovilai]

/retest

ai-retester: The Mongo backup/restore test never got out of the “waiting to start” state – the todolist pod hung in PodInitializing because the scheduler rejected it for violating the cluster’s PodSecurity policy (the container was marked as privileged, used unsupported securityContext flags, and the pod had no required fields). The test timed out (~540 s) waiting for the pod to succeed, so it failed.

[kaovilai]

/retest

ai-retester: The e2e-test-cli-aws-e2e step failed because the Mongo application CSI via CLI test timed out while waiting for the todolist pod to become ready during the OADP end-to-end testing. The "container "todolist" in pod ... is waiting to start: PodInitializing" message confirms, the pod never reached active status for testing.

[kaovilai]

/retest

[weshayutin]

@kaovilai @Joeavaikath you have to rebase :)

[kaovilai]

As far as prow goes, no you don't. :) assuming no conflicts

[weshayutin]

/retest

[kaovilai]

As far as prow goes, no you don't. :) assuming no conflicts

receipts if interested in digging more:

https://github.com/kubernetes-sigs/prow/blob/8cd4af136cc0eeb9ee7ab9fc11cd99f7adba663e/pkg/pod-utils/clone/clone.go#L359

Before a PR is merged, Tide ensures that all jobs configured as required in the presubmits part of the config.yaml file are passing against the latest base branch commit, rerunning the jobs if necessary. No job is required to be configured in which case it’s enough if a PR meets all GitHub search criteria.

emphasis mine.

TL;DR: each test run contains changes from base branch merged into it. Subsequent retests or new test triggers also bring latest base branch changes into it.

[kaovilai]

/retest

ai-retester: The e2e-test-cli-aws test failed because the "MySQL application two Vol CSI via CLI" test case failed, with a pod initialization error, preventing the application from being accessible. The underlying issue appears to be that the "ebs.csi.aws.com" volume snapshots were not ready when the restore was attempted, causing provisioning to fail.

[kaovilai]

/retest

ai-retester: The e2e-test-cli-aws test failed because the MySQL application two Vol CSI via CLI test case failed. The Todolist service could not be reached after restore, likely due to issues with the pod initializing despite the restore completing otherwise. The underlying cause could be due to CSI snapshot restore failures in the persistent data location because, while the restore was marked as being completed at 2025-11-07 17:15:22 +0000 UTC, the service wasn't accessible at 2025-11-07 17:16:02.128.

[coderabbitai]

Walkthrough

Adds OpenShift Console CLI download support: new CLI download controller that manages a CLI server Deployment, Service, Route, and ConsoleCLIDownload; RBAC for console.openshift.io; manager env var and relatedImage; console API scheme registration and controller wiring.

Changes

Cohort / File(s)	Summary
RBAC declarations config/rbac/role.yaml, internal/controller/dataprotectionapplication_controller.go, bundle/manifests/oadp-operator.clusterserviceversion.yaml	Added RBAC rule for apiGroup console.openshift.io, resource consoleclidownloads with verbs create, delete, get, list, patch, update, watch (kubebuilder marker and manifest entries).
Manager / bundle env & images config/manager/manager.yaml, bundle/manifests/oadp-operator.clusterserviceversion.yaml	Injected environment variable RELATED_IMAGE_CONSOLE_CLI_DOWNLOAD = quay.io/konveyor/oadp-cli-binaries:latest into manager container and added corresponding relatedImages entry (console-cli-download).
Console API scheme & init cmd/main.go	Registered OpenShift console.openshift.io/v1 scheme in manager and conditionally added CLI download setup runnable to manager startup.
CLI download controller implementation internal/controller/cli_download_controller.go	Added CLIDownloadSetup runnable: reads RELATED_IMAGE, resolves operator Deployment ownership, reconciles Deployment, Service, Route, and ConsoleCLIDownload resources idempotently; implements route hostname wait/retry and exponential backoff.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Focus review on internal/controller/cli_download_controller.go (reconciliation correctness, owner references, idempotency, retry/backoff behavior).
• Verify RBAC entries in role.yaml, CSV, and kubebuilder markers are consistent and sufficient.
• Confirm RELATED_IMAGE_CONSOLE_CLI_DOWNLOAD propagated correctly in manager.yaml and CSV.
• Check cmd/main.go ordering: console scheme registration and conditional runnable wiring.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between b46f2af and a78c3bb.

📒 Files selected for processing (2)

• cmd/main.go (3 hunks)
• internal/controller/cli_download_controller.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

• internal/controller/cli_download_controller.go
• cmd/main.go

---

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (3)

cmd/main.go (1)

275-284: Guard CLI download setup when WATCH_NAMESPACE is empty

CLIDownloadSetup assumes watchNamespace is the operator namespace and non-empty. When getWatchNamespace() fails and returns "", this runnable will try to create namespaced resources with an empty namespace, which can cause startup failures in “watch all namespaces” scenarios.

Consider short-circuiting here (e.g., skip adding CLIDownloadSetup or make it a no-op) when watchNamespace == "".

internal/controller/cli_download_controller.go (2)

76-122: Reconcile Deployment/Service spec to handle upgrades, not just initial create

reconcileCLIResources only creates the Deployment and Service on IsNotFound and otherwise leaves them untouched. If you later change the CLI server image, ports, or resource requests, existing clusters will keep the old spec indefinitely.

Consider reconciling to a desired spec (or using controllerutil.CreateOrUpdate) so that Deployment and Service are updated when the operator version or image reference changes, while still being idempotent.

---

155-167: Tighten retry logic for route hostname to avoid unbounded goroutines

When route.Spec.Host is empty, the code logs and spawns a goroutine that sleeps 5s and calls reconcileCLIResources again. If the route never gets a hostname (e.g., misconfig or router issue), this will recursively schedule retries forever, leaking goroutines and generating repeated API traffic.

Consider instead:

• Looping with backoff inside the existing call while honoring ctx.Done(), or
• Returning an error and letting a higher-level retry (or manager restart) handle it, or
• Using a bounded retry loop with a maximum number of attempts before giving up with a clear log message.

This keeps behavior predictable and avoids silent background loops.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 5d074fc and b46f2af.

📒 Files selected for processing (6)

• bundle/manifests/oadp-operator.clusterserviceversion.yaml (3 hunks)
• cmd/main.go (3 hunks)
• config/manager/manager.yaml (1 hunks)
• config/rbac/role.yaml (1 hunks)
• internal/controller/cli_download_controller.go (1 hunks)
• internal/controller/dataprotectionapplication_controller.go (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

• config/manager/manager.yaml
• cmd/main.go
• internal/controller/dataprotectionapplication_controller.go
• config/rbac/role.yaml
• internal/controller/cli_download_controller.go
• bundle/manifests/oadp-operator.clusterserviceversion.yaml

🔇 Additional comments (5)

internal/controller/dataprotectionapplication_controller.go (1)

64-76: RBAC for consoleclidownloads is consistent and appropriately scoped

The added kubebuilder RBAC marker for console.openshift.io / consoleclidownloads matches the ClusterRole/CSV updates and gives the operator exactly the verbs it needs for the new Console CLI download flow.

config/manager/manager.yaml (1)

71-100: New RELATED_IMAGE_CONSOLE_CLI_DOWNLOAD env wiring looks correct

The env var is added alongside other RELATED_IMAGE_* entries and aligns with the CSV deployment, so the CLI server image will be available both in-cluster and for local manager.yaml–driven runs.

config/rbac/role.yaml (1)

67-78: ClusterRole permissions for consoleclidownloads are appropriate

Granting full CRUD + watch on console.openshift.io/consoleclidownloads to manager-role lines up with the new controller behavior and other RBAC markers, without widening scope beyond that resource type.

bundle/manifests/oadp-operator.clusterserviceversion.yaml (1)

1048-1059: CSV RBAC, env, and relatedImages updates are coherent

The new consoleclidownloads RBAC, the RELATED_IMAGE_CONSOLE_CLI_DOWNLOAD env on the controller deployment, and the console-cli-download relatedImages entry are all consistent with the rest of the manifests and the new CLI download controller.

Also applies to: 1307-1308, 1479-1480

internal/controller/cli_download_controller.go (1)

34-40: The review comment is incorrect: client.Client does have a Scheme() method

The controller-runtime client.Client interface includes Scheme() *runtime.Scheme. The code at lines 88, 111, and 134 calling c.Client.Scheme() in SetOwnerReference calls is valid and will compile successfully. The struct does not need modification, and the suggested fix is unnecessary.

Likely an incorrect or invalid review comment.

[Joeavaikath]

/retest-required

[openshift-ci]

@Joeavaikath: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/4.19-e2e-test-aws	75f4d68	link	true	/test 4.19-e2e-test-aws
ci/prow/4.19-e2e-test-cli-aws	75f4d68	link	true	/test 4.19-e2e-test-cli-aws

Full PR test history. Your PR dashboard.

Instructions 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.

[kaovilai]

/retest

[Joeavaikath]

All tests passing, wow

[Joeavaikath]

Test after removing this
Traffic from route to svc is http