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

Jump to bottom

Cloud providers should report node hostname address types #67714

Closed
6 tasks done
liggitt opened this issue Aug 22, 2018 · 8 comments · Fixed by #68017
Closed
6 tasks done

Cloud providers should report node hostname address types #67714

liggitt opened this issue Aug 22, 2018 · 8 comments · Fixed by #68017
Assignees
@liggitt
Labels
area/provider/openstack Issues or PRs related to openstack provider sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider.
Milestone
v1.12

Comments

@liggitt
Copy link
Member

liggitt commented Aug 22, 2018 •
edited

7828e5d made cloud providers authoritative for the addresses reported on Node objects, so that the addresses used by the node (and requested as SANs in serving certs) could be verified via cloud provider metadata.

This had the effect of no longer reporting addresses of type Hostname for Node objects for some cloud providers. Cloud providers that have the instance hostname available in metadata should add a type: Hostname address to node status.

This issue tracks the cloud providers updated to do so:
@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Aug 22, 2018
@liggitt liggitt self-assigned this Aug 22, 2018
@liggitt liggitt added the sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. label Aug 22, 2018
@k8s-ci-robot k8s-ci-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Aug 22, 2018
liggitt referenced this issue Aug 22, 2018
@liggitt
Make node status addresses authoritative for kube-apiserver -> kubele…
7af8c6a 
…t connections
@liggitt
Copy link
Member Author

liggitt commented Aug 22, 2018

cc @mikedanese @awly

@dims
Copy link
Member

dims commented Aug 22, 2018

cc @FengyunPan2 @dixudx @hogepodge @dklyle (for openstack)
/sig openstack

@k8s-ci-robot k8s-ci-robot added the area/provider/openstack Issues or PRs related to openstack provider label Aug 22, 2018
@liggitt liggitt mentioned this issue Aug 23, 2018
Merged
@FengyunPan2
Copy link
Contributor

Get it, I will report it for openstack cloud provider.

@FengyunPan2 FengyunPan2 mentioned this issue Aug 23, 2018
Merged
k8s-github-robot pushed a commit that referenced this issue Aug 23, 2018
Merge pull request #67748 from FengyunPan2/report-hostname
d68e1fc 
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Make OpenStack cloud provider report a node hostname address

Related to: #67714
Cloud-provider-reported addresses are authoritative in 1.12, this
preserves default behavior that used the internal dns name as
the "Hostname" address on Node status.

**Release note**:

```release-note
The OpenStack cloud provider now reports a `Hostname` address type for nodes
```
@tpepper
Copy link
Member

tpepper commented Aug 23, 2018

There have been a number of persistent failures in test grid for a few weeks (eg: https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/logs/ci-kubernetes-e2e-gke-gci-new-gci-master-upgrade-master/1423) where the failure signature is:

Error from server: error dialing backend: x509: cannot validate certificate for 10.128.0.2 because it doesn't contain any IP SANs

Related?

@liggitt
Copy link
Member Author

liggitt commented Aug 23, 2018 •
edited

Related?

No, those likely are set up with the kubelet self-signing its serving certificate. The symptom of a cluster affected by this would be the apiserver unable to even reach the kubelet

@mikedanese
Copy link
Member

I think that error is from CRI streaming. From @Random-Liu :

Default config of containerd 1.1 doesn't work with Kubernetes 1.11+. We'll build another cos_containerd image for Kubernetes 1.11.

The problem is that:

  1. In containerd 1.1, we enabled TLS on GCE for security.
  2. In Kubernetes 1.11, we added the kubelet streaming proxy, and use it by default.
  3. Kubelet streaming proxy doesn't use TLS, so it will get x509: certificate signed by unknown authority when talking with containerd 1.1.
  4. In coming containerd 1.2, we've disabled TLS to work with Kubernetes 1.11.

@Random-Liu
Copy link
Member

@mikedanese Those nodes are not running containerd. I think it should be a different issue. :)

@liggitt
Copy link
Member Author

liggitt commented Aug 27, 2018

/milestone v1.12

@k8s-ci-robot k8s-ci-robot added this to the v1.12 milestone Aug 27, 2018
@liggitt liggitt mentioned this issue Aug 28, 2018
Merged
cdkbot-zz pushed a commit to juju-solutions/kubernetes that referenced this issue Aug 28, 2018
Merge pull request kubernetes#67739 from liggitt/hostname-override
a26e1dd 
Automatic merge from submit-queue (batch tested with PRs 67739, 65222). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Honor --hostname-override, report compatible hostname addresses with cloud provider

xref kubernetes#67714

7828e5d made cloud providers authoritative for the addresses reported on Node objects, so that the addresses used by the node (and requested as SANs in serving certs) could be verified via cloud provider metadata.

This had the effect of no longer reporting addresses of type Hostname for Node objects for some cloud providers. Cloud providers that have the instance hostname available in metadata should add a `type: Hostname` address to node status. This is being tracked in kubernetes#67714

This PR does a couple other things to ease the transition to authoritative cloud providers:
* if `--hostname-override` is set on the kubelet, make the kubelet report that `Hostname` address. if it can't be verified via cloud-provider metadata (for cert approval, etc), the kubelet deployer is responsible for fixing the situation by adjusting the kubelet configuration (as they were in 1.11 and previously)
* if `--hostname-override` is not set, *and* the cloud provider didn't report a Hostname address, *and* the auto-detected hostname matches one of the addresses the cloud provider *did* report, make the kubelet report that as a Hostname address. That lets the addresses remain verifiable via cloud provider metadata, while still including a `Hostname` address whenever possible.

/sig node
/sig cloud-provider

/cc @mikedanese

fyi @hh

```release-note
NONE
```
k8s-github-robot pushed a commit that referenced this issue Aug 29, 2018
Merge pull request #67922 from liggitt/vsphere-hostname
1138727 
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md.

Report Hostname node address for vsphere

xref #67714

```release-note
NONE
```
@figo figo mentioned this issue Aug 29, 2018
Merged
@liggitt liggitt closed this as completed Sep 2, 2018
k8s-github-robot pushed a commit that referenced this issue Sep 2, 2018
Merge pull request #68017 from figo/node-status
1ad28be 
Automatic merge from submit-queue (batch tested with PRs 65566, 67959, 68029, 68017, 67263). If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md.

add test to verify vsphere cloud provider report node hostname

**What this PR does / why we need it**:
as in pull #67922 has modify vsphere cloud provider to report
node hostname, this patch is to add the test for it.

also fix an issue at InstanceID(), it suppose to return
cloudprovider.InstanceNotFound when vm not found,  
but it did double error type translation with a retry() logic,  
after removing the retry() (which is not necessary), the logic is simpler, and easier to understand how double translation happened, after the fix,
test TestInstance() can pass


**Which issue(s) this PR fixes** :
Fixes #67714


**Special notes for your reviewer**:
as in the process of create pull to fix crosscloudci/crosscloudci#65,
i saw #67922 already merged, so this pull is just to submit the test part.

**Release note**:

```release-note
none
```


/cc @abrarshivani @BaluDontu @divyenpatel @imkin @kerneltime @luomiao @frapposelli @dougm @liggitt
/sig cloudprovider
@figo figo mentioned this issue Sep 11, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@liggitt liggitt

Labels
area/provider/openstack Issues or PRs related to openstack provider sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider.
Projects
None yet
Milestone
v1.12
Development

Successfully merging a pull request may close this issue.

add test to verify vsphere cloud provider report node hostname figo/kubernetes
7 participants
@dims @liggitt @mikedanese @tpepper @Random-Liu @k8s-ci-robot @FengyunPan2