Skip to content

Commit 4efee35

Browse files
vrutkovsvrutkovs
committed
Refactor vmclusterinfo fetching, add unit tests
1 parent 8c2c745 commit 4efee35

File tree

2 files changed

+211
-17
lines changed

2 files changed

+211
-17
lines changed

‎internal/controller/operator/factory/vmdistributedcluster/vmdistributedcluster.go‎

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -99,25 +99,10 @@ func CreateOrUpdate(ctx context.Context, cr *vmv1alpha1.VMDistributedCluster, rc
9999
// Ensure that all vmusers have a read rule for vmcluster and record vmcluster info in VMDistributedCluster status
100100
cr.Status.VMClusterInfo = make([]vmv1alpha1.VMClusterStatus, len(vmClusters))
101101
for i, vmCluster := range vmClusters {
102-
vmClusterInfo := vmv1alpha1.VMClusterStatus{
103-
VMClusterName: vmCluster.Name,
104-
Generation: vmCluster.Generation,
105-
}
106-
107-
// Find targetref for the cluster in VMUsers
108-
ref, err := findVMUserReadRuleForVMCluster(vmUserObjs, vmCluster)
102+
vmClusterInfo, err := setVMClusterInfo(vmCluster, vmUserObjs, currentCRStatus)
109103
if err != nil {
110-
// Check if targetref already set in vmcluster status
111-
// Exit early if targetref is already set
112-
idx := slices.IndexFunc(currentCRStatus.VMClusterInfo, func(info vmv1alpha1.VMClusterStatus) bool {
113-
return info.VMClusterName == vmCluster.Name
114-
})
115-
if idx == -1 {
116-
return fmt.Errorf("failed to find the rule for vmcluster %s: %w", vmCluster.Name, err), false
117-
}
118-
ref = &currentCRStatus.VMClusterInfo[idx].TargetRef
104+
return err, false
119105
}
120-
vmClusterInfo.TargetRef = *ref.DeepCopy()
121106
cr.Status.VMClusterInfo[i] = vmClusterInfo
122107
}
123108
cr.Status.Zones = cr.Spec.Zones
@@ -363,6 +348,27 @@ func fetchVMUsers(ctx context.Context, rclient client.Client, namespace string,
363348
return vmUsers, nil
364349
}
365350

351+
func setVMClusterInfo(vmCluster *vmv1beta1.VMCluster, vmUserObjs []*vmv1beta1.VMUser, currentCRStatus *vmv1alpha1.VMDistributedClusterStatus) (vmv1alpha1.VMClusterStatus, error) {
352+
vmClusterInfo := vmv1alpha1.VMClusterStatus{
353+
VMClusterName: vmCluster.Name,
354+
Generation: vmCluster.Generation,
355+
}
356+
ref, err := findVMUserReadRuleForVMCluster(vmUserObjs, vmCluster)
357+
if err != nil {
358+
// Check if targetref already set in vmcluster status
359+
// Exit early if targetref is already set
360+
idx := slices.IndexFunc(currentCRStatus.VMClusterInfo, func(info vmv1alpha1.VMClusterStatus) bool {
361+
return info.VMClusterName == vmCluster.Name
362+
})
363+
if idx == -1 {
364+
return vmClusterInfo, fmt.Errorf("failed to find the rule for vmcluster %s: %w", vmCluster.Name, err)
365+
}
366+
ref = &currentCRStatus.VMClusterInfo[idx].TargetRef
367+
}
368+
vmClusterInfo.TargetRef = *ref
369+
return vmClusterInfo, nil
370+
}
371+
366372
func fetchVMAgent(ctx context.Context, rclient client.Client, namespace string, ref corev1.LocalObjectReference) (*vmv1beta1.VMAgent, error) {
367373
if ref.Name == "" {
368374
return nil, errors.New("global vmagent name is not specified")

‎internal/controller/operator/factory/vmdistributedcluster/vmdistributedcluster_test.go‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,3 +1298,191 @@ func TestUpdateVMUserTargetRefs(t *testing.T) {
12981298

12991299
}
13001300
}
1301+
1302+
func TestSetVMClusterInfo(t *testing.T) {
1303+
type args struct {
1304+
vmCluster *vmv1beta1.VMCluster
1305+
vmUserObjs []*vmv1beta1.VMUser
1306+
currentCRStatus *vmv1alpha1.VMDistributedClusterStatus
1307+
}
1308+
tests := []struct {
1309+
name string
1310+
args args
1311+
want vmv1alpha1.VMClusterStatus
1312+
wantErr bool
1313+
}{
1314+
{
1315+
name: "successfully set VMClusterInfo with existing VMUser",
1316+
args: args{
1317+
vmCluster: &vmv1beta1.VMCluster{
1318+
ObjectMeta: metav1.ObjectMeta{
1319+
Name: "test-cluster",
1320+
Namespace: "default",
1321+
},
1322+
Spec: vmv1beta1.VMClusterSpec{},
1323+
},
1324+
vmUserObjs: []*vmv1beta1.VMUser{
1325+
{
1326+
ObjectMeta: metav1.ObjectMeta{
1327+
Name: "test-user",
1328+
Namespace: "default",
1329+
},
1330+
Spec: vmv1beta1.VMUserSpec{
1331+
TargetRefs: []vmv1beta1.TargetRef{
1332+
{
1333+
CRD: &vmv1beta1.CRDRef{
1334+
Name: "test-cluster",
1335+
Kind: "VMCluster/vmselect",
1336+
Namespace: "default",
1337+
},
1338+
TargetPathSuffix: "/select/1",
1339+
},
1340+
},
1341+
},
1342+
},
1343+
},
1344+
currentCRStatus: &vmv1alpha1.VMDistributedClusterStatus{},
1345+
},
1346+
want: vmv1alpha1.VMClusterStatus{
1347+
VMClusterName: "test-cluster",
1348+
Generation: 0,
1349+
TargetRef: vmv1beta1.TargetRef{
1350+
CRD: &vmv1beta1.CRDRef{
1351+
Name: "test-cluster",
1352+
Kind: "VMCluster/vmselect",
1353+
Namespace: "default",
1354+
},
1355+
TargetPathSuffix: "/select/1",
1356+
},
1357+
},
1358+
wantErr: false,
1359+
},
1360+
{
1361+
name: "error when no VMUser found for VMCluster and not in status",
1362+
args: args{
1363+
vmCluster: &vmv1beta1.VMCluster{
1364+
ObjectMeta: metav1.ObjectMeta{
1365+
Name: "test-cluster-no-user",
1366+
Namespace: "default",
1367+
},
1368+
Spec: vmv1beta1.VMClusterSpec{},
1369+
},
1370+
vmUserObjs: []*vmv1beta1.VMUser{},
1371+
currentCRStatus: &vmv1alpha1.VMDistributedClusterStatus{},
1372+
},
1373+
want: vmv1alpha1.VMClusterStatus{
1374+
VMClusterName: "test-cluster-no-user",
1375+
Generation: 0,
1376+
},
1377+
wantErr: true,
1378+
},
1379+
{
1380+
name: "retrieve TargetRef from status when no VMUser found",
1381+
args: args{
1382+
vmCluster: &vmv1beta1.VMCluster{
1383+
ObjectMeta: metav1.ObjectMeta{
1384+
Name: "test-cluster-from-status",
1385+
Namespace: "default",
1386+
},
1387+
Spec: vmv1beta1.VMClusterSpec{},
1388+
},
1389+
vmUserObjs: []*vmv1beta1.VMUser{},
1390+
currentCRStatus: &vmv1alpha1.VMDistributedClusterStatus{
1391+
VMClusterInfo: []vmv1alpha1.VMClusterStatus{
1392+
{
1393+
VMClusterName: "test-cluster-from-status",
1394+
TargetRef: vmv1beta1.TargetRef{
1395+
CRD: &vmv1beta1.CRDRef{
1396+
Name: "test-cluster-from-status",
1397+
Kind: "VMCluster",
1398+
Namespace: "default",
1399+
},
1400+
},
1401+
},
1402+
},
1403+
},
1404+
},
1405+
want: vmv1alpha1.VMClusterStatus{
1406+
VMClusterName: "test-cluster-from-status",
1407+
Generation: 0,
1408+
TargetRef: vmv1beta1.TargetRef{
1409+
CRD: &vmv1beta1.CRDRef{
1410+
Name: "test-cluster-from-status",
1411+
Kind: "VMCluster",
1412+
Namespace: "default",
1413+
},
1414+
},
1415+
},
1416+
wantErr: false,
1417+
},
1418+
{
1419+
name: "retrieve TargetRef from status when VMUser no longer has matching entry",
1420+
args: args{
1421+
vmCluster: &vmv1beta1.VMCluster{
1422+
ObjectMeta: metav1.ObjectMeta{
1423+
Name: "test-cluster-from-status",
1424+
Namespace: "default",
1425+
},
1426+
Spec: vmv1beta1.VMClusterSpec{},
1427+
},
1428+
vmUserObjs: []*vmv1beta1.VMUser{
1429+
{
1430+
ObjectMeta: metav1.ObjectMeta{
1431+
Name: "test-user",
1432+
Namespace: "default",
1433+
},
1434+
Spec: vmv1beta1.VMUserSpec{
1435+
TargetRefs: []vmv1beta1.TargetRef{
1436+
{
1437+
CRD: &vmv1beta1.CRDRef{
1438+
Name: "some-other-cluster",
1439+
Kind: "VMCluster/vmselect",
1440+
Namespace: "default",
1441+
},
1442+
TargetPathSuffix: "/select/1",
1443+
},
1444+
},
1445+
},
1446+
},
1447+
},
1448+
currentCRStatus: &vmv1alpha1.VMDistributedClusterStatus{
1449+
VMClusterInfo: []vmv1alpha1.VMClusterStatus{
1450+
{
1451+
VMClusterName: "test-cluster-from-status",
1452+
TargetRef: vmv1beta1.TargetRef{
1453+
CRD: &vmv1beta1.CRDRef{
1454+
Name: "test-cluster-from-status",
1455+
Kind: "VMCluster",
1456+
Namespace: "default",
1457+
},
1458+
},
1459+
},
1460+
},
1461+
},
1462+
},
1463+
want: vmv1alpha1.VMClusterStatus{
1464+
VMClusterName: "test-cluster-from-status",
1465+
Generation: 0,
1466+
TargetRef: vmv1beta1.TargetRef{
1467+
CRD: &vmv1beta1.CRDRef{
1468+
Name: "test-cluster-from-status",
1469+
Kind: "VMCluster",
1470+
Namespace: "default",
1471+
},
1472+
},
1473+
},
1474+
wantErr: false,
1475+
},
1476+
}
1477+
for _, tt := range tests {
1478+
t.Run(tt.name, func(t *testing.T) {
1479+
got, err := setVMClusterInfo(tt.args.vmCluster, tt.args.vmUserObjs, tt.args.currentCRStatus)
1480+
if tt.wantErr {
1481+
assert.Error(t, err)
1482+
} else {
1483+
assert.NoError(t, err)
1484+
}
1485+
assert.Equal(t, got, tt.want)
1486+
})
1487+
}
1488+
}

0 commit comments

Comments
 (0)