From 0b43cf511fc530c29e4e7060dde27217e90fa35d Mon Sep 17 00:00:00 2001 From: Yerken Date: Wed, 12 Apr 2017 18:13:03 +0200 Subject: [PATCH] cmd flags fixes, fix the bug with multi source (#149) --- main.go | 13 ++++++++++-- pkg/apis/externaldns/types.go | 15 +++++++------- source/multi_source.go | 2 +- source/multi_source_test.go | 2 +- source/store.go | 14 ++++++++++--- source/store_test.go | 39 +++++++++++++++++++++++++---------- 6 files changed, 59 insertions(+), 26 deletions(-) diff --git a/main.go b/main.go index 862d5ac12..659bb695f 100644 --- a/main.go +++ b/main.go @@ -37,6 +37,7 @@ import ( "github.com/kubernetes-incubator/external-dns/provider" "github.com/kubernetes-incubator/external-dns/registry" "github.com/kubernetes-incubator/external-dns/source" + "github.com/spf13/pflag" ) var ( @@ -46,6 +47,9 @@ var ( func main() { cfg := externaldns.NewConfig() if err := cfg.ParseFlags(os.Args); err != nil { + if err == pflag.ErrHelp { + os.Exit(0) + } log.Fatalf("flag parsing error: %v", err) } if cfg.Version { @@ -80,7 +84,12 @@ func main() { source.Register("service", source.NewServiceSource(client, cfg.Namespace, cfg.Compatibility)) source.Register("ingress", source.NewIngressSource(client, cfg.Namespace)) - sources := source.NewMultiSource(source.LookupMultiple(cfg.Sources...)...) + sources, err := source.LookupMultiple(cfg.Sources) + if err != nil { + log.Fatal(err) + } + + multiSource := source.NewMultiSource(sources) var p provider.Provider switch cfg.Provider { @@ -116,7 +125,7 @@ func main() { ctrl := controller.Controller{ Zone: cfg.Zone, - Source: sources, + Source: multiSource, Registry: r, Policy: policy, Interval: cfg.Interval, diff --git a/pkg/apis/externaldns/types.go b/pkg/apis/externaldns/types.go index e41abf472..091070d82 100644 --- a/pkg/apis/externaldns/types.go +++ b/pkg/apis/externaldns/types.go @@ -59,26 +59,25 @@ func NewConfig() *Config { // ParseFlags adds and parses flags from command line func (cfg *Config) ParseFlags(args []string) error { - flags := pflag.NewFlagSet("", pflag.ContinueOnError) + flags := pflag.NewFlagSet("external-dns", pflag.ContinueOnError) flags.BoolVar(&cfg.InCluster, "in-cluster", false, "whether to use in-cluster config") flags.StringVar(&cfg.KubeConfig, "kubeconfig", "", "path to a local kubeconfig file") flags.StringVar(&cfg.Namespace, "namespace", v1.NamespaceAll, "the namespace to look for endpoints; all namespaces by default") flags.StringVar(&cfg.Zone, "zone", "", "the ID of the hosted zone to target") - flags.StringArrayVar(&cfg.Sources, "source", nil, "the sources to gather endpoints from") - flags.StringVar(&cfg.Provider, "provider", "", "the DNS provider to materialize the records in") + flags.StringArrayVar(&cfg.Sources, "source", nil, "the sources to gather endpoints: [service, ingress], e.g. --source service --source ingress") + flags.StringVar(&cfg.Provider, "provider", "", "the DNS provider to materialize the records in: ") flags.StringVar(&cfg.GoogleProject, "google-project", "", "gcloud project to target") - flags.StringVar(&cfg.Policy, "policy", "sync", "the policy to use. options: [\"sync\", \"upsert-only\"]") + flags.StringVar(&cfg.Policy, "policy", "sync", "the policy to use: ") flags.BoolVar(&cfg.Compatibility, "compatibility", false, "enable to process annotation semantics from legacy implementations") flags.StringVar(&cfg.MetricsAddress, "metrics-address", defaultMetricsAddress, "address to expose metrics on") flags.StringVar(&cfg.LogFormat, "log-format", defaultLogFormat, "log format output: ") flags.DurationVar(&cfg.Interval, "interval", time.Minute, "interval between synchronizations") flags.BoolVar(&cfg.Once, "once", false, "run once and exit") - flags.BoolVar(&cfg.DryRun, "dry-run", true, "dry-run mode") + flags.BoolVar(&cfg.DryRun, "dry-run", true, "run without updating DNS provider") flags.BoolVar(&cfg.Debug, "debug", false, "debug mode") flags.BoolVar(&cfg.Version, "version", false, "display the version") flags.StringVar(&cfg.Registry, "registry", "noop", "type of registry for ownership: ") - flags.StringVar(&cfg.RecordOwnerID, "record-owner-id", "", "id of the current external dns for labeling owned records") - flags.StringVar(&cfg.TXTPrefix, "txt-prefix", "", `prefix of the associated TXT records DNS name; if --txt-prefix="abc-", - corresponding txt record for CNAME [example.org] will have DNSName [abc-example.org]. Required for CNAME ownership support`) + flags.StringVar(&cfg.RecordOwnerID, "record-owner-id", "", "id for keeping track of the managed records") + flags.StringVar(&cfg.TXTPrefix, "txt-prefix", "", `prefix assigned to DNS name of the associated TXT record; e.g. for --txt-prefix=abc_ [CNAME example.org] <-> [TXT abc_example.org]`) return flags.Parse(args) } diff --git a/source/multi_source.go b/source/multi_source.go index 913f6df0c..0ea81f126 100644 --- a/source/multi_source.go +++ b/source/multi_source.go @@ -40,6 +40,6 @@ func (ms *multiSource) Endpoints() ([]*endpoint.Endpoint, error) { } // NewMultiSource creates a new multiSource. -func NewMultiSource(children ...Source) Source { +func NewMultiSource(children []Source) Source { return &multiSource{children: children} } diff --git a/source/multi_source_test.go b/source/multi_source_test.go index 99844d0fd..79397495c 100644 --- a/source/multi_source_test.go +++ b/source/multi_source_test.go @@ -69,7 +69,7 @@ func testMultiSourceEndpoints(t *testing.T) { } // Create our object under test and get the endpoints. - source := NewMultiSource(sources...) + source := NewMultiSource(sources) endpoints, err := source.Endpoints() if err != nil { diff --git a/source/store.go b/source/store.go index 84fcd1701..228f6d45d 100644 --- a/source/store.go +++ b/source/store.go @@ -16,6 +16,8 @@ limitations under the License. package source +import "fmt" + var store = map[string]Source{} // Register registers a Source under a given name. @@ -29,10 +31,16 @@ func Lookup(name string) Source { } // LookupMultiple returns multiple Sources given multiple names. -func LookupMultiple(names ...string) (sources []Source) { +func LookupMultiple(names []string) ([]Source, error) { + sources := []Source{} + for _, name := range names { - sources = append(sources, Lookup(name)) + source := Lookup(name) + if source == nil { + return nil, fmt.Errorf("%s source could not be identified", name) + } + sources = append(sources, source) } - return sources + return sources, nil } diff --git a/source/store_test.go b/source/store_test.go index 8b688f891..c5918961c 100644 --- a/source/store_test.go +++ b/source/store_test.go @@ -53,8 +53,10 @@ func testRegisterAndLookup(t *testing.T) { // testLookupMultiple tests that Sources can be looked up by providing multiple names. func testLookupMultiple(t *testing.T) { for _, tc := range []struct { - title string - givenAndExpected map[string]Source + title string + registered map[string]Source + names []string + expectError bool }{ { "multiple registered sources are found by names", @@ -62,24 +64,39 @@ func testLookupMultiple(t *testing.T) { "foo": NewMockSource(nil), "bar": NewMockSource(nil), }, + []string{"foo", "bar"}, + false, + }, + { + "multiple registered sources, one source not registered", + map[string]Source{ + "foo": NewMockSource(nil), + "bar": NewMockSource(nil), + }, + []string{"foo", "baz"}, + true, }, } { t.Run(tc.title, func(t *testing.T) { - for k, v := range tc.givenAndExpected { + for k, v := range tc.registered { Register(k, v) } - names, sources := []string{}, []Source{} - for k, v := range tc.givenAndExpected { - names = append(names, k) - sources = append(sources, v) + lookup, err := LookupMultiple(tc.names) + if !tc.expectError && err != nil { + t.Fatal(err) } - lookup := LookupMultiple(names...) + if tc.expectError { + if err == nil { + t.Fatal("look up should fail if source not registered") + } + t.Skip() + } - for i := range names { - if lookup[i] != sources[i] { - t.Errorf("expected %#v, got %#v", sources[i], lookup[i]) + for i, name := range tc.names { + if lookup[i] != tc.registered[name] { + t.Errorf("expected %#v, got %#v", tc.registered[name], lookup[i]) } } })