From 3611d4f22cf63bd93c6d27a986239cdace2dad2a Mon Sep 17 00:00:00 2001 From: Viktor Kramarenko Date: Wed, 2 Oct 2024 17:45:46 +0300 Subject: [PATCH] final refactoring Signed-off-by: Viktor Kramarenko --- images/agent/src/internal/const.go | 1 + .../controller/lvm_volume_group_discover.go | 13 +---- .../lvm_volume_group_discover_test.go | 14 ++--- .../controller/lvm_volume_group_watcher.go | 4 +- .../lvm_volume_group_watcher_test.go | 2 +- .../lvm_volume_group_set_watcher.go | 54 ++++++++++--------- 6 files changed, 39 insertions(+), 49 deletions(-) diff --git a/images/agent/src/internal/const.go b/images/agent/src/internal/const.go index 5796f36e..7cd7d2a8 100644 --- a/images/agent/src/internal/const.go +++ b/images/agent/src/internal/const.go @@ -52,6 +52,7 @@ const ( ReasonTerminating = "Terminating" ReasonScanFailed = "ScanFailed" ReasonUpdated = "Updated" + ReasonApplied = "Applied" BlockDeviceLabelPrefix = "status.blockdevice.storage.deckhouse.io" diff --git a/images/agent/src/pkg/controller/lvm_volume_group_discover.go b/images/agent/src/pkg/controller/lvm_volume_group_discover.go index a6040f42..a79d002f 100644 --- a/images/agent/src/pkg/controller/lvm_volume_group_discover.go +++ b/images/agent/src/pkg/controller/lvm_volume_group_discover.go @@ -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(), @@ -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) } @@ -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 diff --git a/images/agent/src/pkg/controller/lvm_volume_group_discover_test.go b/images/agent/src/pkg/controller/lvm_volume_group_discover_test.go index 07e419a4..19e1022b 100644 --- a/images/agent/src/pkg/controller/lvm_volume_group_discover_test.go +++ b/images/agent/src/pkg/controller/lvm_volume_group_discover_test.go @@ -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) }) @@ -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) }) @@ -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) }) diff --git a/images/agent/src/pkg/controller/lvm_volume_group_watcher.go b/images/agent/src/pkg/controller/lvm_volume_group_watcher.go index a9327123..8baf7fb6 100644 --- a/images/agent/src/pkg/controller/lvm_volume_group_watcher.go +++ b/images/agent/src/pkg/controller/lvm_volume_group_watcher.go @@ -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 @@ -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 diff --git a/images/agent/src/pkg/controller/lvm_volume_group_watcher_test.go b/images/agent/src/pkg/controller/lvm_volume_group_watcher_test.go index 9698f005..bbb1290e 100644 --- a/images/agent/src/pkg/controller/lvm_volume_group_watcher_test.go +++ b/images/agent/src/pkg/controller/lvm_volume_group_watcher_test.go @@ -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"} diff --git a/images/sds-health-watcher-controller/src/pkg/controller/lvm_volume_group_set_watcher.go b/images/sds-health-watcher-controller/src/pkg/controller/lvm_volume_group_set_watcher.go index 76715906..4635bad6 100644 --- a/images/sds-health-watcher-controller/src/pkg/controller/lvm_volume_group_set_watcher.go +++ b/images/sds-health-watcher-controller/src/pkg/controller/lvm_volume_group_set_watcher.go @@ -3,6 +3,9 @@ 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" @@ -10,11 +13,6 @@ import ( "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" @@ -22,7 +20,11 @@ import ( "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 ( @@ -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)) @@ -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)) }, })) @@ -157,10 +158,11 @@ 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)) - err := provideLVMVolumeGroupsBySetPerNode(ctx, cl, log, metrics, lvgSet, nodes) + err := provideLVMVolumeGroupsPerNode(ctx, cl, log, metrics, lvgSet, nodes) if err != nil { log.Error(err, fmt.Sprintf("[provideLVMVolumeGroupsBySet] unable to provide LVMVolumeGroups by the LVMVolumeGroupSet %s with strategy %s", lvgSet.Name, strategyPerNode)) return err @@ -171,45 +173,45 @@ func provideLVMVolumeGroupsBySet(ctx context.Context, cl client.Client, log logg return nil } -func provideLVMVolumeGroupsBySetPerNode(ctx context.Context, cl client.Client, log logger.Logger, metrics monitoring.Metrics, lvgSet *v1alpha1.LVMVolumeGroupSet, nodes map[string]v1.Node) error { - log.Debug("[provideLVMVolumeGroupsBySetPerNode] tries to get LVMVolumeGroups") +func provideLVMVolumeGroupsPerNode(ctx context.Context, cl client.Client, log logger.Logger, metrics monitoring.Metrics, lvgSet *v1alpha1.LVMVolumeGroupSet, nodes map[string]v1.Node) error { + log.Debug("[provideLVMVolumeGroupsPerNode] tries to get LVMVolumeGroups") currentLVGs, err := GetLVMVolumeGroups(ctx, cl, metrics) if err != nil { - log.Error(err, "[provideLVMVolumeGroupsBySetPerNode] unable to get LVMVolumeGroups") + log.Error(err, "[provideLVMVolumeGroupsPerNode] unable to get LVMVolumeGroups") return err } - log.Debug("[provideLVMVolumeGroupsBySetPerNode] successfully got LVMVolumeGroups") - log.Trace(fmt.Sprintf("[provideLVMVolumeGroupsBySetPerNode] current LVMVolumeGroups: %+v", currentLVGs)) + log.Debug("[provideLVMVolumeGroupsPerNode] successfully got LVMVolumeGroups") + log.Trace(fmt.Sprintf("[provideLVMVolumeGroupsPerNode] current LVMVolumeGroups: %+v", currentLVGs)) for _, node := range nodes { configuredLVG := configureLVGBySet(lvgSet, node) - log.Trace(fmt.Sprintf("[provideLVMVolumeGroupsBySetPerNode] configurated LVMVolumeGroup: %+v", configuredLVG)) + log.Trace(fmt.Sprintf("[provideLVMVolumeGroupsPerNode] configurated LVMVolumeGroup: %+v", configuredLVG)) currentLVG := matchConfiguredLVGWithExistingOne(configuredLVG, currentLVGs) if currentLVG != nil { - log.Debug(fmt.Sprintf("[provideLVMVolumeGroupsBySetPerNode] tries to update the LVMVolumeGroup %s", currentLVG.Name)) + log.Debug(fmt.Sprintf("[provideLVMVolumeGroupsPerNode] tries to update the LVMVolumeGroup %s", currentLVG.Name)) err = updateLVMVolumeGroupByConfiguredFromSet(ctx, cl, currentLVG, configuredLVG) if err != nil { - log.Error(err, fmt.Sprintf("[provideLVMVolumeGroupsBySetPerNode] unable to update the LVMVolumeGroup %s", currentLVG.Name)) + log.Error(err, fmt.Sprintf("[provideLVMVolumeGroupsPerNode] unable to update the LVMVolumeGroup %s", currentLVG.Name)) return err } - log.Info(fmt.Sprintf("[provideLVMVolumeGroupsBySetPerNode] LVMVolumeGroup %s has been successfully updated", currentLVG.Name)) + log.Info(fmt.Sprintf("[provideLVMVolumeGroupsPerNode] LVMVolumeGroup %s has been successfully updated", currentLVG.Name)) } else { - log.Debug(fmt.Sprintf("[provideLVMVolumeGroupsBySetPerNode] tries to create the LVMVolumeGroup %s", configuredLVG.Name)) + log.Debug(fmt.Sprintf("[provideLVMVolumeGroupsPerNode] tries to create the LVMVolumeGroup %s", configuredLVG.Name)) err = createLVMVolumeGroup(ctx, cl, configuredLVG) if err != nil { - log.Error(err, fmt.Sprintf("[provideLVMVolumeGroupsBySetPerNode] unable to create the LVMVolumeGroup %s", configuredLVG.Name)) + log.Error(err, fmt.Sprintf("[provideLVMVolumeGroupsPerNode] unable to create the LVMVolumeGroup %s", configuredLVG.Name)) return err } - log.Info(fmt.Sprintf("[provideLVMVolumeGroupsBySetPerNode] the LVMVolumeGroup %s has been created", configuredLVG.Name)) - log.Debug(fmt.Sprintf("[provideLVMVolumeGroupsBySetPerNode] tries to update the LVMVolumeGroupSet %s status by the created LVMVolumeGroup %s", lvgSet.Name, configuredLVG.Name)) + log.Info(fmt.Sprintf("[provideLVMVolumeGroupsPerNode] the LVMVolumeGroup %s has been created", configuredLVG.Name)) + log.Debug(fmt.Sprintf("[provideLVMVolumeGroupsPerNode] tries to update the LVMVolumeGroupSet %s status by the created LVMVolumeGroup %s", lvgSet.Name, configuredLVG.Name)) err = updateLVMVolumeGroupSetStatusByLVGIfNeeded(ctx, cl, log, lvgSet, configuredLVG, nodes) if err != nil { - log.Error(err, fmt.Sprintf("[provideLVMVolumeGroupsBySetPerNode] unable to update the LVMVolumeGroupSet %s", lvgSet.Name)) + log.Error(err, fmt.Sprintf("[provideLVMVolumeGroupsPerNode] unable to update the LVMVolumeGroupSet %s", lvgSet.Name)) return err } - log.Debug(fmt.Sprintf("[provideLVMVolumeGroupsBySetPerNode] successfully updated the LVMVolumeGroupSet %s status by the created LVMVolumeGroup %s", lvgSet.Name, configuredLVG.Name)) + log.Debug(fmt.Sprintf("[provideLVMVolumeGroupsPerNode] successfully updated the LVMVolumeGroupSet %s status by the created LVMVolumeGroup %s", lvgSet.Name, configuredLVG.Name)) } } @@ -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