Skip to content

Commit 7c4de67

Browse files
committed
ToSquash: Remove useless error from the AlertGetter's return
1 parent 6792377 commit 7c4de67

3 files changed

Lines changed: 21 additions & 37 deletions

File tree

pkg/clusterconditions/promql/alerts.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
)
1717

1818
type Getter interface {
19-
Get(ctx context.Context) (prometheusv1.AlertsResult, error)
19+
Get(ctx context.Context) prometheusv1.AlertsResult
2020
}
2121

2222
func NewAlertGetter(promQLTarget clusterconditions.PromQLTarget) Getter {
@@ -38,13 +38,13 @@ type ocAlertGetter struct {
3838
lastRefresh time.Time
3939
}
4040

41-
func (o *ocAlertGetter) Get(ctx context.Context) (prometheusv1.AlertsResult, error) {
41+
func (o *ocAlertGetter) Get(ctx context.Context) prometheusv1.AlertsResult {
4242
if time.Now().After(o.lastRefresh.Add(o.expiration)) {
4343
if err := o.refresh(ctx); err != nil {
4444
klog.Errorf("Failed to refresh alerts, using stale cache instead: %v", err)
4545
}
4646
}
47-
return o.cached, nil
47+
return o.cached
4848
}
4949

5050
func (o *ocAlertGetter) refresh(ctx context.Context) error {

pkg/cvo/availableupdates.go

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -519,18 +519,15 @@ func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, tran
519519
}
520520

521521
type AlertGetter interface {
522-
Get(ctx context.Context) (prometheusv1.AlertsResult, error)
522+
Get(ctx context.Context) prometheusv1.AlertsResult
523523
}
524524

525-
func (u *availableUpdates) evaluateAlertRisks(ctx context.Context) ([]configv1.ConditionalUpdateRisk, error) {
525+
func (u *availableUpdates) evaluateAlertRisks(ctx context.Context) []configv1.ConditionalUpdateRisk {
526526
if u == nil || u.AlertGetter == nil {
527-
return nil, nil
528-
}
529-
alertsResult, err := u.AlertGetter.Get(ctx)
530-
if err != nil {
531-
return nil, fmt.Errorf("failed to get alerts: %w", err)
527+
return nil
532528
}
533-
return alertsToRisks(alertsResult.Alerts), nil
529+
alertsResult := u.AlertGetter.Get(ctx)
530+
return alertsToRisks(alertsResult.Alerts)
534531
}
535532

536533
func alertsToRisks(alerts []prometheusv1.Alert) []configv1.ConditionalUpdateRisk {
@@ -683,12 +680,7 @@ func (u *availableUpdates) evaluateConditionalUpdates(ctx context.Context) {
683680

684681
var alertRisks []configv1.ConditionalUpdateRisk
685682
if u.ShouldReconcileAcceptRisks() {
686-
if evaluated, err := u.evaluateAlertRisks(ctx); err != nil {
687-
klog.Errorf("Failed to evaluate alert conditions: %v", err)
688-
} else {
689-
alertRisks = evaluated
690-
}
691-
683+
alertRisks = u.evaluateAlertRisks(ctx)
692684
}
693685
u.attachAlertRisksToUpdates(alertRisks)
694686

pkg/cvo/availableupdates_test.go

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -960,22 +960,23 @@ func Test_loadRiskConditions(t *testing.T) {
960960
type mockAlertGetter struct {
961961
ret prometheusv1.AlertsResult
962962
jsonFile string
963+
t *testing.T
963964
}
964965

965-
func (m *mockAlertGetter) Get(_ context.Context) (prometheusv1.AlertsResult, error) {
966+
func (m *mockAlertGetter) Get(_ context.Context) prometheusv1.AlertsResult {
966967
var ret prometheusv1.AlertsResult
967968
if m.jsonFile != "" {
968969
data, err := os.ReadFile(m.jsonFile)
969970
if err != nil {
970-
return ret, err
971+
m.t.Fatal(err)
971972
}
972973
err = json.Unmarshal(data, &ret)
973974
if err != nil {
974-
return ret, err
975+
m.t.Fatal(err)
975976
}
976-
return ret, nil
977+
return ret
977978
}
978-
return m.ret, nil
979+
return m.ret
979980
}
980981

981982
func Test_evaluateAlertConditions(t *testing.T) {
@@ -995,6 +996,7 @@ func Test_evaluateAlertConditions(t *testing.T) {
995996
name: "basic case",
996997
u: &availableUpdates{
997998
AlertGetter: &mockAlertGetter{
999+
t: t,
9981000
ret: prometheusv1.AlertsResult{
9991001
Alerts: []prometheusv1.Alert{
10001002
{
@@ -1088,6 +1090,7 @@ func Test_evaluateAlertConditions(t *testing.T) {
10881090
name: "from file",
10891091
u: &availableUpdates{
10901092
AlertGetter: &mockAlertGetter{
1093+
t: t,
10911094
jsonFile: filepath.Join("testdata", "alerts.json"),
10921095
},
10931096
},
@@ -1110,20 +1113,9 @@ func Test_evaluateAlertConditions(t *testing.T) {
11101113
}
11111114
for _, tt := range tests {
11121115
t.Run(tt.name, func(t *testing.T) {
1113-
actual, actualError := tt.u.evaluateAlertRisks(context.TODO())
1114-
if diff := cmp.Diff(tt.expectedErr, actualError, cmp.Comparer(func(x, y error) bool {
1115-
if x == nil || y == nil {
1116-
return x == nil && y == nil
1117-
}
1118-
return x.Error() == y.Error()
1119-
})); diff != "" {
1120-
t.Errorf("error mismatch (-want +got):\n%s", diff)
1121-
}
1122-
1123-
if actualError == nil {
1124-
if diff := cmp.Diff(tt.expectedAlertRisks, actual); diff != "" {
1125-
t.Errorf("AlertRisks mismatch (-want +got):\n%s", diff)
1126-
}
1116+
actual := tt.u.evaluateAlertRisks(context.TODO())
1117+
if diff := cmp.Diff(tt.expectedAlertRisks, actual); diff != "" {
1118+
t.Errorf("AlertRisks mismatch (-want +got):\n%s", diff)
11271119
}
11281120
})
11291121
}
@@ -1515,7 +1507,7 @@ func Test_evaluateConditionalUpdates(t *testing.T) {
15151507
ConditionalUpdates: tt.conditionUpdates,
15161508
ShouldReconcileAcceptRisks: tt.shouldReconcileAcceptRisks,
15171509
ConditionRegistry: conditionRegistry,
1518-
AlertGetter: &mockAlertGetter{jsonFile: filepath.Join("testdata", "alerts.json")},
1510+
AlertGetter: &mockAlertGetter{t: t, jsonFile: filepath.Join("testdata", "alerts.json")},
15191511
}
15201512
u.evaluateConditionalUpdates(context.TODO())
15211513
if diff := cmp.Diff(tt.expected, u.Updates); diff != "" {

0 commit comments

Comments
 (0)