Skip to content

feat: auto-set LB IP-pinning and sharing-key annotations from allocation IP #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Exonical merged 2 commits into from
May 30, 2026
Merged

feat: auto-set LB IP-pinning and sharing-key annotations from allocation IP #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1408c8e
feat: auto-set LB IP-pinning and sharing-key annotations from allocat…
Exonical May 29, 2026
7457b9d
fix: remove stale LB IP-pinning annotations on Service update
Exonical May 29, 2026
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
6 changes: 6 additions & 0 deletions chart/pelican-wings/templates/configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ data:
lb_annotations:
{{- toYaml .Values.wings.kubernetes.lbAnnotations | nindent 8 }}
{{- end }}
{{- if .Values.wings.kubernetes.lbIPAnnotation }}
lb_ip_annotation: {{ .Values.wings.kubernetes.lbIPAnnotation | quote }}
{{- end }}
{{- if .Values.wings.kubernetes.lbSharingKey }}
lb_sharing_key: {{ .Values.wings.kubernetes.lbSharingKey | quote }}
{{- end }}
storage_mode: {{ .Values.wings.kubernetes.storageMode | quote }}
{{- if .Values.wings.kubernetes.storageClass }}
storage_class: {{ .Values.wings.kubernetes.storageClass | quote }}
Expand Down
8 changes: 8 additions & 0 deletions chart/pelican-wings/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ wings:
# lbAnnotations:
# io.cilium/lb-ipam-pool: "game-servers"
lbAnnotations: {}
# -- Annotation key Wings sets to the server's allocation IP on each LB Service.
# This pins the LB to the IP selected in the Panel so multiple servers share one IP.
# Cilium: "lbipam.cilium.io/ips" | MetalLB: "metallb.universe.tf/loadBalancerIPs"
lbIPAnnotation: ""
# -- Annotation key for LB IP sharing. Wings sets the value to the allocation IP,
# grouping all servers on the same IP under one sharing key.
# Cilium: "lbipam.cilium.io/sharing-key" | MetalLB: "metallb.universe.tf/allow-shared-ip"
lbSharingKey: ""
# -- Storage mode: "hostpath" or "pvc"
storageMode: pvc
# -- StorageClass for PVCs (empty = cluster default)
Expand Down
23 changes: 23 additions & 0 deletions config/config_kubernetes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,29 @@ type KubernetesConfiguration struct {
// metallb.universe.tf/address-pool: "game-servers"
LBAnnotations map[string]string `json:"lb_annotations" yaml:"lb_annotations"`

// LBIPAnnotation is the annotation key that Wings sets to the server's
// allocation IP on each LoadBalancer Service. This pins the LB to the IP
// the user selected in the Panel, enabling multiple game servers to share
// the same external IP (on different ports).
//
// Common values:
// Cilium: "lbipam.cilium.io/ips"
// MetalLB: "metallb.universe.tf/loadBalancerIPs"
//
// When empty (default), no IP-pinning annotation is added.
LBIPAnnotation string `default:"" json:"lb_ip_annotation" yaml:"lb_ip_annotation"`

// LBSharingKey is the annotation key used to allow multiple Services to
// share the same external IP. Wings sets this annotation to the allocation
// IP value, grouping all game servers on the same IP under one sharing key.
//
// Common values:
// Cilium: "lbipam.cilium.io/sharing-key"
// MetalLB: "metallb.universe.tf/allow-shared-ip"
//
// When empty (default), no sharing-key annotation is added.
LBSharingKey string `default:"" json:"lb_sharing_key" yaml:"lb_sharing_key"`

// Tolerations allow game server Pods to be scheduled on tainted nodes.
Tolerations []KubeToleration `json:"tolerations" yaml:"tolerations"`

Expand Down
41 changes: 32 additions & 9 deletions environment/kubernetes/network.go
View file
Open in desktop

@devin-ai-integration devin-ai-integration Bot May 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔴 Stale LB IP-pinning annotations persist on Service update when allocation IP becomes invalid

When updating an existing LoadBalancer Service, the annotation merge at lines 148-155 only adds/overwrites keys from the new annotations map — it never removes annotations that are no longer desired. If a server's allocation IP changes from a valid address (e.g. 23.227.184.222) to 0.0.0.0, 127.0.0.1, or empty, allocationIP() (network.go:362-371) returns "", so the LBIPAnnotation and LBSharingKey keys are not included in the annotations map. However, those annotation keys from the previous EnsureService call remain on the existing Service object, keeping the LoadBalancer pinned to the old/stale IP.

Example scenario
  1. Server allocation IP is 23.227.184.222; EnsureService creates the Service with lbipam.cilium.io/ips: "23.227.184.222"
  2. Admin changes allocation to 0.0.0.0 (wildcard)
  3. EnsureService runs again: allocationIP() returns "", so annotations map only has LBAnnotations entries
  4. Merge loop writes LBAnnotations but the stale lbipam.cilium.io/ips key is never deleted
  5. The LB controller keeps the Service pinned to 23.227.184.222

(Refers to lines 148-155)

Prompt for agents 
In EnsureService in environment/kubernetes/network.go, the update path (lines 148-155) merges annotations into the existing service but never removes stale IP-pinning annotations when the allocation IP becomes invalid (0.0.0.0, 127.0.0.1, or empty).

To fix this, after the merge loop (or as part of the update block), explicitly delete the LBIPAnnotation and LBSharingKey annotation keys from existing.Annotations when allocationIP() returns empty but the config keys are set. For example:

if allocIP == "" {
    if cfg.Kubernetes.LBIPAnnotation != "" {
        delete(existing.Annotations, cfg.Kubernetes.LBIPAnnotation)
    }
    if cfg.Kubernetes.LBSharingKey != "" {
        delete(existing.Annotations, cfg.Kubernetes.LBSharingKey)
    }
}

Note that allocIP is currently scoped inside the if-block at line 102, so you will need to hoist the allocationIP() call to a broader scope (or call it again) so it is accessible in the update path. Also consider the case where the config key itself changes (e.g. LBIPAnnotation changes from one annotation name to another) — the old annotation key from the previous config would also remain stale.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@devin-ai-integration devin-ai-integration Bot May 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch. Fixed in 7457b9d — the update path now replaces annotations entirely (existing.Annotations = annotations) instead of merging, so stale IP-pinning and sharing-key annotations are cleaned up when the allocation IP becomes invalid. Added a test that verifies the stale annotation removal scenario.

Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,21 @@ func (e *Environment) EnsureService(ctx context.Context) error {
var annotations map[string]string
if isLB {
svcType = corev1.ServiceTypeLoadBalancer
if len(cfg.Kubernetes.LBAnnotations) > 0 {
annotations = cfg.Kubernetes.LBAnnotations
annotations = make(map[string]string)
for k, v := range cfg.Kubernetes.LBAnnotations {
annotations[k] = v
}

// Auto-set IP-pinning and sharing-key annotations from the
// server's allocation IP so the LB is bound to the IP the user
// selected in the Panel.
if allocIP := e.allocationIP(); allocIP != "" {
if cfg.Kubernetes.LBIPAnnotation != "" {
annotations[cfg.Kubernetes.LBIPAnnotation] = allocIP
}
if cfg.Kubernetes.LBSharingKey != "" {
annotations[cfg.Kubernetes.LBSharingKey] = allocIP
}
}
}

Expand Down Expand Up @@ -132,13 +145,8 @@ func (e *Environment) EnsureService(ctx context.Context) error {
existing.Labels = labels

e.log().WithField("service", svcName).Infof("updating %s service for server", svcType)
if isLB && len(annotations) > 0 {
if existing.Annotations == nil {
existing.Annotations = make(map[string]string)
}
for k, v := range annotations {
existing.Annotations[k] = v
}
if isLB {
existing.Annotations = annotations
}
_, err = e.client.CoreV1().Services(ns).Update(ctx, existing, metav1.UpdateOptions{})
if err != nil {
Expand Down Expand Up @@ -342,3 +350,18 @@ func sanitizePortName(name string) string {
func portName(proto string, port int) string {
return sanitizePortName(fmt.Sprintf("%s-%d", proto, port))
}

// allocationIP returns the server's default allocation IP if it is a usable
// public/external address. Returns empty for 0.0.0.0, 127.0.0.1, or when
// no default allocation is configured.
func (e *Environment) allocationIP() string {
allocs := e.Configuration.Allocations()
if allocs.DefaultMapping == nil {
return ""
}
ip := allocs.DefaultMapping.Ip
if ip == "" || ip == "0.0.0.0" || ip == "127.0.0.1" {
return ""
}
return ip
}
142 changes: 142 additions & 0 deletions environment/kubernetes/network_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,148 @@ func TestNetwork(t *testing.T) {
svcs, _ := client.CoreV1().Services("pelican").List(context.Background(), metav1.ListOptions{})
g.Assert(len(svcs.Items)).Equal(0)
})

g.It("should auto-set LB IP and sharing-key annotations from allocation IP", func() {
client := fake.NewSimpleClientset()
allocs := environment.Allocations{
DefaultMapping: &environment.DefaultAllocationMapping{
Ip: "23.227.184.222",
Port: 27015,
},
Mappings: map[string][]int{"23.227.184.222": {27015}},
}
env := newTestEnv(client, allocs)

config.Update(func(c *config.Configuration) {
c.Kubernetes.NetworkMode = config.KubeNetworkLoadBalancer
c.Kubernetes.Namespace = "pelican"
c.Kubernetes.LBAnnotations = map[string]string{
"io.cilium/lb-ipam-pool": "game-servers",
}
c.Kubernetes.LBIPAnnotation = "lbipam.cilium.io/ips"
c.Kubernetes.LBSharingKey = "lbipam.cilium.io/sharing-key"
})

err := env.EnsureService(context.Background())
g.Assert(err).IsNil()

svc, err := client.CoreV1().Services("pelican").Get(context.Background(), "gs-test-server-uuid", metav1.GetOptions{})
g.Assert(err).IsNil()
g.Assert(svc.Spec.Type).Equal(corev1.ServiceTypeLoadBalancer)
g.Assert(svc.Annotations["io.cilium/lb-ipam-pool"]).Equal("game-servers")
g.Assert(svc.Annotations["lbipam.cilium.io/ips"]).Equal("23.227.184.222")
g.Assert(svc.Annotations["lbipam.cilium.io/sharing-key"]).Equal("23.227.184.222")
})

g.It("should not set IP annotations when allocation IP is 0.0.0.0", func() {
client := fake.NewSimpleClientset()
allocs := environment.Allocations{
DefaultMapping: &environment.DefaultAllocationMapping{
Ip: "0.0.0.0",
Port: 27015,
},
Mappings: map[string][]int{"0.0.0.0": {27015}},
}
env := newTestEnv(client, allocs)

config.Update(func(c *config.Configuration) {
c.Kubernetes.NetworkMode = config.KubeNetworkLoadBalancer
c.Kubernetes.Namespace = "pelican"
c.Kubernetes.LBIPAnnotation = "lbipam.cilium.io/ips"
c.Kubernetes.LBSharingKey = "lbipam.cilium.io/sharing-key"
})

err := env.EnsureService(context.Background())
g.Assert(err).IsNil()

svc, err := client.CoreV1().Services("pelican").Get(context.Background(), "gs-test-server-uuid", metav1.GetOptions{})
g.Assert(err).IsNil()
_, hasIP := svc.Annotations["lbipam.cilium.io/ips"]
g.Assert(hasIP).IsFalse()
_, hasKey := svc.Annotations["lbipam.cilium.io/sharing-key"]
g.Assert(hasKey).IsFalse()
})

g.It("should not set IP annotations when config keys are empty", func() {
client := fake.NewSimpleClientset()
allocs := environment.Allocations{
DefaultMapping: &environment.DefaultAllocationMapping{
Ip: "23.227.184.222",
Port: 27015,
},
Mappings: map[string][]int{"23.227.184.222": {27015}},
}
env := newTestEnv(client, allocs)

config.Update(func(c *config.Configuration) {
c.Kubernetes.NetworkMode = config.KubeNetworkLoadBalancer
c.Kubernetes.Namespace = "pelican"
c.Kubernetes.LBIPAnnotation = ""
c.Kubernetes.LBSharingKey = ""
})

err := env.EnsureService(context.Background())
g.Assert(err).IsNil()

svc, err := client.CoreV1().Services("pelican").Get(context.Background(), "gs-test-server-uuid", metav1.GetOptions{})
g.Assert(err).IsNil()
_, hasIP := svc.Annotations["lbipam.cilium.io/ips"]
g.Assert(hasIP).IsFalse()
})

g.It("should remove stale IP annotations when allocation IP becomes invalid", func() {
existingSvc := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "gs-test-server-uuid",
Namespace: "pelican",
Annotations: map[string]string{
"io.cilium/lb-ipam-pool": "game-servers",
"lbipam.cilium.io/ips": "23.227.184.222",
"lbipam.cilium.io/sharing-key": "23.227.184.222",
},
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeLoadBalancer,
Ports: []corev1.ServicePort{
{Name: "tcp-27015", Protocol: corev1.ProtocolTCP, Port: 27015},
{Name: "udp-27015", Protocol: corev1.ProtocolUDP, Port: 27015},
},
Selector: map[string]string{"pelican.dev/server-id": "test-server-uuid"},
},
}
client := fake.NewSimpleClientset(existingSvc)
allocs := environment.Allocations{
DefaultMapping: &environment.DefaultAllocationMapping{
Ip: "0.0.0.0",
Port: 27015,
},
Mappings: map[string][]int{"0.0.0.0": {27015}},
}
env := newTestEnv(client, allocs)

config.Update(func(c *config.Configuration) {
c.Kubernetes.NetworkMode = config.KubeNetworkLoadBalancer
c.Kubernetes.Namespace = "pelican"
c.Kubernetes.LBAnnotations = map[string]string{
"io.cilium/lb-ipam-pool": "game-servers",
}
c.Kubernetes.LBIPAnnotation = "lbipam.cilium.io/ips"
c.Kubernetes.LBSharingKey = "lbipam.cilium.io/sharing-key"
})

err := env.EnsureService(context.Background())
g.Assert(err).IsNil()

svc, err := client.CoreV1().Services("pelican").Get(context.Background(), "gs-test-server-uuid", metav1.GetOptions{})
g.Assert(err).IsNil()
// Pool annotation should remain.
g.Assert(svc.Annotations["io.cilium/lb-ipam-pool"]).Equal("game-servers")
// IP-pinning annotations should be removed.
_, hasIP := svc.Annotations["lbipam.cilium.io/ips"]
g.Assert(hasIP).IsFalse()
_, hasKey := svc.Annotations["lbipam.cilium.io/sharing-key"]
g.Assert(hasKey).IsFalse()
})
})

g.Describe("DeleteService", func() {
Expand Down
Loading