Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: missing tags on target manager config field #109

Merged
merged 1 commit into from
Feb 19, 2024
Merged

Conversation

lvlcn-t
Copy link
Collaborator

@lvlcn-t lvlcn-t commented Feb 19, 2024

Motivation

The implementation of #101 introduced a bug with its embedded Config struct to the TargetManagerConfig.

Viper or rather the underlying library mapstructure doesn't unmarshal the embedded fields of the Config struct directly in the TargetManagerConfig struct. It rather uses the internal struct key of it. So the mapstructure library sees the struct like this:

type TargetManagerConfig struct {
	Config Config
	Gitlab GitlabTargetManagerConfig `yaml:"gitlab" mapstructure:"gitlab"`
}

That means that the startup config currently needs a Config key as part of the targetManager key which has the general configuration options for the targetManager. So an example configuration currently needs to look like this:

targetManager:
  Config:
    checkInterval: 1m
    registrationInterval: 1m
    updateInterval: 1m
    unhealthyThreshold: 3m
  gitlab:
    baseUrl: https://gitlab.com
    projectId: 1234

Reference

spf13/viper#797
https://pkg.go.dev/github.com/mitchellh/mapstructure#hdr-Embedded_Structs_and_Squashing

Changes

I've added the missing yaml and mapstructure tags for the embedded struct.

Now the configuration can again look like this:

targetManager:
  checkInterval: 1m
  registrationInterval: 1m
  updateInterval: 1m
  unhealthyThreshold: 3m
  gitlab:
    baseUrl: https://gitlab.com
    projectId: 1234

For additional information look at the commits.

Tests done

  • Unit tests succeeded.
  • E2E tests succeeded.

Manual e2e tests

I've added a debug log after unmarshaling the startup config into its config struct:

$ go run main.go run --config .tmp/start-config.yaml 
Using config file: .tmp/start-config.yaml
{"time":"2024-02-19T17:06:07.44296144+01:00","level":"DEBUG","source":{"function":"github.com/caas-team/sparrow/cmd.NewCmdRun.run.func1","file":"/home/tom/dev/sparrow/cmd/run.go","line":75},"msg":"Config loaded","config":{"SparrowName":"dev-tom.local.sparrow.com","Loader":{"Type":"file","Interval":0,"Http":{"Url":"https://myconfig.example.com/config.yaml","Token":"","Timeout":30000000000,"RetryCfg":{"Count":3,"Delay":10000000000}},"File":{"Path":"./.tmp/run-config.yaml"}},"Api":{"ListeningAddress":":8080"},"TargetManager":{"CheckInterval":60000000000,"RegistrationInterval":60000000000,"UpdateInterval":60000000000,"UnhealthyThreshold":180000000000,"Gitlab":{"BaseURL":"https://gitlab.com","Token":"","ProjectID":123456}}}}
{"time":"2024-02-19T17:06:07.443524529+01:00","level":"INFO","source":{"function":"github.com/caas-team/sparrow/cmd.NewCmdRun.run.func1","file":"/home/tom/dev/sparrow/cmd/run.go","line":81},"msg":"Running sparrow"}
{"time":"2024-02-19T17:06:07.443626652+01:00","level":"INFO","source":{"function":"github.com/caas-team/sparrow/pkg/sparrow/targets.(*gitlabTargetManager).Reconcile","file":"/home/tom/dev/sparrow/pkg/sparrow/targets/gitlab.go","line":77},"msg":"Starting global gitlabTargetManager reconciler"}
{"time":"2024-02-19T17:06:07.443742549+01:00","level":"INFO","source":{"function":"github.com/caas-team/sparrow/pkg/config.(*FileLoader).Run","file":"/home/tom/dev/sparrow/pkg/config/file.go","line":69},"msg":"File Loader disabled"}
{"time":"2024-02-19T17:06:07.443776101+01:00","level":"INFO","source":{"function":"github.com/caas-team/sparrow/pkg/api.(*api).Run.func1","file":"/home/tom/dev/sparrow/pkg/api/api.go","line":76},"msg":"Serving Api","addr":":8080"}

TODO

  • I've assigned this PR to myself
  • I've labeled this PR correctly

@lvlcn-t lvlcn-t added bug Something isn't working area/config Issues/PRs related to the startup/sparrow config area labels Feb 19, 2024
@lvlcn-t lvlcn-t added this to the 0.4.0 milestone Feb 19, 2024
@lvlcn-t lvlcn-t self-assigned this Feb 19, 2024
@lvlcn-t lvlcn-t changed the title fix: tarman config fields fix: config keys of target manager Feb 19, 2024
@lvlcn-t lvlcn-t changed the title fix: config keys of target manager fix: missing tags on target manager config field Feb 19, 2024
@lvlcn-t lvlcn-t merged commit ae8c61e into main Feb 19, 2024
11 checks passed
@lvlcn-t lvlcn-t deleted the fix/tarman-config branch February 19, 2024 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/config Issues/PRs related to the startup/sparrow config area bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants