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

Handle non-standard coredns ConfigMap name in SD controller #3295

Merged
merged 4 commits into from
Dec 9, 2024

Conversation

tpantelis
Copy link
Contributor

@tpantelis tpantelis commented Dec 5, 2024

Includes a couple preliminary commits to refactor constants and add additional unit test.

Fixes submariner-io/lighthouse#1602

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
@submariner-bot
Copy link
Contributor

🤖 Created branch: z_pr3295/tpantelis/coredns_cm
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

@tpantelis tpantelis requested a review from aswinsuryan December 5, 2024 14:49
Fixes submariner-io/lighthouse#1602

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>
cm := &configMaps.Items[i]

_, hasCorefile := cm.Data[Corefile]
if strings.HasSuffix(cm.Name, suffix) && hasCorefile {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on this comment, we could we just check for the existence of the Corefile data entry. @vthapar @aswinsuryan WDYT?

Copy link
Contributor

@aswinsuryan aswinsuryan Dec 6, 2024

Choose a reason for hiding this comment

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

this looks good , since it is listing all the configmaps in DefaultCoreDNSNamespace , we should be safe as there should be only one coredns corefile normally in this namespace.

@submariner-bot submariner-bot added the ready-to-test When a PR is ready for full E2E testing label Dec 6, 2024
@tpantelis tpantelis enabled auto-merge (rebase) December 7, 2024 01:46
@tpantelis tpantelis added the release-note-needed Should be mentioned in the release notes label Dec 7, 2024
@tpantelis tpantelis disabled auto-merge December 9, 2024 12:32
@tpantelis tpantelis merged commit e8d13fd into submariner-io:devel Dec 9, 2024
46 checks passed
@submariner-bot
Copy link
Contributor

🤖 Closed branches: [z_pr3295/tpantelis/coredns_cm]

@tpantelis tpantelis deleted the coredns_cm branch December 10, 2024 15:55
tpantelis added a commit to tpantelis/submariner-website that referenced this pull request Dec 11, 2024
skitt pushed a commit to submariner-io/submariner-website that referenced this pull request Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-test When a PR is ready for full E2E testing release-note-handled release-note-needed Should be mentioned in the release notes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Submariner does not add LightHouse DNS entry in corefile section of configmap in case of RKE2 cluster.
4 participants