Skip to content

Show how to use serverSideApply() properly #6780

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

Closed
cowwoc opened this issue Jan 10, 2025 · 12 comments · Fixed by #6793
Closed

Show how to use serverSideApply() properly #6780

cowwoc opened this issue Jan 10, 2025 · 12 comments · Fixed by #6793
Labels
Milestone

Comments

@cowwoc
Copy link
Contributor

cowwoc commented Jan 10, 2025

Is your enhancement related to a problem? Please describe

https://github.com/fabric8io/kubernetes-client/blob/13b390964ccd813926895af43f4e9ea82f98ffef/doc/FAQ.md reads:

A common pattern for createOrReplace was obtaining the current resource from the api server (possibly via an informer cache), making modifications, then calling createOrReplace. Doing something similar with serverSideApply will fail as the managedFields will be populated. While you may be tempted to simply clear the managedFields and the resourceVersion, this is generally not what you should be doing unless you want your logic to assume ownership of every field on the resource. Instead you should reorganize your code to apply only the desired state that you wish to apply.

I don't understand what is meant in that last sentence...

Describe the solution you'd like

Please provide a short example along the lines of:

Say you want to update X to Y, instead of doing:

bad code

do this:

good code

Either provide this inline in the FAQ or link to it from this paragraph. Thank you in advance.

@cowwoc cowwoc changed the title Provide example of Show how to use serverSideApply() properly Jan 10, 2025
@cowwoc
Copy link
Contributor Author

cowwoc commented Jan 10, 2025

For example, given this monstrocity:

Job job = new JobBuilder().
      withMetadata(new ObjectMetaBuilder().
        withNamespace(NAMESPACE).
        withName(jobName).
        build()).
      withSpec(new JobSpecBuilder().
        withBackoffLimit(0).
        withTtlSecondsAfterFinished(CLEANUP_IN_SECONDS).
        withTemplate(new PodTemplateSpecBuilder().
          withMetadata(new ObjectMetaBuilder().
            withNamespace(NAMESPACE).
            withLabels(Map.of("runId", runId)).
            build()).
          withSpec(new PodSpecBuilder().
            addToContainers(new ContainerBuilder().
              withName(jobName).
              withImage("ghcr.io/bell-sw/liberica-runtime-container:jre-musl").
              addToVolumeMounts(new VolumeMountBuilder().
                withName("script-volume").
                withMountPath("/scripts").
                build()).
              withCommand("sh", "-c", "apk add --no-cache bash && ./scripts/" + scriptName).
              build()).
            addToVolumes(new VolumeBuilder().
              withName("script-volume").
              withConfigMap(new ConfigMapVolumeSourceBuilder().
                withName(script.item().getMetadata().getName()).
                withDefaultMode(0100).
                build()).
              build()).
            withRestartPolicy("Never").
            build()).
          build()).
        withPodFailurePolicy(new PodFailurePolicyBuilder().
          withRules(new PodFailurePolicyRuleBuilder().
            withAction("FailJob").
            withOnExitCodes(new PodFailurePolicyOnExitCodesRequirementBuilder().
              withContainerName(jobName).
              withOperator("NotIn").
              withValues(0).
              build()).
            build()).
          build()).
        build()).
      build();

how would I use a serverSideApply() to only update spec.template.metadata.labels?

@shawkins
Copy link
Contributor

Always apply the whole thing and let the server figure it out. No need to think of it as selectively updating fields.

@cowwoc
Copy link
Contributor Author

cowwoc commented Jan 10, 2025

@shawkins

Are you saying that the following pattern is correct?

Job job = // ...
ScalableResource<Job> jobResource = client.batch().v1().jobs().resource(job);
if (jobResource.get() == null)
  job = jobResource.create();
else
  job = jobResource.serverSideApply();

I just realized that my example is a poor one because Job.spec.template is immutable (it cannot be patched). But I want to understand the general pattern that I should be using for patching non-immutable properties... Is the idea to create the desired state, then execute the above if-else statement?

Thanks.

@shawkins
Copy link
Contributor

No if / else, just serverSideApply.

@cowwoc
Copy link
Contributor Author

cowwoc commented Jan 10, 2025

Okay, thank you. Please consider linking to sample code near the section I referenced in the FAQ.

@shawkins
Copy link
Contributor

Okay, thank you. Please consider linking to sample code near the section I referenced in the FAQ.

If possible please open a PR.

@cowwoc
Copy link
Contributor Author

cowwoc commented Jan 10, 2025

If possible please open a PR.

Normally I would just add a link to https://github.com/fabric8io/kubernetes-client/blob/main/doc/CHEATSHEET.md#server-side-apply but it doesn't make direct use of the serverSideApply() method whereas a few examples down https://github.com/fabric8io/kubernetes-client/blob/main/doc/CHEATSHEET.md#kubernetes-client-dsl-usage I see:

client.pods().inNamespace("default").resource(pod).serverSideApply();

which is more of what I would expect. Is there a reason that https://github.com/fabric8io/kubernetes-client/blob/main/doc/CHEATSHEET.md#server-side-apply advocates the use of PatchContext instead of serverSideApply()?

@shawkins
Copy link
Contributor

Is there a reason that https://github.com/fabric8io/kubernetes-client/blob/main/doc/CHEATSHEET.md#server-side-apply advocates the use of PatchContext instead of serverSideApply()?

That doc probably pre-dates when the first class method was added.

@manusa manusa added the doc label Jan 13, 2025
@manusa
Copy link
Member

manusa commented Jan 13, 2025

Hi @cowwoc
Do you want to open a PR to update the cheatsheet accordingly?

@cowwoc
Copy link
Contributor Author

cowwoc commented Jan 13, 2025

@manusa I just created #6784 but I've got a question regarding editing https://github.com/fabric8io/kubernetes-client/blob/13b390964ccd813926895af43f4e9ea82f98ffef/doc/FAQ.md

There is a note that says:

It is not recommended to use serverSideApply directly against modified existing resources - the intent is to apply only the desired state owned by the applier. See the next topic for more.

and later:

While you may be tempted to simply clear the managedFields and the resourceVersion, this is generally not what you should be doing unless you want your logic to assume ownership of every field on the resource.

Do these concerns still hold with the new serverSideApply() method? That is, if I modify part of an existing resource do I have to worry about my field manager taking ownership of every field in that resource?

https://github.com/fabric8io/kubernetes-client/blob/13b390964ccd813926895af43f4e9ea82f98ffef/doc/FAQ.md might require a bigger cleanup than I am comfortable applying by myself. Are you able to contribute on this front?

@shawkins
Copy link
Contributor

shawkins commented Jan 13, 2025

Do these concerns still hold with the new serverSideApply() method? That is, if I modify part of an existing resource do I have to worry about my field manager taking ownership of every field in that resource?

What is meant here is doing something like:

var something = resource.get();
// edit something, including removing managedFields and the resourceVersion
client.resource(something).serverSideApply(); // you have now assumed ownership over all state that is still present

That is probably not what you want to do. Rather than trying to remove everything you aren't responsible for, instead supply only the desired state known to the current owner - even if that looks like building the same resource from scratch every time.

This is known as reconstructive usage of SSA in the context of operators: https://kubernetes.io/blog/2022/10/20/advanced-server-side-apply/#reconstructive-controllers

Are you able to contribute on this front?

We can continue to collaborate here or on a PR if that works for you.

@cowwoc
Copy link
Contributor Author

cowwoc commented Jan 15, 2025

I pushed a second PR for the FAQ.md

Once merged, I believe we can close this issue. Thank you for your help.

@manusa manusa added this to the 7.1.0 milestone Jan 16, 2025 — with automated-tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants