-
Notifications
You must be signed in to change notification settings - Fork 602
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
if no kubeconfig use inclusterconfig test #5603
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vaikas 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 |
Codecov Report
@@ Coverage Diff @@
## main #5603 +/- ##
=======================================
Coverage 82.79% 82.79%
=======================================
Files 199 199
Lines 6230 6230
=======================================
Hits 5158 5158
Misses 744 744
Partials 328 328 Continue to review full report at Codecov.
|
Now fails with this:
When trying to load the in-cluster. I'll keep playing. |
doh, of course it does because the tests do not run in the cluster. |
@benmoss @salaboy And to enable those tests, the order is now:
|
@vaikas that is the correct order... When I was looking at the code that leads me to my previous PR, I assumed that this order is defined inside the knative client which does the same checks in the same order, and for some reason, these checks are repeated here. Hence I assumed that by removing the if for the default location the checks will be delegated internally to the client that was being created. Can we reuse these checks, so they are not duplicated in every module using |
I just opened knative/pkg#2197, I think this is the right fix, now sure how to be 100% sure this works for in-cluster, but from reading the code in client-go it should. |
/lgtm |
If no kubeconfig is specified, try to use inclusterconfig.
Proposed Changes
Pre-review Checklist
Release Note
Docs