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

Add READY and ADDRESS to gateway printer columns #743

Merged
merged 2 commits into from
Aug 6, 2021
Merged

Add READY and ADDRESS to gateway printer columns #743

merged 2 commits into from
Aug 6, 2021

Conversation

howardjohn
Copy link
Contributor

What type of PR is this?

/kind feature

What this PR does / why we need it:

Add new columns. This allows at glance finding out how to access the gateway, and if its ready to be accessed

Example:

NAME                  CLASS         ADDRESS        READY   AGE
coolgateway           something                            19m
coolgateway-foobar    foobar        10.149.54.25           19m
coolgatewayl-myclass  myclass                      True    19m

Ingress for comparison:

NAME      CLASS    HOSTS              ADDRESS          PORTS     AGE
ingress   <none>   foo.org            172.21.255.155   80        4d22h

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 3, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @howardjohn. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 3, 2021
@robscott
Copy link
Member

robscott commented Aug 3, 2021

This looks great, thanks @howardjohn!

/ok-to-test
/approve

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 3, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: howardjohn, robscott

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 3, 2021
@robscott
Copy link
Member

robscott commented Aug 3, 2021

Actually, missed that there were changes to v1a1 CRDs. Although these are not breaking in any way, I think we should generally consider v1alpha1 frozen.

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 3, 2021
robscott
robscott reviewed Aug 3, 2021
View reviewed changes
apis/v1alpha1/gateway_types.go Outdated
Comment on lines 28 to 29
// +kubebuilder:printcolumn:name="Address",type=string,JSONPath=`.status.addresses[*].value`
// +kubebuilder:printcolumn:name="Ready",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].status`
Copy link
Member

Choose a reason for hiding this comment

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

Suggest removing from v1alpha1 since we haven't made any other v1alpha1 changes since v0.3.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

howardjohn added a commit to howardjohn/istio that referenced this pull request Aug 3, 2021
@howardjohn
Report internal address if external address is not present
3521a24 
Joined with kubernetes-sigs/gateway-api#743,
this allows displaying non-empty results when there is no external IP
for the Gateway (ie its a cluster IP gateway)
@howardjohn howardjohn mentioned this pull request Aug 3, 2021
Merged
@robscott
Copy link
Member

robscott commented Aug 3, 2021

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 3, 2021
istio-testing pushed a commit to istio/istio that referenced this pull request Aug 5, 2021
@howardjohn
Report internal address if external address is not present (#34495)
16e4cba 
Joined with kubernetes-sigs/gateway-api#743,
this allows displaying non-empty results when there is no external IP
for the Gateway (ie its a cluster IP gateway)
@hbagdi
Copy link
Contributor

hbagdi commented Aug 6, 2021

Let's optimize for the regular case.
We can revisit this during beta if required.
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 6, 2021
@k8s-ci-robot k8s-ci-robot merged commit 8359c1c into kubernetes-sigs:master Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robscott robscott robscott left review comments

@bowei bowei Awaiting requested review from bowei

@hbagdi hbagdi Awaiting requested review from hbagdi

Assignees

@hbagdi hbagdi

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@howardjohn @k8s-ci-robot @robscott @hbagdi