numaproj / numaflow

Kubernetes-native platform to run massively parallel data/streaming jobs
https://numaflow.numaproj.io/
Apache License 2.0
1.29k stars 115 forks source link

Fix: Use Merge patch rather than json patch for `pause-timestamp` annotation apply #2078

Closed juliev0 closed 2 months ago

juliev0 commented 2 months ago

It seems if you create a Pipeline with no metadata.annotations set (at least if you are using minimized CRDs), then trying to do this json patch operation fails with the error "the server rejected our request due to an error in our request".

This does not happen if a Merge Patch is used instead.

I tested this in the case of:

For both cases, I experimented with adding lifecycle.desiredPhase: Paused and also with removing it.

For both, I observed the proper annotation set:

metadata:
  annotations:
    numaflow.numaproj.io/pause-timestamp: "2024-09-20T20:31:47Z"

...and then later removed.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 63.80%. Comparing base (ed543ad) to head (498f5bf). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/reconciler/pipeline/controller.go 50.00% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2078 +/- ## ========================================== - Coverage 63.99% 63.80% -0.20% ========================================== Files 321 321 Lines 29572 29572 ========================================== - Hits 18926 18869 -57 - Misses 9628 9689 +61 + Partials 1018 1014 -4 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.