New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NO-ISSUE: Fix hypershift destroy in CI test script #6246
NO-ISSUE: Fix hypershift destroy in CI test script #6246
Conversation
Missed modifying the hypershift destroy command after changing the hypershift cli function to always run in the pod.
@CrystalChun: This pull request explicitly references no jira issue. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
/test ? |
Skipping CI for Draft Pull Request. |
@CrystalChun: The following commands are available to trigger required jobs:
The following commands are available to trigger optional jobs:
Use
In response to this:
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/test-infra repository. |
/test edge-e2e-ai-operator-ztp-capi |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6246 +/- ##
==========================================
- Coverage 68.28% 68.10% -0.19%
==========================================
Files 240 241 +1
Lines 35799 36085 +286
==========================================
+ Hits 24445 24574 +129
- Misses 9198 9343 +145
- Partials 2156 2168 +12 |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CrystalChun, gamli75 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:
Approvers can indicate their approval by writing |
/retest-required |
For context, it might be good to have a link to your original PR here, it's harder to review this without context on how the hypershift command was changed EDIT: I think it's this one? b3b43c2 If my understanding is correct, in this PR a change was made to the If this is the case then this PR looks good to me as you are simply correcting a place where you forgot to prefix a command with Before providing LGTM though, I would like to ask a question about the motivation behind the change you made in the previous PR b3b43c2 Why did you stop embedding |
Thanks for the feedback. I'll keep that in mind for next time.
Yes, this is it.
The connected way didn't require specifying the However, to also support disconnected running in the |
/lgtm |
/retest-required |
@CrystalChun: all tests passed! 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/test-infra repository. I understand the commands that are listed here. |
Missed modifying the hypershift destroy command
after changing the hypershift cli function to always run in the pod.
List all the issues related to this PR
What environments does this code impact?
How was this code tested?
Checklist
docs
, README, etc)Reviewers Checklist