Rev k8s to 1.29 #8805
Rev k8s to 1.29 #8805
Conversation
marvin-tigera
added
release-note-required
|
Need to update all the library versions and "replace" pins too. |
aaaaaaaalex
force-pushed
the
rev-kubernetes-1-29
branch
2 times, most recently
from
08ef870
to
00a31e1
Compare
| // REVIEWER TODO: is this likely correct? Port() func no longer returns error. | ||
| if tgtPort < 1 { | ||
| return errors.Errorf("no port for endpoint %q", ep) |
There was a problem hiding this comment.
feels like this should go. Probably handled where a string is converted to number already?
| Endpoint: net.JoinHostPort(ep.Addr().String(), strconv.Itoa(int(ep.Port()))), | ||
| // IsLocal is not important here | ||
| }, | ||
| NewEndpointInfo(ep.Addr().String(), int(ep.Port()), false, false, false, false, nil), |
There was a problem hiding this comment.
My preference in this place would be to use the struct so that it is a bit more obvious what is being set.
| var _ k8sp.Endpoint = &endpointInfo{} | ||
|
|
||
| // NewEndpointInfo creates a new endpointInfo, returning it as a k8s proxy Endpoint. | ||
| func NewEndpointInfo(ip string, port int, isLocal, ready, serving, terminating bool, zoneHints sets.Set[string]) k8sp.Endpoint { |
There was a problem hiding this comment.
to an extent a nit and I do not think you need to change it in the tests, but for the future, my preferred patter is to have a function that takes manadatory arguments and them variadic functional options like https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis as they are quite a bit more expressive and more readable
There was a problem hiding this comment.
I tend to prefer that too, though in this case I was hoping to keep things somewhat similar to how the k8s-pkg equivalent was handling things. But I guess we have diverged now, anyway, so why not do it our preferred way :)
aaaaaaaalex
force-pushed
the
rev-kubernetes-1-29
branch
from
5f7cc21
to
a60be0e
Compare
Description
Related issues/PRs
Todos
Release Note
Reminder for the reviewer
Make sure that this PR has the correct labels and milestone set.
Every PR needs one
docs-*label.docs-pr-required: This change requires a change to the documentation that has not been completed yet.docs-completed: This change has all necessary documentation completed.docs-not-required: This change has no user-facing impact and requires no docs.Every PR needs one
release-note-*label.release-note-required: This PR has user-facing changes. Most PRs should have this label.release-note-not-required: This PR has no user-facing changes.Other optional labels:
cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.