ingress: Add v1 describers for Ingress and IngressClass #91268
Conversation
|
Skipping CI for Draft Pull Request. |
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
sig/apps
There was a problem hiding this comment.
Here is the basic descriptors that I have started for v1. I still have to handle Resource Backends gracefully and add a few more test cases, but wanted to get some early feedback.
For the backend.Resource part, do either of you have suggestions on how the CRD output should be presented? Should it just be Backend and then similar output to the IngressClassDescribe? I'm still noodling over what the most readable method would be.
| sNum := int64(backend.Service.Port.Number) | ||
| sp.Name = strconv.FormatInt(sNum, 10) |
There was a problem hiding this comment.
match the logic above where we check if backend.Service.Port.Number matches sp.Port, and if so, set spName = sp.Name
| sNum := int64(backend.Service.Port.Number) | ||
| sp.Name = strconv.FormatInt(sNum, 10) | ||
| } else { | ||
| sp.Name = backend.Service.Port.Name |
There was a problem hiding this comment.
match the logic above where we check if backend.ServicePort.StrVal == sp.Name { and if so, set spName = sp.Name
| } | ||
| ns = metav1.NamespaceSystem | ||
| } | ||
| if def.Service != nil { |
There was a problem hiding this comment.
I would unnest this, make it apply to all backend types, and factor out the backend description to apply to both service and resource backends:
w.Write(LEVEL_0, "Default backend:\t%s\n", i.describeBackendV1(ns, def))
...
w.Write(LEVEL_2, "\t%s \t%s\n", path.Path, i.describeBackendV1(ing.Namespace, &path.Backend))
inside describeBackendV1:
- for service backends, return the old
%s (%s)format - for resource backends, return
<apiGroup>.<kind> <name>or something similar
There was a problem hiding this comment.
@liggitt Any chance you can run the describe test and spot where I'm making a mistake
make test WHAT=./staging/src/k8s.io/kubectl/pkg/describe/...
There was a problem hiding this comment.
NVM looks like my editor collapsed these return lines and I missed that
|
/assign @liggitt |
cmluciano
force-pushed
the
cml/ingv1describe
branch
from
65613b8
to
c514594
Compare
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
This is ready for review @robscott Can you take a look and make sure the IngressClass descripters look good? |
|
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
|
/unhold |
k8s-ci-robot
removed
the
do-not-merge/hold
|
/hold Temporary hold to get prow/tide to get back on its feet. Feel free to remove hold in a few hours. |
k8s-ci-robot
added
the
do-not-merge/hold
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
|
/hold Temporary hold to get prow/tide to get back on its feet. Feel free to remove hold in a few hours. |
k8s-ci-robot
added
the
do-not-merge/hold
|
/test all |
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
|
/retest |
1 similar comment
|
/retest |
|
needs rebase |
Ingressv1 Get is attempted for Ingresses and IngressClasses and falls back to Ingressv1beta1 if there is a failure. Signed-off-by: Christopher M. Luciano <cmluciano@us.ibm.com>
cmluciano
force-pushed
the
cml/ingv1describe
branch
from
bb113f7
to
31520cc
Compare
k8s-ci-robot
added
size/XL
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cmluciano, liggitt 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 |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
/lgtm |
|
@cmluciano: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
|
/retest Review the full test history for this PR. Silence the bot with an |
What type of PR is this?
/kind api-change
What this PR does / why we need it:
Ingressv1 Get is attempted for Ingresses and IngressClasses
and falls back to Ingressv1beta1 if there is a failure.
Which issue(s) this PR fixes:
Based on #89778
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
Ingress KEP
kubernetes/enhancements#1453