Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Default cluster dns-domain to empty string in e2e_node suite #98025

Merged
merged 1 commit into from Jan 16, 2021
Merged

Default cluster dns-domain to empty string in e2e_node suite #98025

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/e2e_node/e2e_node_suite_test.go
View file
Open in desktop
Expand Up @@ -84,7 +84,7 @@ func registerNodeFlags(flags *flag.FlagSet) {
flags.StringVar(&framework.TestContext.SystemSpecName, "system-spec-name", "", "The name of the system spec (e.g., gke) that's used in the node e2e test. The system specs are in test/e2e_node/system/specs/. This is used by the test framework to determine which tests to run for validating the system requirements.")
flags.Var(cliflag.NewMapStringString(&framework.TestContext.ExtraEnvs), "extra-envs", "The extra environment variables needed for node e2e tests. Format: a list of key=value pairs, e.g., env1=val1,env2=val2")
flags.StringVar(&framework.TestContext.SriovdpConfigMapFile, "sriovdp-configmap-file", "", "The name of the SRIOV device plugin Config Map to load.")
flag.StringVar(&framework.TestContext.ClusterDNSDomain, "dns-domain", "cluster.local", "The DNS Domain of the cluster.")
flag.StringVar(&framework.TestContext.ClusterDNSDomain, "dns-domain", "", "The DNS Domain of the cluster.")
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for doing this.
I see the test failures like https://prow.k8s.io/view/gcs/kubernetes-jenkins/logs/ci-kubernetes-node-kubelet-features/1349429706050506752 as an example.
One of failed tests sets hostFQDN like

kubernetes/test/e2e_node/pod_hostnamefqdn_test.go

Line 117 in 5c58b22

hostFQDN := fmt.Sprintf("%s.%s.%s.svc.%s", pod.ObjectMeta.Name, subdomain, f.Namespace.Name, framework.TestContext.ClusterDNSDomain)

If framework.TestContext.ClusterDNSDomain becomes empty like this PR, the hostFQDN becomes like foo-pod.foo-subdomain.foo-ns.svc. without cluster.local.
The FQDN is different from common cases I feel like https://kubernetes.io/docs/tasks/administer-cluster/dns-debugging-resolution/
Can we enablecluster.local instead?

/cc @oomichi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oomichi we could, although I am hesitant to require that for these tests to run successfully as that is not the default for the kubelet. If we were to set to cluster.local we would do so here: https://github.com/kubernetes/test-infra/blob/c094f147c10f9b667472e81a5b36e12a61446e37/config/jobs/kubernetes/sig-node/node-kubelet.yaml#L127

Copy link
Member

Choose a reason for hiding this comment

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

I see, thanks for your explanation.
I guess it is ideal to change the default value to "cluster.local" on kubelet side at the end of day, but current change is fine for me to solve test failure at this time.

/triage accepted
/lgtm

}

func init() {
Expand Down