Skip to content

Commit

Permalink
Since emitClusterSync is run unconditionally, even in environment
Browse files Browse the repository at this point in the history
without Hive, make sure that it doesn't panic when the monitor's
hive.ClusterManager is nil

Compare to preexisting code in emitHiveRegistrationStatus
  • Loading branch information
kimorris27 authored and rhamitarora committed Jan 21, 2025
1 parent a0e64cf commit af250d5
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 16 deletions.
6 changes: 6 additions & 0 deletions pkg/monitor/cluster/clustersync.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ import (
)

func (mon *Monitor) emitClusterSync(ctx context.Context) error {
if mon.hiveClusterManager == nil {
// TODO(hive): remove this once we have Hive everywhere
mon.log.Info("skipping: no hive cluster manager")
return nil
}

clusterSync, err := mon.hiveClusterManager.GetClusterSync(ctx, mon.doc)
if err != nil {
return err
Expand Down
58 changes: 42 additions & 16 deletions pkg/monitor/cluster/clustersync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,28 +10,39 @@ import (
"time"

hivev1alpha1 "github.com/openshift/hive/apis/hiveinternal/v1alpha1"
"github.com/sirupsen/logrus"
"github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/assert"
"go.uber.org/mock/gomock"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/Azure/ARO-RP/pkg/hive"
mock_hive "github.com/Azure/ARO-RP/pkg/util/mocks/hive"
mock_metrics "github.com/Azure/ARO-RP/pkg/util/mocks/metrics"
)

func TestEmitClusterSync(t *testing.T) {
for _, tt := range []struct {
name string
clusterSync *hivev1alpha1.ClusterSync
getClusterSyncErr error
expectedError error
expectedGauges []struct {
name string
withClusterManager bool
clusterSync *hivev1alpha1.ClusterSync
getClusterSyncErr error
expectedError error
expectedGauges []struct {
name string
value int64
labels map[string]string
}
wantLog string
}{
{
name: "SyncSets and SelectorSyncSets have elements",
name: "Don't do anything because Monitor does not have a Hive ClusterManager",
withClusterManager: false,
wantLog: "skipping: no hive cluster manager",
},
{
name: "SyncSets and SelectorSyncSets have elements",
withClusterManager: true,
clusterSync: &hivev1alpha1.ClusterSync{
Status: hivev1alpha1.ClusterSyncStatus{
SyncSets: []hivev1alpha1.SyncStatus{
Expand Down Expand Up @@ -81,7 +92,8 @@ func TestEmitClusterSync(t *testing.T) {
},
},
{
name: "SyncSets and SelectorSyncSets have success and failure",
name: "SyncSets and SelectorSyncSets have success and failure",
withClusterManager: true,
clusterSync: &hivev1alpha1.ClusterSync{
Status: hivev1alpha1.ClusterSyncStatus{
SyncSets: []hivev1alpha1.SyncStatus{
Expand Down Expand Up @@ -131,7 +143,8 @@ func TestEmitClusterSync(t *testing.T) {
},
},
{
name: "SyncSets and SelectorSyncSets are nil",
name: "SyncSets and SelectorSyncSets are nil",
withClusterManager: true,
clusterSync: &hivev1alpha1.ClusterSync{
Status: hivev1alpha1.ClusterSyncStatus{
SyncSets: nil,
Expand All @@ -146,9 +159,10 @@ func TestEmitClusterSync(t *testing.T) {
}{},
},
{
name: "GetSyncSetResources returns error",
getClusterSyncErr: errors.New("some error"),
expectedError: errors.New("some error"),
name: "GetSyncSetResources returns error",
withClusterManager: true,
getClusterSyncErr: errors.New("some error"),
expectedError: errors.New("some error"),
expectedGauges: []struct {
name string
value int64
Expand All @@ -160,24 +174,36 @@ func TestEmitClusterSync(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

mockHiveClusterManager := mock_hive.NewMockClusterManager(ctrl)
ctx := context.Background()

var mockHiveClusterManager hive.ClusterManager
if tt.withClusterManager {
_mockHiveClusterManager := mock_hive.NewMockClusterManager(ctrl)
_mockHiveClusterManager.EXPECT().GetClusterSync(ctx, gomock.Any()).Return(tt.clusterSync, tt.getClusterSyncErr).AnyTimes()
mockHiveClusterManager = _mockHiveClusterManager
}

m := mock_metrics.NewMockEmitter(ctrl)
logger, hook := test.NewNullLogger()
log := logrus.NewEntry(logger)

mockMonitor := &Monitor{
hiveClusterManager: mockHiveClusterManager,
m: m,
log: log,
}

ctx := context.Background()

mockHiveClusterManager.EXPECT().GetClusterSync(ctx, mockMonitor.doc).Return(tt.clusterSync, tt.getClusterSyncErr).AnyTimes()

for _, gauge := range tt.expectedGauges {
m.EXPECT().EmitGauge(gauge.name, gauge.value, gauge.labels).Times(1)
}

err := mockMonitor.emitClusterSync(ctx)
assert.Equal(t, tt.expectedError, err)

if tt.wantLog != "" {
x := hook.LastEntry()
assert.Equal(t, tt.wantLog, x.Message)
}
})
}
}

0 comments on commit af250d5

Please sign in to comment.