[feat: gw-api] Creating Target Group + TGB from Gateway spec #4150
Conversation
k8s-ci-robot added the cncf-cla: yes k8s-ci-robot added approved 
zac-nixon force-pushed the znixon/gw-tg-creation branch from 7379608 to 132e5a3 Compare k8s-ci-robot added the needs-rebase 
zac-nixon force-pushed the znixon/gw-tg-creation branch from 132e5a3 to 75892c3 Compare k8s-ci-robot removed the needs-rebase 
| // enableProxyProtocolV2 [Network LoadBalancers] Indicates whether proxy protocol version 2 is enabled. | ||
| // By default, proxy protocol is disabled. | ||
| // +optional | ||
| EnableProxyProtocolV2 *bool `json:"enableProxyProtocolV2,omitempty"` |
There was a problem hiding this comment.
Lets infer this from the attributes value instead of creating a dedicated field.
| return output, nil | ||
| } | ||
| | ||
| func (builder *targetGroupBuilderImpl) getTargetProps(routeDescriptor routeutils.RouteDescriptor, backend routeutils.Backend) *elbv2gw.TargetGroupProps { |
There was a problem hiding this comment.
NIT: getTargetGroupProps
| // TODO, auto infer? | ||
| if targetGroupProps == nil || targetGroupProps.Protocol == nil { | ||
| // infer this somehow!? | ||
| // use the backend config to get the protocol type. |
There was a problem hiding this comment.
This should be based on its route right instead of backend config?
There was a problem hiding this comment.
I think it depends. For example, there is no way to say that a TargetGroup should use the TCP_UDP protocol just from the route type. I'll add some auto infer using the route, and we can come back in our specific L4 phase to include TCP_UDP support.
| return | ||
| } | ||
| | ||
| if _, ok := (*attributeMap)[shared_constants.TGAttributeProxyProtocolV2Enabled]; !ok { |
There was a problem hiding this comment.
Lets not add any default values from our side for attributes. We dont want any client side handling of any attribute defaults. This is leftover code from service as they had dedicated annotations for few attributes.
| | ||
| baseBuilder.logger.Info("Got this route details", "routes", routes) | ||
| /* Target Groups */ | ||
| // TODO - Figure out how to map this back to a listener? |
There was a problem hiding this comment.
We will build listeners after the LB so while building listeners we will be building their corresponding actions on rules(for ALB) and default actions for NLB, we will build their tgs.
| /lgtm |
k8s-ci-robot added the lgtm 
| [APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: shraddhabang, zac-nixon 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 |
shraddhabang added the tide/merge-method-squash …tes-sigs#4150) * [gw api] tg creation * fixes to get tg + tgb working * make logging less noisy * refactor duplicated consts * tg tests * fix interval and timeout to appease alb * use shared constant for health check port * refactor multicluster to target group props * refactor to use route kind enum * infer target group type from route
…tes-sigs#4150) * [gw api] tg creation * fixes to get tg + tgb working * make logging less noisy * refactor duplicated consts * tg tests * fix interval and timeout to appease alb * use shared constant for health check port * refactor multicluster to target group props * refactor to use route kind enum * infer target group type from route
…tes-sigs#4150) * [gw api] tg creation * fixes to get tg + tgb working * make logging less noisy * refactor duplicated consts * tg tests * fix interval and timeout to appease alb * use shared constant for health check port * refactor multicluster to target group props * refactor to use route kind enum * infer target group type from route
3 participants
Issue
Description
Checklist
README.md, or thedocsdirectory)BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯