fix(ipv6): follow the cluster families for the syslog-ng bind and the metrics bind

Three points from review.

The syslog-ng source bound "::" whenever enabledIPv6 was set. That reaches IPv4
clients only through v4-mapped addresses, which net.ipv6.bindv6only turns off.
On a node with that setting and a cluster without IPv6, the Service is IPv4 and
the source had no IPv4 listener, so ingestion failed. The bind now follows the
same cluster answer the Services use.

The buffer-metrics sidecars hard-coded their listen address, so a bind set on
bufferVolumeMetrics was accepted and dropped. They pass it through now. An unset
bind still yields the wildcard address.

A cluster with no IPv6 range is re-probed instead of cached, so a range added to
a running cluster is picked up without restarting the operator.

Signed-off-by: Bence Csati <bence.csati@axoflow.com>

Author: csatib02

controllers/logging/logging_controller.go

@@ -246,7 +246,7 @@ func (r *LoggingReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	syslogNGExternal, syslogNGSpec := loggingResources.GetSyslogNGSpec()
 	if syslogNGSpec != nil {
 		logging.AggregatorLevelConfigCheck(syslogNGSPec.ConfigCheck)
-		syslogNGConfig, secretList, err := r.clusterConfigurationSyslogNG(loggingResources)
+		syslogNGConfig, secretList, err := r.clusterConfigurationSyslogNG(ctx, loggingResources)
 		if err != nil {
 			// TODO: move config generation into Syslog-NG reconciler
 			reconcilers = append(reconcilers, func(ctx context.Context) (*reconcile.Result, error) {
@@ -461,7 +461,7 @@ func (r *LoggingReconciler) clusterConfigurationFluentd(resources model.LoggingR
 	return output.String(), &slf.Secrets, nil
 }
 
-func (r *LoggingReconciler) clusterConfigurationSyslogNG(resources model.LoggingResources) (string, *secret.MountSecrets, error) {
+func (r *LoggingReconciler) clusterConfigurationSyslogNG(ctx context.Context, resources model.LoggingResources) (string, *secret.MountSecrets, error) {
 	if cfg := resources.Logging.Spec.FlowConfigOverride; cfg != "" {
 		return cfg, nil, nil
 	}
@@ -482,6 +482,7 @@ func (r *LoggingReconciler) clusterConfigurationSyslogNG(resources model.Logging
 		Flows:                resources.SyslogNG.Flows,
 		SecretLoaderFactory:  &slf,
 		SourcePort:           syslogng.ServicePort,
+		ClusterHasIPv6:       len(r.clusterIPFamilies(ctx, &resources.Logging, syslogngSpec.EnabledIPv6)) > 0,
 		SyslogNGSpec:         syslogngSpec,
 		SkipInvalidResources: resources.Logging.Spec.SkipInvalidResources,
 		Logger:               r.Log,

pkg/resources/fluentbit/daemonset.go

@@ -314,7 +314,8 @@ func (r *Reconciler) bufferMetricsSidecarContainer() *corev1.Container {
 		if r.fluentbitSpec.BufferVolumeMetrics.Port != 0 {
 			port = r.fluentbitSpec.BufferVolumeMetrics.Port
 		}
-		portParam := fmt.Sprintf("--web.listen-address=:%d", port)
+		// An unset bind keeps the wildcard address, which listens on both families.
+		portParam := fmt.Sprintf("--web.listen-address=%s:%d", r.fluentbitSpec.BufferVolumeMetrics.Bind, port)
 		args := []string{portParam}
 		if len(r.fluentbitSpec.BufferVolumeArgs) != 0 {
 			args = append(args, r.fluentbitSpec.BufferVolumeArgs...)

pkg/resources/fluentd/statefulset.go

@@ -411,7 +411,8 @@ func (r *Reconciler) bufferMetricsSidecarContainer() *corev1.Container {
 		if r.fluentdSpec.BufferVolumeMetrics.Port != 0 {
 			port = r.fluentdSpec.BufferVolumeMetrics.Port
 		}
-		portParam := fmt.Sprintf("--web.listen-address=:%d", port)
+		// An unset bind keeps the wildcard address, which listens on both families.
+		portParam := fmt.Sprintf("--web.listen-address=%s:%d", r.fluentdSpec.BufferVolumeMetrics.Bind, port)
 		args := []string{portParam}
 		if len(r.fluentdSpec.BufferVolumeArgs) != 0 {
 			args = append(args, r.fluentdSpec.BufferVolumeArgs...)

pkg/resources/ipfamily/detector.go

@@ -70,13 +70,15 @@ func (d *Detector) Families(ctx context.Context, namespace string) []corev1.IPFa
 		return nil
 	}
 
-	switch {
-	case hasIPv6 && hasIPv4:
-		d.families = []corev1.IPFamily{corev1.IPv6Protocol, corev1.IPv4Protocol}
-	case hasIPv6:
-		d.families = []corev1.IPFamily{corev1.IPv6Protocol}
-	default:
-		d.log.Info("cluster has no IPv6 service range, leaving the IP families to the cluster")
+	if !hasIPv6 {
+		// Re-probed every time, so a range added to a running cluster is picked up without a restart.
+		d.log.V(1).Info("cluster has no IPv6 service range, leaving the IP families to the cluster")
+		return nil
+	}
+
+	d.families = []corev1.IPFamily{corev1.IPv6Protocol}
+	if hasIPv4 {
+		d.families = append(d.families, corev1.IPv4Protocol)
 	}
 	d.resolved = true
 	return d.families

pkg/resources/ipfamily/detector_test.go

@@ -48,10 +48,10 @@ func TestFamilies(t *testing.T) {
 			resolved: true,
 		},
 		{
-			name:     "an IPv4-only cluster is left to choose for itself",
+			name:     "an IPv4-only cluster is left to choose for itself, and re-probed",
 			results:  map[corev1.IPFamily]error{corev1.IPv6Protocol: invalidFamilyErr(), corev1.IPv4Protocol: nil},
 			expected: nil,
-			resolved: true,
+			resolved: false,
 		},
 		{
 			name: "a webhook rejecting every service leaves the answer unknown",

pkg/resources/syslogng/statefulset.go

@@ -269,7 +269,8 @@ func (r *Reconciler) bufferMetricsSidecarContainer() *corev1.Container {
 		if r.syslogNGSpec.BufferVolumeMetrics.Port != 0 {
 			port = r.syslogNGSpec.BufferVolumeMetrics.Port
 		}
-		portParam := fmt.Sprintf("--web.listen-address=:%d", port)
+		// An unset bind keeps the wildcard address, which listens on both families.
+		portParam := fmt.Sprintf("--web.listen-address=%s:%d", r.syslogNGSpec.BufferVolumeMetrics.Bind, port)
 		args := []string{portParam, "--collector.disable-defaults", "--collector.filesystem", "--collector.textfile", "--collector.textfile.directory=/prometheus/node_exporter/textfile_collector/"}
 
 		nodeExporterCmd := fmt.Sprintf("nodeexporter -> ./bin/node_exporter %v", strings.Join(args, " "))

pkg/sdk/logging/model/syslogng/config/config.go

@@ -55,6 +55,7 @@ type Input struct {
 	Flows                []v1beta1.SyslogNGFlow
 	SecretLoaderFactory  SecretLoaderFactory
 	SourcePort           int
+	ClusterHasIPv6       bool
 	SkipInvalidResources bool
 	Logger               logr.Logger
 }
@@ -185,7 +186,9 @@ func configRenderer(in Input) (render.Renderer, error) {
 	// syslog-ng defaults to ip(0.0.0.0) ip-protocol(4), so an IPv6-primary Service would route to a
 	// port nothing is listening on. "::" still accepts IPv4-mapped clients where bindv6only is off.
 	sourceIP, sourceIPProtocol := "", 0
-	if in.SyslogNGSpec.EnabledIPv6 {
+	// Binding IPv6 only reaches IPv4 clients through v4-mapped addresses, which net.ipv6.bindv6only
+	// turns off. A cluster without IPv6 gives the source an IPv4 Service, so leave it on IPv4 there.
+	if in.SyslogNGSpec.EnabledIPv6 && in.ClusterHasIPv6 {
 		sourceIP, sourceIPProtocol = "::", 6
 	}
 

pkg/sdk/logging/model/syslogng/config/config_test.go

@@ -1202,10 +1202,26 @@ func TestRenderConfigIntoBindsTheSourceForIPv6(t *testing.T) {
 				Name:                "test",
 				Namespace:           "config-test",
 				SyslogNGSpec:        &v1beta1.SyslogNGSpec{EnabledIPv6: test.enabledIPv6},
+				ClusterHasIPv6:      true,
 				SecretLoaderFactory: &TestSecretLoaderFactory{},
 			}, &buf)
 			require.NoError(t, err)
 			require.Contains(t, buf.String(), test.wantSource)
 		})
 	}
 }
+
+// Binding IPv6 only reaches IPv4 clients through v4-mapped addresses, which bindv6only disables.
+func TestRenderConfigIntoKeepsIPv4WhenTheClusterHasNoIPv6(t *testing.T) {
+	var buf strings.Builder
+	err := RenderConfigInto(Input{
+		SourcePort:          601,
+		Name:                "test",
+		Namespace:           "config-test",
+		SyslogNGSpec:        &v1beta1.SyslogNGSpec{EnabledIPv6: true},
+		ClusterHasIPv6:      false,
+		SecretLoaderFactory: &TestSecretLoaderFactory{},
+	}, &buf)
+	require.NoError(t, err)
+	require.Contains(t, buf.String(), `network(flags("no-parse") port(601) transport("tcp"));`)
+}