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.

Sign up for GitHub

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

Fix vSphere Cloud Provider to handle upgrade from k8s version less than v1.9.4 to v1.9.4+ #62919

Merged
merged 1 commit into from Apr 23, 2018
Merged

Fix vSphere Cloud Provider to handle upgrade from k8s version less than v1.9.4 to v1.9.4+ #62919

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/cloudprovider/providers/vsphere/BUILD
View file
Open in desktop
Expand Up @@ -20,6 +20,7 @@ go_library(
"//pkg/cloudprovider/providers/vsphere/vclib:go_default_library",
"//pkg/cloudprovider/providers/vsphere/vclib/diskmanagers:go_default_library",
"//pkg/controller:go_default_library",
"//pkg/util/version:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
"//vendor/github.com/vmware/govmomi/vim25:go_default_library",
"//vendor/github.com/vmware/govmomi/vim25/mo:go_default_library",
Expand Down
6 changes: 5 additions & 1 deletion pkg/cloudprovider/providers/vsphere/nodemanager.go
View file
Open in desktop
Expand Up @@ -76,7 +76,11 @@ func (nm *NodeManager) DiscoverNode(node *v1.Node) error {
var globalErr *error

queueChannel = make(chan *VmSearch, QUEUE_SIZE)
nodeUUID := GetUUIDFromProviderID(node.Spec.ProviderID)
nodeUUID, err := GetNodeUUID(node)
if err != nil {
glog.Errorf("Node Discovery failed to get node uuid for node %s with error: %v", node.Name, err)
return err
}

glog.V(4).Infof("Discovering node %s with uuid %s", node.ObjectMeta.Name, nodeUUID)

Expand Down
3 changes: 2 additions & 1 deletion pkg/cloudprovider/providers/vsphere/vsphere.go
View file
Open in desktop
Expand Up @@ -587,7 +587,8 @@ func (vs *VSphere) InstanceExistsByProviderID(ctx context.Context, providerID st
return false, err
}
for _, node := range nodes {
if node.VMUUID == GetUUIDFromProviderID(providerID) {
// ProviderID is UUID for nodes v1.9.3+
if node.VMUUID == GetUUIDFromProviderID(providerID) || node.NodeName == providerID {
nodeName = node.NodeName
break
}
Expand Down
41 changes: 38 additions & 3 deletions pkg/cloudprovider/providers/vsphere/vsphere_util.go
View file
Open in desktop
Expand Up @@ -29,13 +29,14 @@ import (

"fmt"

"path/filepath"

"github.com/vmware/govmomi/vim25/mo"
"io/ioutil"
"k8s.io/api/core/v1"
k8stypes "k8s.io/apimachinery/pkg/types"
"k8s.io/kubernetes/pkg/cloudprovider/providers/vsphere/vclib"
"k8s.io/kubernetes/pkg/cloudprovider/providers/vsphere/vclib/diskmanagers"
"k8s.io/kubernetes/pkg/util/version"
"path/filepath"
)

const (
Expand Down Expand Up @@ -472,7 +473,12 @@ func (vs *VSphere) checkDiskAttached(ctx context.Context, nodes []k8stypes.NodeN
if err != nil {
return nodesToRetry, err
}
nodeUUID := strings.ToLower(GetUUIDFromProviderID(node.Spec.ProviderID))
nodeUUID, err := GetNodeUUID(&node)
if err != nil {
glog.Errorf("Node Discovery failed to get node uuid for node %s with error: %v", node.Name, err)
return nodesToRetry, err
}
nodeUUID = strings.ToLower(nodeUUID)
glog.V(9).Infof("Verifying volume for node %s with nodeuuid %q: %s", nodeName, nodeUUID, vmMoMap)
vclib.VerifyVolumePathsForVM(vmMoMap[nodeUUID], nodeVolumes[nodeName], convertToString(nodeName), attached)
}
Expand Down Expand Up @@ -542,3 +548,32 @@ func GetVMUUID() (string, error) {
func GetUUIDFromProviderID(providerID string) string {
return strings.TrimPrefix(providerID, ProviderPrefix)
}

func IsUUIDSupportedNode(node *v1.Node) (bool, error) {
newVersion, err := version.ParseSemantic("v1.9.4")
Copy link
Member

Choose a reason for hiding this comment

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

@divyenpatel @abrarshivani This change seems problematic, could we have not done this in a different way? Such as if - we can't find node using ProviderID then we could use SystemUUID? Some kubernetes distributions such as Openshift do not necessarily match kubernetes versioning schemes and hence the fix will not work for them.

Copy link
Member

Choose a reason for hiding this comment

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

@gnufied in that case IsUUIDSupportedNode() is returning error?
If that is the case, then we need to correct func GetNodeUUID(node *v1.Node) (string, error) function.

Can you provide Openshift versioning schemes?

 

Copy link
Member

Choose a reason for hiding this comment

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

@divyenpatel no IsUUIDSupportedNode will always return true in Kubernetes distributions which do not necessarily track kube's version naming. Basically - openshift-3.9 == kube-1.9 but after initial release openshift does not follow release pattern of kubernetes and hence openshift-3.9.40 will still report kube-1.9.0..

if err != nil {
glog.Errorf("Failed to determine whether node %+v is old with error %v", node, err)
return false, err
}
nodeVersion, err := version.ParseSemantic(node.Status.NodeInfo.KubeletVersion)
if err != nil {
glog.Errorf("Failed to determine whether node %+v is old with error %v", node, err)
return false, err
}
if nodeVersion.LessThan(newVersion) {
return true, nil
}
return false, nil
}

func GetNodeUUID(node *v1.Node) (string, error) {
oldNode, err := IsUUIDSupportedNode(node)
if err != nil {
glog.Errorf("Failed to get node UUID for node %+v with error %v", node, err)
return "", err
}
if oldNode {
return node.Status.NodeInfo.SystemUUID, nil
}
return GetUUIDFromProviderID(node.Spec.ProviderID), nil
}