AppWrapper e2e: Remove creationTimestamp from marshalled template #630
Conversation
| @@ -145,6 +147,10 @@ func runMnistPyTorchAppWrapper(t *testing.T, accelerator string, numberOfGpus in | |||
| }, | |||
| } | |||
|
|
|||
| raw, _ := json.Marshal(job) | |||
| // Workaround to remove creationTimestamp which is brought by json marshaller, this field breaks Kueue reconciliation | |||
| patchedRaw := strings.ReplaceAll(string(raw), `"metadata":{"creationTimestamp":null},`, "") | |||
There was a problem hiding this comment.
Could this be encapsulated into the Raw helper function?
There was a problem hiding this comment.
I think it belongs in Raw eventually.
A related point is the go dependencies in codeflare-common are very dated. Kueue, KubeRay, and AppWrapper are way behind what is current in codeflare-operator. Probably updating them is worth doing.
There was a problem hiding this comment.
I can add it there as an option
There was a problem hiding this comment.
Added the replace into dedicated helper function.
I didn't want to add it to the Raw as the manipulation of the content needs to be done on a struct pointer, so in the end I have manipulated with RawExtension anyway. In such case RawExtension can be modified outside of Raw.
sutaakar
force-pushed
the
appwrapper-fix
branch
from
4d7bfb6 to
498c2bf
Compare
sutaakar
force-pushed
the
appwrapper-fix
branch
from
498c2bf to
4e62564
Compare
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sutaakar The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit cb032ef
into
project-codeflare:main
Issue link
What changes have been made
Remove creationTimestamp from marshalled template for AppWrapper. This creationTimestamp is brought by marshaller (it is not omitted because it is defined as struct instead of pointer in k8s API). This field breaks Kueue reconciliation in AppWrapper for Kueue 0.8.3.
Verification steps
The change was verified in #628
Checks