Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,16 @@ func main() {

p2pEndpointDomain := os.Getenv("SEI_P2P_ENDPOINT_DOMAIN")

nlbTargetType := os.Getenv("SEI_NLB_TARGET_TYPE")
switch nlbTargetType {
case "":
nlbTargetType = nodedeploymentcontroller.DefaultNLBTargetType
case nodedeploymentcontroller.NLBTargetTypeIP, nodedeploymentcontroller.NLBTargetTypeInstance:
default:
setupLog.Error(nil, "Invalid SEI_NLB_TARGET_TYPE — expected \"ip\" or \"instance\"", "value", nlbTargetType)
os.Exit(1)
}

//nolint:staticcheck // migrating to events.EventRecorder API is a separate effort
recorder := mgr.GetEventRecorderFor("seinodedeployment-controller")
if err := (&nodedeploymentcontroller.SeiNodeDeploymentReconciler{
Expand All @@ -226,6 +236,7 @@ func main() {
GatewayDomain: platformCfg.GatewayDomain,
GatewayPublicDomain: platformCfg.GatewayPublicDomain,
P2PEndpointDomain: p2pEndpointDomain,
NLBTargetType: nlbTargetType,
PlanExecutor: &planner.Executor[*seiv1alpha1.SeiNodeDeployment]{
ConfigFor: func(ctx context.Context, group *seiv1alpha1.SeiNodeDeployment) task.ExecutionConfig {
var assemblerNode *seiv1alpha1.SeiNode
Expand Down
9 changes: 9 additions & 0 deletions internal/controller/nodedeployment/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ type SeiNodeDeploymentReconciler struct {
// from SEI_P2P_ENDPOINT_DOMAIN. Empty disables the P2P endpoint path.
P2PEndpointDomain string

// NLBTargetType picks the AWS NLB target mode for per-pod P2P endpoint
// Services. "ip" registers pod IPs directly — correct on VPC-CNI
// clusters (prod, dev). "instance" routes via NodePort — required on
// clusters where pod IPs aren't VPC-routable (e.g. harbor, where
// Cilium cluster-pool uses 100.64.0.0/14). Wired from SEI_NLB_TARGET_TYPE
// in cmd/main.go, which defaults the field to DefaultNLBTargetType
// when the env var is unset — the reconciler reads this field as-is.
NLBTargetType string

// PlanExecutor drives group-level task plans (e.g. genesis assembly).
PlanExecutor planner.PlanExecutor[*seiv1alpha1.SeiNodeDeployment]
}
Expand Down
30 changes: 25 additions & 5 deletions internal/controller/nodedeployment/p2p_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,18 @@ const (
awsLBCrossZoneAnnotation = "service.beta.kubernetes.io/aws-load-balancer-attributes"
awsLBTypeValue = "external"
awsLBSchemeValue = "internet-facing"
awsLBTargetTypeValue = "ip"
awsLBCrossZoneAttributeStr = "load_balancing.cross_zone.enabled=true"

// NLBTargetTypeIP registers pod IPs directly with the NLB target group.
// Correct on VPC-CNI clusters where pod IPs are VPC-routable.
NLBTargetTypeIP = "ip"
// NLBTargetTypeInstance routes NLB traffic via the node's NodePort and
// relies on kube-proxy / Cilium socket-LB to forward to the pod.
// Required on clusters where pod IPs aren't VPC-routable.
NLBTargetTypeInstance = "instance"
// DefaultNLBTargetType matches the prior hardcoded behaviour.
DefaultNLBTargetType = NLBTargetTypeIP

p2pEndpointServiceSuffix = "-p2p"
)

Expand Down Expand Up @@ -85,7 +94,7 @@ func p2pEndpointServiceName(group *seiv1alpha1.SeiNodeDeployment, ordinal int) s
func (r *SeiNodeDeploymentReconciler) reconcileP2PEndpoints(ctx context.Context, group *seiv1alpha1.SeiNodeDeployment) error {
desiredNames := make(map[string]struct{}, group.Spec.Replicas)
for i := range int(group.Spec.Replicas) {
desired := generateP2PEndpointService(group, i, r.p2pEndpointHostname(group, i))
desired := generateP2PEndpointService(group, i, r.p2pEndpointHostname(group, i), r.NLBTargetType)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Envtest reconciler missing NLBTargetType causes test failure

High Severity

The envtest reconciler in suite_test.go (line 202) does not initialize NLBTargetType, so it defaults to "". When reconcileP2PEndpoints passes r.NLBTargetType to generateP2PEndpointService, the annotation aws-load-balancer-nlb-target-type becomes empty string, and the AllocateLoadBalancerNodePorts logic skips the == NLBTargetTypeIP branch (producing nil instead of *false). The envtest at p2p_endpoint_test.go:98-99 asserts the annotation equals "ip" — this assertion will fail, breaking CI.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit dfc998a. Configure here.

desired.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Service"))
if err := ctrl.SetControllerReference(group, desired, r.Scheme); err != nil {
return fmt.Errorf("setting owner reference on P2P endpoint Service %s: %w", desired.Name, err)
Expand Down Expand Up @@ -181,7 +190,7 @@ func (r *SeiNodeDeploymentReconciler) deleteP2PEndpointForOrdinal(ctx context.Co
return nil
}

func generateP2PEndpointService(group *seiv1alpha1.SeiNodeDeployment, ordinal int, hostname string) *corev1.Service {
func generateP2PEndpointService(group *seiv1alpha1.SeiNodeDeployment, ordinal int, hostname, targetType string) *corev1.Service {
name := p2pEndpointServiceName(group, ordinal)
childName := seiNodeName(group, ordinal)

Expand All @@ -195,11 +204,22 @@ func generateP2PEndpointService(group *seiv1alpha1.SeiNodeDeployment, ordinal in
externalDNSHostnameAnnotation: hostname,
awsLBTypeAnnotation: awsLBTypeValue,
awsLBSchemeAnnotation: awsLBSchemeValue,
awsLBTargetTypeAnnotation: awsLBTargetTypeValue,
awsLBTargetTypeAnnotation: targetType,
awsLBCrossZoneAnnotation: awsLBCrossZoneAttributeStr,
managedByAnnotation: controllerName,
}

// target-type=ip registers pod IPs directly; NodePort is not used so
// we explicitly disable allocation to preserve the limited 30000-32767
// range. target-type=instance requires a NodePort for the NLB→node→pod
// hop (kube-proxy / Cilium socket-LB), so leave allocation at the
// kube default (true) by passing nil.
var allocateNodePorts *bool
if targetType == NLBTargetTypeIP {
f := false
allocateNodePorts = &f
}

return &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Expand All @@ -210,7 +230,7 @@ func generateP2PEndpointService(group *seiv1alpha1.SeiNodeDeployment, ordinal in
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeLoadBalancer,
ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeLocal,
AllocateLoadBalancerNodePorts: new(bool),
AllocateLoadBalancerNodePorts: allocateNodePorts,
Selector: map[string]string{
noderesource.NodeLabel: childName,
},
Expand Down
22 changes: 21 additions & 1 deletion internal/controller/nodedeployment/p2p_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func TestGeneratePublishableService_Annotations(t *testing.T) {
snd := &seiv1alpha1.SeiNodeDeployment{
ObjectMeta: metav1.ObjectMeta{Name: testChainAtlantic, Namespace: "sei-test-1"},
}
svc := generateP2PEndpointService(snd, 0, "atlantic-2-0-p2p.atlantic-2.prod.platform.sei.io")
svc := generateP2PEndpointService(snd, 0, "atlantic-2-0-p2p.atlantic-2.prod.platform.sei.io", NLBTargetTypeIP)

g.Expect(svc.Annotations).To(HaveKeyWithValue(
"external-dns.alpha.kubernetes.io/hostname",
Expand All @@ -147,4 +147,24 @@ func TestGeneratePublishableService_Annotations(t *testing.T) {
g.Expect(svc.Spec.Ports).To(HaveLen(1))
g.Expect(svc.Spec.Ports[0].Port).To(Equal(seiconfig.PortP2P))
g.Expect(svc.Spec.Selector).To(HaveKeyWithValue(noderesource.NodeLabel, "atlantic-2-0"))

// target-type=ip → NodePort allocation explicitly disabled (preserve
// the 30000-32767 range; pod IP is the NLB target, no NodePort hop).
g.Expect(svc.Spec.AllocateLoadBalancerNodePorts).NotTo(BeNil())
g.Expect(*svc.Spec.AllocateLoadBalancerNodePorts).To(BeFalse())
}

func TestGeneratePublishableService_InstanceTargetType(t *testing.T) {
g := NewWithT(t)
snd := &seiv1alpha1.SeiNodeDeployment{
ObjectMeta: metav1.ObjectMeta{Name: testChainAtlantic, Namespace: "sei-test-1"},
}
svc := generateP2PEndpointService(snd, 0, "atlantic-2-0-p2p.atlantic-2.harbor.platform.sei.io", NLBTargetTypeInstance)

g.Expect(svc.Annotations).To(HaveKeyWithValue(
"service.beta.kubernetes.io/aws-load-balancer-nlb-target-type", "instance"))

// target-type=instance needs a NodePort for the NLB→node→pod hop.
// Leaving the field nil lets kube default to true.
g.Expect(svc.Spec.AllocateLoadBalancerNodePorts).To(BeNil())
}
Loading