This repository was archived by the owner on Sep 7, 2022. It is now read-only.
Provision vsphere volumes as per zone and attach labels #517
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| } | ||
|
|
||
| func (nm *NodeManager) UnRegisterNode(node *v1.Node) error { | ||
| klog.V(4).Infof("UnRegister node called: %v", *node) |
| } | ||
|
|
||
| func (nm *NodeManager) RediscoverNode(nodeName k8stypes.NodeName) error { | ||
| klog.V(4).Infof("Rediscover node called: %v", nodeName) |
| } | ||
| } | ||
| if !found { | ||
| msg := fmt.Sprintf("The specified datastore %s does not match the provided availability zones : %s", volumeOptions.Datastore, volumeOptions.Zone) |
There was a problem hiding this comment.
You could use fmt.Errorf(...) instead.
|
|
||
| // Create cloudprovider.Zone type from selectedZones | ||
| var cpZones []cloudprovider.Zone | ||
| for _, selectedZone := range selectedZones { |
There was a problem hiding this comment.
I think duplicates should be handled here. Not a functionality problem, but for a better performance.
There was a problem hiding this comment.
Duplicate validation is already taken care as part of SC creation. Hence, need not be handled here.
|
Addressed Subbu's comments. |
skarthiksrinivas
force-pushed
the
vsphere_volume_zone
branch
from
6723a1e to
5b53c5d
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind bug
What this PR does / why we need it:
Currently vsphere cloud provider (VCP) insists on provisioning a volume only on a globally shared datastore. Hence, in a zoned environment, even in presence of a shared datastore within a specific zone, the volume provisioning can fail if that datastore is not shared across all the zones hosting kubernetes nodes. This change fixes this issue by considering the zone information provided in allowedTopologies for selection of the datastore. If allowedTopologies is not provided, the current behaviour is retained as-is.
Further, after a volume is created on vSphere, the zone label does not get applied on it today. Without this information on the volume, a pod that uses this volume could get scheduled to a different zone where this volume is not visible/accessible. This issue can be solved by just labeling the volume with the zone information. This PR computes the zone of a volume in the PersistentVolumeLabel admission controller since vsphere is still an in-tree cloud provider and the out-of-tree plugin in still the works.
Which issue(s) this PR fixes:
Fixes kubernetes#67703
Does this PR introduce a user-facing change?:
Yes