Fix GetReference function

pkg/controller/volume/scheduling/scheduler_binder_test.go

@@ -53,7 +53,6 @@ var (
 	boundPVC            = makeTestPVC("bound-pvc", "1G", "", pvcBound, "pv-bound", "1", &waitClass)
 	boundPVC2           = makeTestPVC("bound-pvc2", "1G", "", pvcBound, "pv-bound2", "1", &waitClass)
 	boundPVCNode1a      = makeTestPVC("unbound-pvc", "1G", "", pvcBound, "pv-node1a", "1", &waitClass)
-	badPVC              = makeBadPVC()
 	immediateUnboundPVC = makeTestPVC("immediate-unbound-pvc", "1G", "", pvcUnbound, "", "1", &immediateClass)
 	immediateBoundPVC   = makeTestPVC("immediate-bound-pvc", "1G", "", pvcBound, "pv-bound-immediate", "1", &immediateClass)
 
@@ -556,28 +555,6 @@ func makeTestPVC(name, size, node string, pvcBoundState int, pvName, resourceVer
 	return pvc
 }
 
-func makeBadPVC() *v1.PersistentVolumeClaim {
-	fs := v1.PersistentVolumeFilesystem
-	return &v1.PersistentVolumeClaim{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:            "bad-pvc",
-			Namespace:       "testns",
-			UID:             types.UID("pvc-uid"),
-			ResourceVersion: "1",
-			// Don't include SefLink, so that GetReference will fail
-		},
-		Spec: v1.PersistentVolumeClaimSpec{
-			Resources: v1.ResourceRequirements{
-				Requests: v1.ResourceList{
-					v1.ResourceName(v1.ResourceStorage): resource.MustParse("1G"),
-				},
-			},
-			StorageClassName: &waitClass,
-			VolumeMode:       &fs,
-		},
-	}
-}
-
 func makeTestPV(name, node, capacity, version string, boundToPVC *v1.PersistentVolumeClaim, className string) *v1.PersistentVolume {
 	fs := v1.PersistentVolumeFilesystem
 	pv := &v1.PersistentVolume{
@@ -1046,12 +1023,6 @@ func TestAssumePodVolumes(t *testing.T) {
 			expectedBindings:      []*bindingInfo{makeBinding(unboundPVC, pvNode1aBound)},
 			expectedProvisionings: []*v1.PersistentVolumeClaim{},
 		},
-		"claimref-failed": {
-			podPVCs:    []*v1.PersistentVolumeClaim{unboundPVC},
-			bindings:   []*bindingInfo{makeBinding(unboundPVC, pvNode1a), makeBinding(badPVC, pvNode1b)},
-			pvs:        []*v1.PersistentVolume{pvNode1a, pvNode1b},
-			shouldFail: true,
-		},
 		"tmpupdate-failed": {
 			podPVCs:    []*v1.PersistentVolumeClaim{unboundPVC},
 			bindings:   []*bindingInfo{makeBinding(unboundPVC, pvNode1a), makeBinding(unboundPVC2, pvNode1b)},

pkg/kubelet/container/ref_test.go

@@ -133,13 +133,6 @@ func TestGenerateContainerRef(t *testing.T) {
 			success: true,
 		},
 		{
-			name:      "no-selflink",
-			pod:       &noSelfLinkPod,
-			container: &v1.Container{},
-			expected:  nil,
-			success:   false,
-		},
-		{
 			name: "defaulted-selflink",
 			pod:  &defaultedSelfLinkPod,
 			container: &v1.Container{

staging/src/k8s.io/client-go/tools/reference/ref.go

@@ -19,8 +19,6 @@ package reference
 import (
 	"errors"
 	"fmt"
-	"net/url"
-	"strings"
 
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -30,8 +28,7 @@ import (
 
 var (
 	// Errors that could be returned by GetReference.
-	ErrNilObject  = errors.New("can't reference a nil object")
-	ErrNoSelfLink = errors.New("selfLink was empty, can't make reference")
+	ErrNilObject = errors.New("can't reference a nil object")
 )
 
 // GetReference returns an ObjectReference which refers to the given
@@ -47,20 +44,6 @@ func GetReference(scheme *runtime.Scheme, obj runtime.Object) (*v1.ObjectReferen
 		return ref, nil
 	}
 
-	gvk := obj.GetObjectKind().GroupVersionKind()
-
-	// if the object referenced is actually persisted, we can just get kind from meta
-	// if we are building an object reference to something not yet persisted, we should fallback to scheme
-	kind := gvk.Kind
-	if len(kind) == 0 {
-		// TODO: this is wrong
-		gvks, _, err := scheme.ObjectKinds(obj)
-		if err != nil {
-			return nil, err
-		}
-		kind = gvks[0].Kind
-	}
-
 	// An object that implements only List has enough metadata to build a reference
 	var listMeta metav1.Common
 	objectMeta, err := meta.Accessor(obj)
@@ -73,29 +56,29 @@ func GetReference(scheme *runtime.Scheme, obj runtime.Object) (*v1.ObjectReferen
 		listMeta = objectMeta
 	}
 
-	// if the object referenced is actually persisted, we can also get version from meta
-	version := gvk.GroupVersion().String()
-	if len(version) == 0 {
-		selfLink := listMeta.GetSelfLink()
-		if len(selfLink) == 0 {
-			return nil, ErrNoSelfLink
-		}
-		selfLinkUrl, err := url.Parse(selfLink)
+	gvk := obj.GetObjectKind().GroupVersionKind()
+
+	// If object meta doesn't contain data about kind and/or version,
+	// we are falling back to scheme.
+	//
+	// TODO: This doesn't work for CRDs, which are not registered in scheme.
+	if gvk.Empty() {
+		gvks, _, err := scheme.ObjectKinds(obj)
 		if err != nil {
 			return nil, err
 		}
-		// example paths: /<prefix>/<version>/*
-		parts := strings.Split(selfLinkUrl.Path, "/")
-		if len(parts) < 4 {
-			return nil, fmt.Errorf("unexpected self link format: '%v'; got version '%v'", selfLink, version)
-		}
-		if parts[1] == "api" {
-			version = parts[2]
-		} else {
-			version = parts[2] + "/" + parts[3]
+		if len(gvks) == 0 || gvks[0].Empty() {
+			return nil, fmt.Errorf("unexpected gvks registered for object %T: %v", obj, gvks)
 		}
+		// TODO: The same object can be registered for multiple group versions
+		// (although in practise this doesn't seem to be used).
+		// In such case, the version set may not be correct.
+		gvk = gvks[0]
 	}
 
+	kind := gvk.Kind
+	version := gvk.GroupVersion().String()
+
 	// only has list metadata
 	if objectMeta == nil {
 		return &v1.ObjectReference{

staging/src/k8s.io/client-go/tools/reference/ref_test.go

@@ -37,29 +37,31 @@ func TestGetReferenceRefVersion(t *testing.T) {
 	tests := []struct {
 		name               string
 		input              *TestRuntimeObj
+		groupVersion       schema.GroupVersion
 		expectedRefVersion string
 	}{
 		{
-			name: "api from selflink",
+			name: "v1 GV from scheme",
 			input: &TestRuntimeObj{
-				ObjectMeta: metav1.ObjectMeta{SelfLink: "/api/v1/namespaces"},
+				ObjectMeta: metav1.ObjectMeta{SelfLink: "/bad-selflink/unused"},
 			},
+			groupVersion:       schema.GroupVersion{Group: "", Version: "v1"},
 			expectedRefVersion: "v1",
 		},
 		{
-			name: "foo.group/v3 from selflink",
+			name: "foo.group/v3 GV from scheme",
 			input: &TestRuntimeObj{
-				ObjectMeta: metav1.ObjectMeta{SelfLink: "/apis/foo.group/v3/namespaces"},
+				ObjectMeta: metav1.ObjectMeta{SelfLink: "/bad-selflink/unused"},
 			},
+			groupVersion:       schema.GroupVersion{Group: "foo.group", Version: "v3"},
 			expectedRefVersion: "foo.group/v3",
 		},
 	}
 
-	scheme := runtime.NewScheme()
-	scheme.AddKnownTypes(schema.GroupVersion{Group: "this", Version: "is ignored"}, &TestRuntimeObj{})
-
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
+			scheme := runtime.NewScheme()
+			scheme.AddKnownTypes(test.groupVersion, &TestRuntimeObj{})
 			ref, err := GetReference(scheme, test.input)
 			if err != nil {
 				t.Fatal(err)