java-operator-sdk icon indicating copy to clipboard operation
java-operator-sdk copied to clipboard

How to selectively define the fields to reconcile with DependentResources

Open andreaTP opened this issue 3 years ago • 4 comments

Bug Report

Defining a DependenResource of type e.g. Job is slightly problematic at a first attempt. The reason is that the Job .spec.template field is immutable and any update operation slightly changing it result in an error.

Here I'm tentatively disabling all the update methods: https://github.com/andreaTP/bink8s/blob/0177a66e4b31225c0b5a91e5694e80a3d3127df4/src/main/java/io/bink8s/controller/workflow/ImageBuildJobDependentResource.java#L85-L97

I'm not sure what would be the best course of action here, e.g. Labels can still be updated/reconciled, but we should make sure the .spec.template remains untouched. The problematic bit is that .spec.template contains elements that are filled after the object is created e.g.:

spec:
  template:
    metadata:
      creationTimestamp: null
      labels:
        controller-uid: 7ec0fb0f-d0fa-4795-b051-233d8d40f19c

I'm looking for guidance on how to handle this situation properly 🙏

andreaTP avatar Sep 16 '22 17:09 andreaTP

We don't have a Job specific matcher yet, but this can be probably just fixed if you override the matcher.

What is your exact use case?

Asking because recreating a use case is not supported, what might be an use case some times.

csviri avatar Sep 19 '22 06:09 csviri

I would like to simply skip the update if .spec.template differ, and, to make sure I never update that subpath (as it will result in an error all the time).

andreaTP avatar Sep 19 '22 08:09 andreaTP

Ahh ok, this is defintely should be possible to do even now, just overriding the match and the update function.

However might be easier if the desired would receive the actual resource too if present. cc @metacosm - will prepare a PR for this.

csviri avatar Sep 19 '22 08:09 csviri

@andreaTP I linked a PR, we can discuss that. Basically now this is possible just more hacky, with that it should be doable much more elegantly. Probably the best would be to add to the same branch an integration test for this use case.

csviri avatar Sep 19 '22 08:09 csviri

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] avatar Nov 19 '22 03:11 github-actions[bot]

This issue was closed because it has been stalled for 14 days with no activity.

github-actions[bot] avatar Dec 04 '22 02:12 github-actions[bot]

I was updating the API for this use case, see related PR. But realized that getSecondaryResource always can be called. So you can just call that set the spec.template part from there if present. In this case it is not changed. Not sure if we should still change the API of desired. Will create an example at least.

csviri avatar Jan 31 '23 11:01 csviri

See sample here: https://github.com/java-operator-sdk/java-operator-sdk/pull/1746

I think this is good enough, no need to explicitly put the resource in to desrire(...) api. Unless you think otherwise @andreaTP or @metacosm

csviri avatar Jan 31 '23 12:01 csviri