Skip to content
Open
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
29 changes: 29 additions & 0 deletions manifests/0000_70_dns-operator_00.crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,35 @@ spec:
type: object
type: array
type: object
nodeServices:
description: |-
nodeServices specifies K8s services for which entries should be added
to /etc/hosts by the node resolver. These services will be added in addition to
the default "image-registry.openshift-image-registry.svc" service.
For each service reference, entries will be created using the format "<name>.<namespace>.svc"
and an alias with the CLUSTER_DOMAIN suffix will also be added.
items:
description: DNSNodeService represents a Kubernetes service by name
and namespace for node services.
properties:
name:
description: name is the name of the service.
maxLength: 63
minLength: 1
pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$
type: string
namespace:
description: namespace is the namespace of the service.
maxLength: 63
minLength: 1
pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$
type: string
required:
- name
- namespace
type: object
maxItems: 20
type: array
operatorLogLevel:
default: Normal
description: 'operatorLogLevel controls the logging level of the DNS
Expand Down
32 changes: 26 additions & 6 deletions pkg/operator/controller/controller_dns_node_resolver_daemonset.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package controller
import (
"context"
"fmt"
"strings"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
Expand All @@ -23,11 +24,9 @@ import (
)

const (
// services is a comma- or space-delimited list of services for which
// entries should be added to /etc/hosts. NOTE: For now, ensure these
// are relative names; for each relative name, an alias with the
// CLUSTER_DOMAIN suffix will also be added.
services = "image-registry.openshift-image-registry.svc"
// defaultService is the default service that should always be included
// in the services list for the node resolver
defaultService = "image-registry.openshift-image-registry.svc"

// workloadPartitioningManagement contains the management workload annotation
workloadPartitioningManagement = "target.workload.openshift.io/management"
Expand All @@ -38,6 +37,27 @@ var (
nodeResolverScript = manifests.NodeResolverScript()
)

// buildServicesList combines the default service with additional services from the DNS spec.
// The default service is always included first, followed by any additional services.
func buildServicesList(dns *operatorv1.DNS) string {
// Start with the default service
services := []string{defaultService}

// Add any additional services from the spec
if len(dns.Spec.NodeServices) > 0 {
for _, service := range dns.Spec.NodeServices {
Copy link
Member

Choose a reason for hiding this comment

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

@candita Will this ensure that the list, assuming items aren't changing, are in a stable order or do we need to sort them to prevent churn?

// Build service name in format: name.namespace.svc
if service.Name != "" && service.Namespace != "" {
serviceName := service.Name + "." + service.Namespace + ".svc"
services = append(services, serviceName)
}
}
}

// Join all services with commas for the environment variable
return strings.Join(services, ",")
}

// ensureNodeResolverDaemonset ensures the node resolver daemonset exists if it
// should or does not exist if it should not exist. Returns a Boolean
// indicating whether the daemonset exists, the daemonset if it does exist, and
Expand Down Expand Up @@ -82,7 +102,7 @@ func desiredNodeResolverDaemonSet(dns *operatorv1.DNS, clusterIP, clusterDomain,
maxUnavailable := intstr.FromString("33%")
envs := []corev1.EnvVar{{
Name: "SERVICES",
Value: services,
Value: buildServicesList(dns),
}}
if len(clusterIP) > 0 {
envs = append(envs, corev1.EnvVar{
Expand Down
138 changes: 138 additions & 0 deletions pkg/operator/controller/controller_dns_node_resolver_daemonset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,71 @@ import (
"k8s.io/apimachinery/pkg/util/intstr"
)

// TestBuildServicesList tests the buildServicesList function with various DNS configurations
func TestBuildServicesList(t *testing.T) {
testCases := []struct {
name string
nodeServices []operatorv1.DNSNodeService
expected string
}{
{
name: "no additional services",
nodeServices: nil,
expected: "image-registry.openshift-image-registry.svc",
},
{
name: "empty additional services slice",
nodeServices: []operatorv1.DNSNodeService{},
expected: "image-registry.openshift-image-registry.svc",
},
{
name: "single additional service",
nodeServices: []operatorv1.DNSNodeService{{Name: "my-service", Namespace: "my-namespace"}},
expected: "image-registry.openshift-image-registry.svc,my-service.my-namespace.svc",
},
{
name: "multiple additional services",
nodeServices: []operatorv1.DNSNodeService{{Name: "service1", Namespace: "namespace1"}, {Name: "service2", Namespace: "namespace2"}, {Name: "service3", Namespace: "namespace3"}},
expected: "image-registry.openshift-image-registry.svc,service1.namespace1.svc,service2.namespace2.svc,service3.namespace3.svc",
},
{
name: "service with empty name is filtered out",
nodeServices: []operatorv1.DNSNodeService{{Name: "", Namespace: "my-namespace"}},
expected: "image-registry.openshift-image-registry.svc",
},
{
name: "service with empty namespace is filtered out",
nodeServices: []operatorv1.DNSNodeService{{Name: "my-service", Namespace: ""}},
expected: "image-registry.openshift-image-registry.svc",
},
{
name: "mixed valid and invalid services",
nodeServices: []operatorv1.DNSNodeService{{Name: "service1", Namespace: "namespace1"}, {Name: "", Namespace: "namespace2"}, {Name: "service3", Namespace: "namespace3"}},
expected: "image-registry.openshift-image-registry.svc,service1.namespace1.svc,service3.namespace3.svc",
},
{
name: "all invalid services filtered out",
nodeServices: []operatorv1.DNSNodeService{{Name: "", Namespace: "namespace1"}, {Name: "service2", Namespace: ""}},
expected: "image-registry.openshift-image-registry.svc",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
dns := &operatorv1.DNS{
Spec: operatorv1.DNSSpec{
NodeServices: tc.nodeServices,
},
}

result := buildServicesList(dns)
if result != tc.expected {
t.Errorf("expected %q, got %q", tc.expected, result)
}
})
}
}

// TestDesiredNodeResolverDaemonset verifies that desiredNodeResolverDaemonSet
// returns the expected daemonset.
func TestDesiredNodeResolverDaemonset(t *testing.T) {
Expand Down Expand Up @@ -53,6 +118,79 @@ func TestDesiredNodeResolverDaemonset(t *testing.T) {
} else if clusterDomain != domain {
t.Errorf("expected CLUSTER_DOMAIN env for dns node resolver image %q, got %q", clusterDomain, domain)
}
services, ok := envs["SERVICES"]
if !ok {
t.Errorf("SERVICES env for dns node resolver image not found")
} else if services != "image-registry.openshift-image-registry.svc" {
t.Errorf("expected SERVICES env for dns node resolver image %q, got %q", "image-registry.openshift-image-registry.svc", services)
}
}
}

// TestDesiredNodeResolverDaemonsetWithNodeServices verifies that desiredNodeResolverDaemonSet
// correctly handles node services in the SERVICES environment variable.
func TestDesiredNodeResolverDaemonsetWithNodeServices(t *testing.T) {
clusterDomain := "cluster.local"
clusterIP := "172.30.77.10"
openshiftCLIImage := "openshift/origin-cli:test"

testCases := []struct {
name string
nodeServices []operatorv1.DNSNodeService
expectedServices string
}{
{
name: "with single node service",
nodeServices: []operatorv1.DNSNodeService{{Name: "my-api", Namespace: "my-namespace"}},
expectedServices: "image-registry.openshift-image-registry.svc,my-api.my-namespace.svc",
},
{
name: "with multiple node services",
nodeServices: []operatorv1.DNSNodeService{{Name: "service1", Namespace: "ns1"}, {Name: "service2", Namespace: "ns2"}},
expectedServices: "image-registry.openshift-image-registry.svc,service1.ns1.svc,service2.ns2.svc",
},
{
name: "with valid services only",
nodeServices: []operatorv1.DNSNodeService{{Name: "clean-service", Namespace: "namespace"}, {Name: "another-service", Namespace: "ns"}},
expectedServices: "image-registry.openshift-image-registry.svc,clean-service.namespace.svc,another-service.ns.svc",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
dns := &operatorv1.DNS{
ObjectMeta: metav1.ObjectMeta{
Name: DefaultDNSController,
},
Spec: operatorv1.DNSSpec{
NodeServices: tc.nodeServices,
},
}

if want, ds, err := desiredNodeResolverDaemonSet(dns, clusterIP, clusterDomain, openshiftCLIImage); err != nil {
t.Errorf("invalid node resolver daemonset: %v", err)
} else if !want {
t.Error("expected the node resolver daemonset desired to be true, got false")
} else if len(ds.Spec.Template.Spec.Containers) != 1 {
t.Errorf("expected number of daemonset containers 1, got %d", len(ds.Spec.Template.Spec.Containers))
} else {
c := ds.Spec.Template.Spec.Containers[0]

// Check environment variables
envs := map[string]string{}
for _, e := range c.Env {
envs[e.Name] = e.Value
}

// Verify SERVICES environment variable contains expected services
services, ok := envs["SERVICES"]
if !ok {
t.Errorf("SERVICES env for dns node resolver image not found")
} else if services != tc.expectedServices {
t.Errorf("expected SERVICES env for dns node resolver image %q, got %q", tc.expectedServices, services)
}
}
})
}
}

Expand Down
30 changes: 30 additions & 0 deletions vendor/github.com/openshift/api/operator/v1/types_dns.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.