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

Add support for Kustomize Strategic Merge Patch on ScaledJobs #5751

Closed
YorickH opened this issue Apr 26, 2024 · 3 comments
Closed

Add support for Kustomize Strategic Merge Patch on ScaledJobs #5751

YorickH opened this issue Apr 26, 2024 · 3 comments
Assignees
Labels
feature-request All issues for new features that have not been committed to needs-discussion stale All issues that are marked as stale due to inactivity

Comments

@YorickH
Copy link

YorickH commented Apr 26, 2024

Proposal

In order for Kustomize to patch CRDs properly, we need to publish openAPI schema containing the following metadata in the ScaledJob CRD definitions.

  • x-kubernetes-patch-merge-key
  • x-kubernetes-patch-strategy

Use-Case

We have been running ScaledObjects for about 3 years now and recently we've been considering switching to ScaledJobs for some use cases.

We use Kustomize extensively with a multi-level layering system, even with a centralized shared repository with the base objects.

While experimenting with implementing it into our setup I noticed that patching ScaledJobs (we add default environment variables for monitoring purposes by default, while we add more environment variables on a service level. Or patching things like an entrypoint on the service level) was not doing a strategic merge.

So I went on a hunt and quickly realized that strategic merges need to be supported by the CRD. We prefer using strategic merge patches over JSON patch for readability and ease of use.

To solve this issue we would have 3 options:

  1. Switch to a JSON patch for ScaledJobs which would make it a readability hell
  2. Generate our own (full) OpenAPI schema, run the necessary patches for supporting strategic merges, and use the openapi.path to specify the correct OpenAPI schema to customize.
  • This is what official Kustomize docs suggest as a fix
  • The problem with this method is that it overwrites the existing known OpenAPI schema, losing other options for strategic merge. An issue to support merging OpenAPI schema definitions has been open for 2 years now.
  • To help with that issue, we could use this project to generate and patch the OpenAPI schema based on our own cluster and use that file as input. Obviously this adds more maintenance in the case you want to upgrade components, API's or clusters as you would need to maintain that list as best as possible.
  1. Ask the operator developers to add those merge keys to the CRD's in the correct locations to support StrategicMerge. This is something that for example argo-rollouts implemented

Is this a feature you are interested in implementing yourself?

Maybe

Anything else?

Happy to make my first contribution but I hardly have any experience in golang. But I noticed there is some JSON patching going on which I'm well familiar with but it depends on how we would see this being implemented.

@YorickH YorickH added feature-request All issues for new features that have not been committed to needs-discussion labels Apr 26, 2024
Copy link

stale bot commented Jun 25, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale All issues that are marked as stale due to inactivity label Jun 25, 2024
@zroubalik zroubalik removed the stale All issues that are marked as stale due to inactivity label Jun 25, 2024
Copy link

stale bot commented Aug 27, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale All issues that are marked as stale due to inactivity label Aug 27, 2024
Copy link

stale bot commented Sep 5, 2024

This issue has been automatically closed due to inactivity.

@stale stale bot closed this as completed Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request All issues for new features that have not been committed to needs-discussion stale All issues that are marked as stale due to inactivity
Projects
None yet
Development

No branches or pull requests

3 participants