Refactored seeding flow (#11)

Signed-off-by: Sotirios Mantziaris <[email protected]>

.travis.yml

@@ -8,8 +8,8 @@ install: true
 
 before_script:
   - rm -rf consul
-  - wget https://releases.hashicorp.com/consul/1.4.3/consul_1.4.3_linux_amd64.zip  
-  - unzip "consul_1.4.3_linux_amd64.zip"
+  - wget https://releases.hashicorp.com/consul/1.4.4/consul_1.4.4_linux_amd64.zip -O consul_1.4.4_linux_amd64.zip
+  - unzip "consul_1.4.4_linux_amd64.zip"
   - ./consul agent -server -bootstrap-expect 1 -data-dir /tmp/consul -dev -bind=$(hostname -I | awk '{print $1}' | xargs) -http-port 8501 -log-level=err &
   - curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOPATH)/bin v1.15.0
 

README.md

@@ -27,17 +27,20 @@ The above defines the following fields:
 - Age, which will be seeded with the value `18`, and if exists, overridden with whatever value the env var `ENV_AGE` holds
 - IsAdmin, which will be seeded with the value `true`, and if exists, overridden with whatever value the env var `ENV_AGE` holds and then from consul if the consul seeder and/or watcher are provided.
 
-`Harvester` works as follows given a config struct:
+`Harvester` has a seeding phase and an optional monitoring phase.
 
-- Seeding phase
-  - Apply the seed tag value, if present
-  - Apply the value contained in the env var, if present
-  - Apply the value return from Consul, if present and harvester is setup
-- Monitoring phase (Consul only)
-  - Monitor a key and apply if tag key matches
-  - Monitor a key-prefix and apply if tag key matches
+## Seeding phase
+
+- Apply the seed tag value, if present
+- Apply the value contained in the env var, if present
+- Apply the value returned from Consul, if present and harvester is setup to seed from consul
 
-## Seeder
+Conditions where seeding fails:
+
+- If at the end of the seeding phase one or more fields have not been seeded
+- If the seed value is invalid
+
+### Seeder
 
 `Harvester` allows the creation of custom getters which are used by the seeder and implement the following interface:
 
@@ -49,7 +52,12 @@ type Getter interface {
 
 Seed and env tags are supported by default, the Consul getter has to be setup when creating a `Harvester` with the builder.
 
-## Monitor
+## Monitoring phase (Consul only)
+
+- Monitor a key and apply if tag key matches
+- Monitor a key-prefix and apply if tag key matches
+
+### Monitor
 
 `Harvester` allows for dynamically changing the config value by monitoring a source. The following sources are available:
 
@@ -65,7 +73,7 @@ The `Harvester` builder pattern is used to create a `Harvester` instance. The bu
 - Consul monitor, for setting up monitoring from Consul
 
 ```go
-    h, err := New(tt.args.cfg).
+    h, err := New(cfg).
                 WithConsulSeed("address", "dc", "token").
                 WithConsulMonitor("address", "dc", "token", items...).
                 Create()

seed/consul/getter.go

@@ -41,13 +41,14 @@ func New(addr, dc, token string, timeout time.Duration) (*Getter, error) {
 }
 
 // Get the specific key value from consul.
-func (g *Getter) Get(key string) (string, error) {
+func (g *Getter) Get(key string) (*string, error) {
 	pair, _, err := g.kv.Get(key, &api.QueryOptions{Datacenter: g.dc, Token: g.token})
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 	if pair == nil {
-		return "", nil
+		return nil, nil
 	}
-	return string(pair.Value), nil
+	val := string(pair.Value)
+	return &val, nil
 }

seed/consul/getter_integration_test.go

@@ -40,19 +40,20 @@ func TestMain(m *testing.M) {
 }
 
 func TestGetter_Get(t *testing.T) {
+	one := "1"
 	type args struct {
 		key  string
 		addr string
 	}
 	tests := []struct {
 		name    string
 		args    args
-		want    string
+		want    *string
 		wantErr bool
 	}{
-		{name: "success", args: args{addr: addr, key: "get_key1"}, want: "1", wantErr: false},
-		{name: "missing key", args: args{addr: addr, key: "get_key2"}, want: "", wantErr: false},
-		{name: "wrong address", args: args{addr: "xxx", key: "get_key1"}, want: "", wantErr: true},
+		{name: "success", args: args{addr: addr, key: "get_key1"}, want: &one, wantErr: false},
+		{name: "missing key", args: args{addr: addr, key: "get_key2"}, want: nil, wantErr: false},
+		{name: "wrong address", args: args{addr: "xxx", key: "get_key1"}, want: nil, wantErr: true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

seed/seed.go

@@ -2,15 +2,17 @@ package seed
 
 import (
 	"errors"
+	"fmt"
 	"os"
+	"strings"
 
 	"github.com/taxibeat/harvester/config"
 	"github.com/taxibeat/harvester/log"
 )
 
 // Getter interface for fetching a value for a specific key.
 type Getter interface {
-	Get(key string) (string, error)
+	Get(key string) (*string, error)
 }
 
 // Param parameters for setting a getter for a specific source.
@@ -43,14 +45,17 @@ func New(pp ...Param) *Seeder {
 
 // Seed the provided config with values for their sources.
 func (s *Seeder) Seed(cfg *config.Config) error {
+	seedMap := make(map[*config.Field]bool, len(cfg.Fields))
 	for _, f := range cfg.Fields {
+		seedMap[f] = false
 		val, ok := f.Sources[config.SourceSeed]
 		if ok {
 			err := cfg.Set(f.Name, val, f.Kind)
 			if err != nil {
 				return err
 			}
-			log.Infof("seed value %s applied on %s", val, f.Name)
+			log.Infof("seed value %s applied on field %s", val, f.Name)
+			seedMap[f] = true
 		}
 		key, ok := f.Sources[config.SourceEnv]
 		if ok {
@@ -60,9 +65,10 @@ func (s *Seeder) Seed(cfg *config.Config) error {
 				if err != nil {
 					return err
 				}
-				log.Infof("env var value %s applied on %s", val, f.Name)
+				log.Infof("env var value %s applied on field %s", val, f.Name)
+				seedMap[f] = true
 			} else {
-				log.Warnf("env var %s did not exist for %s", key, f.Name)
+				log.Warnf("env var %s did not exist for field %s", key, f.Name)
 			}
 		}
 		key, ok = f.Sources[config.SourceConsul]
@@ -73,14 +79,29 @@ func (s *Seeder) Seed(cfg *config.Config) error {
 			}
 			value, err := gtr.Get(key)
 			if err != nil {
-				return err
+				log.Errorf("failed to get consul key %s for field %s: %v", key, f.Name, err)
+				continue
+			}
+			if value == nil {
+				log.Warnf("consul key %s did not exist for field %s", key, f.Name)
+				continue
 			}
-			err = cfg.Set(f.Name, value, f.Kind)
+			err = cfg.Set(f.Name, *value, f.Kind)
 			if err != nil {
 				return err
 			}
-			log.Infof("consul value %s applied on %s", val, f.Name)
+			log.Infof("consul value %s applied on filed %s", val, f.Name)
+			seedMap[f] = true
 		}
 	}
+	sb := strings.Builder{}
+	for f, seeded := range seedMap {
+		if !seeded {
+			sb.WriteString(fmt.Sprintf("field %s not seeded", f.Name))
+		}
+	}
+	if sb.Len() > 0 {
+		return errors.New(sb.String())
+	}
 	return nil
 }

seed/seed_test.go

@@ -49,6 +49,8 @@ func TestSeeder_Seed(t *testing.T) {
 	require.NoError(t, err)
 	invalidBoolCfg, err := config.New(&testInvalidBool{})
 	require.NoError(t, err)
+	missingCfg, err := config.New(&testMissingValue{})
+	require.NoError(t, err)
 	prmSuccess, err := NewParam(config.SourceConsul, &testConsulGet{})
 	require.NoError(t, err)
 	prmError, err := NewParam(config.SourceConsul, &testConsulGet{err: true})
@@ -68,7 +70,8 @@ func TestSeeder_Seed(t *testing.T) {
 	}{
 		{name: "success", fields: fields{consulParam: prmSuccess}, args: args{cfg: goodCfg}, wantErr: false},
 		{name: "consul get nil", args: args{cfg: goodCfg}, wantErr: true},
-		{name: "consul get error", fields: fields{consulParam: prmError}, args: args{cfg: goodCfg}, wantErr: true},
+		{name: "consul get error but previous seed successful", fields: fields{consulParam: prmError}, args: args{cfg: goodCfg}, wantErr: false},
+		{name: "consul missing value", fields: fields{consulParam: prmSuccess}, args: args{cfg: missingCfg}, wantErr: true},
 		{name: "invalid int", args: args{cfg: invalidIntCfg}, wantErr: true},
 		{name: "invalid float", args: args{cfg: invalidFloatCfg}, wantErr: true},
 		{name: "invalid bool", fields: fields{consulParam: prmSuccess}, args: args{cfg: invalidBoolCfg}, wantErr: true},
@@ -114,16 +117,25 @@ type testInvalidBool struct {
 	HasJob bool `consul:"/config/XXX"`
 }
 
+type testMissingValue struct {
+	HasJob bool `consul:"/config/YYY"`
+}
+
 type testConsulGet struct {
 	err bool
 }
 
-func (tcg *testConsulGet) Get(key string) (string, error) {
+func (tcg *testConsulGet) Get(key string) (*string, error) {
 	if tcg.err {
-		return "", errors.New("TEST")
+		return nil, errors.New("TEST")
+	}
+	if key == "/config/YYY" {
+		return nil, nil
 	}
+	val := "XXX"
 	if key == "/config/XXX" {
-		return "XXX", nil
+		return &val, nil
 	}
-	return "true", nil
+	val = "true"
+	return &val, nil
 }