diff --git a/pkg/k8s/deployer.go b/pkg/k8s/deployer.go index b7102557a7..9c70dd4545 100644 --- a/pkg/k8s/deployer.go +++ b/pkg/k8s/deployer.go @@ -176,6 +176,7 @@ func (d *Deployer) Deploy(ctx context.Context, f fn.Function) (fn.DeploymentResu existingService, err := serviceClient.Get(ctx, f.Name, metav1.GetOptions{}) if err == nil { svc.ResourceVersion = existingService.ResourceVersion + preserveServiceNetworkFields(svc, existingService) if _, err = serviceClient.Update(ctx, svc, metav1.UpdateOptions{}); err != nil { return fn.DeploymentResult{}, fmt.Errorf("failed to update service: %w", err) } @@ -524,6 +525,15 @@ func (d *Deployer) generateService(f fn.Function, namespace string, daprInstalle return service, nil } +// preserveServiceNetworkFields carries forward the existing Service's +// network configuration when preparing a generated Service for update. +func preserveServiceNetworkFields(target, existing *corev1.Service) { + target.Spec.ClusterIP = existing.Spec.ClusterIP + target.Spec.ClusterIPs = existing.Spec.ClusterIPs + target.Spec.IPFamilies = existing.Spec.IPFamilies + target.Spec.IPFamilyPolicy = existing.Spec.IPFamilyPolicy +} + // CheckResourcesArePresent returns error if Secrets or ConfigMaps // referenced in input sets are not deployed on the cluster in the specified namespace func CheckResourcesArePresent(ctx context.Context, namespace string, referencedSecrets, referencedConfigMaps, referencedPVCs *sets.Set[string], referencedServiceAccount, imagePullSecret string) error { diff --git a/pkg/k8s/deployer_test.go b/pkg/k8s/deployer_test.go index 020abc2d9d..5ecd1aecf2 100644 --- a/pkg/k8s/deployer_test.go +++ b/pkg/k8s/deployer_test.go @@ -2,11 +2,15 @@ package k8s import ( "os" + "reflect" "strings" "testing" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/utils/ptr" fn "knative.dev/func/pkg/functions" ) @@ -563,3 +567,129 @@ func Test_ProcessVolumes_ValidPath(t *testing.T) { t.Errorf("expected mount path /etc/secret, got %s", mounts[0].MountPath) } } + +func TestPreserveServiceNetworkFields(t *testing.T) { + tests := []struct { + name string + existing *corev1.Service + }{ + { + name: "IPv4 single-stack", + existing: &corev1.Service{ + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + ClusterIP: "10.96.1.42", + ClusterIPs: []string{"10.96.1.42"}, + IPFamilies: []corev1.IPFamily{corev1.IPv4Protocol}, + IPFamilyPolicy: ptr.To(corev1.IPFamilyPolicySingleStack), + }, + }, + }, + { + name: "IPv6 single-stack", + existing: &corev1.Service{ + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + ClusterIP: "fd00::1:42", + ClusterIPs: []string{"fd00::1:42"}, + IPFamilies: []corev1.IPFamily{corev1.IPv6Protocol}, + IPFamilyPolicy: ptr.To(corev1.IPFamilyPolicySingleStack), + }, + }, + }, + { + name: "dual-stack", + existing: &corev1.Service{ + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + ClusterIP: "10.96.1.42", + ClusterIPs: []string{"10.96.1.42", "fd00::1:42"}, + IPFamilies: []corev1.IPFamily{corev1.IPv4Protocol, corev1.IPv6Protocol}, + IPFamilyPolicy: ptr.To(corev1.IPFamilyPolicyPreferDualStack), + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + target := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-func", + Namespace: "default", + Labels: map[string]string{"new": "label"}, + Annotations: map[string]string{"new": "annotation"}, + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + Selector: map[string]string{"new": "selector"}, + Ports: []corev1.ServicePort{ + {Name: "http", Port: 8080, Protocol: corev1.ProtocolTCP}, + }, + }, + } + + preserveServiceNetworkFields(target, tt.existing) + + if target.Spec.ClusterIP != tt.existing.Spec.ClusterIP { + t.Errorf("expected ClusterIP %q, got %q", tt.existing.Spec.ClusterIP, target.Spec.ClusterIP) + } + if !reflect.DeepEqual(target.Spec.ClusterIPs, tt.existing.Spec.ClusterIPs) { + t.Errorf("expected ClusterIPs %v, got %v", tt.existing.Spec.ClusterIPs, target.Spec.ClusterIPs) + } + if !reflect.DeepEqual(target.Spec.IPFamilies, tt.existing.Spec.IPFamilies) { + t.Errorf("expected IPFamilies %v, got %v", tt.existing.Spec.IPFamilies, target.Spec.IPFamilies) + } + if !reflect.DeepEqual(target.Spec.IPFamilyPolicy, tt.existing.Spec.IPFamilyPolicy) { + t.Errorf("expected IPFamilyPolicy %v, got %v", tt.existing.Spec.IPFamilyPolicy, target.Spec.IPFamilyPolicy) + } + + // Mutable fields already present on target must remain unchanged + if target.Labels["new"] != "label" { + t.Errorf("expected target labels unchanged, got %v", target.Labels) + } + if target.Annotations["new"] != "annotation" { + t.Errorf("expected target annotations unchanged, got %v", target.Annotations) + } + if target.Spec.Selector["new"] != "selector" { + t.Errorf("expected target selector unchanged, got %v", target.Spec.Selector) + } + if len(target.Spec.Ports) != 1 || target.Spec.Ports[0].Port != 8080 { + t.Errorf("expected target ports unchanged, got %v", target.Spec.Ports) + } + }) + } +} + +func TestGenerateServiceLeavesNetworkFieldsUnset(t *testing.T) { + d := &Deployer{} + f := fn.Function{ + Name: "test-func", + Runtime: "go", + } + deployment := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-func", + UID: "test-uid", + }, + } + + svc, err := d.generateService(f, "default", false, deployment) + if err != nil { + t.Fatalf("generateService failed: %v", err) + } + + // For new service creation, ClusterIP must be left unset so Kubernetes can allocate it + if svc.Spec.ClusterIP != "" { + t.Errorf("expected empty ClusterIP on newly generated service, got %q", svc.Spec.ClusterIP) + } + if svc.Spec.ClusterIPs != nil { + t.Errorf("expected nil ClusterIPs on newly generated service, got %v", svc.Spec.ClusterIPs) + } + if svc.Spec.IPFamilies != nil { + t.Errorf("expected nil IPFamilies on newly generated service, got %v", svc.Spec.IPFamilies) + } + if svc.Spec.IPFamilyPolicy != nil { + t.Errorf("expected nil IPFamilyPolicy on newly generated service, got %v", svc.Spec.IPFamilyPolicy) + } +}