Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(customresources): use a 3-way merge patch instead of strategic merge #1095

Merged
merged 1 commit into from
May 5, 2020

Conversation

metral
Copy link
Contributor

@metral metral commented May 4, 2020

Proposed changes

CustomResources do not support JSON strategic merges on patch updates.

Instead, they require a JSON 3-way merge patch to properly apply the patch.

CustomResource kinds now are treated separately from other known k8s kinds in
order to apply the proper patch.

A new CoreOS prometheus-operator test with a step update is included to validate this update path.

Related issues (optional)

Fixes #1061

CustomResources do not support [JSON strategic merges](https://tools.ietf.org/html/rfc6902) on patch updates.

Instead, they require a [JSON 3-way merge patch](https://tools.ietf.org/html/rfc7386) to properly apply the patch.

CustomResource kinds now are treated separately from other known k8s kinds in
order to apply the proper patch.

A new [CoreOS prometheus-operator](https://github.com/coreos/prometheus-operator) test with a step update is included to validate this update path.
Copy link
Member

@lblackstone lblackstone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Just to confirm, the new test fails without this change?

@metral
Copy link
Contributor Author

metral commented May 5, 2020

Just to confirm, the new test fails without this change?

Yes, that's right.

--

Before the change

Initial update with index.ts, then update to step1/index.ts results in error:

image


After the change

The step1/index.ts update succeeds, and the CR spec is updated:
spec.endpoints.relabelings[] updates to -> spec.endpoints.metricReloadings[]

image

@metral metral merged commit ae15a2a into master May 5, 2020
@pulumi-bot pulumi-bot deleted the metral/cr-merge-patch branch May 5, 2020 04:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CustomResource kinds fail to update
2 participants