Skip to content

Commit f81d6ee

Browse files
onutconutc
committed
fix(binding): preserve replace contract and retries
1 parent bddfca1 commit f81d6ee

6 files changed

Lines changed: 690 additions & 91 deletions

File tree

‎api/internal_bindings.go‎

Lines changed: 82 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -316,13 +316,21 @@ func (s *server) replaceInternalBinding(
316316
if binding == nil {
317317
return nil, false, nil
318318
}
319-
replayed := strings.TrimSpace(binding.Spec.DesiredRevision) == strings.TrimSpace(targetRevision)
320-
if binding.Spec.DesiredRevision != strings.TrimSpace(targetRevision) {
321-
binding.Spec.DesiredRevision = strings.TrimSpace(targetRevision)
319+
desiredRevision := strings.TrimSpace(targetRevision)
320+
replayed := strings.TrimSpace(binding.Spec.DesiredRevision) == desiredRevision
321+
needsUpdate := false
322+
if binding.Spec.DesiredRevision != desiredRevision {
323+
binding.Spec.DesiredRevision = desiredRevision
324+
needsUpdate = true
325+
}
326+
if !bindingReadyOnDesiredRevision(binding) {
322327
if binding.Annotations == nil {
323328
binding.Annotations = map[string]string{}
324329
}
325330
binding.Annotations[spritzv1.BindingReconcileRequestedAtAnnotationKey] = time.Now().UTC().Format(time.RFC3339Nano)
331+
needsUpdate = true
332+
}
333+
if needsUpdate {
326334
if err := s.client.Update(ctx, binding); err != nil {
327335
return nil, false, err
328336
}
@@ -334,31 +342,94 @@ func (s *server) replaceInternalBinding(
334342
return &stored, replayed, nil
335343
}
336344

337-
func replacementRuntimeFromBinding(binding *spritzv1.SpritzBinding) *spritzv1.Spritz {
345+
func replacementRuntimeFromBinding(binding *spritzv1.SpritzBinding, fallbackName string) *spritzv1.Spritz {
338346
if binding == nil {
339347
return nil
340348
}
341349
ref := binding.Status.CandidateInstanceRef
342-
if ref == nil && binding.Status.ActiveInstanceRef != nil && strings.TrimSpace(binding.Status.ObservedRevision) == strings.TrimSpace(binding.Spec.DesiredRevision) {
350+
if ref == nil && bindingReadyOnDesiredRevision(binding) {
343351
ref = binding.Status.ActiveInstanceRef
344352
}
345-
if ref == nil {
353+
if ref != nil {
354+
revision := strings.TrimSpace(ref.Revision)
355+
if revision == "" {
356+
revision = bindingObservedRevision(binding)
357+
}
358+
if revision == "" {
359+
revision = strings.TrimSpace(binding.Spec.DesiredRevision)
360+
}
361+
return &spritzv1.Spritz{
362+
ObjectMeta: metav1.ObjectMeta{
363+
Namespace: ref.Namespace,
364+
Name: ref.Name,
365+
Annotations: map[string]string{
366+
targetRevisionAnnotationKey: revision,
367+
},
368+
},
369+
Status: spritzv1.SpritzStatus{
370+
Phase: strings.TrimSpace(ref.Phase),
371+
},
372+
}
373+
}
374+
name := strings.TrimSpace(fallbackName)
375+
if name == "" {
376+
name = predictedBindingCandidateName(binding)
377+
}
378+
if name == "" {
346379
return nil
347380
}
348-
revision := strings.TrimSpace(ref.Revision)
381+
revision := strings.TrimSpace(binding.Spec.DesiredRevision)
349382
if revision == "" {
350-
revision = strings.TrimSpace(binding.Status.ObservedRevision)
383+
revision = bindingObservedRevision(binding)
351384
}
352385
return &spritzv1.Spritz{
353386
ObjectMeta: metav1.ObjectMeta{
354-
Namespace: ref.Namespace,
355-
Name: ref.Name,
387+
Namespace: binding.Namespace,
388+
Name: name,
356389
Annotations: map[string]string{
357390
targetRevisionAnnotationKey: revision,
358391
},
359392
},
360393
Status: spritzv1.SpritzStatus{
361-
Phase: strings.TrimSpace(ref.Phase),
394+
Phase: "Provisioning",
362395
},
363396
}
364397
}
398+
399+
func bindingReadyOnDesiredRevision(binding *spritzv1.SpritzBinding) bool {
400+
if binding == nil || binding.Status.ActiveInstanceRef == nil {
401+
return false
402+
}
403+
if binding.Status.CandidateInstanceRef != nil {
404+
return false
405+
}
406+
return bindingObservedRevision(binding) == strings.TrimSpace(binding.Spec.DesiredRevision)
407+
}
408+
409+
func bindingObservedRevision(binding *spritzv1.SpritzBinding) string {
410+
if binding == nil {
411+
return ""
412+
}
413+
if revision := strings.TrimSpace(binding.Status.ObservedRevision); revision != "" {
414+
return revision
415+
}
416+
if binding.Status.ActiveInstanceRef != nil {
417+
return strings.TrimSpace(binding.Status.ActiveInstanceRef.Revision)
418+
}
419+
return ""
420+
}
421+
422+
func predictedBindingCandidateName(binding *spritzv1.SpritzBinding) string {
423+
if binding == nil {
424+
return ""
425+
}
426+
if binding.Status.CandidateInstanceRef != nil {
427+
return strings.TrimSpace(binding.Status.CandidateInstanceRef.Name)
428+
}
429+
return spritzv1.BindingRuntimeNameForSequence(
430+
strings.TrimSpace(binding.Spec.BindingKey),
431+
binding.Spec.Template.NamePrefix,
432+
binding.Spec.Template.PresetID,
433+
binding.Status.NextRuntimeSequence+1,
434+
)
435+
}

‎api/internal_bindings_test.go‎

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,3 +165,235 @@ func TestInternalReplaceSpritzUsesBindingLifecycleWhenRuntimeIsOwnedByBinding(t
165165
t.Fatalf("expected reconcile annotation to be set, got %#v", stored.Annotations)
166166
}
167167
}
168+
169+
func TestInternalReplaceSpritzSchedulesBindingReplacementBeforeCandidateExists(t *testing.T) {
170+
targetRevision := "sha256:rev-2"
171+
binding := &spritzv1.SpritzBinding{
172+
ObjectMeta: metav1.ObjectMeta{
173+
Name: "channel-installation-binding-1",
174+
Namespace: "spritz-production",
175+
},
176+
Spec: spritzv1.SpritzBindingSpec{
177+
BindingKey: "channel-installation-binding-1",
178+
DesiredRevision: "sha256:rev-1",
179+
Template: spritzv1.SpritzBindingTemplate{
180+
PresetID: "zeno",
181+
NamePrefix: "zeno",
182+
},
183+
},
184+
Status: spritzv1.SpritzBindingStatus{
185+
ObservedRevision: "sha256:rev-1",
186+
ActiveInstanceRef: &spritzv1.SpritzBindingInstanceRef{
187+
Namespace: "spritz-production",
188+
Name: "zeno-acme",
189+
Revision: "sha256:rev-1",
190+
Phase: "Ready",
191+
},
192+
},
193+
}
194+
source := &spritzv1.Spritz{
195+
ObjectMeta: metav1.ObjectMeta{
196+
Name: "zeno-acme",
197+
Namespace: "spritz-production",
198+
OwnerReferences: []metav1.OwnerReference{{
199+
APIVersion: spritzv1.GroupVersion.String(),
200+
Kind: "SpritzBinding",
201+
Name: binding.Name,
202+
}},
203+
},
204+
Status: spritzv1.SpritzStatus{Phase: "Ready"},
205+
}
206+
s := newInternalSpritzesTestServer(t, source)
207+
if err := s.client.Create(context.Background(), binding); err != nil {
208+
t.Fatalf("failed to create binding: %v", err)
209+
}
210+
e := echo.New()
211+
s.registerRoutes(e)
212+
213+
body := `{
214+
"targetRevision": "sha256:rev-2",
215+
"idempotencyKey": "replace-1",
216+
"replacement": {
217+
"principal": {"id": "channel-gateway"},
218+
"request": {
219+
"presetId": "zeno",
220+
"ownerId": "user-123",
221+
"requestId": "replace-1",
222+
"source": "channel-gateway",
223+
"spec": {}
224+
}
225+
}
226+
}`
227+
req := httptest.NewRequest(http.MethodPost, "/api/internal/v1/spritzes/spritz-production/zeno-acme:replace", strings.NewReader(body))
228+
req.Header.Set("Authorization", "Bearer spritz-internal-token")
229+
req.Header.Set("Content-Type", "application/json")
230+
rec := httptest.NewRecorder()
231+
e.ServeHTTP(rec, req)
232+
233+
if rec.Code != http.StatusAccepted {
234+
t.Fatalf("expected 202, got %d: %s", rec.Code, rec.Body.String())
235+
}
236+
var firstPayload struct {
237+
Data internalReplaceSpritzResponse `json:"data"`
238+
}
239+
if err := json.Unmarshal(rec.Body.Bytes(), &firstPayload); err != nil {
240+
t.Fatalf("failed to decode first replace response: %v", err)
241+
}
242+
if firstPayload.Data.Replacement.InstanceID == "" {
243+
t.Fatalf("expected a predicted replacement name, got %#v", firstPayload.Data)
244+
}
245+
if firstPayload.Data.Replacement.TargetRevision != targetRevision {
246+
t.Fatalf("expected target revision %q, got %#v", targetRevision, firstPayload.Data)
247+
}
248+
if firstPayload.Data.Replacement.Ready {
249+
t.Fatalf("expected predicted replacement to be unready")
250+
}
251+
if firstPayload.Data.Replayed {
252+
t.Fatalf("expected first request to be non-replayed")
253+
}
254+
255+
replayReq := httptest.NewRequest(http.MethodPost, "/api/internal/v1/spritzes/spritz-production/zeno-acme:replace", strings.NewReader(body))
256+
replayReq.Header.Set("Authorization", "Bearer spritz-internal-token")
257+
replayReq.Header.Set("Content-Type", "application/json")
258+
replayRec := httptest.NewRecorder()
259+
e.ServeHTTP(replayRec, replayReq)
260+
261+
if replayRec.Code != http.StatusAccepted {
262+
t.Fatalf("expected replay to stay accepted, got %d: %s", replayRec.Code, replayRec.Body.String())
263+
}
264+
var replayPayload struct {
265+
Data internalReplaceSpritzResponse `json:"data"`
266+
}
267+
if err := json.Unmarshal(replayRec.Body.Bytes(), &replayPayload); err != nil {
268+
t.Fatalf("failed to decode replay replace response: %v", err)
269+
}
270+
if replayPayload.Data.Replacement.InstanceID != firstPayload.Data.Replacement.InstanceID {
271+
t.Fatalf("expected replay to keep the same replacement identity, got first=%#v replay=%#v", firstPayload.Data, replayPayload.Data)
272+
}
273+
if !replayPayload.Data.Replayed {
274+
t.Fatalf("expected replay request to be marked replayed")
275+
}
276+
277+
actorID := replaceReservationActorIDForTest("spritz-production", "zeno-acme")
278+
record, found, err := s.idempotencyReservations().get(context.Background(), actorID, "replace-1")
279+
if err != nil {
280+
t.Fatalf("failed to load replace reservation: %v", err)
281+
}
282+
if !found {
283+
t.Fatalf("expected replace reservation to be stored")
284+
}
285+
if !record.completed {
286+
t.Fatalf("expected replace reservation to be completed after the first response")
287+
}
288+
if record.name != firstPayload.Data.Replacement.InstanceID {
289+
t.Fatalf("expected reservation name %q, got %#v", firstPayload.Data.Replacement.InstanceID, record)
290+
}
291+
}
292+
293+
func TestInternalReplaceSpritzBindingLifecycleRejectsIdempotencyConflicts(t *testing.T) {
294+
binding := &spritzv1.SpritzBinding{
295+
ObjectMeta: metav1.ObjectMeta{
296+
Name: "channel-installation-binding-1",
297+
Namespace: "spritz-production",
298+
},
299+
Spec: spritzv1.SpritzBindingSpec{
300+
BindingKey: "channel-installation-binding-1",
301+
DesiredRevision: "sha256:rev-1",
302+
Template: spritzv1.SpritzBindingTemplate{
303+
PresetID: "zeno",
304+
NamePrefix: "zeno",
305+
},
306+
},
307+
Status: spritzv1.SpritzBindingStatus{
308+
ObservedRevision: "sha256:rev-1",
309+
ActiveInstanceRef: &spritzv1.SpritzBindingInstanceRef{
310+
Namespace: "spritz-production",
311+
Name: "zeno-acme",
312+
Revision: "sha256:rev-1",
313+
Phase: "Ready",
314+
},
315+
CandidateInstanceRef: &spritzv1.SpritzBindingInstanceRef{
316+
Namespace: "spritz-production",
317+
Name: "zeno-replacement",
318+
Revision: "sha256:rev-2",
319+
Phase: "Provisioning",
320+
},
321+
},
322+
}
323+
source := &spritzv1.Spritz{
324+
ObjectMeta: metav1.ObjectMeta{
325+
Name: "zeno-acme",
326+
Namespace: "spritz-production",
327+
OwnerReferences: []metav1.OwnerReference{{
328+
APIVersion: spritzv1.GroupVersion.String(),
329+
Kind: "SpritzBinding",
330+
Name: binding.Name,
331+
}},
332+
},
333+
Status: spritzv1.SpritzStatus{Phase: "Ready"},
334+
}
335+
s := newInternalSpritzesTestServer(t, source)
336+
if err := s.client.Create(context.Background(), binding); err != nil {
337+
t.Fatalf("failed to create binding: %v", err)
338+
}
339+
e := echo.New()
340+
s.registerRoutes(e)
341+
342+
firstBody := `{
343+
"targetRevision": "sha256:rev-2",
344+
"idempotencyKey": "replace-1",
345+
"replacement": {
346+
"principal": {"id": "channel-gateway"},
347+
"request": {
348+
"presetId": "zeno",
349+
"ownerId": "user-123",
350+
"requestId": "replace-1",
351+
"source": "channel-gateway",
352+
"spec": {}
353+
}
354+
}
355+
}`
356+
firstReq := httptest.NewRequest(http.MethodPost, "/api/internal/v1/spritzes/spritz-production/zeno-acme:replace", strings.NewReader(firstBody))
357+
firstReq.Header.Set("Authorization", "Bearer spritz-internal-token")
358+
firstReq.Header.Set("Content-Type", "application/json")
359+
firstRec := httptest.NewRecorder()
360+
e.ServeHTTP(firstRec, firstReq)
361+
if firstRec.Code != http.StatusAccepted {
362+
t.Fatalf("expected first replace to succeed, got %d: %s", firstRec.Code, firstRec.Body.String())
363+
}
364+
365+
conflictingBody := `{
366+
"targetRevision": "sha256:rev-3",
367+
"idempotencyKey": "replace-1",
368+
"replacement": {
369+
"principal": {"id": "channel-gateway"},
370+
"request": {
371+
"presetId": "zeno",
372+
"ownerId": "user-123",
373+
"requestId": "replace-1-conflict",
374+
"source": "channel-gateway",
375+
"spec": {}
376+
}
377+
}
378+
}`
379+
conflictReq := httptest.NewRequest(http.MethodPost, "/api/internal/v1/spritzes/spritz-production/zeno-acme:replace", strings.NewReader(conflictingBody))
380+
conflictReq.Header.Set("Authorization", "Bearer spritz-internal-token")
381+
conflictReq.Header.Set("Content-Type", "application/json")
382+
conflictRec := httptest.NewRecorder()
383+
e.ServeHTTP(conflictRec, conflictReq)
384+
385+
if conflictRec.Code != http.StatusConflict {
386+
t.Fatalf("expected 409, got %d: %s", conflictRec.Code, conflictRec.Body.String())
387+
}
388+
if !strings.Contains(conflictRec.Body.String(), "different request") {
389+
t.Fatalf("expected idempotency conflict, got %s", conflictRec.Body.String())
390+
}
391+
392+
var stored spritzv1.SpritzBinding
393+
if err := s.client.Get(context.Background(), client.ObjectKey{Namespace: binding.Namespace, Name: binding.Name}, &stored); err != nil {
394+
t.Fatalf("failed to reload binding: %v", err)
395+
}
396+
if stored.Spec.DesiredRevision != "sha256:rev-2" {
397+
t.Fatalf("expected binding to keep the original desired revision, got %#v", stored.Spec)
398+
}
399+
}

0 commit comments

Comments
 (0)