fix(load-balancer): apply the service UID label to existing Load Balancers (#1325)

Set the service UID after copying the existing labels so it replaces a
stale
value. Unrelated labels on the Load Balancer are still preserved.

This is reachable whenever a Load Balancer is adopted with a stale UID
label:
recreating a Service under the same name gives it a new UID, and
`EnsureLoadBalancer` finds the existing Load Balancer through its name
fallback,
which also covers importing Load Balancers created by other means.

Author: lukasmetzner

internal/hcops/load_balancer.go

@@ -345,10 +345,11 @@ func (l *LoadBalancerOps) changeHCLBInfo(ctx context.Context, lb *hcloud.LoadBal
 
 	if lb.Labels[LabelServiceUID] != string(svc.ObjectMeta.UID) {
 		// Make a defensive copy of labels. This way we do not modify lb unless
-		// updating is really successful.
+		// updating is really successful. The service UID is set after copying,
+		// so that it replaces a stale value instead of being overwritten by it.
 		labels := make(map[string]string, len(lb.Labels)+1)
-		labels[LabelServiceUID] = string(svc.ObjectMeta.UID)
 		maps.Copy(labels, lb.Labels)
+		labels[LabelServiceUID] = string(svc.ObjectMeta.UID)
 		opts.Labels = labels
 		update = true
 	}

internal/hcops/load_balancer_test.go

@@ -1247,6 +1247,50 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) {
 				assert.Equal(t, "some-value", tt.initialLB.Labels["some-label"])
 			},
 		},
+		{
+			name:       "replace stale service UID label",
+			serviceUID: "12",
+			initialLB: &hcloud.LoadBalancer{
+				ID: 12,
+				Labels: map[string]string{
+					hcops.LabelServiceUID: "stale-uid",
+					"some-label":          "some-value",
+				},
+				PublicNet: hcloud.LoadBalancerPublicNet{
+					Enabled: true,
+				},
+			},
+			mock: func(_ *testing.T, tt *LBReconcilementTestCase) {
+				updated := *tt.initialLB
+				updated.Labels = map[string]string{
+					hcops.LabelServiceUID: tt.serviceUID,
+					"some-label":          "some-value",
+				}
+				opts := hcloud.LoadBalancerUpdateOpts{
+					Labels: map[string]string{
+						hcops.LabelServiceUID: tt.serviceUID,
+						"some-label":          "some-value",
+					},
+				}
+				tt.fx.LBClient.
+					On("Update", tt.fx.Ctx, tt.initialLB, opts).
+					Return(&updated, nil, nil)
+			},
+			perform: func(t *testing.T, tt *LBReconcilementTestCase) {
+				changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service)
+				assert.NoError(t, err)
+				assert.True(t, changed)
+				assert.Equal(t, tt.serviceUID, tt.initialLB.Labels[hcops.LabelServiceUID])
+				assert.Equal(t, "some-value", tt.initialLB.Labels["some-label"])
+
+				// The stale label must actually be replaced, otherwise every
+				// reconcile issues the same update again.
+				changed, err = tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service)
+				assert.NoError(t, err)
+				assert.False(t, changed)
+				tt.fx.LBClient.AssertNumberOfCalls(t, "Update", 1)
+			},
+		},
 		{
 			name:       "rename load balancer",
 			serviceUID: "11",