Skip to content

Commit cb6fc6e

Browse files
killianmuldoonsbueringer
authored andcommitted
remove status and message aggregation
1 parent ab1d6ad commit cb6fc6e

File tree

2 files changed

+29
-48
lines changed

2 files changed

+29
-48
lines changed

internal/runtime/client/client.go

Lines changed: 7 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -202,39 +202,19 @@ func (c *client) CallAllExtensions(ctx context.Context, hook runtimecatalog.Hook
202202
}
203203

204204
// Aggregate all responses into a single object.
205-
response = aggregateResponses(responses)
205+
aggregateSuccessfulResponses(response, responses)
206206

207207
if err != nil {
208208
return errors.Wrap(err, "failed to aggregate responses")
209209
}
210210
return nil
211211
}
212212

213-
func aggregateResponses(responses map[string]runtimehooksv1.ResponseObject) runtimehooksv1.ResponseObject {
214-
var aggregated runtimehooksv1.ResponseObject
215-
216-
for name, resp := range responses {
217-
// If this is the first response use it as the aggregated response, format the message correctly and continue.
218-
if aggregated == nil {
219-
aggregated = resp
220-
if resp.GetMessage() != "" {
221-
aggregated.SetMessage(fmt.Sprintf("%s: %s", name, resp.GetMessage()))
222-
}
223-
continue
224-
}
225-
226-
if resp.GetMessage() != "" {
227-
// Add a new line if the aggregated message is not empty.
228-
if aggregated.GetMessage() != "" {
229-
aggregated.SetMessage(fmt.Sprintf("%s\n", aggregated.GetMessage()))
230-
}
231-
aggregated.SetMessage(fmt.Sprintf("%s%s: %s", aggregated.GetMessage(), name, resp.GetMessage()))
232-
}
233-
// If any single hook fails the overall Response status is Failure.
234-
if resp.GetStatus() == runtimehooksv1.ResponseStatusFailure {
235-
aggregated.SetStatus(runtimehooksv1.ResponseStatusFailure)
236-
}
237-
213+
func aggregateSuccessfulResponses(aggregated runtimehooksv1.ResponseObject, responses map[string]runtimehooksv1.ResponseObject) {
214+
// At this point the Status should always be ResponseStatusSuccess and the Message should be empty.
215+
aggregated.SetMessage("")
216+
aggregated.SetStatus(runtimehooksv1.ResponseStatusSuccess)
217+
for _, resp := range responses {
238218
if retryable, ok := resp.(runtimehooksv1.Retryable); ok {
239219
if aggregatedRetryable, aggregatedOK := aggregated.(runtimehooksv1.Retryable); aggregatedOK {
240220
aggregatedRetryable.SetRetryAfterSeconds(
@@ -243,7 +223,7 @@ func aggregateResponses(responses map[string]runtimehooksv1.ResponseObject) runt
243223
}
244224
}
245225
}
246-
return aggregated
226+
fmt.Print(aggregated)
247227
}
248228

249229
func lowestNonZeroRetryAfterSeconds(i, j int32) int32 {

internal/runtime/client/client_test.go

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -710,60 +710,61 @@ func Test_aggregateResponses(t *testing.T) {
710710
{
711711
name: "Return input value with only one response",
712712
responses: map[string]runtimehooksv1.ResponseObject{
713-
"first": fakeResponse(runtimehooksv1.ResponseStatusFailure, "This is a failure of a hook", 5),
713+
"first": fakeResponse(5),
714714
},
715-
want: fakeResponse(runtimehooksv1.ResponseStatusFailure, "first: This is a failure of a hook", 5),
715+
want: fakeResponse(5),
716716
},
717717
{
718718
name: "Success if no response is a failure",
719719
responses: map[string]runtimehooksv1.ResponseObject{
720-
"first": fakeResponse(runtimehooksv1.ResponseStatusSuccess, "", 5),
721-
"second": fakeResponse(runtimehooksv1.ResponseStatusSuccess, "", 1),
722-
"third": fakeResponse(runtimehooksv1.ResponseStatusSuccess, "", 2),
723-
"fourth": fakeResponse(runtimehooksv1.ResponseStatusSuccess, "a failure", 5),
720+
"first": fakeResponse(5),
721+
"second": fakeResponse(1),
722+
"third": fakeResponse(2),
723+
"fourth": fakeResponse(5),
724724
},
725-
want: fakeResponse(runtimehooksv1.ResponseStatusSuccess, "fourth: a failure", 1),
725+
want: fakeResponse(1),
726726
},
727727
{
728728
name: "Failure if any response is a failure",
729729
responses: map[string]runtimehooksv1.ResponseObject{
730-
"first": fakeResponse(runtimehooksv1.ResponseStatusSuccess, "", 5),
731-
"second": fakeResponse(runtimehooksv1.ResponseStatusSuccess, "", 1),
732-
"third": fakeResponse(runtimehooksv1.ResponseStatusSuccess, "", 2),
733-
"fourth": fakeResponse(runtimehooksv1.ResponseStatusFailure, "a failure", 5),
730+
"first": fakeResponse(5),
731+
"second": fakeResponse(1),
732+
"third": fakeResponse(2),
733+
"fourth": fakeResponse(5),
734734
},
735-
want: fakeResponse(runtimehooksv1.ResponseStatusFailure, "fourth: a failure", 1),
735+
want: fakeResponse(1),
736736
},
737737

738738
{
739739
name: "Return lowest non-zero response for retry seconds",
740740
responses: map[string]runtimehooksv1.ResponseObject{
741-
"first": fakeResponse(runtimehooksv1.ResponseStatusFailure, "This is a failure of a hook", 5),
742-
"second": fakeResponse(runtimehooksv1.ResponseStatusFailure, "another failure", 1),
741+
"first": fakeResponse(5),
742+
"second": fakeResponse(1),
743743
},
744-
want: fakeResponse(runtimehooksv1.ResponseStatusFailure, "first: This is a failure of a hook\nsecond: another failure", 1),
744+
want: fakeResponse(1),
745745
},
746746
}
747747
for _, tt := range tests {
748748
t.Run(tt.name, func(t *testing.T) {
749-
got := aggregateResponses(tt.responses)
750-
if !reflect.DeepEqual(got, tt.want) {
751-
t.Errorf("aggregateResponses() got = %v, want %v", got, tt.want)
749+
response := &fakev1alpha1.FakeRetryableResponse{TypeMeta: metav1.TypeMeta{Kind: "FakeResponse", APIVersion: "v1alpha1"}}
750+
aggregateSuccessfulResponses(response, tt.responses)
751+
if !reflect.DeepEqual(response, tt.want) {
752+
t.Errorf("aggregateSuccessfulResponses() got = %v, want %v", response, tt.want)
752753
}
753754
})
754755
}
755756
}
756757

757-
func fakeResponse(status runtimehooksv1.ResponseStatus, message string, retryAfterSeconds int32) *fakev1alpha1.FakeRetryableResponse {
758+
func fakeResponse(retryAfterSeconds int32) *fakev1alpha1.FakeRetryableResponse {
758759
return &fakev1alpha1.FakeRetryableResponse{
759760
TypeMeta: metav1.TypeMeta{
760761
Kind: "FakeResponse",
761762
APIVersion: "v1alpha1",
762763
},
763764
RetryableResponse: runtimehooksv1.RetryableResponse{
764765
CommonResponse: runtimehooksv1.CommonResponse{
765-
Message: message,
766-
Status: status,
766+
Message: "",
767+
Status: runtimehooksv1.ResponseStatusSuccess,
767768
},
768769
RetryAfterSeconds: retryAfterSeconds,
769770
},

0 commit comments

Comments
 (0)