Skip to content

K
k3s
Commit 349a99b8 authored by Daniel Smith's avatar Daniel Smith
Browse files
Options

Refactor the patch handler for readability

This is the combination of a series of changes which individually don't make any behavioral changes. The original commits are preserved in my own fork in the refactor-patch-complete branch, as when squashed this is impossible to review. This turned a big function with lots of parameters and closures into an object with multiple functions, fewer closures and more well documented state transitions.
parent aab5cb4c
Expand all Show whitespace changes
Inline Side-by-side
Showing with 33 additions and 16 deletions
staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest_test.go
View file @ 349a99b8
...@@ -293,7 +293,7 @@ func (tc *patchTestCase) Run(t *testing.T) { ...@@ -293,7 +293,7 @@ func (tc *patchTestCase) Run(t *testing.T) {
convertor := runtime.UnsafeObjectConvertor(scheme) convertor := runtime.UnsafeObjectConvertor(scheme)
kind := examplev1.SchemeGroupVersion.WithKind("Pod") kind := examplev1.SchemeGroupVersion.WithKind("Pod")
resource := examplev1.SchemeGroupVersion.WithResource("pods") resource := examplev1.SchemeGroupVersion.WithResource("pods")
versionedObj := &examplev1.Pod{} schemaReferenceObj := &examplev1.Pod{}
for _, patchType := range []types.PatchType{types.JSONPatchType, types.MergePatchType, types.StrategicMergePatchType} { for _, patchType := range []types.PatchType{types.JSONPatchType, types.MergePatchType, types.StrategicMergePatchType} {
// This needs to be reset on each iteration. // This needs to be reset on each iteration.
...@@ -323,7 +323,7 @@ func (tc *patchTestCase) Run(t *testing.T) { ...@@ -323,7 +323,7 @@ func (tc *patchTestCase) Run(t *testing.T) {
patch := []byte{} patch := []byte{}
switch patchType { switch patchType {
case types.StrategicMergePatchType: case types.StrategicMergePatchType:
patch, err = strategicpatch.CreateTwoWayMergePatch(originalObjJS, changedJS, versionedObj) patch, err = strategicpatch.CreateTwoWayMergePatch(originalObjJS, changedJS, schemaReferenceObj)
if err != nil { if err != nil {
t.Errorf("%s: unexpected error: %v", tc.name, err) t.Errorf("%s: unexpected error: %v", tc.name, err)
continue continue
...@@ -338,18 +338,33 @@ func (tc *patchTestCase) Run(t *testing.T) { ...@@ -338,18 +338,33 @@ func (tc *patchTestCase) Run(t *testing.T) {
} }
resultObj, err := patchResource( p := patcher{
ctx, namer: namer,
admissionMutation, creater: creater,
rest.ValidateAllObjectFunc, defaulter: defaulter,
admissionValidation, unsafeConvertor: convertor,
1*time.Second, kind: kind,
versionedObj, resource: resource,
testPatcher,
name, createValidation: rest.ValidateAllObjectFunc,
patchType, updateValidation: admissionValidation,
patch, admissionCheck: admissionMutation,
namer, creater, defaulter, convertor, kind, resource, codec, utiltrace.New("Patch"+name))
codec: codec,
timeout: 1 * time.Second,
schemaReferenceObj: schemaReferenceObj,
restPatcher: testPatcher,
name: name,
patchType: patchType,
patchJS: patch,
trace: utiltrace.New("Patch" + name),
}
resultObj, err := p.patchResource(ctx)
if len(tc.expectedError) != 0 { if len(tc.expectedError) != 0 {
if err == nil || err.Error() != tc.expectedError { if err == nil || err.Error() != tc.expectedError {
t.Errorf("%s: expected error %v, but got %v", tc.name, tc.expectedError, err) t.Errorf("%s: expected error %v, but got %v", tc.name, tc.expectedError, err)
...@@ -407,11 +422,11 @@ func TestNumberConversion(t *testing.T) { ...@@ -407,11 +422,11 @@ func TestNumberConversion(t *testing.T) {
}, },
} }
versionedObjToUpdate := &examplev1.Pod{} versionedObjToUpdate := &examplev1.Pod{}
versionedObj := &examplev1.Pod{} schemaReferenceObj := &examplev1.Pod{}
patchJS := []byte(`{"spec":{"terminationGracePeriodSeconds":42,"activeDeadlineSeconds":120}}`) patchJS := []byte(`{"spec":{"terminationGracePeriodSeconds":42,"activeDeadlineSeconds":120}}`)
err := strategicPatchObject(codec, defaulter, currentVersionedObject, patchJS, versionedObjToUpdate, versionedObj) err := strategicPatchObject(codec, defaulter, currentVersionedObject, patchJS, versionedObjToUpdate, schemaReferenceObj)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
...@@ -538,6 +553,8 @@ func TestPatchResourceWithConflict(t *testing.T) { ...@@ -538,6 +553,8 @@ func TestPatchResourceWithConflict(t *testing.T) {
expectedError: `Operation cannot be fulfilled on pods.example.apiserver.k8s.io "foo": existing 2, new 1`, expectedError: `Operation cannot be fulfilled on pods.example.apiserver.k8s.io "foo": existing 2, new 1`,
} }
// See issue #63104 for discussion of how much sense this makes.
tc.startingPod.Name = name tc.startingPod.Name = name
tc.startingPod.Namespace = namespace tc.startingPod.Namespace = namespace
tc.startingPod.UID = uid tc.startingPod.UID = uid
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment