Skip to content

Commit 981b709

Browse files
committed
WIP
Signed-off-by: Daniel Franz <dfranz@redhat.com>
1 parent 85e8cbf commit 981b709

File tree

4 files changed

+66
-26
lines changed

4 files changed

+66
-26
lines changed

.github/workflows/e2e.yaml

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ jobs:
2323

2424
e2e-kind:
2525
runs-on: ubuntu-latest
26+
outputs:
27+
alerts-found: ${{ steps.alert_output.outputs.alerts-found }}
2628
steps:
2729
- uses: actions/checkout@v5
2830
with:
@@ -34,7 +36,11 @@ jobs:
3436

3537
- name: Run e2e tests
3638
run: ARTIFACT_PATH=/tmp/artifacts E2E_SUMMARY_OUTPUT=$GITHUB_STEP_SUMMARY make test-e2e
37-
39+
- name: Mark for alerts
40+
id: alert_output
41+
if: |
42+
contains(github.step_summary, 'Firing Alerts')
43+
run: echo "alerts-found=true" >> "$GITHUB_OUTPUT"
3844
- uses: actions/upload-artifact@v4
3945
if: failure()
4046
with:
@@ -50,6 +56,8 @@ jobs:
5056

5157
experimental-e2e:
5258
runs-on: ubuntu-latest
59+
outputs:
60+
alerts-found: ${{ steps.alert_output.outputs.alerts-found }}
5361
steps:
5462
- uses: actions/checkout@v5
5563
with:
@@ -61,7 +69,11 @@ jobs:
6169

6270
- name: Run e2e tests
6371
run: ARTIFACT_PATH=/tmp/artifacts E2E_SUMMARY_OUTPUT=$GITHUB_STEP_SUMMARY make test-experimental-e2e
64-
72+
- name: Mark for alerts
73+
id: alert_output
74+
if: |
75+
contains(github.step_summary, 'Firing Alerts')
76+
run: echo "alerts-found=true" >> "$GITHUB_OUTPUT"
6577
- uses: actions/upload-artifact@v4
6678
if: failure()
6779
with:
@@ -111,3 +123,41 @@ jobs:
111123
name: upgrade-experimental-e2e-artifacts
112124
path: /tmp/artifacts/
113125

126+
report-performance:
127+
runs-on: ubuntu-latest
128+
needs: [ experimental-e2e, e2e-kind ]
129+
steps:
130+
- name: Find Previous Alert Comment
131+
id: find_comment
132+
uses: peter-evans/find-comment@v3
133+
with:
134+
issue-number: ${{ github.event.pull_request.number }}
135+
comment-author: 'github-actions[bot]'
136+
body-includes: '**Alerts Found**'
137+
138+
- name: Delete comment
139+
uses: detomarco/delete-comments@1.1.0
140+
if: |
141+
needs.e2e-kind.outputs.alerts-found == false &&
142+
needs.experimental-e2e.outputs.alerts-found == false
143+
with:
144+
comment-id: ${{ steps.find_comment.outputs.comment-id }}
145+
146+
- name: Post Failure Comment
147+
uses: peter-evans/create-or-update-comment@v4
148+
if: |
149+
needs.e2e-kind.outputs.alerts-found == true ||
150+
needs.experimental-e2e.outputs.alerts-found == true
151+
with:
152+
issue-number: ${{ github.event.pull_request.number }}
153+
comment-id: ${{ steps.find_comment.outputs.comment-id }}
154+
edit-mode: replace
155+
body: |
156+
**Alerts Found**
157+
158+
/hold
159+
160+
A hold has been placed on this PR due to performance alerts during the CI's e2e run.
161+
View the summary [here](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for detaills.
162+
163+
To acknowledge this warning and continue anyway, leave an `/unhold` comment.

internal/shared/util/testutils/summary.go

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,14 @@ type xychart struct {
4242
}
4343

4444
type githubSummary struct {
45-
client api.Client
46-
Pods []string
47-
alertsFiring bool
45+
client api.Client
46+
Pods []string
4847
}
4948

5049
func NewSummary(c api.Client, pods ...string) githubSummary {
5150
return githubSummary{
52-
client: c,
53-
Pods: pods,
54-
alertsFiring: false,
51+
client: c,
52+
Pods: pods,
5553
}
5654
}
5755

@@ -139,7 +137,6 @@ func (s *githubSummary) Alerts() (string, error) {
139137
switch a.State {
140138
case v1.AlertStateFiring:
141139
firingAlerts = append(firingAlerts, aConv)
142-
s.alertsFiring = true
143140
case v1.AlertStatePending:
144141
pendingAlerts = append(pendingAlerts, aConv)
145142
// Ignore AlertStateInactive; the alerts endpoint doesn't return them
@@ -176,34 +173,30 @@ func executeTemplate(templateFile string, obj any) (string, error) {
176173
// The markdown is template-driven; the summary methods are called from within the
177174
// template. This allows us to add or change queries (hopefully) without needing to
178175
// touch code. The summary will be output to a file supplied by the env target.
179-
func PrintSummary(path string) error {
176+
func PrintSummary(path string) {
180177
if path == "" {
181178
fmt.Printf("No summary output path specified; skipping")
182-
return nil
179+
return
183180
}
184181

185182
client, err := api.NewClient(api.Config{
186183
Address: defaultPromUrl,
187184
})
188185
if err != nil {
189186
fmt.Printf("warning: failed to initialize promQL client: %v", err)
190-
return nil
187+
return
191188
}
192189

193190
summary := NewSummary(client, "operator-controller", "catalogd")
194191
summaryMarkdown, err := executeTemplate(summaryTemplate, &summary)
195192
if err != nil {
196193
fmt.Printf("warning: failed to generate e2e test summary: %v", err)
197-
return nil
194+
return
198195
}
199196
err = os.WriteFile(path, []byte(summaryMarkdown), 0o600)
200197
if err != nil {
201198
fmt.Printf("warning: failed to write e2e test summary output to %s: %v", path, err)
202-
return nil
199+
return
203200
}
204201
fmt.Printf("Test summary output to %s successful\n", path)
205-
if summary.alertsFiring {
206-
return fmt.Errorf("performance alerts encountered during test run; please check e2e test summary for details")
207-
}
208-
return nil
209202
}

internal/shared/util/testutils/templates/alert.md.tmpl

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66
| State | {{ .State }} |
77
{{- end}}
88

9+
{{ if .FiringAlerts }}
910
### Firing Alerts
1011
{{ range .FiringAlerts }}
1112
{{ template "alert" .}}
12-
{{ end }}
13+
{{ end }}{{ end }}
14+
{{ if .PendingAlerts }}
1315
### Pending Alerts
1416
{{ range .PendingAlerts }}
1517
{{ template "alert" .}}
16-
{{ end }}
18+
{{ end }}{{ end }}

test/e2e/e2e_suite_test.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,7 @@ func TestMain(m *testing.M) {
4040
if path == "" {
4141
fmt.Printf("Note: E2E_SUMMARY_OUTPUT is unset; skipping summary generation")
4242
} else {
43-
err = utils.PrintSummary(path)
44-
if err != nil {
45-
// Fail the run if alerts are found
46-
fmt.Printf("%v", err)
47-
os.Exit(1)
48-
}
43+
utils.PrintSummary(path)
4944
}
5045
os.Exit(res)
5146
}

0 commit comments

Comments
 (0)