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

Allow virtual-kubelet to use cluster domain #8

Merged
merged 1 commit into from
Aug 10, 2020

Conversation

grahamhayes
Copy link
Contributor

This allows --cluster-domain to be passed to virtual kubelet like a
traditional kublet, and use this to generate search-domains for
/etc/resolv.conf

  • Only apply to pods with DNSClusterFirst to match kubelet
  • Merge search-domains with any set in the dnsConfig
  • Set ndots to the default 5

Related: #641

Depends-On: virtual-kubelet/virtual-kubelet#642

Signed-off-by: Graham Hayes [email protected]

@grahamhayes grahamhayes changed the title Allow virtual-kubelet to use cluster domain WIP:Allow virtual-kubelet to use cluster domain Jun 19, 2019
@grahamhayes
Copy link
Contributor Author

Marking as WIP, as virtual-kubelet/virtual-kubelet#642 needs to merge first, and then update the go.mod file to the commit with the new flag.

@grahamhayes grahamhayes force-pushed the use-cluster-dns branch 2 times, most recently from 002ee23 to 9567a9a Compare June 20, 2019 11:06
@grahamhayes grahamhayes changed the title WIP:Allow virtual-kubelet to use cluster domain Allow virtual-kubelet to use cluster domain Jun 20, 2019
@srrengar srrengar requested a review from ibabou August 21, 2019 23:35
@grahamhayes grahamhayes force-pushed the use-cluster-dns branch 2 times, most recently from 10ed07d to 7c7f076 Compare August 26, 2019 11:33
@grahamhayes
Copy link
Contributor Author

@srrengar / @ibabou This commit is good to go, the dependency in vk/vk merged, and the go.mod was updated to add in the flag.

If you have any questions, ping me (here, or on chat) and I can walk through the use case and reproduction steps.

Copy link
Contributor

@ibabou ibabou left a comment

Choose a reason for hiding this comment

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

The change looks fine. Thanks!

aci.go Outdated

containerGroup.ContainerGroupProperties.Extensions = []*aci.Extension{p.kubeProxyExtension}
containerGroup.ContainerGroupProperties.DNSConfig = p.getDNSConfig(pod.Spec.DNSPolicy, pod.Spec.DNSConfig)
containerGroup.ContainerGroupProperties.DNSConfig = p.getDNSConfig(pod)
Copy link
Contributor

Choose a reason for hiding this comment

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

typo, a duplicate

@cpuguy83
Copy link
Contributor

cpuguy83 commented Sep 3, 2019

Sorry this needs a rebase now that we've updated to vk 1.0

@grahamhayes
Copy link
Contributor Author

@cpuguy83 / @ibabou should I keep rebasing this? this new rebase looks like it will take a bit more time (#13 changed the flow for some files / calls) , and I will need to retest - it is a non trivial time investment. It seems like something a customer using vk will run into (it was one of the first things we ran into), but has been open for an extended period.

@cpuguy83
Copy link
Contributor

cpuguy83 commented Sep 4, 2019

@grahamhayes Looks like it's good to go as soon as there's a rebase. Originally just been waiting for someone from ACI to review which is now ok :)

@mgsnuno
Copy link

mgsnuno commented Jul 23, 2020

As described in #46 and since everything seems to be ready to merge, when can we close and have a new release with this? Thank you

@mgsnuno
Copy link

mgsnuno commented Jul 29, 2020

@grahamhayes @cpuguy83 when can we merge this and have a new release? it would be much appreciated. thank you

@mgsnuno
Copy link

mgsnuno commented Aug 7, 2020

sorry to keep insisting but I've been having random timeouts due to dns issues with virtual-kubelet (while explicitly setting dnsConfig), while with aks with VMs I never encountered those. And if this gets released then I can test if it got improved.

Any timeline on this? Thank you.

This allows `--cluster-domain` to be passed to virtual kubelet like a
traditional kublet, and use this to generate search-domains for
`/etc/resolv.conf`

* Only apply to pods with `DNSClusterFirst` to match kubelet
* Merge search-domains with any set in the `dnsConfig`
* Set `ndots` to the default 5

Related: virtual-kubelet#641

Depends-On: virtual-kubelet/virtual-kubelet#642

Signed-off-by: Graham Hayes <[email protected]>
@cpuguy83 cpuguy83 merged commit 7cf0e9e into virtual-kubelet:master Aug 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants