diff --git a/CHANGELOG.md b/CHANGELOG.md index 27baffe8..9f0fc525 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ bugs in actually-released versions. This happens uniformly, regardless of the platform. So also on Linux, which has a case-sensitive filesystem, this matching is done in a case-insensitive way. It is very unlikely that a Flamenco configuration has two separate variables, for paths that only differ in their case. - Fix issue where jobs could get stuck in `pause-requested` state. Such jobs are now also detected at startup of the Manager, and sent to `paused` when possible. +- Log a warning at startup when the `blender` variable is missing, or has no value for one or more platforms. ## 3.6 - released 2024-12-01 diff --git a/cmd/flamenco-manager/main.go b/cmd/flamenco-manager/main.go index 71ee0e6d..4a183576 100644 --- a/cmd/flamenco-manager/main.go +++ b/cmd/flamenco-manager/main.go @@ -4,9 +4,7 @@ package main import ( "context" - "errors" "flag" - "io/fs" "net" "net/url" "os" @@ -117,15 +115,10 @@ func main() { func runFlamencoManager() bool { // Load configuration. configService := config.NewService() - err := configService.Load() - if err != nil && !errors.Is(err, fs.ErrNotExist) { - log.Error().Err(err).Msg("loading configuration") - } - if cliArgs.setupAssistant { configService.ForceFirstRun() } - isFirstRun, err := configService.IsFirstRun() + isFirstRun, err := configService.Load() switch { case err != nil: log.Fatal().Err(err).Msg("unable to determine whether this is the first run of Flamenco or not") diff --git a/internal/manager/api_impl/interfaces.go b/internal/manager/api_impl/interfaces.go index d6943d8d..08da47a6 100644 --- a/internal/manager/api_impl/interfaces.go +++ b/internal/manager/api_impl/interfaces.go @@ -184,8 +184,8 @@ type ConfigService interface { // options (like Shaman). EffectiveStoragePath() string - // IsFirstRun returns true if this is likely to be the first run of Flamenco. - IsFirstRun() (bool, error) + // Load returns true if this is likely to be the first run of Flamenco. + Load() (bool, error) // ForceFirstRun forces IsFirstRun() to return true. This is used to force the // setup assistant on a configured system. diff --git a/internal/manager/api_impl/meta.go b/internal/manager/api_impl/meta.go index 02b134b1..5a4e44e1 100644 --- a/internal/manager/api_impl/meta.go +++ b/internal/manager/api_impl/meta.go @@ -39,7 +39,7 @@ func (f *Flamenco) GetVersion(e echo.Context) error { } func (f *Flamenco) GetConfiguration(e echo.Context) error { - isFirstRun, err := f.config.IsFirstRun() + isFirstRun, err := f.config.Load() if err != nil { logger := requestLogger(e) logger.Error().Err(err).Msg("error investigating configuration") 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 8a19e7e0..13f1e4cb 100644 --- a/internal/manager/api_impl/mocks/api_impl_mock.gen.go +++ b/internal/manager/api_impl/mocks/api_impl_mock.gen.go @@ -953,19 +953,19 @@ func (mr *MockConfigServiceMockRecorder) Get() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockConfigService)(nil).Get)) } -// IsFirstRun mocks base method. -func (m *MockConfigService) IsFirstRun() (bool, error) { +// Load mocks base method. +func (m *MockConfigService) Load() (bool, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "IsFirstRun") + ret := m.ctrl.Call(m, "Load") ret0, _ := ret[0].(bool) ret1, _ := ret[1].(error) return ret0, ret1 } -// IsFirstRun indicates an expected call of IsFirstRun. -func (mr *MockConfigServiceMockRecorder) IsFirstRun() *gomock.Call { +// Load indicates an expected call of Load. +func (mr *MockConfigServiceMockRecorder) Load() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsFirstRun", reflect.TypeOf((*MockConfigService)(nil).IsFirstRun)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Load", reflect.TypeOf((*MockConfigService)(nil).Load)) } // NewVariableExpander mocks base method. diff --git a/internal/manager/config/config.go b/internal/manager/config/config.go index 9b183514..0129c923 100644 --- a/internal/manager/config/config.go +++ b/internal/manager/config/config.go @@ -246,6 +246,7 @@ func (c *Conf) processAfterLoading(override ...func(c *Conf)) { c.constructVariableLookupTable() c.checkDatabase() c.checkVariables() + c.checkBlenderVariable() } // MockCurrentGOOSForTests can be used in unit tests to make the variable @@ -602,6 +603,37 @@ func (c *Conf) checkVariables() { } } +func (c *Conf) checkBlenderVariable() { + blenderKey := "blender" + blender, ok := c.Variables[blenderKey] + if !ok { + log.Error().Msg("config: variable 'blender' not found, Flamenco will not be able to run Blender") + return + } + + missingPlatforms := []string{} + for _, value := range blender.Values { + if value.Value == "" { + missingPlatforms = append(missingPlatforms, string(value.Platform)) + } + } + + logger := log.With(). + Str("filename", configFilename). + Logger() + switch len(missingPlatforms) { + case 0: // Fine. + case 1: + logger.Warn(). + Str("platform", missingPlatforms[0]). + Msgf("config: variable %q has no value, Flamenco will not be able to run Blender on this platform", blenderKey) + default: // More than one missing platform. + logger.Warn(). + Strs("platforms", missingPlatforms). + Msgf("config: variable %q has no value, Flamenco will not be able to run Blender on these platforms", blenderKey) + } +} + func (c *Conf) checkDatabase() { c.DatabaseDSN = strings.TrimSpace(c.DatabaseDSN) } diff --git a/internal/manager/config/service.go b/internal/manager/config/service.go index 0ce13489..2293f7bc 100644 --- a/internal/manager/config/service.go +++ b/internal/manager/config/service.go @@ -22,13 +22,16 @@ func NewService() *Service { } } -// IsFirstRun returns true if this is likely to be the first run of Flamenco. -func (s *Service) IsFirstRun() (bool, error) { - if s.forceFirstRun { - return true, nil - } +func (s *Service) ForceFirstRun() { + s.forceFirstRun = true +} +// Load parses the flamenco-manager.yaml file, and returns whether this is +// likely to be the first run or not. +func (s *Service) Load() (bool, error) { config, err := getConf() + s.config = config + switch { case errors.Is(err, fs.ErrNotExist): // No configuration means first run. @@ -38,20 +41,7 @@ func (s *Service) IsFirstRun() (bool, error) { } // No shared storage configured means first run. - return config.SharedStoragePath == "", nil -} - -func (s *Service) ForceFirstRun() { - s.forceFirstRun = true -} - -func (s *Service) Load() error { - config, err := getConf() - if err != nil { - return err - } - s.config = config - return nil + return s.forceFirstRun || config.SharedStoragePath == "", nil } func (s *Service) Get() *Conf {