From 6763f09849e30e0e628a83509b1342eb276a6609 Mon Sep 17 00:00:00 2001 From: Ageev Pavel Date: Sat, 26 Dec 2020 20:55:06 +0300 Subject: [PATCH 1/3] Fix #8 - Fix bug with deregister non pinchy services - Update user guide documentation - Add logs in file source - Add logs in consul registry - Add docker-compose example for pinhcy - Remove image push to github docker registry - Push docker image to docker hub registry for branches (latest only for tags) --- .github/workflows/main.yml | 38 ++-------- Makefile | 27 ++++++- cmd/pinchy/internal/command.go | 3 + cmd/pinchy/internal/provider.go | 2 +- .../source/file/{example.yaml => example.yml} | 0 .../docker-compose/consul/docker-compose.yml | 0 .../docker-compose/pinchy/docker-compose.yml | 15 ++++ docs/registry/consul.md | 2 +- docs/source/file.md | 15 +--- docs/user-guide.md | 71 +++++++++++++++---- go.mod | 2 +- go.sum | 10 --- .../extension/registry/consul/provider.go | 8 ++- internal/extension/registry/consul/wire.go | 7 +- internal/extension/source/file/provider.go | 6 +- internal/extension/source/file/wire.go | 4 +- pkg/core/logger.go | 31 ++------ pkg/core/manager.go | 18 +++-- pkg/core/manager_test.go | 5 +- pkg/core/registry/consul/registry.go | 30 ++++++-- pkg/core/registry/consul/registry_test.go | 44 ++++++++---- pkg/core/service.go | 20 +++--- pkg/core/source/file/source.go | 7 +- pkg/core/source/file/source_test.go | 23 ++++-- 24 files changed, 234 insertions(+), 154 deletions(-) rename configs/source/file/{example.yaml => example.yml} (100%) rename {deploy => deployments}/docker-compose/consul/docker-compose.yml (100%) create mode 100644 deployments/docker-compose/pinchy/docker-compose.yml diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index c528a19..cbcd08d 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -44,7 +44,7 @@ jobs: name: Build binary runs-on: ubuntu-18.04 needs: test - if: github.event_name == 'push' && contains(github.ref, 'refs/tags/') + if: github.event_name == 'push' strategy: matrix: goos: [ darwin, linux, windows ] @@ -82,7 +82,7 @@ jobs: name: Push docker image runs-on: ubuntu-18.04 needs: binary - if: github.event_name == 'push' && contains(github.ref, 'refs/tags/') + if: github.event_name == 'push' strategy: matrix: goos: [ linux ] @@ -101,50 +101,26 @@ jobs: with: ref: ${{ github.ref }} head_ref: ${{ github.head_ref }} - - name: Build docker image for github registry - run: | - make docker-image-build DOCKER_TAG=${{ steps.get-tag-reference.outputs.tag }} - - name: Push docker image to github registry - run: | - make docker-image-push \ - DOCKER_USER=${{ github.actor }} \ - DOCKER_PASSWORD=${{ secrets.GITHUB_TOKEN }} \ - DOCKER_TAG=${{ steps.get-tag-reference.outputs.tag }} - - name: Build docker image for github registry as latest - run: | - make docker-image-build DOCKER_TAG="latest" - - name: Push docker image to github registry as latest - run: | - make docker-image-push \ - DOCKER_USER=${{ github.actor }} \ - DOCKER_PASSWORD=${{ secrets.GITHUB_TOKEN }} \ - DOCKER_TAG="latest" - name: Build docker image for docker hub registry run: | make docker-image-build \ - DOCKER_IMAGE=${{ github.repository }} \ DOCKER_TAG=${{ steps.get-tag-reference.outputs.tag }} - name: Push docker image to docker hub registry run: | make docker-image-push \ - DOCKER_REGISTRY=docker.io \ - DOCKER_IMAGE=${{ github.repository }} \ DOCKER_USER=${{ secrets.DOCKER_HUB_USERNAME }} \ DOCKER_PASSWORD=${{ secrets.DOCKER_HUB_PASSWORD }} \ DOCKER_TAG=${{ steps.get-tag-reference.outputs.tag }} - name: Build docker image for docker hub registry as latest - run: | - make docker-image-build \ - DOCKER_IMAGE=${{ github.repository }} \ - DOCKER_TAG="latest" + run: make docker-image-build + if: github.event_name == 'push' && contains(github.ref, 'refs/tags/') - name: Push docker image to docker hub registry as latest run: | make docker-image-push \ - DOCKER_REGISTRY=docker.io \ - DOCKER_IMAGE=${{ github.repository }} \ DOCKER_USER=${{ secrets.DOCKER_HUB_USERNAME }} \ - DOCKER_PASSWORD=${{ secrets.DOCKER_HUB_PASSWORD }} \ - DOCKER_TAG="latest" + DOCKER_PASSWORD=${{ secrets.DOCKER_HUB_PASSWORD }} + if: github.event_name == 'push' && contains(github.ref, 'refs/tags/') + release: name: Upload release asset runs-on: ubuntu-18.04 diff --git a/Makefile b/Makefile index 41e6f1b..81de697 100644 --- a/Makefile +++ b/Makefile @@ -8,11 +8,11 @@ CGO_ENABLED?=0 BUIlD_VERSION?=latest -DOCKER_REGISTRY?=docker.pkg.github.com -DOCKER_IMAGE?=${DOCKER_REGISTRY}/insidieux/pinchy/${APP_NAME} +DOCKER_REGISTRY?=docker.io +DOCKER_IMAGE?=insidieux/${APP_NAME} +DOCKER_TAG?=latest DOCKER_USER= DOCKER_PASSWORD= -DOCKER_TAG?=latest ifeq (, $(shell which docker)) $(error "Binary docker not found in $(PATH)") @@ -111,6 +111,12 @@ build: .PHONY: docker-image-build docker-image-build: +ifndef DOCKER_IMAGE + $(error DOCKER_IMAGE is not set) +endif +ifndef DOCKER_TAG + $(error DOCKER_TAG is not set) +endif @docker rmi ${DOCKER_IMAGE}:${DOCKER_TAG} || true @docker build \ -f ${PWD}/build/docker/cmd/pinchy/Dockerfile \ @@ -119,6 +125,21 @@ docker-image-build: .PHONY: docker-image-push docker-image-push: +ifndef DOCKER_REGISTRY + $(error DOCKER_REGISTRY is not set) +endif +ifndef DOCKER_USER + $(error DOCKER_USER is not set) +endif +ifndef DOCKER_PASSWORD + $(error DOCKER_PASSWORD is not set) +endif +ifndef DOCKER_IMAGE + $(error DOCKER_IMAGE is not set) +endif +ifndef DOCKER_TAG + $(error DOCKER_TAG is not set) +endif @docker login -u ${DOCKER_USER} -p ${DOCKER_PASSWORD} ${DOCKER_REGISTRY} @docker push ${DOCKER_IMAGE}:${DOCKER_TAG} diff --git a/cmd/pinchy/internal/command.go b/cmd/pinchy/internal/command.go index d790b50..978da8c 100644 --- a/cmd/pinchy/internal/command.go +++ b/cmd/pinchy/internal/command.go @@ -65,6 +65,9 @@ func NewCommand(version string) *cobra.Command { } watchCommand.Flags().Duration(`scheduler.interval`, time.Minute, `Interval between manager runs (1s, 1m, 5m, 1h and others)`) registryCmd.PersistentFlags().Bool(`manager.continue-on-error`, false, `Omit errors during process manager`) + registryCmd.PersistentFlags().Bool(`manager.exit-on-error`, false, `Stop manager process on first error and by pass it to command line`) + _ = registryCmd.PersistentFlags().MarkDeprecated(`manager.continue-on-error`, `Flag "manager.continue-on-error" is deprecated, use "manager.exit-on-error" instead`) + _ = registryCmd.PersistentFlags().MarkHidden(`manager.continue-on-error`) registryCmd.PersistentFlags().AddFlagSet(registryProvider.Flags()) registryCmd.AddCommand(onceCommand) registryCmd.AddCommand(watchCommand) diff --git a/cmd/pinchy/internal/provider.go b/cmd/pinchy/internal/provider.go index e9e22e7..899f23c 100644 --- a/cmd/pinchy/internal/provider.go +++ b/cmd/pinchy/internal/provider.go @@ -68,7 +68,7 @@ func provideSource(commandViper *viper.Viper, factory source.Factory, logger cor // Provider for core.Source func provideManagerExitOnError(commandViper *viper.Viper) core.ManagerExitOnError { - return core.ManagerExitOnError(commandViper.GetBool(`manager.continue-on-error`)) + return core.ManagerExitOnError(commandViper.GetBool(`manager.exit-on-error`)) } // Provider for time.Ticker diff --git a/configs/source/file/example.yaml b/configs/source/file/example.yml similarity index 100% rename from configs/source/file/example.yaml rename to configs/source/file/example.yml diff --git a/deploy/docker-compose/consul/docker-compose.yml b/deployments/docker-compose/consul/docker-compose.yml similarity index 100% rename from deploy/docker-compose/consul/docker-compose.yml rename to deployments/docker-compose/consul/docker-compose.yml diff --git a/deployments/docker-compose/pinchy/docker-compose.yml b/deployments/docker-compose/pinchy/docker-compose.yml new file mode 100644 index 0000000..5fe503b --- /dev/null +++ b/deployments/docker-compose/pinchy/docker-compose.yml @@ -0,0 +1,15 @@ +version: '3' + +services: + pinchy: + image: insidieux/pinchy:v1.0.0 + volumes: + - ./configs/source/file:/etc/pinchy + command: + - 'file' + - 'consul' + - '--source.path' + - '/etc/pinchy/example.yml' + - '--registry.address' + - 'http://consul:8500' + - 'once' diff --git a/docs/registry/consul.md b/docs/registry/consul.md index 070c159..a9ab0a3 100644 --- a/docs/registry/consul.md +++ b/docs/registry/consul.md @@ -3,5 +3,5 @@ ## Available flags ``` ---registry.address %full-http-url-to-consul-api% +--registry.address string Consul http api address (default "127.0.0.1:8500") ``` diff --git a/docs/source/file.md b/docs/source/file.md index 21c48b8..475f878 100644 --- a/docs/source/file.md +++ b/docs/source/file.md @@ -3,18 +3,9 @@ ## Available flags ``` ---source.path %full-path-to-yml-file% +--source.path string YML file config path (default "$HOME/services.yml") ``` -## config.yml example +## services.yml example -```yaml -- name: service-name - id: service-id - port: 80 - tags: - - tag-1 - - tag-2 - meta: - key: value -``` +Example services.yml file be found in [configs](./../../configs/source/file/example.yml) file. diff --git a/docs/user-guide.md b/docs/user-guide.md index a1ef1b8..f624c3a 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -4,7 +4,8 @@ ### Github Release -Visit the [releases page](https://github.com/insidieux/pinchy/releases/latest) to download one of the pre-built binaries for your platform. +Visit the [releases page](https://github.com/insidieux/pinchy/releases/latest) to download one of the pre-built binaries +for your platform. ### Docker @@ -14,13 +15,6 @@ Use the [Docker image](https://hub.docker.com/repository/docker/insidieux/pinchy docker pull insidieux/pinchy ``` -or - -```shell -echo PASSWORD_FILE | docker login docker.pkg.github.com --username USERNAME --password-stdin -docker pull docker.pkg.github.com/insidieux/pinchy -``` - ### go get Alternatively, you can use the go get method: @@ -36,29 +30,76 @@ Ensure that `$GOPATH/bin` is added to your `$PATH`. ### Binary ```shell -pinchy ... +pinchy %source% %registry% %mode% [flags] ``` ### Docker ```shell -docker run insidieux/pinchy +docker run insidieux/pinchy:latest %source% %registry% %mode% [flags] ``` -or +### Docker-compose run -```shell -docker run docker.pkg.github.com/insidieux/pinchy/pinchy +Example docker-compose file be found in [deployment](./../deployments/docker-compose/pinchy/docker-compose.yml) +directory + +## Modes + +`once` mode run sync process only single time + +`watch` mode run sync process repeatedly with constant `schedule.interval` + +## Command common flags + +``` +--logger.level string Log level (default "info") +--manager.exit-on-error Stop manager process on first error and by pass it to command line +``` + +### Watch mode + +``` +--scheduler.interval duration Interval between manager runs (1s, 1m, 5m, 1h and others) (default 1m0s) ``` -### Available source types +### Source and Registry flags + +Flags for chosen `source` and `registry` are described in a related documentation for sources and registry types. + +## Available source types - [file] [file]: ./source/file.md -### Available registry types +## Available registry types - [consul] [consul]: ./registry/consul.md + +## Examples + +### Once + +``` +pinchy \ + file \ + consul \ + once \ + --source.path /etc/pinchy/services.yml \ + --registry.address http://127.0.0.1:8500 +``` + +### Watch + +``` +pinchy \ + file \ + consul \ + watch \ + --source.path /etc/pinchy/services.yml \ + --registry.address http://127.0.0.1:8500 \ + --scheduler.interval 5s +``` diff --git a/go.mod b/go.mod index 924e056..8d1f520 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,6 @@ go 1.15 require ( github.com/agrea/ptr v0.0.0-20180711073057-77a518d99b7b - github.com/go-playground/validator/v10 v10.4.0 github.com/google/wire v0.4.0 github.com/hashicorp/consul/api v1.7.0 github.com/hashicorp/go-cleanhttp v0.5.1 @@ -18,6 +17,7 @@ require ( github.com/spf13/viper v1.7.1 github.com/stretchr/testify v1.6.1 github.com/thoas/go-funk v0.7.0 + golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 // indirect golang.org/x/net v0.0.0-20200519113804-d87ec0cfa476 // indirect golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e // indirect gopkg.in/yaml.v2 v2.3.0 // indirect diff --git a/go.sum b/go.sum index 542338d..3f79ce3 100644 --- a/go.sum +++ b/go.sum @@ -51,14 +51,6 @@ github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9 github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE= github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk= -github.com/go-playground/assert/v2 v2.0.1 h1:MsBgLAaY856+nPRTKrp3/OZK38U/wa0CcBYNjji3q3A= -github.com/go-playground/assert/v2 v2.0.1/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4= -github.com/go-playground/locales v0.13.0 h1:HyWk6mgj5qFqCT5fjGBuRArbVDfE4hi8+e8ceBS/t7Q= -github.com/go-playground/locales v0.13.0/go.mod h1:taPMhCMXrRLJO55olJkUXHZBHCxTMfnGwq/HNwmWNS8= -github.com/go-playground/universal-translator v0.17.0 h1:icxd5fm+REJzpZx7ZfpaD876Lmtgy7VtROAbHHXk8no= -github.com/go-playground/universal-translator v0.17.0/go.mod h1:UkSxE5sNxxRwHyU+Scu5vgOQjsIJAF8j9muTVoKLVtA= -github.com/go-playground/validator/v10 v10.4.0 h1:72qIR/m8ybvL8L5TIyfgrigqkrw7kVYAvjEvpT85l70= -github.com/go-playground/validator/v10 v10.4.0/go.mod h1:nlOn6nFhuKACm19sB/8EGNn9GlaMV7XkbRSipzJ0Ii4= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4= @@ -155,8 +147,6 @@ github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfn github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= -github.com/leodido/go-urn v1.2.0 h1:hpXL4XnriNwQ/ABnpepYM/1vCLWNDfUNts8dX3xTG6Y= -github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII= github.com/magiconair/properties v1.8.1 h1:ZC2Vc7/ZFkGmsVC9KvOjumD+G5lXy2RtTKyzRKO2BQ4= github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU= diff --git a/internal/extension/registry/consul/provider.go b/internal/extension/registry/consul/provider.go index df6f6b0..52ca31a 100644 --- a/internal/extension/registry/consul/provider.go +++ b/internal/extension/registry/consul/provider.go @@ -16,6 +16,8 @@ const ( registryName = `consul` flagConsulAddress = `address` + + defaultCommonTag = `pinchy` ) type ( @@ -33,7 +35,7 @@ func init() { } } -func newClientConfig(v *viper.Viper, transport *http.Transport) (*api.Config, error) { +func provideClientConfig(v *viper.Viper, transport *http.Transport) (*api.Config, error) { flag := registry.MakeFlagName(flagConsulAddress) address := v.GetString(flag) if address == `` { @@ -50,7 +52,7 @@ func provideConsulClientFactory() factory { return api.NewClient } -func newClient(cfg *api.Config, factory factory) (client, error) { +func provideClient(cfg *api.Config, factory factory) (client, error) { c, err := factory(cfg) if err != nil { return nil, errors.Wrap(err, `failed to create consul client`) @@ -58,6 +60,6 @@ func newClient(cfg *api.Config, factory factory) (client, error) { return c, nil } -func newAgent(c client) pkgConsul.Agent { +func provideAgent(c client) pkgConsul.Agent { return c.Agent() } diff --git a/internal/extension/registry/consul/wire.go b/internal/extension/registry/consul/wire.go index ba2b6b8..a5b00f1 100644 --- a/internal/extension/registry/consul/wire.go +++ b/internal/extension/registry/consul/wire.go @@ -13,11 +13,12 @@ import ( func NewRegistry(*viper.Viper) (core.Registry, func(), error) { panic(wire.Build( cleanhttp.DefaultPooledTransport, - newClientConfig, + provideClientConfig, provideConsulClientFactory, - newClient, - newAgent, + provideClient, + provideAgent, consul.NewRegistry, + wire.Value(consul.Tag(defaultCommonTag)), wire.Bind(new(core.Registry), new(*consul.Registry)), )) } diff --git a/internal/extension/source/file/provider.go b/internal/extension/source/file/provider.go index f78e05d..6a712b5 100644 --- a/internal/extension/source/file/provider.go +++ b/internal/extension/source/file/provider.go @@ -18,20 +18,20 @@ const ( func init() { set := pflag.NewFlagSet(sourceName, pflag.ExitOnError) - set.String(source.MakeFlagName(flagFilePath), `$HOME/services.yml`, `services.yml config path`) + set.String(source.MakeFlagName(flagFilePath), `$HOME/services.yml`, `YML file config path`) if err := source.Register(sourceName, set, NewSource); err != nil { panic(err) } } -func newReader() afero.Afero { +func provideReader() afero.Afero { return afero.Afero{ Fs: afero.NewReadOnlyFs(afero.NewOsFs()), } } -func newPath(v *viper.Viper) (pkgFile.Path, error) { +func providePath(v *viper.Viper) (pkgFile.Path, error) { flag := source.MakeFlagName(flagFilePath) path := v.GetString(flag) if path == `` { diff --git a/internal/extension/source/file/wire.go b/internal/extension/source/file/wire.go index b05322f..0adbc16 100644 --- a/internal/extension/source/file/wire.go +++ b/internal/extension/source/file/wire.go @@ -13,9 +13,9 @@ import ( func NewSource(*viper.Viper) (core.Source, func(), error) { panic(wire.Build( - newReader, + provideReader, wire.Bind(new(pkgFile.Reader), new(afero.Afero)), - newPath, + providePath, pkgFile.NewSource, wire.Bind(new(core.Source), new(*pkgFile.Source)), )) diff --git a/pkg/core/logger.go b/pkg/core/logger.go index 359ae6a..60424a8 100644 --- a/pkg/core/logger.go +++ b/pkg/core/logger.go @@ -1,34 +1,13 @@ package core +import ( + "github.com/sirupsen/logrus" +) + type ( // LoggerInterface provides common log methods. This is replica for logrus.FieldLogger. LoggerInterface interface { - Debugf(format string, args ...interface{}) - Infof(format string, args ...interface{}) - Printf(format string, args ...interface{}) - Warnf(format string, args ...interface{}) - Warningf(format string, args ...interface{}) - Errorf(format string, args ...interface{}) - Fatalf(format string, args ...interface{}) - Panicf(format string, args ...interface{}) - - Debug(args ...interface{}) - Info(args ...interface{}) - Print(args ...interface{}) - Warn(args ...interface{}) - Warning(args ...interface{}) - Error(args ...interface{}) - Fatal(args ...interface{}) - Panic(args ...interface{}) - - Debugln(args ...interface{}) - Infoln(args ...interface{}) - Println(args ...interface{}) - Warnln(args ...interface{}) - Warningln(args ...interface{}) - Errorln(args ...interface{}) - Fatalln(args ...interface{}) - Panicln(args ...interface{}) + logrus.FieldLogger } // Loggable determine possibility to inject LoggerInterface. Can be used to wire Source and Registry implementations diff --git a/pkg/core/manager.go b/pkg/core/manager.go index da9a15f..3622876 100644 --- a/pkg/core/manager.go +++ b/pkg/core/manager.go @@ -47,18 +47,22 @@ func NewManager(source Source, registry Registry, logger LoggerInterface, exitOn // - Remove orphan Services // - Register Services fetched from Source func (m *Manager) Run(ctx context.Context) error { + m.logger.Infoln(`Fetching services from source`) incoming, err := m.source.Fetch(ctx) if err != nil { return errors.Wrap(err, `failed to fetch services from source`) } + m.logger.Infoln(`Fetching services from registry`) registered, err := m.registry.Fetch(ctx) if err != nil { return errors.Wrap(err, `failed to fetch services from registry`) } + m.logger.Infoln(`Checking difference between registered services and incoming list`) orphan := m.findOrphan(incoming, registered) if len(orphan) > 0 { + m.logger.Infof(`Deleting %d orphan services`, len(orphan)) if err := m.deregisterServices(ctx, orphan); err != nil { err := errors.Wrap(err, `failed to deregister services`) m.logger.Error(err.Error()) @@ -67,11 +71,15 @@ func (m *Manager) Run(ctx context.Context) error { } } } - if err := m.registerServices(ctx, incoming); err != nil { - err := errors.Wrap(err, `failed to register services`) - m.logger.Error(err.Error()) - if m.exitOnError { - return err + + if len(incoming) > 0 { + m.logger.Infoln(`Registering services in registry`) + if err := m.registerServices(ctx, incoming); err != nil { + err := errors.Wrap(err, `failed to register services`) + m.logger.Error(err.Error()) + if m.exitOnError { + return err + } } } return nil diff --git a/pkg/core/manager_test.go b/pkg/core/manager_test.go index 7e149dc..9885085 100644 --- a/pkg/core/manager_test.go +++ b/pkg/core/manager_test.go @@ -56,6 +56,7 @@ type managerRunTestSuite struct { func (s *managerRunTestSuite) SetupTest() { s.manager = new(Manager) + s.manager.logger, _ = test.NewNullLogger() } func (s *managerRunTestSuite) TestErrorFetchFromSource() { @@ -91,11 +92,9 @@ func (s *managerRunTestSuite) TestErrorDeregisterOrphan() { registryMock := new(MockRegistry) registryMock.On(`Fetch`, ctx).Return(Services{{Name: `service-2`}}, nil) registryMock.On(`Deregister`, ctx, `service-2`).Return(errors.New(`expected error`)) - logger, _ := test.NewNullLogger() s.manager.source = sourceMock s.manager.registry = registryMock - s.manager.logger = logger s.manager.exitOnError = true err := s.manager.Run(ctx) @@ -111,11 +110,9 @@ func (s *managerRunTestSuite) TestErrorRegister() { registryMock := new(MockRegistry) registryMock.On(`Fetch`, ctx).Return(Services{}, nil) registryMock.On(`Register`, ctx, service).Return(errors.New(`expected error`)) - logger, _ := test.NewNullLogger() s.manager.source = sourceMock s.manager.registry = registryMock - s.manager.logger = logger s.manager.exitOnError = true err := s.manager.Run(ctx) diff --git a/pkg/core/registry/consul/registry.go b/pkg/core/registry/consul/registry.go index 7fa9dba..4691d09 100644 --- a/pkg/core/registry/consul/registry.go +++ b/pkg/core/registry/consul/registry.go @@ -2,40 +2,50 @@ package consul import ( "context" + "fmt" "github.com/agrea/ptr" "github.com/hashicorp/consul/api" "github.com/insidieux/pinchy/pkg/core" "github.com/pkg/errors" + "github.com/thoas/go-funk" ) type ( // Agent interface provide common function for work with Consul HTTP API Agent interface { - Services() (map[string]*api.AgentService, error) + ServicesWithFilter(filter string) (map[string]*api.AgentService, error) ServiceRegister(service *api.AgentServiceRegistration) error ServiceDeregister(serviceID string) error } + // Tag is a common tag for query and register services in registry + Tag string + // Registry is implementation of core.Registry interface Registry struct { - agent Agent + agent Agent + logger core.LoggerInterface + tag Tag } ) // NewRegistry provide Registry as core.Registry implementation -func NewRegistry(agent Agent) *Registry { +func NewRegistry(agent Agent, tag Tag) *Registry { return &Registry{ agent: agent, + tag: tag, } } // Fetch make request for Agent.Services and try to cast result to core.Services func (r *Registry) Fetch(_ context.Context) (core.Services, error) { - registered, err := r.agent.Services() + r.logger.Infoln(`Send services filter consul agent request`) + registered, err := r.agent.ServicesWithFilter(fmt.Sprintf(`("%s" in Tags)`, r.tag)) if err != nil { return nil, errors.Wrap(err, `failed to fetch registered services info`) } + r.logger.Infoln(`Prepare registered services list`) result := make([]*core.Service, 0) for _, item := range registered { service := &core.Service{ @@ -59,6 +69,7 @@ func (r *Registry) Fetch(_ context.Context) (core.Services, error) { // Deregister make request for Agent.ServiceDeregister by core.Service RegistrationID func (r *Registry) Deregister(_ context.Context, serviceID string) error { + r.logger.Infof(`Send service deregister consul agent request for service "%s"`, serviceID) if err := r.agent.ServiceDeregister(serviceID); err != nil { return errors.Wrapf(err, `failed deregister service by service id "%s"`, serviceID) } @@ -67,6 +78,7 @@ func (r *Registry) Deregister(_ context.Context, serviceID string) error { // Register make request for Agent.ServiceRegister for core.Service func (r *Registry) Register(ctx context.Context, service *core.Service) error { + r.logger.Infof(`Validate service "%s"`, service.RegistrationID()) if err := service.Validate(ctx); err != nil { return errors.Wrap(err, `service has validation error before registration`) } @@ -81,14 +93,22 @@ func (r *Registry) Register(ctx context.Context, service *core.Service) error { if service.Port != nil { asr.Port = *service.Port } + asr.Tags = append(asr.Tags, string(r.tag)) if service.Tags != nil { - asr.Tags = *service.Tags + asr.Tags = append(asr.Tags, *service.Tags...) } + asr.Tags = funk.UniqString(asr.Tags) if service.Meta != nil { asr.Meta = *service.Meta } + r.logger.Infof(`Send service register consul agent request for service "%s"`, service.RegistrationID()) if err := r.agent.ServiceRegister(asr); err != nil { return errors.Wrapf(err, `failed register service by service id "%s"`, service.RegistrationID()) } return nil } + +// WithLogger is implementation of core.Loggable interface +func (r *Registry) WithLogger(logger core.LoggerInterface) { + r.logger = logger +} diff --git a/pkg/core/registry/consul/registry_test.go b/pkg/core/registry/consul/registry_test.go index 80b2b58..b466b37 100644 --- a/pkg/core/registry/consul/registry_test.go +++ b/pkg/core/registry/consul/registry_test.go @@ -8,6 +8,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/insidieux/pinchy/pkg/core" "github.com/pkg/errors" + "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" ) @@ -30,6 +31,10 @@ func TestRegistry_Register(t *testing.T) { suite.Run(t, new(registryRegisterTestSuite)) } +func TestRegistry_WithLogger(t *testing.T) { + suite.Run(t, new(registryWithLoggerTestSuite)) +} + // --- Suites --- type newRegistryTestSuite struct { @@ -37,9 +42,9 @@ type newRegistryTestSuite struct { } func (s *newRegistryTestSuite) TestNewSource() { - got := NewRegistry(nil) + got := NewRegistry(nil, ``) s.Implements((*core.Registry)(nil), got) - s.Equal(&Registry{nil}, got) + s.Equal(&Registry{nil, nil, ``}, got) } type registryFetchTestSuite struct { @@ -50,11 +55,12 @@ type registryFetchTestSuite struct { func (s *registryFetchTestSuite) SetupTest() { s.agent = new(MockAgent) - s.registry = NewRegistry(s.agent) + s.registry = NewRegistry(s.agent, `test`) + s.registry.logger, _ = test.NewNullLogger() } func (s *registryFetchTestSuite) TestErrorAgentFetch() { - s.agent.On(`Services`).Return(nil, errors.New(`expected error`)) + s.agent.On(`ServicesWithFilter`, mock.Anything).Return(nil, errors.New(`expected error`)) s.registry.agent = s.agent services, err := s.registry.Fetch(context.Background()) @@ -63,7 +69,7 @@ func (s *registryFetchTestSuite) TestErrorAgentFetch() { } func (s *registryFetchTestSuite) TestSuccess() { - s.agent.On(`Services`).Return(map[string]*api.AgentService{ + s.agent.On(`ServicesWithFilter`, mock.Anything).Return(map[string]*api.AgentService{ `name`: { ID: `id`, Service: `name`, @@ -100,7 +106,8 @@ type registryDeregisterTestSuite struct { func (s *registryDeregisterTestSuite) SetupTest() { s.agent = new(MockAgent) - s.registry = NewRegistry(s.agent) + s.registry = NewRegistry(s.agent, `test`) + s.registry.logger, _ = test.NewNullLogger() } func (s *registryDeregisterTestSuite) TestErrorAgentDeregister() { @@ -125,7 +132,8 @@ type registryRegisterTestSuite struct { func (s *registryRegisterTestSuite) SetupTest() { s.agent = new(MockAgent) - s.registry = NewRegistry(s.agent) + s.registry = NewRegistry(s.agent, `test`) + s.registry.logger, _ = test.NewNullLogger() } func (s *registryRegisterTestSuite) TestErrorServiceValidation() { @@ -162,6 +170,16 @@ func (s *registryRegisterTestSuite) TestSuccess() { s.NoError(err) } +type registryWithLoggerTestSuite struct { + suite.Suite +} + +func (s *registryWithLoggerTestSuite) TestWithLogger() { + logger, _ := test.NewNullLogger() + src := NewRegistry(nil, ``) + src.WithLogger(logger) +} + // --- Mocks --- // MockAgent is an autogenerated mock type for the Agent type @@ -197,13 +215,13 @@ func (_m *MockAgent) ServiceRegister(service *api.AgentServiceRegistration) erro return r0 } -// Services provides a mock function with given fields: -func (_m *MockAgent) Services() (map[string]*api.AgentService, error) { +// ServicesWithFilter provides a mock function with given fields: filter +func (_m *MockAgent) ServicesWithFilter(filter string) (map[string]*api.AgentService, error) { ret := _m.Called() var r0 map[string]*api.AgentService - if rf, ok := ret.Get(0).(func() map[string]*api.AgentService); ok { - r0 = rf() + if rf, ok := ret.Get(0).(func(string) map[string]*api.AgentService); ok { + r0 = rf(filter) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(map[string]*api.AgentService) @@ -211,8 +229,8 @@ func (_m *MockAgent) Services() (map[string]*api.AgentService, error) { } var r1 error - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(filter) } else { r1 = ret.Error(1) } diff --git a/pkg/core/service.go b/pkg/core/service.go index 083cd81..7a3a7e4 100644 --- a/pkg/core/service.go +++ b/pkg/core/service.go @@ -3,20 +3,16 @@ package core import ( "context" - "github.com/go-playground/validator/v10" + "github.com/pkg/errors" "github.com/spf13/cast" "github.com/thoas/go-funk" ) -var ( - validation = validator.New() -) - type ( // Service contains all the necessary information for further registration in Registry Service struct { - Name string `json:"," validate:"required"` - Address string `json:"," validate:"required"` + Name string `json:","` + Address string `json:","` ID *string `json:",omitempty"` Port *int `json:",omitempty"` Tags *[]string `json:",omitempty"` @@ -28,8 +24,14 @@ type ( ) // Validate process validation to check required fields for Service, such as Service.Name and Service.Address -func (s *Service) Validate(ctx context.Context) error { - return validation.StructCtx(ctx, s) +func (s *Service) Validate(_ context.Context) error { + if s.Name == `` { + return errors.New(`service field "name" is required and cannot be empty`) + } + if s.Address == `` { + return errors.Errorf(`service "%s" field "address" is required and cannot be empty`, s.Name) + } + return nil } // RegistrationID generate identification for registration in Registry. diff --git a/pkg/core/source/file/source.go b/pkg/core/source/file/source.go index 167013e..47efc52 100644 --- a/pkg/core/source/file/source.go +++ b/pkg/core/source/file/source.go @@ -39,20 +39,23 @@ func NewSource(reader Reader, filename Path) *Source { // - validate core.Service // - return core.Services func (s *Source) Fetch(ctx context.Context) (core.Services, error) { + s.logger.Infof(`Reading file "%s"`, s.filename) contents, err := s.reader.ReadFile(string(s.filename)) if err != nil { return nil, errors.Wrap(err, `failed read content from config file`) } + s.logger.Infoln(`Decoding yml config`) items := make([]*core.Service, 0) if err := yaml.Unmarshal(contents, &items); err != nil { return nil, errors.Wrap(err, `failed unmarshal content from config file`) } + s.logger.Infoln(`Collecting services list with service validation`) result := make([]*core.Service, 0) - for _, item := range items { + for index, item := range items { if err := item.Validate(ctx); err != nil { - s.logger.Warningln(errors.Wrap(err, `failed to validate service`).Error()) + s.logger.Warningln(errors.Wrapf(err, `Failed to validate service #%d`, index).Error()) continue } result = append(result, item) diff --git a/pkg/core/source/file/source_test.go b/pkg/core/source/file/source_test.go index 8817247..9a50da7 100644 --- a/pkg/core/source/file/source_test.go +++ b/pkg/core/source/file/source_test.go @@ -23,6 +23,10 @@ func TestSource_Fetch(t *testing.T) { suite.Run(t, new(sourceFetchTestSuite)) } +func TestSource_WithLogger(t *testing.T) { + suite.Run(t, new(sourceWithLoggerTestSuite)) +} + // --- Suites --- type newSourceTestSuite struct { @@ -39,11 +43,13 @@ type sourceFetchTestSuite struct { suite.Suite source *Source reader afero.Afero + hook *test.Hook } func (s *sourceFetchTestSuite) SetupTest() { s.reader = afero.Afero{Fs: afero.NewMemMapFs()} s.source = NewSource(s.reader, `filename`) + s.source.logger, s.hook = test.NewNullLogger() } func (s *sourceFetchTestSuite) TestErrorRead() { @@ -85,15 +91,12 @@ func (s *sourceFetchTestSuite) TestSkipServiceValidationCase() { panic(errors.Wrap(err, `failed to write to in-memory file`)) } - logger, hook := test.NewNullLogger() - s.source.WithLogger(logger) - services, err := s.source.Fetch(context.Background()) s.NotNil(services) s.NoError(err) s.Equal(core.Services{}, services) - s.Equal(hook.LastEntry().Level, logrus.WarnLevel) - s.Contains(hook.LastEntry().Message, `failed to validate service`) + s.Equal(s.hook.LastEntry().Level, logrus.WarnLevel) + s.Equal(s.hook.LastEntry().Message, `Failed to validate service #0: service "service" field "address" is required and cannot be empty`) } func (s *sourceFetchTestSuite) TestSuccess() { @@ -124,3 +127,13 @@ func (s *sourceFetchTestSuite) TestSuccess() { s.NoError(err) s.Equal(expected, services) } + +type sourceWithLoggerTestSuite struct { + suite.Suite +} + +func (s *sourceWithLoggerTestSuite) TestWithLogger() { + logger, _ := test.NewNullLogger() + src := NewSource(nil, `filename`) + src.WithLogger(logger) +} From d75c407d5c2e79bce60a728260a42e6a8fe2e190 Mon Sep 17 00:00:00 2001 From: Ageev Pavel Date: Mon, 28 Dec 2020 12:33:00 +0300 Subject: [PATCH 2/3] Add log to scheduler --- pkg/core/scheduler.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/core/scheduler.go b/pkg/core/scheduler.go index c2c79c2..58cb411 100644 --- a/pkg/core/scheduler.go +++ b/pkg/core/scheduler.go @@ -33,6 +33,7 @@ func (s *Scheduler) Run(ctx context.Context) { case <-ctx.Done(): return case <-s.ticker.C: + s.logger.Infoln(`Running manager`) if err := s.manager.Run(ctx); err != nil { s.logger.Errorln(errors.Wrap(err, `failed to process manager run`).Error()) } From 8f49f1c77334d0a9db577299c0dcf429e1d5bf07 Mon Sep 17 00:00:00 2001 From: Ageev Pavel Date: Mon, 28 Dec 2020 12:39:20 +0300 Subject: [PATCH 3/3] Remove log from scheduler --- pkg/core/scheduler.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/core/scheduler.go b/pkg/core/scheduler.go index 58cb411..c2c79c2 100644 --- a/pkg/core/scheduler.go +++ b/pkg/core/scheduler.go @@ -33,7 +33,6 @@ func (s *Scheduler) Run(ctx context.Context) { case <-ctx.Done(): return case <-s.ticker.C: - s.logger.Infoln(`Running manager`) if err := s.manager.Run(ctx); err != nil { s.logger.Errorln(errors.Wrap(err, `failed to process manager run`).Error()) }