cmd flags fixes, fix the bug with multi source (#149)

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,

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: <aws|google>")
 	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: <sync|upsert-only>")
 	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: <text|json>")
 	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: <noop|txt>")
-	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)
 }

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}
 }

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 {

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
 }

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])
 				}
 			}
 		})