-
Notifications
You must be signed in to change notification settings - Fork 8.1k
AGE column for istioctl #6013
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
AGE column for istioctl #6013
Conversation
General question: There's been quite a few PR's recently making istioctl behave like kubectl. Why not use kubectl directly? |
@ayj My original inspiration for istioctl get improvements was so that there could be calculated custom fields summarizing deployment configuration. See #5538 I implemented the custom fields, but to make the PRs small and manageable I broke the implementation into several PRs and the last and most interesting one is not yet delivered. I believe there is some easy low-hanging fruit with istioctl improvements. Today I spent 90 minutes debugging a clients 404 issues. Turns out an Istio VirtualService had been modified with
kubectl can't do any validation; at least with istioctl we now know something is wrong. My hope is that a few short changes to istioctl will help medium-skilled operators from shooting themselves in the foot. (I realize the particular validation problem described here is better fixed by #1894 ) |
Thanks for taking time to improve the overall user experience. My comment was more in reference to issues (I mistakenly said PRs) that have suggested feature enhancements to bring istioctl in-line with kubectl, e.g.
Server-side validation (#1894) will be enabled by default in 1.0 at which point we should remove client-side validation from istioctl. Server-side validation is also available in 0.8 but disabled by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Codecov Report
@@ Coverage Diff @@
## master #6013 +/- ##
======================================
+ Coverage 67% 67% +1%
======================================
Files 345 345
Lines 30684 30611 -73
======================================
- Hits 20452 20444 -8
+ Misses 9397 9330 -67
- Partials 835 837 +2
Continue to review full report at Codecov.
|
Closing; I will create a new PR after I find a fool-proof way to add a dependency. |
Marking this WIP because I suspect the failure in ci/circleci: e2e-pilot-auth-v1alpha3-v2 is real. I introduce a new field, model.ConfigMeta.CreationTimestamp and I suspect that some code compares ConfigMeta with DeepEqual() and the timestamps cause the comparisons that previously returned |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ayj, esnible 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 |
@esnible: The following test failed, say
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. |
istioctl get should include an AGE column similar to kubectl.