From 0ff8ed7585cf487089a8bb02cfa48fe411f12f3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Fri, 8 Jul 2022 11:34:22 +0200 Subject: [PATCH] Manager: implement the `getVariables` OpenAPI operation --- internal/manager/api_impl/meta.go | 20 ++++++ internal/manager/api_impl/meta_test.go | 60 ++++++++++++++++ .../api_impl/mocks/api_impl_mock.gen.go | 14 ++++ internal/manager/api_impl/varrepl.go | 1 + internal/manager/config/config.go | 70 +++++++++++++++---- internal/manager/config/service.go | 3 + 6 files changed, 153 insertions(+), 15 deletions(-) create mode 100644 internal/manager/api_impl/meta_test.go diff --git a/internal/manager/api_impl/meta.go b/internal/manager/api_impl/meta.go index 54d32928..def7512c 100644 --- a/internal/manager/api_impl/meta.go +++ b/internal/manager/api_impl/meta.go @@ -7,6 +7,7 @@ import ( "net/http" "git.blender.org/flamenco/internal/appinfo" + "git.blender.org/flamenco/internal/manager/config" "git.blender.org/flamenco/pkg/api" "github.com/labstack/echo/v4" ) @@ -24,3 +25,22 @@ func (f *Flamenco) GetConfiguration(e echo.Context) error { StorageLocation: f.config.EffectiveStoragePath(), }) } + +func (f *Flamenco) GetVariables(e echo.Context, audience api.ManagerVariableAudience, platform string) error { + variables := f.config.ResolveVariables( + config.VariableAudience(audience), + config.VariablePlatform(platform), + ) + + apiVars := api.ManagerVariables{ + AdditionalProperties: make(map[string]api.ManagerVariable), + } + for name, variable := range variables { + apiVars.AdditionalProperties[name] = api.ManagerVariable{ + IsTwoway: variable.IsTwoWay, + Value: variable.Value, + } + } + + return e.JSON(http.StatusOK, apiVars) +} diff --git a/internal/manager/api_impl/meta_test.go b/internal/manager/api_impl/meta_test.go new file mode 100644 index 00000000..af59eac7 --- /dev/null +++ b/internal/manager/api_impl/meta_test.go @@ -0,0 +1,60 @@ +package api_impl + +// SPDX-License-Identifier: GPL-3.0-or-later + +import ( + "net/http" + "testing" + + "git.blender.org/flamenco/internal/manager/config" + "git.blender.org/flamenco/pkg/api" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" +) + +func TestGetVariables(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + mf := newMockedFlamenco(mockCtrl) + + // Test Linux Worker. + { + resolvedVarsLinuxWorker := make(map[string]config.ResolvedVariable) + resolvedVarsLinuxWorker["jobs"] = config.ResolvedVariable{ + IsTwoWay: true, + Value: "Linux value", + } + resolvedVarsLinuxWorker["blender"] = config.ResolvedVariable{ + IsTwoWay: false, + Value: "/usr/local/blender", + } + + mf.config.EXPECT(). + ResolveVariables(config.VariableAudienceWorkers, config.VariablePlatformLinux). + Return(resolvedVarsLinuxWorker) + + echoCtx := mf.prepareMockedRequest(nil) + err := mf.flamenco.GetVariables(echoCtx, api.ManagerVariableAudienceWorkers, "linux") + assert.NoError(t, err) + assertResponseJSON(t, echoCtx, http.StatusOK, api.ManagerVariables{ + AdditionalProperties: map[string]api.ManagerVariable{ + "blender": {Value: "/usr/local/blender", IsTwoway: false}, + "jobs": {Value: "Linux value", IsTwoway: true}, + }, + }) + } + + // Test unknown platform User. + { + resolvedVarsUnknownPlatform := make(map[string]config.ResolvedVariable) + mf.config.EXPECT(). + ResolveVariables(config.VariableAudienceUsers, config.VariablePlatform("troll")). + Return(resolvedVarsUnknownPlatform) + + echoCtx := mf.prepareMockedRequest(nil) + err := mf.flamenco.GetVariables(echoCtx, api.ManagerVariableAudienceUsers, "troll") + assert.NoError(t, err) + assertResponseJSON(t, echoCtx, http.StatusOK, api.ManagerVariables{}) + } +} diff --git a/internal/manager/api_impl/mocks/api_impl_mock.gen.go b/internal/manager/api_impl/mocks/api_impl_mock.gen.go index 0e638040..a99a69c9 100644 --- a/internal/manager/api_impl/mocks/api_impl_mock.gen.go +++ b/internal/manager/api_impl/mocks/api_impl_mock.gen.go @@ -699,6 +699,20 @@ func (mr *MockConfigServiceMockRecorder) Get() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockConfigService)(nil).Get)) } +// ResolveVariables mocks base method. +func (m *MockConfigService) ResolveVariables(arg0 config.VariableAudience, arg1 config.VariablePlatform) map[string]config.ResolvedVariable { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ResolveVariables", arg0, arg1) + ret0, _ := ret[0].(map[string]config.ResolvedVariable) + return ret0 +} + +// ResolveVariables indicates an expected call of ResolveVariables. +func (mr *MockConfigServiceMockRecorder) ResolveVariables(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResolveVariables", reflect.TypeOf((*MockConfigService)(nil).ResolveVariables), arg0, arg1) +} + // MockTaskStateMachine is a mock of TaskStateMachine interface. type MockTaskStateMachine struct { ctrl *gomock.Controller diff --git a/internal/manager/api_impl/varrepl.go b/internal/manager/api_impl/varrepl.go index 6d62f65e..d1d192fa 100644 --- a/internal/manager/api_impl/varrepl.go +++ b/internal/manager/api_impl/varrepl.go @@ -10,6 +10,7 @@ import ( type VariableReplacer interface { ExpandVariables(valueToExpand string, audience config.VariableAudience, platform config.VariablePlatform) string + ResolveVariables(audience config.VariableAudience, platform config.VariablePlatform) map[string]config.ResolvedVariable } // replaceTaskVariables performs variable replacement for worker tasks. diff --git a/internal/manager/config/config.go b/internal/manager/config/config.go index e50bbb02..7395fa33 100644 --- a/internal/manager/config/config.go +++ b/internal/manager/config/config.go @@ -149,6 +149,12 @@ type VariableValue struct { Value string `yaml:"value" json:"value"` } +// ResolvedVariable represents info about the variable, resolved for a specific platform & audience. +type ResolvedVariable struct { + IsTwoWay bool + Value string +} + // getConf parses flamenco-manager.yaml and returns its contents as a Conf object. func getConf() (Conf, error) { return loadConf(configFilename) @@ -381,21 +387,8 @@ func updateMap[K comparable, V any](target map[K]V, updateWith map[K]V) { // ExpandVariables converts "{variable name}" to the value that belongs to the given audience and platform. func (c *Conf) ExpandVariables(valueToExpand string, audience VariableAudience, platform VariablePlatform) string { - platformsForAudience := c.VariablesLookup[audience] - if platformsForAudience == nil { - log.Warn(). - Str("valueToExpand", valueToExpand). - Str("audience", string(audience)). - Str("platform", string(platform)). - Msg("no variables defined for this audience") - return valueToExpand - } - - varsForPlatform := map[string]string{} - updateMap(varsForPlatform, platformsForAudience[VariablePlatformAll]) - updateMap(varsForPlatform, platformsForAudience[platform]) - - if varsForPlatform == nil { + varsForPlatform := c.getVariables(audience, platform) + if len(varsForPlatform) == 0 { log.Warn(). Str("valueToExpand", valueToExpand). Str("audience", string(audience)). @@ -410,9 +403,56 @@ func (c *Conf) ExpandVariables(valueToExpand string, audience VariableAudience, valueToExpand = strings.Replace(valueToExpand, placeholder, varvalue, -1) } + // TODO: this needs to go through multiple variable replacements, to make sure + // that, for example, the `{jobs}` variable gets the two-way variables applied + // as well. + return valueToExpand } +// GetVariables returns the variable values for this (audience, platform) combination. +// If no variables are found, just returns an empty map. If a value is defined +// for both the "all" platform and specifically the given platform, the specific +// platform definition wins. +func (c *Conf) getVariables(audience VariableAudience, platform VariablePlatform) map[string]string { + platformsForAudience := c.VariablesLookup[audience] + if len(platformsForAudience) == 0 { + return make(map[string]string) + } + + varsForPlatform := map[string]string{} + updateMap(varsForPlatform, platformsForAudience[VariablePlatformAll]) + updateMap(varsForPlatform, platformsForAudience[platform]) + return varsForPlatform +} + +// ResolveVariables returns the variables for this (audience, platform) combination. +// If no variables are found, just returns an empty map. If a value is defined +// for both the "all" platform and specifically the given platform, the specific +// platform definition wins. +func (c *Conf) ResolveVariables(audience VariableAudience, platform VariablePlatform) map[string]ResolvedVariable { + varsForPlatform := c.getVariables(audience, platform) + + resolvedVars := make(map[string]ResolvedVariable) + for name, value := range varsForPlatform { + resolvedVar := ResolvedVariable{Value: value} + + // Find the 'IsTwoway' property by finding the actual foundVar, which can be + // defined in two places. + if foundVar, ok := c.Variables[name]; ok { + resolvedVar.IsTwoWay = foundVar.IsTwoWay + } else if foundVar, ok := c.implicitVariables[name]; ok { + resolvedVar.IsTwoWay = foundVar.IsTwoWay + } else { + log.Error().Str("variable", name).Msg("unable to find this variable, where did it come from?") + } + + resolvedVars[name] = resolvedVar + } + + return resolvedVars +} + // checkVariables performs some basic checks on variable definitions. // All errors are logged, not returned. func (c *Conf) checkVariables() { diff --git a/internal/manager/config/service.go b/internal/manager/config/service.go index c59df1de..88fbbb6a 100644 --- a/internal/manager/config/service.go +++ b/internal/manager/config/service.go @@ -45,6 +45,9 @@ func (s *Service) Save() error { func (s *Service) ExpandVariables(valueToExpand string, audience VariableAudience, platform VariablePlatform) string { return s.config.ExpandVariables(valueToExpand, audience, platform) } +func (s *Service) ResolveVariables(audience VariableAudience, platform VariablePlatform) map[string]ResolvedVariable { + return s.config.ResolveVariables(audience, platform) +} func (s *Service) EffectiveStoragePath() string { return s.config.EffectiveStoragePath() }