Skip to content
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.

Sign up for GitHub

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

Jump to bottom

kubectl get ingress output remove rules #26684

Merged
merged 1 commit into from
Jun 5, 2016
Merged

kubectl get ingress output remove rules #26684

merged 1 commit into from
Jun 5, 2016
+32 −33

Conversation

adohe-zz
Copy link

@adohe-zz adohe-zz commented Jun 2, 2016 •
edited
Loading

fixes #22837
@bprashanth ptal.

Now the output is:

[tony@localhost kubernetes]$ kubectl get ingress
NAME      RULE      BACKEND   ADDRESS   AGE
test      -                             5s
[tony@localhost kubernetes]$ kubectl get ingress --output=wide
NAME      RULE      BACKEND   ADDRESS   AGE
test      -                             9s
[tony@localhost kubernetes]$ kubectl describe ingress test
Name:           test
Namespace:      default
Address:        
Default backend:    default-http-backend:80 (<none>)
Rules:
  Host      Path    Backends
  ----      ----    --------
  foo.bar.com   
            /foo    s1:80 (<none>)
            /bar    s2:80 (<none>)
  zoo.bar.com   
            /foo    s1:80 (<none>)
            /bar    s2:80 (<none>)
Annotations:
No events.

The get output is more clean.

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Jun 2, 2016
@bprashanth
Copy link
Contributor

Now the columns don't make sense. How about:

NAME      HOSTS                     PORTS           ADDRESS       AGE
test      foo.bar.com + 1 more...    80      123.1.123.12             9s

where we add the +1 more part using the same length algorithm we use for endpoints.
And if there's a TLS section in the Ingress yaml:

NAME      HOSTS                     PORTS                 ADDRESS       AGE
test      foo.bar.com + 1 more...    80, 443           123.1.123.12    9s

And if there are no hosts, eg:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: test
spec:
  backend:
    serviceName: echoheaders-https
    servicePort: 80

we get:

NAME      HOSTS          PORTS              ADDRESS       AGE
test             *     80, 443         123.1.123.12    9s

also, what does describe with that last Ingress show?
I think it did something awkward last time i checked.

@adohe-zz
Copy link
Author

adohe-zz commented Jun 3, 2016

@bprashanth update this, ptal.

If there are no hosts, we use <none> instead of *, which we use in other places.
Also the describe output of no hosts:

Name:           test-ingress
Namespace:      default
Address:        
Default backend:    testsvc:80 (<none>)
Rules:
  Host  Path    Backends
  ----  ----    --------
Annotations:
No events.

@bprashanth
Copy link
Contributor

Hmm, * is better, none implies that we won't accept anything, but in fact leaving the host field blank means we accept all hosts. The same applies for paths, the ingress shown previously:

apiVersion: extensions/v1beta1
kind: Ingress
metadata:
  name: test
spec:
  backend:
    serviceName: echoheaders-https
    servicePort: 80

Has no paths, which means we apply the wildcard and send all traffic to that backend.

If you have the time to fix the describe output, that's wrong too. It should be:

Name:           test-ingress
Namespace:      default
Address:        
Default backend:    testsvc:80 (<none>)
Rules:
  Host  Path    Backends
  ----  ----    --------
  *       *        testsvc:80
Annotations:
No events.

Or we should not display the table at all.

@adohe-zz
Copy link
Author

adohe-zz commented Jun 4, 2016

I will open a new PR to fix the describe output.

@adohe-zz adohe-zz mentioned this pull request Jun 4, 2016
Merged
bprashanth
bprashanth reviewed Jun 4, 2016
View reviewed changes
pkg/kubectl/resource_printer.go
@@ -420,7 +420,7 @@ var replicationControllerColumns = []string{"NAME", "DESIRED", "CURRENT", "AGE"}
var replicaSetColumns = []string{"NAME", "DESIRED", "CURRENT", "AGE"}
var jobColumns = []string{"NAME", "DESIRED", "SUCCESSFUL", "AGE"}
var serviceColumns = []string{"NAME", "CLUSTER-IP", "EXTERNAL-IP", "PORT(S)", "AGE"}
var ingressColumns = []string{"NAME", "RULE", "BACKEND", "ADDRESS", "AGE"}
var ingressColumns = []string{"NAME", "HOSTS", "PORTS", "ADDRESS", "AGE"}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can you reorder to address, ports? because I'm guessing pople are going to try piping this through shell commands and reordering the two would be an extra step.

Copy link
Author

Choose a reason for hiding this comment

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

Good suggestion 👍

@bprashanth
Copy link
Contributor

Just one nit. Thanks for helping out!

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 4, 2016
k8s-github-robot pushed a commit that referenced this pull request Jun 4, 2016
Merge pull request #26843 from AdoHe/describe_ingress
9bf1ce4 
Automatic merge from submit-queue

update ingress describe output

refer #26684 for more detail. @bprashanth ptal.
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 5, 2016
@bprashanth bprashanth added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Jun 5, 2016
@bprashanth bprashanth added this to the v1.3 milestone Jun 5, 2016
@k8s-bot
Copy link

k8s-bot commented Jun 5, 2016

GCE e2e build/test passed for commit 9d28bab.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 56af9ac into kubernetes:master Jun 5, 2016
@adohe-zz adohe-zz deleted the kubectl_ingress branch June 5, 2016 13:42
@code-sleuth code-sleuth mentioned this pull request Oct 23, 2018
Closed
@zhouya0 zhouya0 mentioned this pull request May 22, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@bprashanth bprashanth

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.3
Development

Successfully merging this pull request may close these issues.

kubectl get ingress should print consistently formatted columns
7 participants
@adohe-zz @bprashanth @k8s-bot @k8s-github-robot @smarterclayton @googlebot @adohe