Skip to content

Commit

Permalink
final refactoring
Browse files Browse the repository at this point in the history 
Signed-off-by: Viktor Kramarenko <[email protected]>
  • Loading branch information
@ViktorKram
ViktorKram committed Oct 2, 2024
1 parent bcef4fe commit 7d8dedb
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 33 deletions.
1 change: 1 addition & 0 deletions images/agent/src/internal/const.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const (
ReasonTerminating = "Terminating"
ReasonScanFailed = "ScanFailed"
ReasonUpdated = "Updated"
ReasonApplied = "Applied"

BlockDeviceLabelPrefix = "status.blockdevice.storage.deckhouse.io"

Expand Down
13 changes: 2 additions & 11 deletions images/agent/src/pkg/controller/lvm_volume_group_discover.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ func GetLVMVolumeGroupCandidates(log logger.Logger, sdsCache *cache.Cache, bds m

for _, vg := range vgWithTag {
allocateSize := getVGAllocatedSize(vg)
health, message := checkVGHealth(sortedBDs, vgIssues, pvIssues, lvIssues, vg)
health, message := checkVGHealth(vgIssues, pvIssues, lvIssues, vg)

candidate := internal.LVMVolumeGroupCandidate{
LVMVGName: generateLVMVGName(),
Expand Down Expand Up @@ -449,13 +449,9 @@ func getVGAllocatedSize(vg internal.VGData) resource.Quantity {
return allocatedSize
}

func checkVGHealth(blockDevices map[string][]v1alpha1.BlockDevice, vgIssues map[string]string, pvIssues map[string][]string, lvIssues map[string]map[string]string, vg internal.VGData) (health, message string) {
func checkVGHealth(vgIssues map[string]string, pvIssues map[string][]string, lvIssues map[string]map[string]string, vg internal.VGData) (health, message string) {
issues := make([]string, 0, len(vgIssues)+len(pvIssues)+len(lvIssues)+1)

if bds, exist := blockDevices[vg.VGName+vg.VGUUID]; !exist || len(bds) == 0 {
issues = append(issues, fmt.Sprintf("[ERROR] Unable to get BlockDevice resources for VG, name: %s ; uuid: %s", vg.VGName, vg.VGUUID))
}

if vgIssue, exist := vgIssues[vg.VGName+vg.VGUUID]; exist {
issues = append(issues, vgIssue)
}
Expand Down Expand Up @@ -643,11 +639,6 @@ func configureCandidateNodeDevices(log logger.Logger, pvs map[string][]internal.
PVUUID: pv.PVUuid,
}

//if bd, exist := bdPathStatus[pv.PVName]; exist {
// device.DevSize = *resource.NewQuantity(bd.Status.Size.Value(), resource.BinarySI)
// device.BlockDevice = bd.Name
//}

device.DevSize = *resource.NewQuantity(bd.Status.Size.Value(), resource.BinarySI)
device.BlockDevice = bd.Name

Expand Down
14 changes: 5 additions & 9 deletions images/agent/src/pkg/controller/lvm_volume_group_discover_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,12 @@ func TestLVMVolumeGroupDiscover(t *testing.T) {
vgName = "testVg"
vgUUID = "testUuid"
)
bds := map[string][]v1alpha1.BlockDevice{
vgName + vgUUID: {{}},
}
vgIssues := map[string]string{}
pvIssues := map[string][]string{}
lvIssues := map[string]map[string]string{}
vg := internal.VGData{VGName: vgName, VGUUID: vgUUID}

health, _ := checkVGHealth(bds, vgIssues, pvIssues, lvIssues, vg)
health, _ := checkVGHealth(vgIssues, pvIssues, lvIssues, vg)
assert.Equal(t, health, internal.LVMVGHealthOperational)
})

Expand All @@ -93,15 +90,14 @@ func TestLVMVolumeGroupDiscover(t *testing.T) {
vgName = "testVg"
vgUUID = "testUuid"
)
bds := map[string][]v1alpha1.BlockDevice{
vgName + vgUUID: {},
vgIssues := map[string]string{
vgName + vgUUID: "some-issue",
}
vgIssues := map[string]string{}
pvIssues := map[string][]string{}
lvIssues := map[string]map[string]string{}
vg := internal.VGData{VGName: vgName, VGUUID: vgUUID}

health, _ := checkVGHealth(bds, vgIssues, pvIssues, lvIssues, vg)
health, _ := checkVGHealth(vgIssues, pvIssues, lvIssues, vg)
assert.Equal(t, health, internal.LVMVGHealthNonOperational)
})

Expand Down Expand Up @@ -325,7 +321,7 @@ func TestLVMVolumeGroupDiscover(t *testing.T) {
mp := map[string][]v1alpha1.BlockDevice{vgName + vgUUID: bds}
ar := map[string][]internal.PVData{vgName + vgUUID: pvs}

actual := configureCandidateNodeDevices(ar, mp, vg, nodeName)
actual := configureCandidateNodeDevices(log, ar, mp, vg, nodeName)

assert.Equal(t, expected, actual)
})
Expand Down
4 changes: 2 additions & 2 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 @@ -430,7 +430,7 @@ func reconcileLVGUpdateFunc(ctx context.Context, cl client.Client, log logger.Lo
}

log.Debug(fmt.Sprintf("[reconcileLVGUpdateFunc] tries to add a condition %s to the LVMVolumeGroup %s", internal.TypeVGConfigurationApplied, lvg.Name))
err = updateLVGConditionIfNeeded(ctx, cl, log, lvg, v1.ConditionTrue, internal.TypeVGConfigurationApplied, "Applied", "configuration has been applied")
err = updateLVGConditionIfNeeded(ctx, cl, log, lvg, v1.ConditionTrue, internal.TypeVGConfigurationApplied, internal.ReasonApplied, "configuration has been applied")
if err != nil {
log.Error(err, fmt.Sprintf("[reconcileLVGUpdateFunc] unable to add a condition %s to the LVMVolumeGroup %s", internal.TypeVGConfigurationApplied, lvg.Name))
return true, err
Expand Down Expand Up @@ -519,7 +519,7 @@ func reconcileLVGCreateFunc(ctx context.Context, cl client.Client, log logger.Lo
log.Debug(fmt.Sprintf("[reconcileLVGCreateFunc] successfully created thin-pools for the LVMVolumeGroup %s", lvg.Name))
}

err = updateLVGConditionIfNeeded(ctx, cl, log, lvg, v1.ConditionTrue, internal.TypeVGConfigurationApplied, "Success", "all configuration has been applied")
err = updateLVGConditionIfNeeded(ctx, cl, log, lvg, v1.ConditionTrue, internal.TypeVGConfigurationApplied, internal.ReasonApplied, "all configuration has been applied")
if err != nil {
log.Error(err, fmt.Sprintf("[RunLVMVolumeGroupWatcherController] unable to add a condition %s to the LVMVolumeGroup %s", internal.TypeVGConfigurationApplied, lvg.Name))
return true, err
Expand Down
2 changes: 1 addition & 1 deletion images/agent/src/pkg/controller/lvm_volume_group_watcher_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1086,7 +1086,7 @@ func TestLVMVolumeGroupWatcherCtrl(t *testing.T) {
newLVG.Status.Conditions = []v1.Condition{
{
Type: internal.TypeVGConfigurationApplied,
Reason: internal.ReasonCreating,
Reason: internal.ReasonApplied,
},
}
newLVG.Labels = map[string]string{LVGMetadateNameLabelKey: "some-other-name"}
Expand Down
22 changes: 12 additions & 10 deletions images/sds-health-watcher-controller/src/pkg/controller/lvm_volume_group_set_watcher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,28 @@ package controller
import (
"context"
"fmt"
"reflect"
"time"

"github.com/deckhouse/sds-node-configurator/api/v1alpha1"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/util/workqueue"
"reflect"
"sds-health-watcher-controller/config"
"sds-health-watcher-controller/internal"
"sds-health-watcher-controller/pkg/logger"
"sds-health-watcher-controller/pkg/monitoring"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"sigs.k8s.io/controller-runtime/pkg/source"
"time"

"sds-health-watcher-controller/config"
"sds-health-watcher-controller/internal"
"sds-health-watcher-controller/pkg/logger"
"sds-health-watcher-controller/pkg/monitoring"
)

const (
Expand Down Expand Up @@ -79,13 +81,13 @@ func RunLVMVolumeGroupSetWatcher(
}

err = c.Watch(source.Kind(mgr.GetCache(), &v1alpha1.LVMVolumeGroupSet{}, handler.TypedFuncs[*v1alpha1.LVMVolumeGroupSet, reconcile.Request]{
CreateFunc: func(ctx context.Context, e event.TypedCreateEvent[*v1alpha1.LVMVolumeGroupSet], q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
CreateFunc: func(_ context.Context, e event.TypedCreateEvent[*v1alpha1.LVMVolumeGroupSet], q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
log.Info(fmt.Sprintf("[RunLVMVolumeGroupSetWatcher] createFunc got a create event for the LVMVolumeGroupSet, name: %s", e.Object.GetName()))
request := reconcile.Request{NamespacedName: types.NamespacedName{Namespace: e.Object.GetNamespace(), Name: e.Object.GetName()}}
q.Add(request)
log.Info(fmt.Sprintf("[RunLVMVolumeGroupSetWatcher] createFunc added a request for the LVMVolumeGroupSet %s to the Reconcilers queue", e.Object.GetName()))
},
UpdateFunc: func(ctx context.Context, e event.TypedUpdateEvent[*v1alpha1.LVMVolumeGroupSet], q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
UpdateFunc: func(_ context.Context, e event.TypedUpdateEvent[*v1alpha1.LVMVolumeGroupSet], q workqueue.TypedRateLimitingInterface[reconcile.Request]) {
log.Info(fmt.Sprintf("[RunLVMVolumeGroupSetWatcher] UpdateFunc got a update event for the LVMVolumeGroupSet %s", e.ObjectNew.GetName()))
if !shouldLVGSetWatcherReconcileUpdateEvent(e.ObjectOld, e.ObjectNew) {
log.Info(fmt.Sprintf("[RunLVMVolumeGroupSetWatcher] update event for the LVMVolumeGroupSet %s should not be reconciled as not target changed were made", e.ObjectNew.Name))
Expand All @@ -94,7 +96,6 @@ func RunLVMVolumeGroupSetWatcher(

request := reconcile.Request{NamespacedName: types.NamespacedName{Namespace: e.ObjectNew.GetNamespace(), Name: e.ObjectNew.GetName()}}
q.Add(request)

log.Info(fmt.Sprintf("[RunLVMVolumeGroupSetWatcher] updateFunc added a request for the LVMVolumeGroupSet %s to the Reconcilers queue", e.ObjectNew.Name))
},
}))
Expand Down Expand Up @@ -157,6 +158,7 @@ func reconcileLVMVolumeGroupSet(ctx context.Context, cl client.Client, log logge
}

func provideLVMVolumeGroupsBySet(ctx context.Context, cl client.Client, log logger.Logger, metrics monitoring.Metrics, lvgSet *v1alpha1.LVMVolumeGroupSet, nodes map[string]v1.Node) error {
//nolint:gocritic
switch lvgSet.Spec.Strategy {
case strategyPerNode:
log.Debug(fmt.Sprintf("[provideLVMVolumeGroupsBySet] the LVMVolumeGroupSet %s has strategy %s, tries to provide the LVMVolumeGroups", lvgSet.Name, strategyPerNode))
Expand Down Expand Up @@ -310,7 +312,7 @@ func GetNodes(ctx context.Context, cl client.Client, metrics monitoring.Metrics,
}

func updateLVMVolumeGroupSetPhaseIfNeeded(ctx context.Context, cl client.Client, log logger.Logger, lvgSet *v1alpha1.LVMVolumeGroupSet, phase, reason string) error {
log.Debug(fmt.Sprintf("[updateLVMVolumeGroupSetPhaseIfNeeded] tries to update the LVMVolumeGroupSet %s status phase to %s and reason to %s", lvgSet, phase, reason))
log.Debug(fmt.Sprintf("[updateLVMVolumeGroupSetPhaseIfNeeded] tries to update the LVMVolumeGroupSet %s status phase to %s and reason to %s", lvgSet.Name, phase, reason))
if lvgSet.Status.Phase == phase && lvgSet.Status.Reason == reason {
log.Debug(fmt.Sprintf("[updateLVMVolumeGroupSetPhaseIfNeeded] no need to update phase or reason of the LVMVolumeGroupSet %s as they are same", lvgSet.Name))
return nil
Expand Down

0 comments on commit 7d8dedb

Please sign in to comment.