Support for multiple targets #243
Conversation
|
@linki: GitHub didn't allow me to request PR reviews from the following users: sethpollack. Note that only kubernetes-incubator members can review this PR, and authors cannot review their own PRs. In response to this:
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
added
the
cncf-cla: yes
|
Coincidentally this is exactly what I was working on, since this feature is desperately needed by us. I even took the same approach with using a string slice as targets in endpoints etc. |
|
@linki @dereulenspiegel I have another PR for multiple targets, with a few changes, I'll push it up soon. |
|
@linki @dereulenspiegel It depends on this PR #248 |
provider/aws.go
Outdated
| EvaluateTargetHealth: aws.Bool(evaluateTargetHealth), | ||
| } | ||
| } else { | ||
| change.ResourceRecordSet.Type = aws.String(suitableType(endpoint)) | ||
| change.ResourceRecordSet.TTL = aws.Int64(recordTTL) | ||
| change.ResourceRecordSet.ResourceRecords = []*route53.ResourceRecord{ | ||
| { | ||
| Value: aws.String(endpoint.Target), | ||
| Value: aws.String(endpoint.Targets[0]), |
There was a problem hiding this comment.
I think this needs to iterate over the Targets available and add them each as a separate ResourceRecord.
There was a problem hiding this comment.
Exactly! This was just to make it somehow compile.
|
FWIW this also affects Ingress services if the ingress controller has multiple IP addresses. For example this can easily happen with the nginx ingress controller. |
|
@sethpollack @linki I started looking at this briefly last night to look at the #315 I primarily didn't want to spend time merging these and resolving conflicts from master if it's going to be something that needs to be deferred indefinitely since it would result in a ton of merge conflicts. I'm curious if either of you are continuing to work on this or if there is a list of major gotchas that we should be looking out for. On solution for now would be to take what @linki has already started and work on getting the current functionality (no multiples) working with the Curious what your thoughts are on a way to get a basis for functional expansion though |
linki
added
work-in-progress
do-not-merge
linki
changed the title
k8s-ci-robot
added
the
size/XXL
|
Nothing to see here anymore. It's obsolete. Another round of "thank you" to all the contributors that worked on the multiple targets feature! 🎉 |
…sigs#243) * Ensure that go modules are enabled in all helper scripts This ensures that they can be run individually. * Add missing go-cmp dependency to go.{mod,sum} * Limit scope when enabling go modules
This is a work in progress to allow endpoints to have multiple targets, e.g. for NodePort services.
Issue: #239
/cc @sethpollack