Updating spec mutation logic of daemonset, statefulset, and deployment with mergeWithOverwriteWithEmptyValue #3324
Conversation
…y to update in chainsaw test
…th mergeWithOverwriteWithEmptyValue
…y-operator into 2947-updating-ds-sf-depl-mutation
davidhaja
changed the title
davidhaja
changed the title
davidhaja
changed the title
…y-operator into 2947-updating-ds-sf-depl-mutation
There was a problem hiding this comment.
Just one thought, otherwise I think this looks great. Thank you! @swiatekm @pavolloffay could you take a look here as well please?
…y-operator into 2947-updating-ds-sf-depl-mutation
| @@ -0,0 +1,16 @@ | |||
| # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | |||
| change_type: enhancement | |||
There was a problem hiding this comment.
I wonder if this shouldn't be a bug_fix. The current behaviour looks like a bug to me.
| @@ -0,0 +1,14 @@ | |||
| apiVersion: apps/v1 | |||
There was a problem hiding this comment.
Any particular reason not to put all the assertions in one file for each step, like we do for other tests? This isn't a criticism per se, I'm honestly wondering if this makes the tests easier to maintain. WDYT @open-telemetry/operator-approvers ?
There was a problem hiding this comment.
Hm... I see the value in that, this certainly increases our surface area for testing. I do think we're generally due for some e2e test maintenance though so maybe we can do that in a follow up?
There was a problem hiding this comment.
We can discuss during the next SIG meeting. But I agree we shouldn't hold up this PR.
|
@davidhaja thank you very much for your contribution here 🙇 |
Description:
Previously the opentelemetry operator couldn't remove fields from the spec of deployments, statefulsets, and daemonsets because of the
mergeWithOverridemerging strategy. (except the nodeselector field due to #2941).After this PR, the operator will use the
mergeWithOverwriteWithEmptyValuemerging logic on the spec of the resources, which will allow to remove fields if they are also removed from the opentelemetrycollector CR.Link to tracking Issue(s): #2947
Testing:
Documentation:
N/A
Limitation:
The
daemonset.metadata.annotations,statefulset.metadata.annotations,deployment.metadata.annotationsis still being mutated withmergeWithOverride, therefore the operator cannot remove annotations from there.This isn't the case with
daemonset.spec.template.metadata.annotations,statefulset.spec.template.metadata.annotations,deployment.spec.template.metadata.annotationssince they are in thespecof the resources.