Addressing comments

.travis.yml

@@ -8,6 +8,4 @@ env:
 sudo: required
 dist: trusty
 script:
- - make lint
- - make test-ci-unit
- - make m3ctl
+ - make all

Makefile

@@ -42,6 +42,9 @@ services-linux-amd64:
 
 $(foreach SERVICE,$(SERVICES),$(eval $(SERVICE_RULES)))
 
+all: lint test-ci-unit services
+	@echo Made all successfully
+
 lint:
 	@which golint > /dev/null || go get -u github.com/golang/lint/golint
 	$(lint_check)

handler/common.go

@@ -22,6 +22,7 @@ package handler
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
 )
 
@@ -38,15 +39,19 @@ type NotSupportdResponse struct {
 }
 
 // MethodNotSupportedResponse writes a method not supported response to the writer
-func MethodNotSupportedResponse(w http.ResponseWriter, r *http.Request, supported []string) {
+func MethodNotSupportedResponse(w http.ResponseWriter, r *http.Request, supported []string) error {
 	w.WriteHeader(http.StatusMethodNotAllowed)
 	response := NotSupportdResponse{
 		Error:            "Method not supported",
 		URI:              r.RequestURI,
 		MethodName:       r.Method,
 		SupportedMethods: supported}
-	body, _ := json.Marshal(response)
+	body, err := json.Marshal(response)
+	if err != nil {
+		return fmt.Errorf("Could not generate MethodNotSupportedResponse for %s - %s", r.Method, r.RequestURI)
+	}
 	w.Write(body)
+	return nil
 }
 
 // InitJSONResponse sets common headers for m3ctl on a ResponseWriter

handler/common_test.go

@@ -45,9 +45,7 @@ func TestMethodNotSupportedResponse(t *testing.T) {
 
 	rawResult := make([]byte, rr.Body.Len())
 	_, err := rr.Body.Read(rawResult)
-	if err != nil {
-		t.Errorf("Encountered error parsing reponse")
-	}
+	require.NoError(t, err, "Encountered error parsing reponse")
 
 	var actualResult NotSupportdResponse
 	json.Unmarshal(rawResult, &actualResult)

handler/controller.go

@@ -20,14 +20,10 @@
 
 package handler
 
-import (
-	"net/http"
-
-	"github.com/m3db/m3x/instrument"
-)
+import "net/http"
 
 // Controller defines functions around a controllable entity
 type Controller interface {
 	// Registers the handlers for this controll with the given ServeMux
-	RegisterHandlers(*http.ServeMux, instrument.Options) error
+	RegisterHandlers(*http.ServeMux) error
 }

handler/health/health.go

@@ -27,17 +27,18 @@ import (
 	"time"
 
 	"github.com/m3db/m3ctl/handler"
+
 	"github.com/m3db/m3x/instrument"
 )
 
 const (
-	ok   = "OK"
-	fail = "FAIL"
+	ok          = "OK"
+	fail        = "FAIL"
+	healthURL   = "/health"
+	unknownName = "unkown"
 )
 
-var (
-	healthURL = "/health"
-)
+var ()
 
 type health struct {
 	Host         string        `json:"host"`
@@ -47,7 +48,14 @@ type health struct {
 }
 
 // Controller sets up the handlers for health endpoint
-type Controller struct{}
+type controller struct {
+	instruments instrument.Options
+}
+
+// NewController creates a new rules controller
+func NewController(instrument instrument.Options) handler.Controller {
+	return &controller{instruments: instrument}
+}
 
 // Returns health check status msg plus additional message
 func healthCheck() string {
@@ -57,7 +65,7 @@ func healthCheck() string {
 func gatherHostInfo() string {
 	host, err := os.Hostname()
 	if err != nil {
-		host = "unknown"
+		host = unknownName
 	}
 	return host
 }
@@ -81,8 +89,8 @@ func m3ctlHealthCheck(w http.ResponseWriter, r *http.Request) {
 }
 
 // RegisterHandlers registers health handlers
-func (c Controller) RegisterHandlers(mux *http.ServeMux, inst instrument.Options) error {
-	log := inst.Logger()
+func (c *controller) RegisterHandlers(mux *http.ServeMux) error {
+	log := c.instruments.Logger()
 	mux.HandleFunc(healthURL, m3ctlHealthCheck)
 	log.Infof("Registered health endpoints")
 	return nil

handler/rules/global.go

@@ -20,8 +20,10 @@
 
 package rules
 
-import "net/http"
-import "fmt"
+import (
+	"fmt"
+	"net/http"
+)
 
 func getNamespaces(w http.ResponseWriter, r *http.Request) {
 	w.WriteHeader(http.StatusNotImplemented)

handler/rules/rules.go

@@ -23,59 +23,74 @@ package rules
 import (
 	"net/http"
 
-	"github.com/m3db/m3ctl/handler"
 	"github.com/m3db/m3x/instrument"
+
+	"github.com/m3db/m3ctl/handler"
 )
 
-var (
+const (
 	getAllURL          = "/rules/namespaces"
 	createOrDeleteURL  = "/rules/namespace"
 	modifyNamespaceURL = "/rules/namespace/"
 )
 
 // Controller sets up the handlers for rules
-type Controller struct{}
+type controller struct {
+	instruments instrument.Options
+}
+
+// NewController creates a new rules controller
+func NewController(instrument instrument.Options) handler.Controller {
+	return &controller{instruments: instrument}
+}
+
+func (c *controller) handleNotSupported(w http.ResponseWriter, r *http.Request, supported []string) {
+	err := handler.MethodNotSupportedResponse(w, r, []string{http.MethodGet})
+	if err != nil {
+		c.instruments.Logger().Errorf("%v", err)
+	}
+}
 
 // RegisterHandlers registers rule handlers
-func (c Controller) RegisterHandlers(mux *http.ServeMux, inst instrument.Options) error {
-	log := inst.Logger()
-	mux.HandleFunc(getAllURL, handleGetAll)
-	mux.HandleFunc(createOrDeleteURL, handleCreateDelete)
-	mux.HandleFunc(modifyNamespaceURL, handleModify)
+func (c *controller) RegisterHandlers(mux *http.ServeMux) error {
+	log := c.instruments.Logger()
+	mux.HandleFunc(getAllURL, c.handleGetAll)
+	mux.HandleFunc(createOrDeleteURL, c.handleCreateDelete)
+	mux.HandleFunc(modifyNamespaceURL, c.handleModify)
 	log.Infof("Registered rules endpoints")
 	return nil
 }
 
-func handleGetAll(w http.ResponseWriter, r *http.Request) {
+func (c *controller) handleGetAll(w http.ResponseWriter, r *http.Request) {
 	handler.InitJSONResponse(w)
 	switch r.Method {
 	case http.MethodGet:
 		getNamespaces(w, r)
 	default:
-		handler.MethodNotSupportedResponse(w, r, []string{http.MethodGet})
+		c.handleNotSupported(w, r, []string{http.MethodGet})
 	}
 }
 
-func handleCreateDelete(w http.ResponseWriter, r *http.Request) {
+func (c *controller) handleCreateDelete(w http.ResponseWriter, r *http.Request) {
 	handler.InitJSONResponse(w)
 	switch r.Method {
 	case http.MethodPost:
 		createNamespace(w, r)
 	case http.MethodDelete:
 		deleteNamespace(w, r)
 	default:
-		handler.MethodNotSupportedResponse(w, r, []string{http.MethodPost, http.MethodDelete})
+		c.handleNotSupported(w, r, []string{http.MethodPost, http.MethodDelete})
 	}
 }
 
-func handleModify(w http.ResponseWriter, r *http.Request) {
+func (c *controller) handleModify(w http.ResponseWriter, r *http.Request) {
 	handler.InitJSONResponse(w)
 	switch r.Method {
 	case http.MethodGet:
 		getRulesInNamespace(w, r)
 	case http.MethodPut:
 		setRulesInNamespace(w, r)
 	default:
-		handler.MethodNotSupportedResponse(w, r, []string{http.MethodGet, http.MethodPut})
+		c.handleNotSupported(w, r, []string{http.MethodGet, http.MethodPut})
 	}
 }

server/handlers.go

@@ -23,22 +23,14 @@ package server
 import (
 	"net/http"
 
-	"github.com/m3db/m3ctl/handler"
 	"github.com/m3db/m3ctl/handler/health"
 	"github.com/m3db/m3ctl/handler/rules"
-	"github.com/m3db/m3x/instrument"
-)
 
-var (
-	controllers = []handler.Controller{
-		rules.Controller{},
-		health.Controller{},
-	}
+	"github.com/m3db/m3x/instrument"
 )
 
 func registerHandlers(mux *http.ServeMux, instruments instrument.Options) error {
-	for _, controller := range controllers {
-		controller.RegisterHandlers(mux, instruments)
-	}
+	rules.NewController(instruments).RegisterHandlers(mux)
+	health.NewController(instruments).RegisterHandlers(mux)
 	return nil
 }

server/options.go

@@ -58,6 +58,15 @@ type options struct {
 	writeTimeout   time.Duration
 }
 
+// NewOptions creates a new set of server options
+func NewOptions() Options {
+	return &options{
+		readTimeout:    defaultReadTimeout,
+		writeTimeout:   defaultWriteTimeout,
+		instrumentOpts: instrument.NewOptions(),
+	}
+}
+
 func (o *options) SetInstrumentOptions(value instrument.Options) Options {
 	opts := *o
 	opts.instrumentOpts = value
@@ -68,15 +77,6 @@ func (o *options) InstrumentOptions() instrument.Options {
 	return o.instrumentOpts
 }
 
-// NewOptions creates a new set of server options
-func NewOptions() Options {
-	return &options{
-		readTimeout:    defaultReadTimeout,
-		writeTimeout:   defaultWriteTimeout,
-		instrumentOpts: instrument.NewOptions(),
-	}
-}
-
 func (o *options) SetReadTimeout(value time.Duration) Options {
 	opts := *o
 	opts.readTimeout = value

services/m3ctl/main/main.go

@@ -28,12 +28,13 @@ import (
 	"syscall"
 	"time"
 
-	etcdclient "github.com/m3db/m3cluster/client/etcd"
-	"github.com/m3db/m3ctl/server"
-	"github.com/m3db/m3ctl/services/m3ctl/serve"
+	"github.com/m3db/m3cluster/client/etcd"
 	"github.com/m3db/m3x/config"
 	"github.com/m3db/m3x/instrument"
 	"github.com/m3db/m3x/log"
+
+	"github.com/m3db/m3ctl/server"
+	"github.com/m3db/m3ctl/services/m3ctl/serve"
 )
 
 type serverConfig struct {
@@ -63,7 +64,7 @@ func (c *serverConfig) NewHTTPServerOptions(
 }
 
 type kvClientConfiguration struct {
-	Etcd *etcdclient.Configuration `yaml:"etcd"`
+	Etcd *etcd.Configuration `yaml:"etcd"`
 }
 
 type configuration struct {