Fix GetReference function

This commit is contained in:
wojtekt 2019-08-05 08:37:55 +02:00
parent 56d7920e8f
commit 399d09ce4a
4 changed files with 27 additions and 78 deletions

View File

@ -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)},

View File

@ -132,13 +132,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,

View File

@ -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{

View File

@ -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)