Bugfix ignore and copy from obj #37
Conversation
also rename few variables to communicate better context Signed-off-by: Ishan Khare <[email protected]>
Signed-off-by: Ishan Khare <[email protected]>
also add README for deploying plugin Signed-off-by: Ishan Khare <[email protected]>
ishankhare07
force-pushed
the
bugfix-ignore-and-copyFromObj
branch
from
06cb45c
to
989ece9
Compare
| @@ -46,7 +46,7 @@ func ApplyPatches(obj1, obj2 client.Object, patchesConf []*config.Patch, reverse | |||
|
|
|||
| // remove ignore paths from patched object | |||
| for _, p := range reversePatchesConf { | |||
| if p.Path == "" || (p.Ignore != nil && !*p.Ignore) { | |||
There was a problem hiding this comment.
Not sure if that is correct, the logic currently is that its true by default, this would make it false by default, so only in the case of ignore: false we want to skip applying the delete patch
| // apply patches on from object | ||
| err = patches.ApplyPatches(toObjCopied, toObj, patchesConfig, reversePatchesConfig, nameResolver) | ||
| // apply patches on toObjCopied | ||
| err = patches.ApplyPatches(toObjCopied, fromObj, patchesConfig, reversePatchesConfig, nameResolver) |
There was a problem hiding this comment.
this is needed in cases where we want to populated controller applied default back into the virtual obj. i.e.
vObj.field1 = pObj.field1.
Currently the behavior of copyFromObject is like vObj.field1 = vObj.field2
But consider the scenario where
vobj
=====
spec.template.timeoutSeconds: nil (unset)
|
|
v
-------------------------------------------
pobj
spec.template.timeoutSeconds: 300 ----------> set by controller to default
I would want to (expect that) copyFromObject in reversePatches as defined
- op: copyFromObject
fromPath: spec.template.spec.timeoutSeconds
path: spec.template.spec.timeoutSeconds
conditions:
- empty: true
path: spec.template.spec.timeoutSeconds
would copy across physical <-> virtual boundary, but since both the args to ApplyPatches originated from same source obj, it was not happening
| @@ -78,15 +79,15 @@ func (s *patcher) ApplyPatches(ctx context.Context, fromObj, toObj client.Object | |||
| return outObject, nil | |||
| } | |||
|
|
|||
| func (s *patcher) ApplyReversePatches(ctx context.Context, fromObj, otherObj client.Object, reversePatchConfig []*config.Patch, nameResolver patches.NameResolver) (controllerutil.OperationResult, error) { | |||
| originalUnstructured, err := toUnstructured(fromObj) | |||
| func (s *patcher) ApplyReversePatches(ctx context.Context, destObj, sourceObj client.Object, reversePatchConfig []*config.Patch, nameResolver patches.NameResolver) (controllerutil.OperationResult, error) { | |||
There was a problem hiding this comment.
IMHO original name was confusing. So unless I understand the function incorrectly, the new name is much better.
Also add knative-serving plugin example