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

Consider newer topology labels in zone controller #11

Merged
merged 1 commit into from
Sep 27, 2020

Conversation

stoyanr
Copy link
Contributor

@stoyanr stoyanr commented Sep 24, 2020

This PR adds code to consider the newer topology labels in the zone controller, in addition to the older failure-domain ones. According to https://kubernetes.io/docs/reference/kubernetes-api/labels-annotations-taints/, starting in v1.17, the failure-domain.beta.kubernetes.io/region label is deprecated in favor of topology.kubernetes.io/region, and the failure-domain.beta.kubernetes.io/zone label is deprecated in favor of topology.kubernetes.io/zone.

Since the cloud provider still vendors Kubernetes 1.16, the newer labels of the underkube nodes will be considered when determining the region and zone, but not actually added to the overkube nodes (as this is done by the vendored code). Vendoring a newer Kubernetes version is unfortunately close to impossible since kubevirt.io/client-go is still based on 1.16 and there are conflicts, see also #10.

@kubevirt-bot kubevirt-bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. size/XS labels Sep 24, 2020
@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. and removed dco-signoff: no Indicates the PR's author has not DCO signed all their commits. labels Sep 24, 2020
@stoyanr
Copy link
Contributor Author

stoyanr commented Sep 24, 2020

/cc @afritzler @gonzolino

@kubevirt-bot
Copy link
Contributor

@stoyanr: GitHub didn't allow me to request PR reviews from the following users: afritzler.

Note that only kubevirt members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @afritzler @gonzolino

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@stoyanr
Copy link
Contributor Author

stoyanr commented Sep 24, 2020

@gonzolino The Travis build fails with:

go: error loading module requirements
Makefile:25: recipe for target 'clean' failed

I don't think this failure has anything to do with my changes. I would like to retrigger the build, but I don't seem to be able to do this from Travis. Is there a way?

@stoyanr
Copy link
Contributor Author

stoyanr commented Sep 24, 2020

/retest

Copy link
Contributor

@gonzolino gonzolino left a comment

Choose a reason for hiding this comment

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

I couldn't reproduce the travis error on my local machine...

Please adress the comment from teh review, then we can trigger a retest. It's probably just some error on travis.

pkg/cloudprovider/kubevirt/zones.go Show resolved Hide resolved
pkg/cloudprovider/kubevirt/zones.go Show resolved Hide resolved
Signed-off-by: Stoyan Rachev <s.rachev@sap.com>
@gonzolino gonzolino merged commit 3125575 into kubevirt:master Sep 27, 2020
@stoyanr stoyanr deleted the add-topology-labels branch September 28, 2020 07:01
nirarg referenced this pull request in nirarg/cloud-provider-kubevirt Aug 31, 2022
Add "io.openshift.release.operator" label to the Dockerfile
@kubevirt-bot kubevirt-bot mentioned this pull request Feb 6, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants