Skip to content

Commit

Permalink
some fixes
Browse files Browse the repository at this point in the history 
Signed-off-by: Aleksandr Zimin <[email protected]>
  • Loading branch information
@AleksZimin
AleksZimin committed Nov 4, 2024
1 parent 26b3764 commit 006596b
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 35 deletions.
9 changes: 6 additions & 3 deletions images/agent/src/pkg/controller/block_device.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func BlockDeviceReconcile(ctx context.Context, cl client.Client, log logger.Logg
return false
}

apiBlockDevices, err := GetAPIBlockDevices(ctx, cl, metrics, nil)
apiBlockDevices, err := GetAPIBlockDevices(ctx, cl, metrics, nil, "")
if err != nil {
log.Error(err, "[RunBlockDeviceController] unable to GetAPIBlockDevices")
return true
Expand Down Expand Up @@ -164,8 +164,8 @@ func hasBlockDeviceDiff(blockDevice v1alpha1.BlockDevice, candidate internal.Blo
}

// GetAPIBlockDevices returns map of BlockDevice resources with BlockDevice as a key. You might specify a selector to get a subset or
// leave it as nil to get all the resources.
func GetAPIBlockDevices(ctx context.Context, cl client.Client, metrics monitoring.Metrics, selector *metav1.LabelSelector) (map[string]v1alpha1.BlockDevice, error) {
// leave it as nil to get all the resources. Also you can specify a node name to get only resources that are related to the node. If nodeName is empty, all resources that match the selector will be returned.
func GetAPIBlockDevices(ctx context.Context, cl client.Client, metrics monitoring.Metrics, selector *metav1.LabelSelector, nodeName string) (map[string]v1alpha1.BlockDevice, error) {
list := &v1alpha1.BlockDeviceList{}
s, err := metav1.LabelSelectorAsSelector(selector)
if err != nil {
Expand All @@ -185,6 +185,9 @@ func GetAPIBlockDevices(ctx context.Context, cl client.Client, metrics monitorin

result := make(map[string]v1alpha1.BlockDevice, len(list.Items))
for _, item := range list.Items {
if nodeName != "" && item.Status.NodeName != nodeName {
continue
}
result[item.Name] = item
}

Expand Down
69 changes: 54 additions & 15 deletions images/agent/src/pkg/controller/block_device_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func TestBlockDeviceCtrl(t *testing.T) {
},
}

actualBd, err := GetAPIBlockDevices(ctx, cl, metrics, lvg.Spec.BlockDeviceSelector)
actualBd, err := GetAPIBlockDevices(ctx, cl, metrics, lvg.Spec.BlockDeviceSelector, lvg.Spec.Local.NodeName)
if assert.NoError(t, err) {
assert.Equal(t, 2, len(actualBd))

Expand All @@ -132,42 +132,76 @@ func TestBlockDeviceCtrl(t *testing.T) {
assert.False(t, ok)
}
})

t.Run("bds_exist_only_match_labels_return_bds", func(t *testing.T) {
const (
name1 = "name11"
name2 = "name22"
name3 = "name33"
hostName = "test-host"
name1 = "name11"
name2 = "name22"
name3 = "name33"
name4 = "name44"
hostName = "test-host"
otherHostName = "other-host"
)

bds := []v1alpha1.BlockDevice{
{
ObjectMeta: metav1.ObjectMeta{
Name: name1,
Labels: map[string]string{
"kubernetes.io/hostname": hostName,
"kubernetes.io/metadata.name": name1,
"kubernetes.io/hostname": hostName,
"kubernetes.io/metadata.name": name1,
"status.blockdevice.storage.deckhouse.io/size": "1G",
},
},
Status: v1alpha1.BlockDeviceStatus{
Size: resource.MustParse("1G"),
NodeName: hostName,
Consumable: true,
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: name2,
Labels: map[string]string{
"kubernetes.io/hostname": hostName,
"kubernetes.io/metadata.name": name2,
"kubernetes.io/hostname": hostName,
"kubernetes.io/metadata.name": name2,
"status.blockdevice.storage.deckhouse.io/size": "1G",
},
},
Status: v1alpha1.BlockDeviceStatus{
Size: resource.MustParse("1G"),
NodeName: hostName,
Consumable: true,
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: name3,
Labels: map[string]string{
"kubernetes.io/hostname": "other-host",
"kubernetes.io/metadata.name": name3,
"kubernetes.io/hostname": hostName,
"kubernetes.io/metadata.name": name3,
"status.blockdevice.storage.deckhouse.io/size": "2G",
},
},
Status: v1alpha1.BlockDeviceStatus{
Size: resource.MustParse("1G"),
NodeName: hostName,
Consumable: true,
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: name4,
Labels: map[string]string{
"kubernetes.io/hostname": otherHostName,
"kubernetes.io/metadata.name": name4,
"status.blockdevice.storage.deckhouse.io/size": "1G",
},
},
Status: v1alpha1.BlockDeviceStatus{
Size: resource.MustParse("1G"),
NodeName: otherHostName,
Consumable: true,
},
},
}

Expand All @@ -190,12 +224,15 @@ func TestBlockDeviceCtrl(t *testing.T) {
lvg := &v1alpha1.LVMVolumeGroup{
Spec: v1alpha1.LVMVolumeGroupSpec{
BlockDeviceSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{"kubernetes.io/hostname": hostName},
MatchLabels: map[string]string{"status.blockdevice.storage.deckhouse.io/size": "1G"},
},
Local: v1alpha1.LVMVolumeGroupLocalSpec{
NodeName: hostName,
},
},
}

actualBd, err := GetAPIBlockDevices(ctx, cl, metrics, lvg.Spec.BlockDeviceSelector)
actualBd, err := GetAPIBlockDevices(ctx, cl, metrics, lvg.Spec.BlockDeviceSelector, lvg.Spec.Local.NodeName)
if assert.NoError(t, err) {
assert.Equal(t, 2, len(actualBd))

Expand All @@ -205,6 +242,8 @@ func TestBlockDeviceCtrl(t *testing.T) {
assert.True(t, ok)
_, ok = actualBd[name3]
assert.False(t, ok)
_, ok = actualBd[name4]
assert.False(t, ok)
}
})

Expand Down Expand Up @@ -276,7 +315,7 @@ func TestBlockDeviceCtrl(t *testing.T) {
},
}

actualBd, err := GetAPIBlockDevices(ctx, cl, metrics, lvg.Spec.BlockDeviceSelector)
actualBd, err := GetAPIBlockDevices(ctx, cl, metrics, lvg.Spec.BlockDeviceSelector, lvg.Spec.Local.NodeName)
if assert.NoError(t, err) {
assert.Equal(t, 2, len(actualBd))
_, ok := actualBd[name1]
Expand Down
8 changes: 4 additions & 4 deletions images/agent/src/pkg/controller/controller_reconcile_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ var _ = Describe("Storage Controller", func() {
})

It("GetAPIBlockDevices", func() {
listDevice, err := controller.GetAPIBlockDevices(ctx, cl, testMetrics, nil)
listDevice, err := controller.GetAPIBlockDevices(ctx, cl, testMetrics, nil, "")
Expect(err).NotTo(HaveOccurred())
Expect(listDevice).NotTo(BeNil())
Expect(len(listDevice)).To(Equal(1))
Expand Down Expand Up @@ -115,7 +115,7 @@ var _ = Describe("Storage Controller", func() {
MachineID: "1234",
}

resources, err := controller.GetAPIBlockDevices(ctx, cl, testMetrics, nil)
resources, err := controller.GetAPIBlockDevices(ctx, cl, testMetrics, nil, "")
Expect(err).NotTo(HaveOccurred())
Expect(resources).NotTo(BeNil())
Expect(len(resources)).To(Equal(1))
Expand All @@ -127,7 +127,7 @@ var _ = Describe("Storage Controller", func() {
err = controller.UpdateAPIBlockDevice(ctx, cl, testMetrics, oldResource, newCandidate)
Expect(err).NotTo(HaveOccurred())

resources, err = controller.GetAPIBlockDevices(ctx, cl, testMetrics, nil)
resources, err = controller.GetAPIBlockDevices(ctx, cl, testMetrics, nil, "")
Expect(err).NotTo(HaveOccurred())
Expect(resources).NotTo(BeNil())
Expect(len(resources)).To(Equal(1))
Expand All @@ -147,7 +147,7 @@ var _ = Describe("Storage Controller", func() {
})
Expect(err).NotTo(HaveOccurred())

devices, err := controller.GetAPIBlockDevices(context.Background(), cl, testMetrics, nil)
devices, err := controller.GetAPIBlockDevices(context.Background(), cl, testMetrics, nil, "")
Expect(err).NotTo(HaveOccurred())
for name := range devices {
Expect(name).NotTo(Equal(deviceName))
Expand Down
2 changes: 1 addition & 1 deletion images/agent/src/pkg/controller/lvm_volume_group_discover.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func LVMVolumeGroupDiscoverReconcile(ctx context.Context, cl client.Client, metr
log.Debug("[RunLVMVolumeGroupDiscoverController] no current LVMVolumeGroups found")
}

blockDevices, err := GetAPIBlockDevices(ctx, cl, metrics, nil)
blockDevices, err := GetAPIBlockDevices(ctx, cl, metrics, nil, cfg.NodeName)
if err != nil {
log.Error(err, "[RunLVMVolumeGroupDiscoverController] unable to GetAPIBlockDevices")
for _, lvg := range currentLVMVGs {
Expand Down
6 changes: 3 additions & 3 deletions images/agent/src/pkg/controller/lvm_volume_group_watcher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ func RunLVMVolumeGroupWatcherController(
log.Debug(fmt.Sprintf("[RunLVMVolumeGroupWatcherController] successfully removed the label %s from the LVMVolumeGroup %s", internal.LVGUpdateTriggerLabel, lvg.Name))
}

log.Debug(fmt.Sprintf("[RunLVMVolumeGroupWatcherController] tries to get block device resources for the LVMVolumeGroup %s by the selector %v", lvg.Name, lvg.Spec.BlockDeviceSelector))
blockDevices, err := GetAPIBlockDevices(ctx, cl, metrics, lvg.Spec.BlockDeviceSelector)
log.Debug(fmt.Sprintf("[RunLVMVolumeGroupWatcherController] tries to get block device resources for the LVMVolumeGroup %s by the selector %v on the node %s", lvg.Name, lvg.Spec.BlockDeviceSelector, lvg.Spec.Local.NodeName))
blockDevices, err := GetAPIBlockDevices(ctx, cl, metrics, lvg.Spec.BlockDeviceSelector, lvg.Spec.Local.NodeName)
if err != nil {
log.Error(err, fmt.Sprintf("[RunLVMVolumeGroupWatcherController] unable to get BlockDevices. Retry in %s", cfg.BlockDeviceScanIntervalSec.String()))
err = updateLVGConditionIfNeeded(ctx, cl, log, lvg, v1.ConditionFalse, internal.TypeVGConfigurationApplied, "NoBlockDevices", fmt.Sprintf("unable to get block devices resources, err: %s", err.Error()))
Expand All @@ -129,7 +129,7 @@ func RunLVMVolumeGroupWatcherController(

return reconcile.Result{RequeueAfter: cfg.BlockDeviceScanIntervalSec}, nil
}
log.Debug(fmt.Sprintf("[RunLVMVolumeGroupWatcherController] successfully got block device resources for the LVMVolumeGroup %s by the selector %v", lvg.Name, lvg.Spec.BlockDeviceSelector))
log.Debug(fmt.Sprintf("[RunLVMVolumeGroupWatcherController] successfully got block device resources for the LVMVolumeGroup %s by the selector %v on the node %s", lvg.Name, lvg.Spec.BlockDeviceSelector, lvg.Spec.Local.NodeName))
log.Trace(fmt.Sprintf("[RunLVMVolumeGroupWatcherController] block devices: %v", blockDevices))

valid, reason := validateSpecBlockDevices(lvg, blockDevices)
Expand Down
14 changes: 5 additions & 9 deletions images/agent/src/pkg/controller/lvm_volume_group_watcher_func.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,19 +294,15 @@ func validateSpecBlockDevices(lvg *v1alpha1.LVMVolumeGroup, blockDevices map[str
}
}

bdFromOurNode := make([]string, 0, len(blockDevices))
bdFromOtherNode := make([]string, 0, len(blockDevices))
for _, bd := range blockDevices {
if bd.Status.NodeName == lvg.Spec.Local.NodeName {
bdFromOurNode = append(bdFromOurNode, bd.Name)
if bd.Status.NodeName != lvg.Spec.Local.NodeName {
bdFromOtherNode = append(bdFromOtherNode, bd.Name)
}
}

if len(bdFromOurNode) == 0 {
bdNames := make([]string, 0, len(blockDevices))
for _, bd := range blockDevices {
bdNames = append(bdNames, bd.Name)
}
return false, fmt.Sprintf("none of specified BlockDevices %s were found on the node %s", lvg.Spec.Local.NodeName, strings.Join(bdNames, ","))
if len(bdFromOtherNode) != 0 {
return false, fmt.Sprintf("block devices %s have different node names from LVMVolumeGroup Local.NodeName", strings.Join(bdFromOtherNode, ","))
}

return true, ""
Expand Down

0 comments on commit 006596b

Please sign in to comment.