Merge pull request #12 from cybozu-go/cmd

Use the command framework

Author: yutannihilation

.travis.yml

@@ -1,7 +1,7 @@
 sudo: false
 language: go
 go:
-  - 1.6
+  - 1.7
   - tip
 
 before_install:
@@ -24,5 +24,5 @@ deploy:
     - ${GO_APT_MIRROR}
   skip_cleanup: true
   on:
-    go: 1.6
+    go: 1.7
     tags: true

CHANGELOG.md

@@ -0,0 +1,35 @@
+# Change Log
+
+All notable changes to this project will be documented in this file.
+
+## [Unreleased]
+### Added
+- aptuitl now adopts [github.com/cybozu-go/cmd][cmd] framework.  
+  As a result, commands implement [the common spec][spec].
+- [cacher] added `listen_address` configuration parameter to specify listening address (#9).
+- [cacher] added `log` configuration section to specify logging options.
+- [mirror] added `log` configuration section to specify logging options.
+
+### Changed
+- aptutil now requires Go 1.7 or better.
+
+### Removed
+- [cacher] `-s` and `-l` command-line flags are gone.
+- [mirror] `-s` command-line flag is gone.
+
+## [1.1.0]
+### Changed
+- Update docs (kudos to @xipmix).
+- [cacher] extend Release file check interval from 15 to 600 seconds (#8).
+
+## [1.0.1]
+### Changed
+- [mirror] ignore Sources if `include_source` is not specified in mirror.toml.  
+  This works as a workaround for some badly configured web servers.
+
+
+[cmd]: https://github.com/cybozu-go/cmd
+[spec]: https://github.com/cybozu-go/cmd/blob/master/README.md#specifications
+[Unreleased]: https://github.com/cybozu-go/aptutil/compare/v1.1.0...HEAD
+[1.1.0]: https://github.com/cybozu-go/aptutil/compare/v1.0.1...v1.1.0
+[1.0.1]: https://github.com/cybozu-go/aptutil/compare/v1.0.0...v1.0.1

README.md

@@ -46,7 +46,7 @@ Usage
 Build
 -----
 
-Use Go 1.6 or better.
+Use Go 1.7 or better.
 
 Run the command below exactly as shown, including the ellipsis.
 They are significant - see `go help packages`.

cacher/cacher.go

@@ -5,6 +5,7 @@ package cacher
 
 import (
 	"bytes"
+	"context"
 	"io/ioutil"
 	"net/http"
 	"net/url"
@@ -18,8 +19,6 @@ import (
 	"github.com/cybozu-go/aptutil/apt"
 	"github.com/cybozu-go/log"
 	"github.com/pkg/errors"
-	"golang.org/x/net/context"
-	"golang.org/x/net/context/ctxhttp"
 )
 
 const (
@@ -60,15 +59,11 @@ type Cacher struct {
 
 // NewCacher constructs Cacher.
 func NewCacher(ctx context.Context, config *Config) (*Cacher, error) {
-	checkInterval := time.Duration(config.CheckInterval) * time.Second
-	if checkInterval == 0 {
-		checkInterval = defaultCheckInterval * time.Second
+	if config.CheckInterval == 0 {
+		return nil, errors.New("invaild check_interval")
 	}
-
+	checkInterval := time.Duration(config.CheckInterval) * time.Second
 	cachePeriod := time.Duration(config.CachePeriod) * time.Second
-	if cachePeriod == 0 {
-		cachePeriod = defaultCachePeriod * time.Second
-	}
 
 	metaDir := filepath.Clean(config.MetaDirectory)
 	if !filepath.IsAbs(metaDir) {
@@ -84,10 +79,10 @@ func NewCacher(ctx context.Context, config *Config) (*Cacher, error) {
 		return nil, errors.New("meta_dir and cache_dir must be different")
 	}
 
-	capacity := uint64(config.CacheCapacity) * gib
-	if capacity == 0 {
-		capacity = defaultCacheCapacity * gib
+	if config.CacheCapacity <= 0 {
+		return nil, errors.New("cache_capacity must be > 0")
 	}
+	capacity := uint64(config.CacheCapacity) * gib
 
 	meta := NewStorage(metaDir, 0)
 	cache := NewStorage(cacheDir, capacity)
@@ -206,7 +201,7 @@ func (c *Cacher) maintRelease(p string, withGPG bool) {
 
 	if log.Enabled(log.LvDebug) {
 		log.Debug("maintRelease", map[string]interface{}{
-			"_path": p,
+			"path": p,
 		})
 	}
 
@@ -287,11 +282,19 @@ func (c *Cacher) download(p string, u *url.URL, valid *apt.FileInfo) {
 	ctx, cancel := context.WithTimeout(c.ctx, requestTimeout)
 	defer cancel()
 
-	resp, err := ctxhttp.Get(ctx, c.client, u.String())
+	req := &http.Request{
+		Method:     "GET",
+		URL:        u,
+		Proto:      "HTTP/1.1",
+		ProtoMajor: 1,
+		ProtoMinor: 1,
+		Header:     make(http.Header),
+	}
+	resp, err := c.client.Do(req.WithContext(ctx))
 	if err != nil {
 		log.Warn("GET failed", map[string]interface{}{
-			"_url": u.String(),
-			"_err": err.Error(),
+			"url":   u.String(),
+			"error": err.Error(),
 		})
 		return
 	}
@@ -305,16 +308,16 @@ func (c *Cacher) download(p string, u *url.URL, valid *apt.FileInfo) {
 
 	if err != nil {
 		log.Warn("GET failed", map[string]interface{}{
-			"_url": u.String(),
-			"_err": err.Error(),
+			"url":   u.String(),
+			"error": err.Error(),
 		})
 		return
 	}
 
 	fi := apt.MakeFileInfo(p, body)
 	if valid != nil && !valid.Same(fi) {
 		log.Warn("downloaded data is not valid", map[string]interface{}{
-			"_url": u.String(),
+			"url": u.String(),
 		})
 		return
 	}
@@ -331,8 +334,8 @@ func (c *Cacher) download(p string, u *url.URL, valid *apt.FileInfo) {
 		fil, err = apt.ExtractFileInfo(t[1], bytes.NewReader(body))
 		if err != nil {
 			log.Error("invalid meta data", map[string]interface{}{
-				"_path": p,
-				"_err":  err.Error(),
+				"path":  p,
+				"error": err.Error(),
 			})
 			// do not return; we accept broken meta data as is.
 		}
@@ -344,8 +347,8 @@ func (c *Cacher) download(p string, u *url.URL, valid *apt.FileInfo) {
 
 	if err := storage.Insert(body, fi); err != nil {
 		log.Error("could not save an item", map[string]interface{}{
-			"_path": p,
-			"_err":  err.Error(),
+			"path":  p,
+			"error": err.Error(),
 		})
 		// panic because go-apt-cacher cannot continue working
 		panic(err)
@@ -363,7 +366,7 @@ func (c *Cacher) download(p string, u *url.URL, valid *apt.FileInfo) {
 	}
 	c.info[p] = fi
 	log.Info("downloaded and cached", map[string]interface{}{
-		"_path": p,
+		"path": p,
 	})
 }
 
@@ -401,7 +404,7 @@ RETRY:
 		case ErrNotFound:
 		default:
 			log.Error("lookup failure", map[string]interface{}{
-				"_err": err.Error(),
+				"error": err.Error(),
 			})
 			return http.StatusInternalServerError, nil, err
 		}

cacher/config.go

@@ -1,6 +1,9 @@
 package cacher
 
+import "github.com/cybozu-go/cmd"
+
 const (
+	defaultAddress       = ":3142"
 	defaultCheckInterval = 600
 	defaultCachePeriod   = 3
 	defaultCacheCapacity = 1
@@ -11,12 +14,17 @@ const (
 //
 // Use https://github.com/BurntSushi/toml as follows:
 //
-//    var config cacher.Config
-//    md, err := toml.DecodeFile("/path/to/config.toml", &config)
+//    config := cacher.NewConfig()
+//    md, err := toml.DecodeFile("/path/to/config.toml", config)
 //    if err != nil {
 //        ...
 //    }
 type Config struct {
+	// Addr is the listening address of HTTP server.
+	//
+	// Default is ":3142".
+	Addr string `toml:"listen_address"`
+
 	// CheckInterval specifies interval in seconds to check updates for
 	// Release/InRelease files.
 	//
@@ -49,6 +57,20 @@ type Config struct {
 	// Zero disables limit on the number of connections.
 	MaxConns int `toml:"max_conns"`
 
+	// Log is cmd.LogConfig
+	Log cmd.LogConfig `toml:"log"`
+
 	// Mapping specifies mapping between prefixes and APT URLs.
 	Mapping map[string]string `toml:"mapping"`
 }
+
+// NewConfig creates Config with default values.
+func NewConfig() *Config {
+	return &Config{
+		Addr:          defaultAddress,
+		CheckInterval: defaultCheckInterval,
+		CachePeriod:   defaultCachePeriod,
+		CacheCapacity: defaultCacheCapacity,
+		MaxConns:      defaultMaxConns,
+	}
+}

cacher/config_test.go

@@ -9,7 +9,7 @@ import (
 func TestConfig(t *testing.T) {
 	t.Parallel()
 
-	var config Config
+	config := NewConfig()
 	md, err := toml.DecodeFile("t/cacher.toml", &config)
 	if err != nil {
 		t.Fatal(err)
@@ -34,8 +34,12 @@ func TestConfig(t *testing.T) {
 	if config.CacheCapacity != 21 {
 		t.Error(`config.CacheCapacity != 21`)
 	}
-	if config.MaxConns != 3 {
-		t.Error(`config.MaxConns != 3`)
+	if config.MaxConns != defaultMaxConns {
+		t.Error(`config.MaxConns != defaultMaxConns`)
+	}
+
+	if config.Log.Level != "error" {
+		t.Error(`config.Log.Level != "error"`)
 	}
 
 	if config.Mapping["ubuntu"] != "http://archive.ubuntu.com/ubuntu" {

cacher/handler.go

@@ -24,12 +24,11 @@ func (c cacheHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	accepted := time.Now()
 	p := path.Clean(r.URL.Path[1:])
 
 	if log.Enabled(log.LvDebug) {
 		log.Debug("request path", map[string]interface{}{
-			"_path": p,
+			"path": p,
 		})
 	}
 
@@ -48,13 +47,13 @@ func (c cacheHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		if r.Method == "GET" {
 			var zeroTime time.Time
 			http.ServeContent(w, r, path.Base(p), zeroTime, f)
-			goto LOG
+			return
 		}
 		stat, err := f.Stat()
 		if err != nil {
 			status = http.StatusInternalServerError
 			http.Error(w, err.Error(), status)
-			goto LOG
+			return
 		}
 		ct := mime.TypeByExtension(path.Ext(p))
 		if ct == "" {
@@ -64,14 +63,4 @@ func (c cacheHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Length", strconv.FormatInt(stat.Size(), 10))
 		w.WriteHeader(http.StatusOK)
 	}
-
-LOG:
-	took := time.Now().Sub(accepted)
-	log.Info("[http]", map[string]interface{}{
-		"_method":      r.Method,
-		"_elapsed":     took.String(),
-		"_path":        p,
-		"_status":      status,
-		"_remote_addr": r.RemoteAddr,
-	})
 }

cacher/server.go

@@ -1,47 +1,22 @@
 package cacher
 
 import (
-	_log "log"
-	"net"
 	"net/http"
-	"time"
 
-	"github.com/cybozu-go/log"
-	"github.com/facebookgo/httpdown"
-	"golang.org/x/net/context"
+	"github.com/cybozu-go/cmd"
 )
 
-const (
-	defaultReadTimeout  = 10 * time.Second
-	defaultWriteTimeout = 10 * time.Second
-)
-
-// Serve runs REST API server until ctx.Done() is closed.
-func Serve(ctx context.Context, l net.Listener, c *Cacher) error {
-	hd := httpdown.HTTP{}
-	logger := _log.New(log.DefaultLogger().Writer(log.LvError), "[http]", 0)
-	s := &http.Server{
-		Handler:      cacheHandler{c},
-		ReadTimeout:  defaultReadTimeout,
-		WriteTimeout: defaultWriteTimeout,
-		ErrorLog:     logger,
+// NewServer returns HTTPServer implements go-apt-cacher handlers.
+func NewServer(c *Cacher, config *Config) *cmd.HTTPServer {
+	addr := config.Addr
+	if len(addr) == 0 {
+		addr = defaultAddress
 	}
-	hs := hd.Serve(s, l)
-
-	waiterr := make(chan error, 1)
-	go func() {
-		defer close(waiterr)
-		waiterr <- hs.Wait()
-	}()
-
-	select {
-	case err := <-waiterr:
-		return err
 
-	case <-ctx.Done():
-		if err := hs.Stop(); err != nil {
-			return err
-		}
-		return <-waiterr
+	return &cmd.HTTPServer{
+		Server: &http.Server{
+			Addr:    addr,
+			Handler: cacheHandler{c},
+		},
 	}
 }