From 84488cfab4208bf6244283db4963a2053b087d43 Mon Sep 17 00:00:00 2001 From: Aleksander Mistewicz Date: Thu, 8 Jun 2017 17:22:02 +0200 Subject: [PATCH] Implement filtered worker listing WorkerList first applies Capabilities (Caps) and then Group filter. Currently Caps require exact match, i.e. all keys must be found and values must be equal. It is planned to support ranges and lists of values. Change-Id: I705c7b2f3e7a0be5506c5e6cfef5cd32af274fc9 Signed-off-by: Aleksander Mistewicz Reviewed-on: https://mcdsrvbld02.digital.local/review/49061 Reviewed-by: Lukasz Wojciechowski Tested-by: Lukasz Wojciechowski Reviewed-by: Maciej Wereski Tested-by: Maciej Wereski --- workers/worker_list_test.go | 149 +++++++++++++++++++++++++++++++++++++++++++- workers/workers.go | 95 +++++++++++++++++++++++++++- workers/workers_test.go | 10 +-- 3 files changed, 244 insertions(+), 10 deletions(-) diff --git a/workers/worker_list_test.go b/workers/worker_list_test.go index 96b85cb..d7b2500 100644 --- a/workers/worker_list_test.go +++ b/workers/worker_list_test.go @@ -271,7 +271,7 @@ var _ = Describe("WorkerList", func() { It("should work to SetGroup", func() { var group Groups = []Group{ - Group("group1"), + "group1", } By("setting it") @@ -285,5 +285,152 @@ var _ = Describe("WorkerList", func() { Expect(wl.workers[worker].Groups).To(BeNil()) }) }) + + Describe("ListWorkers", func() { + var refWorkerList []WorkerInfo + + registerAndSetGroups := func(groups Groups, caps Capabilities) WorkerInfo { + capsUUID := uuid.NewV4().String() + caps[UUID] = capsUUID + err := wl.Register(caps) + Expect(err).ToNot(HaveOccurred()) + workerID := WorkerUUID(capsUUID) + + err = wl.SetGroups(workerID, groups) + Expect(err).ToNot(HaveOccurred()) + + return *wl.workers[workerID] + } + + BeforeEach(func() { + refWorkerList = make([]WorkerInfo, 1) + // Add worker with minimal caps and empty groups. + refWorkerList[0] = *wl.workers[worker] + // Add worker with both groups and caps declared. + refWorkerList = append(refWorkerList, registerAndSetGroups( + Groups{"all", "small_1", "small_2"}, + Capabilities{ + "target": "yes", + "display": "yes", + })) + // Add worker similar to the second one, but without caps. + refWorkerList = append(refWorkerList, registerAndSetGroups( + Groups{"all", "small_1", "small_2"}, + Capabilities{}, + )) + // Add worker similar to the second one, but without groups. + refWorkerList = append(refWorkerList, registerAndSetGroups( + Groups{}, + Capabilities{ + "target": "yes", + "display": "yes", + })) + // Add worker similar to the second one, but with display set to no. + refWorkerList = append(refWorkerList, registerAndSetGroups( + Groups{"all", "small_1", "small_2"}, + Capabilities{ + "target": "yes", + "display": "no", + })) + // Add worker similar to the second one, but absent from small_1 group. + refWorkerList = append(refWorkerList, registerAndSetGroups( + Groups{"all", "small_2"}, + Capabilities{ + "target": "yes", + "display": "yes", + })) + }) + + testWorkerList := func(groups Groups, caps Capabilities, + present, absent []WorkerInfo) { + workers, err := wl.ListWorkers(groups, caps) + Expect(err).ToNot(HaveOccurred()) + for _, workerInfo := range present { + Expect(workers).To(ContainElement(workerInfo)) + } + for _, workerInfo := range absent { + Expect(workers).ToNot(ContainElement(workerInfo)) + } + } + + It("should return all workers when parameters are nil", func() { + testWorkerList(nil, nil, refWorkerList, nil) + }) + + It("should return all workers when parameters are empty", func() { + testWorkerList(Groups{}, Capabilities{}, refWorkerList, nil) + }) + + Describe("filterCaps", func() { + It("should return all workers satisfying defined caps", func() { + By("Returning all workers with display") + testWorkerList(Groups{}, + Capabilities{"display": "yes"}, + []WorkerInfo{refWorkerList[1], refWorkerList[3], refWorkerList[5]}, + []WorkerInfo{refWorkerList[0], refWorkerList[2], refWorkerList[4]}) + + By("Returning all workers without display") + testWorkerList(Groups{}, + Capabilities{"display": "no"}, + []WorkerInfo{refWorkerList[4]}, + []WorkerInfo{refWorkerList[0], refWorkerList[1], + refWorkerList[2], refWorkerList[3], refWorkerList[5]}) + }) + + It("should return empty list if no worker matches the caps", func() { + workers, err := wl.ListWorkers(Groups{}, + Capabilities{ + "non-existing-caps": "", + }) + Expect(err).ToNot(HaveOccurred()) + Expect(workers).To(BeEmpty()) + }) + }) + + Describe("filterGroups", func() { + It("should return all workers satisfying defined groups", func() { + By("Returning all workers in group all") + testWorkerList(Groups{"all"}, + nil, + []WorkerInfo{refWorkerList[1], refWorkerList[2], + refWorkerList[4], refWorkerList[5]}, + []WorkerInfo{refWorkerList[0], refWorkerList[3]}) + + By("Returning all workers in group small_1") + testWorkerList(Groups{"small_1"}, + nil, + []WorkerInfo{refWorkerList[1], refWorkerList[2], refWorkerList[4]}, + []WorkerInfo{refWorkerList[0], refWorkerList[3], refWorkerList[5]}) + }) + + It("should return empty list if no worker matches the group", func() { + workers, err := wl.ListWorkers(Groups{"non-existing-group"}, nil) + Expect(err).ToNot(HaveOccurred()) + Expect(workers).To(BeEmpty()) + }) + }) + + It("should work with many groups and caps defined", func() { + By("Returning all targets with display in both groups") + testWorkerList(Groups{"small_1", "small_2"}, + Capabilities{ + "target": "yes", + "display": "yes", + }, + []WorkerInfo{refWorkerList[1], refWorkerList[5]}, + []WorkerInfo{refWorkerList[0], refWorkerList[2], + refWorkerList[3], refWorkerList[4]}) + + By("Returning all targets without display in group all and small_1") + testWorkerList(Groups{"all", "small_1"}, + Capabilities{ + "target": "yes", + "display": "no", + }, + []WorkerInfo{refWorkerList[4]}, + []WorkerInfo{refWorkerList[0], refWorkerList[1], + refWorkerList[2], refWorkerList[3], refWorkerList[5]}) + }) + }) }) }) diff --git a/workers/workers.go b/workers/workers.go index 220950b..1192b28 100644 --- a/workers/workers.go +++ b/workers/workers.go @@ -117,9 +117,102 @@ func (wl *WorkerList) Deregister(uuid WorkerUUID) error { return nil } +// convertToSlice converts given map to slice. +// It is a helper function of ListWorkers. +func convertToSlice(workers map[WorkerUUID]*WorkerInfo) []WorkerInfo { + all := make([]WorkerInfo, 0, len(workers)) + for _, worker := range workers { + all = append(all, *worker) + } + return all +} + +// isCapsMatching returns true if all capabilities in src are satisfied by capabilities in dest +// and false in any other case. +// +// TODO Caps matching is a complex problem and it should be changed to satisfy usecases below: +// * matching any of the values and at least one: +// "SERIAL": "57600,115200" should be satisfied by "SERIAL": "9600, 38400, 57600" +// * match value in range: +// "VOLTAGE": "2.9-3.6" should satisfy "VOLTAGE": "3.3" +func isCapsMatching(src, dest Capabilities) bool { + for srcKey, srcValue := range src { + destValue, ok := dest[srcKey] + if !ok { + // Key is not present in the worker's caps + return false + } + if srcValue != destValue { + // Capability values do not match + return false + } + } + return true +} + +// removeFromSlice replaces i-th element with the last one and returns slice shorter by one. +func removeFromSlice(workers []WorkerInfo, i int) []WorkerInfo { + l := len(workers) - 1 // Index of last element of the slice. + if i != l { + workers[i] = workers[l] + } + return workers[:l] +} + +// filterCaps returns all workers matching given capabilities. +// It is a helper function of ListWorkers. +func filterCaps(workers []WorkerInfo, caps Capabilities) []WorkerInfo { + if caps == nil || len(caps) == 0 { + return workers + } + // range is not used here as workers reference and parameter i + // are modified within the loop. + for i := 0; i < len(workers); i++ { + worker := &workers[i] + if !isCapsMatching(caps, worker.Caps) { + workers = removeFromSlice(workers, i) + i-- // Ensure that no worker will be skipped. + } + } + return workers +} + +// filterGroups returns all workers matching given groups. +// It is a helper function of ListWorkers. +func filterGroups(workers []WorkerInfo, groups Groups) []WorkerInfo { + if groups == nil || len(groups) == 0 { + return workers + } + groupsMatcher := make(map[Group]interface{}) + for _, group := range groups { + groupsMatcher[group] = nil + } + // range is not used here as workers reference and parameter i + // are modified within the loop. + for i := 0; i < len(workers); i++ { + worker := &workers[i] + accept := false + for _, workerGroup := range worker.Groups { + _, ok := groupsMatcher[workerGroup] + if ok { + accept = true + break + } + } + if !accept { + workers = removeFromSlice(workers, i) + i-- // Ensure that no worker will be skipped. + } + } + return workers +} + // ListWorkers is an implementation of ListWorkers from Workers interface. +// It lists all workers when both: +// * any of the groups is matching (or groups is nil) +// * all of the caps is matching (or caps is nil) func (wl *WorkerList) ListWorkers(groups Groups, caps Capabilities) ([]WorkerInfo, error) { - return nil, ErrNotImplemented + return filterGroups(filterCaps(convertToSlice(wl.workers), caps), groups), nil } // GetWorkerInfo is an implementation of GetWorkerInfo from Workers interface. diff --git a/workers/workers_test.go b/workers/workers_test.go index f9674ee..cf3c5c3 100644 --- a/workers/workers_test.go +++ b/workers/workers_test.go @@ -32,16 +32,10 @@ var _ = Describe("WorkerList", func() { It("should return ErrNotImplemented", func() { var ( - err error - uuid WorkerUUID = "" - caps Capabilities = nil - groups Groups = nil + err error + uuid WorkerUUID = "" ) - By("ListWorkers") - _, err = wl.ListWorkers(groups, caps) - Expect(err).To(Equal(ErrNotImplemented)) - By("GetWorkerInfo") _, err = wl.GetWorkerInfo(uuid) Expect(err).To(Equal(ErrNotImplemented)) -- 2.7.4