Skip to content

Conversation

Joeavaikath
Copy link
Contributor

@Joeavaikath Joeavaikath commented Aug 8, 2025

Why the changes were made

Moves the install to within the cli e2e test suite.

How to test the changes made

cli e2e should run without any issues. ci-Dockerfile only has kubectl install now.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 8, 2025
Copy link

openshift-ci bot commented Aug 8, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@Joeavaikath Joeavaikath marked this pull request as ready for review August 12, 2025 18:45
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 12, 2025
@openshift-ci openshift-ci bot requested review from kaovilai and sseago August 12, 2025 18:46
kaovilai
kaovilai previously approved these changes Aug 12, 2025
Copy link
Member

@kaovilai kaovilai left a comment

Choose a reason for hiding this comment

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

Relevant changes from migtools/oadp-cli#43

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 12, 2025
weshayutin
weshayutin previously approved these changes Aug 12, 2025
Copy link
Contributor

@weshayutin weshayutin left a comment

Choose a reason for hiding this comment

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

make install ASSUME_DEFAULT=true
Building kubectl-oadp for current platform (linux/amd64)...
✅ Built kubectl-oadp successfully!
Installing kubectl-oadp to /home/whayutin/.local/bin...
✅ Installed to /home/whayutin/.local/bin

✅ PATH already configured


📋 Configuration:
Setting Velero namespace to: openshift-adp
✅ Client config initialized

📋 Next steps:
  1. Test admin commands: kubectl oadp backup get
  2. Test non-admin commands: kubectl oadp nonadmin backup get
  3. Manage NABSL requests: kubectl oadp nabsl get
  4. Change namespace: kubectl oadp client config set namespace=<namespace>

@Joeavaikath
Copy link
Contributor Author

/retest

@kaovilai
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 12, 2025
@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD 164bf42 and 2 for PR HEAD 9f01806 in total

@openshift-ci openshift-ci bot removed lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 12, 2025
@Joeavaikath Joeavaikath force-pushed the cli-install-oadp-cli branch from 8d6d194 to 7934b9b Compare August 13, 2025 02:54
@kaovilai
Copy link
Member

/retest

@kaovilai
Copy link
Member

      failed to create backup via CLI: exit status 1, output: An error occurred: namespaces "velero" not found

@Joeavaikath
Copy link
Contributor Author

      failed to create backup via CLI: exit status 1, output: An error occurred: namespaces "velero" not found

this is weird...locally it does set the right namespace

@kaovilai
Copy link
Member

see more in slack DM. but maybe need this in CI?

❯ cat ~/.config/velero/config.json 
{"namespace":"openshift-adp"}

@kaovilai
Copy link
Member

Step e2e-test-cli-aws-e2e succeeded after 54m32s.

False alarm from flaky steps post cli e2e

/retest

Refactor OADP CLI installation in CI Dockerfile and e2e tests

- Removed direct installation of oadp-cli from the Dockerfile.
- Introduced a new CLISetup struct to handle the cloning, building, and installation of oadp-cli in a more structured manner.
- Updated e2e tests to utilize the new CLISetup for verifying OADP CLI availability and configuration.
@Joeavaikath Joeavaikath force-pushed the cli-install-oadp-cli branch from 584943f to f21c30d Compare August 14, 2025 05:13
@mpryc
Copy link
Contributor

mpryc commented Aug 14, 2025

/retest-required

@kaovilai kaovilai mentioned this pull request Aug 14, 2025
- Changed the installation method from moving to copying the kubectl-oadp binary to /usr/local/bin.
- Updated log messages to reflect the change in operation.
- Improved error messages for better clarity on failure points.
@Joeavaikath
Copy link
Contributor Author

/retest

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 14, 2025
Comment on lines +175 to +183
if targetPath == fmt.Sprintf("%s/bin/kubectl-oadp", os.Getenv("HOME")) {
homeBin := fmt.Sprintf("%s/bin", os.Getenv("HOME"))
currentPath := os.Getenv("PATH")
if !strings.Contains(currentPath, homeBin) {
newPath := fmt.Sprintf("%s:%s", homeBin, currentPath)
os.Setenv("PATH", newPath)
log.Printf("Added %s to PATH", homeBin)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

For the curious this is what fixed CI

Copy link
Contributor

Choose a reason for hiding this comment

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

well done @kaovilai and @Joeavaikath :)

@kaovilai
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 14, 2025
Copy link

openshift-ci bot commented Aug 14, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Joeavaikath, kaovilai, shubham-pampattiwar

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [kaovilai,shubham-pampattiwar]

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

@Joeavaikath
Copy link
Contributor Author

/retest

@kaovilai
Copy link
Member

I think we can stop retesting and override the hcp tests since its nil pointer deref unrelated.

@openshift-ci-robot
Copy link

/retest-required

Remaining retests: 0 against base HEAD d9ff173 and 2 for PR HEAD 6dc85c6 in total

@kaovilai
Copy link
Member

/override ci/prow/4.19-e2e-test-hcp-aws ci/prow/4.20-e2e-test-hcp-aws

Copy link

openshift-ci bot commented Aug 15, 2025

@kaovilai: Overrode contexts on behalf of kaovilai: ci/prow/4.19-e2e-test-hcp-aws, ci/prow/4.20-e2e-test-hcp-aws

In response to this:

/override ci/prow/4.19-e2e-test-hcp-aws ci/prow/4.20-e2e-test-hcp-aws

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.

Copy link

openshift-ci bot commented Aug 15, 2025

@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-hcp-aws 6dc85c6 link true /test 4.19-e2e-test-hcp-aws
ci/prow/4.20-e2e-test-hcp-aws 6dc85c6 link false /test 4.20-e2e-test-hcp-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.

@openshift-merge-bot openshift-merge-bot bot merged commit ea426b0 into openshift:oadp-dev Aug 15, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants