-
Notifications
You must be signed in to change notification settings - Fork 120
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
chore: patch instead of update and bugfix #2059
Conversation
Signed-off-by: Derek Wang <[email protected]>
@@ -236,6 +236,8 @@ func (mr *monoVertexReconciler) orchestratePods(ctx context.Context, monoVtx *df | |||
monoVtx.Status.CurrentHash = monoVtx.Status.UpdateHash | |||
} else { // Update scenario | |||
if updatedReplicas >= desiredReplicas { | |||
monoVtx.Status.UpdatedReplicas = uint32(desiredReplicas) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixing 4.)
@@ -259,6 +259,8 @@ func (r *vertexReconciler) orchestratePods(ctx context.Context, vertex *dfv1.Ver | |||
vertex.Status.CurrentHash = vertex.Status.UpdateHash | |||
} else { // Update scenario | |||
if updatedReplicas >= desiredReplicas { | |||
vertex.Status.UpdatedReplicas = uint32(desiredReplicas) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixing 4.)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2059 +/- ##
==========================================
+ Coverage 63.11% 63.38% +0.26%
==========================================
Files 319 319
Lines 29180 29253 +73
==========================================
+ Hits 18416 18541 +125
+ Misses 9750 9695 -55
- Partials 1014 1017 +3 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Derek Wang <[email protected]>
@@ -292,7 +295,9 @@ func (r *pipelineReconciler) reconcileFixedResources(ctx context.Context, pl *df | |||
r.recorder.Eventf(pl, corev1.EventTypeNormal, "CreateVertexSuccess", "Created vertex %s successfully", vertexName) | |||
} else { | |||
if oldObj.GetAnnotations()[dfv1.KeyHash] != newObj.GetAnnotations()[dfv1.KeyHash] { // need to update | |||
originalReplicas := oldObj.Spec.Replicas |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixing 3.)
pkg/reconciler/isbsvc/controller.go
Outdated
if old == nil { | ||
return true | ||
func needsToPatchFinalizers(old, new *dfv1.InterStepBufferService) bool { | ||
if old == nil { // This is a weird scenario, nothing we can do. Theoretically it will never happen. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since it is a weird scenario should be log so it might help whoever is debugging?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cleaned it up since there's no need to add this function.
@@ -51,6 +51,8 @@ import ( | |||
|
|||
const ( | |||
finalizerName = dfv1.ControllerPipeline | |||
|
|||
pauseTimestampPath = `/metadata/annotations/numaflow.numaproj.io~1pause-timestamp` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
~1
what does that mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: Derek Wang <[email protected]>
Signed-off-by: Derek Wang <[email protected]>
Signed-off-by: Derek Wang <[email protected]>
Closes: #2054
cpu: 2000m
is updated tocpu: 2
)min
.updateHash != currentHash
, it then it gets stuck.