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.

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

Have kops-controller create headless k8s services for dns=none clusters #16335

Closed
wants to merge 3 commits into from

Conversation

rifelpet
Copy link
Member

Fixes #16332

Like gossip clusters, dns=none clusters wont be able to resolve these DNS names which causes e2e test failures for OIDC tests. This ensures the OIDC test pods can resolve the DNS names.

Example failure: https://testgrid.k8s.io/kops-grid#kops-grid-calico-amzn2-k26

I0204 15:43:18.460415 1 log.go:198] Get "https://api.internal.e2e-e2e-kops-grid-calico-amzn2-k26.test-cncf-aws.k8s.io/.well-known/openid-configuration": dial tcp: lookup api.internal.e2e-e2e-kops-grid-calico-amzn2-k26.test-cncf-aws.k8s.io on 100.64.0.10:53: no such host

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 10, 2024
@k8s-ci-robot k8s-ci-robot requested a review from hakman February 10, 2024 04:40
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from rifelpet. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot requested a review from zetaab February 10, 2024 04:40
@rifelpet rifelpet changed the title Rename kops-controller functions to cover non-gossip clusters @rifelpet Have kops-controller create headless k8s services for dns=none clusters Feb 10, 2024
@k8s-ci-robot k8s-ci-robot added the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Feb 10, 2024
@rifelpet rifelpet changed the title @rifelpet Have kops-controller create headless k8s services for dns=none clusters Have kops-controller create headless k8s services for dns=none clusters Feb 10, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Feb 10, 2024
@rifelpet
Copy link
Member Author

/retest

Like gossip clusters, dns=none clusters wont be able to resolve these DNS names which causes e2e test failures for OIDC tests. This ensures the e2e test pods can resolve these DNS names
@rifelpet
Copy link
Member Author

/retest

@rifelpet
Copy link
Member Author

/cc @hakman @justinsb

All of our "kops latest" grid jobs are failing because of this OIDC test
https://testgrid.k8s.io/kops-grid#kops-grid-calico-u2004-k28

@hakman
Copy link
Member

hakman commented Feb 11, 2024

@rifelpet I believe the fix for the issue is more on the line of changing this condition to include dns=none, instead of the headless service. What do you think?

{{- if GossipName }}
hosts /rootfs/etc/hosts k8s.local {
ttl 30
fallthrough
}
{{- end }}

@hakman
Copy link
Member

hakman commented Feb 11, 2024

@rifelpet Is this still needed?

@rifelpet
Copy link
Member Author

@rifelpet Is this still needed?

To be honest, I'm not sure. This comment #12792 (comment) suggests that this was meant to replace the /etc/hosts mount, but we seem to have both in place. Ideally we can figure out if these headless services are used and remove them if not. If we do want to keep them around, I'm inclined to keep their behavior in sync with the /etc/hosts strategy which would mean merging this.

Perhaps a good topic for office hours.
/kind office-hours

@rifelpet
Copy link
Member Author

in office hours we decided this isn't needed. we will remove the kops-controller headless services at the same time that we eventually remove gossip support

@rifelpet rifelpet closed this Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/addons area/kops-controller cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/office-hours size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DNS None clusters fails OIDC e2e test
3 participants