Exposing dnsPolicy and dnsConfig on the Strimzi api PodTemplate to allow custom name resolution for Pod resources #10949

gertschouten · 2024-12-12T23:59:14Z

Type of change

Enhancement / new feature

Description

By appending Strimzi's common PodTemplate in the Strimzi api with additional properties dnsPolicy and dnsConfig the system configuration file /etc/recolv.conf for Strimzi Pod resources can be controlled, thereby deciding how domain name resolution should be performed on a resource level.
Strimzi's CRDs will thereby allow user input for name resolution customization.
WorkloadUtils in the Strimzi cluster operator for PodBuilder and PodTemplateSpecBuilder propagate these properties to Pods controlled by the operator.
Provisioned Pods will thereby utilize name resolution configuration based on the user input if specified and fall back to defaults, similar to the current situation, in case no dnsConfig or dnsPolicy is specified by the user.

Checklist

Please go through this checklist and make sure all applicable tasks have been done

Write tests
Make sure all tests pass
Update documentation
Check RBAC rights for Kubernetes / OpenShift roles
Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
Reference relevant issue(s) and close them after merging
Update CHANGELOG.md
Supply screenshots for visual changes, such as Grafana dashboards

scholzj · 2024-12-13T00:11:19Z

You should update the title and the description of the PR. You also seem to have some bad commits in there. The Pr has to be done against the main branch with the code from there. That might be also what broker the DCO signoff.

scholzj

We in general deal with the PR only once the proposal is approved. But I left one comment. No need to address it now, you can do it only once the proposal is approved. It would be also great to fix all the unnecessary whitespace changes that make the reviews harder, but will also mess up the Git history etc. (e.g. in the pom.xml files, the values.yaml, etc.)

You will also need to run make crd_install to get the CRD changes copied out to the Helm Chart as well. And you will need to fix the DCO signoff. The instructions should be under the Details link next to the DCO signoff status in the PR.

scholzj · 2024-12-13T10:03:20Z

api/src/main/java/io/strimzi/api/kafka/model/common/template/PodTemplate.java

@@ -53,6 +54,8 @@ public class PodTemplate implements HasMetadataTemplate, UnknownPropertyPreservi
    private String priorityClassName;
    private String schedulerName;
    private List<HostAlias> hostAliases;
+    private PodDNSConfig dnsConfig;
+    private String dnsPolicy;


I think this should have an Enum for the supported values. Check for example how

strimzi-kafka-operator/api/src/main/java/io/strimzi/api/kafka/model/common/template/ExternalTrafficPolicy.java

Line 12 in 0c334d8

public enum ExternalTrafficPolicy {

is used.

Addressed in 9bd12a2

scholzj

I left some more nits. Also, there have been some changes to the main branch in between - could oyu please rebase it to address the conflicts? Thanks.

This might be also worth a record in the CHANGELOG.md file?

cluster-operator/src/main/java/io/strimzi/operator/cluster/model/WorkloadUtils.java

scholzj · 2024-12-20T16:35:00Z

cluster-operator/src/main/java/io/strimzi/operator/cluster/model/WorkloadUtils.java

@@ -339,6 +341,8 @@ public static PodTemplateSpec createPodTemplateSpec(
                    .withPriorityClassName(template != null ? template.getPriorityClassName() : null)
                    .withSchedulerName(template != null && template.getSchedulerName() != null ? template.getSchedulerName() : "default-scheduler")
                    .withHostAliases(template != null ? template.getHostAliases() : null)
+                    .withDnsPolicy(template != null ? template.getDnsPolicy() != null ? template.getDnsPolicy().toValue() : null : null)


Same as above.

Addressed in fffc5db

scholzj · 2024-12-20T16:35:11Z

cluster-operator/src/main/java/io/strimzi/operator/cluster/model/WorkloadUtils.java

@@ -402,6 +406,8 @@ public static Pod createPod(
                    .withPriorityClassName(template != null ? template.getPriorityClassName() : null)
                    .withSchedulerName(template != null && template.getSchedulerName() != null ? template.getSchedulerName() : "default-scheduler")
                    .withHostAliases(template != null ? template.getHostAliases() : null)
+                    .withDnsPolicy(template != null ? template.getDnsPolicy() != null ? template.getDnsPolicy().toValue() : null : null)


Same as above.

Addressed in fffc5db

Signed-off-by: gertschouten <[email protected]>

…el/WorkloadUtils.java Co-authored-by: Jakub Scholz <[email protected]> Signed-off-by: gertschouten <[email protected]>

Signed-off-by: gertschouten <[email protected]>

…using the template sections Signed-off-by: gertschouten <[email protected]>

scholzj · 2024-12-22T00:47:31Z

/azp run regression

azure-pipelines · 2024-12-22T00:47:43Z

Azure Pipelines successfully started running 1 pipeline(s).

scholzj

LGTM. Thanks for the PR.

ppatierno

LGTM, thanks for the PR! I left a couple of nits so when they are fixed I guess we can merge this.

api/src/main/java/io/strimzi/api/kafka/model/common/template/DnsPolicy.java

api/src/main/java/io/strimzi/api/kafka/model/common/template/PodTemplate.java

cluster-operator/src/main/java/io/strimzi/operator/cluster/model/WorkloadUtils.java

Signed-off-by: gertschouten <[email protected]>

ppatierno · 2024-12-23T10:49:08Z

/azp run regression

azure-pipelines · 2024-12-23T10:49:19Z

Azure Pipelines successfully started running 1 pipeline(s).

gertschouten mentioned this pull request Dec 13, 2024

[Enhancement]: Nameserver #10835

Closed

scholzj linked an issue Dec 13, 2024 that may be closed by this pull request

[Enhancement]: Nameserver #10835

Closed

gertschouten force-pushed the issue-10835 branch 2 times, most recently from b9e2cb2 to dddc11d Compare December 13, 2024 09:17

gertschouten changed the title ~~Issue 10835~~ Exposing dnsPolicy and dnsConfig on the Strimzi api PodTemplate to allow custom name resolution for Pod resources Dec 13, 2024

scholzj reviewed Dec 13, 2024

View reviewed changes

gertschouten force-pushed the issue-10835 branch from dddc11d to 00fc892 Compare December 14, 2024 18:12

scholzj added this to the 0.46.0 milestone Dec 15, 2024

scholzj reviewed Dec 20, 2024

View reviewed changes

gertschouten and others added 6 commits December 21, 2024 20:18

expose dnsConfig and dnsPolicy

70e601f

Signed-off-by: gertschouten <[email protected]>

updated helm

cb78558

Signed-off-by: gertschouten <[email protected]>

dnsPolicy as enum

204c9a4

Signed-off-by: gertschouten <[email protected]>

Update cluster-operator/src/main/java/io/strimzi/operator/cluster/mod…

05c44b7

…el/WorkloadUtils.java Co-authored-by: Jakub Scholz <[email protected]> Signed-off-by: gertschouten <[email protected]>

Simplify conditional dnsPolicy in builders

fffc5db

Signed-off-by: gertschouten <[email protected]>

updated changelog to reflect the support for dnsConfig and dnsPolicy …

3e09203

…using the template sections Signed-off-by: gertschouten <[email protected]>

gertschouten force-pushed the issue-10835 branch from 0346098 to 3e09203 Compare December 21, 2024 19:53

scholzj approved these changes Dec 22, 2024

View reviewed changes

scholzj requested a review from ppatierno December 22, 2024 10:40

ppatierno approved these changes Dec 23, 2024

View reviewed changes

gertschouten added 3 commits December 23, 2024 11:00

Reformulate dnsConfig description

7180de1

Signed-off-by: gertschouten <[email protected]>

Snake case for DnsPolicy enumeration

99b2341

Signed-off-by: gertschouten <[email protected]>

newline WorkloadUtils

ae6756d

Signed-off-by: gertschouten <[email protected]>

scholzj merged commit 039c911 into strimzi:main Dec 23, 2024
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exposing dnsPolicy and dnsConfig on the Strimzi api PodTemplate to allow custom name resolution for Pod resources #10949

Exposing dnsPolicy and dnsConfig on the Strimzi api PodTemplate to allow custom name resolution for Pod resources #10949

gertschouten commented Dec 12, 2024 •

edited

Loading

scholzj commented Dec 13, 2024

scholzj left a comment

scholzj Dec 13, 2024

gertschouten Dec 15, 2024

scholzj left a comment

scholzj Dec 20, 2024

gertschouten Dec 21, 2024

scholzj Dec 20, 2024

gertschouten Dec 21, 2024

scholzj commented Dec 22, 2024

azure-pipelines bot commented Dec 22, 2024

scholzj left a comment

ppatierno left a comment

ppatierno commented Dec 23, 2024

azure-pipelines bot commented Dec 23, 2024

Exposing dnsPolicy and dnsConfig on the Strimzi api PodTemplate to allow custom name resolution for Pod resources #10949

Exposing dnsPolicy and dnsConfig on the Strimzi api PodTemplate to allow custom name resolution for Pod resources #10949

Conversation

gertschouten commented Dec 12, 2024 • edited Loading

Type of change

Description

Checklist

scholzj commented Dec 13, 2024

scholzj left a comment

Choose a reason for hiding this comment

scholzj Dec 13, 2024

Choose a reason for hiding this comment

gertschouten Dec 15, 2024

Choose a reason for hiding this comment

scholzj left a comment

Choose a reason for hiding this comment

scholzj Dec 20, 2024

Choose a reason for hiding this comment

gertschouten Dec 21, 2024

Choose a reason for hiding this comment

scholzj Dec 20, 2024

Choose a reason for hiding this comment

gertschouten Dec 21, 2024

Choose a reason for hiding this comment

scholzj commented Dec 22, 2024

azure-pipelines bot commented Dec 22, 2024

scholzj left a comment

Choose a reason for hiding this comment

ppatierno left a comment

Choose a reason for hiding this comment

ppatierno commented Dec 23, 2024

azure-pipelines bot commented Dec 23, 2024

gertschouten commented Dec 12, 2024 •

edited

Loading