Merge pull request kubevirt#546 from mpolednik/service-iface

improve flag (command line option) handling code in services

cmd/virt-api/virt-api.go

@@ -20,23 +20,16 @@
 package main
 
 import (
-	"flag"
-
-	"github.com/spf13/pflag"
-
 	klog "kubevirt.io/kubevirt/pkg/log"
+	"kubevirt.io/kubevirt/pkg/service"
 	"kubevirt.io/kubevirt/pkg/virt-api"
 )
 
 func main() {
 	klog.InitializeLogging("virt-api")
-	swaggerui := flag.String("swagger-ui", "third_party/swagger-ui", "swagger-ui location")
-	host := flag.String("listen", "0.0.0.0", "Address and port where to listen on")
-	port := flag.Int("port", 8183, "Port to listen on")
-	pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
-	pflag.Parse()
 
-	app := virt_api.NewVirtAPIApp(*host, *port, *swaggerui)
+	app := virt_api.VirtAPIApp{}
+	service.Setup(&app)
 	app.Compose()
 	app.ConfigureOpenAPIService()
 	app.Run()

cmd/virt-handler/virt-handler.go

@@ -20,7 +20,6 @@
 package main
 
 import (
-	"flag"
 	"fmt"
 	"net/http"
 	"os"
@@ -29,7 +28,7 @@ import (
 
 	"github.com/emicklei/go-restful"
 	"github.com/libvirt/libvirt-go"
-	"github.com/spf13/pflag"
+	flag "github.com/spf13/pflag"
 	k8sv1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
@@ -56,37 +55,46 @@ import (
 	watchdog "kubevirt.io/kubevirt/pkg/watchdog"
 )
 
-const defaultWatchdogTimeout = 30 * time.Second
+const (
+	defaultWatchdogTimeout = 30 * time.Second
+
+	// Default port that virt-handler listens on.
+	defaultPort = 8185
+
+	// Default address that virt-handler listens on.
+	defaultHost = "0.0.0.0"
+
+	// The URI connection string supplied to libvirt. By default, we connect to system-mode daemon of QEMU.
+	libvirtUri = "qemu:///system"
+
+	hostOverride = ""
+
+	virtShareDir = "/var/run/kubevirt"
+
+	ephemeralDiskDir = "/var/run/libvirt/kubevirt-ephemeral-disk"
+)
 
 type virtHandlerApp struct {
-	Service                 *service.Service
+	service.ServiceListen
+	service.ServiceLibvirt
 	HostOverride            string
-	LibvirtUri              string
 	VirtShareDir            string
 	EphemeralDiskDir        string
 	WatchdogTimeoutDuration time.Duration
 }
 
-func newVirtHandlerApp(host *string, port *int, hostOverride *string, libvirtUri *string, virtShareDir *string, ephemeralDiskDir *string, watchdogTimeoutDuration *time.Duration) *virtHandlerApp {
-	if *hostOverride == "" {
+var _ service.Service = &virtHandlerApp{}
+
+func (app *virtHandlerApp) Run() {
+	// HostOverride should default to os.Hostname(), to make sure we handle errors ensure it here.
+	if app.HostOverride == "" {
 		defaultHostName, err := os.Hostname()
 		if err != nil {
 			panic(err)
 		}
-		*hostOverride = defaultHostName
+		app.HostOverride = defaultHostName
 	}
 
-	return &virtHandlerApp{
-		Service:                 service.NewService("virt-handler", *host, *port),
-		HostOverride:            *hostOverride,
-		LibvirtUri:              *libvirtUri,
-		VirtShareDir:            *virtShareDir,
-		EphemeralDiskDir:        *ephemeralDiskDir,
-		WatchdogTimeoutDuration: *watchdogTimeoutDuration,
-	}
-}
-
-func (app *virtHandlerApp) Run() {
 	logger := log.Log
 	logger.V(1).Level(log.INFO).Log("hostname", app.HostOverride)
 
@@ -189,23 +197,37 @@ func (app *virtHandlerApp) Run() {
 	ws.Route(ws.GET("/api/v1/namespaces/{namespace}/virtualmachines/{name}/console").To(console.Console))
 	ws.Route(ws.GET("/api/v1/namespaces/{namespace}/virtualmachines/{name}/migrationHostInfo").To(migrationHostInfo.MigrationHostInfo))
 	restful.DefaultContainer.Add(ws)
-	server := &http.Server{Addr: app.Service.Address(), Handler: restful.DefaultContainer}
+	server := &http.Server{Addr: app.Address(), Handler: restful.DefaultContainer}
 	server.ListenAndServe()
 }
 
+func (app *virtHandlerApp) AddFlags() {
+	app.InitFlags()
+
+	app.BindAddress = defaultHost
+	app.Port = defaultPort
+	app.LibvirtUri = libvirtUri
+
+	app.AddCommonFlags()
+	app.AddLibvirtFlags()
+
+	flag.StringVar(&app.HostOverride, "hostname-override", hostOverride,
+		"Name under which the node is registered in kubernetes, where this virt-handler instance is running on")
+
+	flag.StringVar(&app.VirtShareDir, "kubevirt-share-dir", virtShareDir,
+		"Shared directory between virt-handler and virt-launcher")
+
+	flag.StringVar(&app.EphemeralDiskDir, "ephemeral-disk-dir", ephemeralDiskDir,
+		"Base directory for ephemeral disk data")
+
+	flag.DurationVar(&app.WatchdogTimeoutDuration, "watchdog-timeout", defaultWatchdogTimeout,
+		"Watchdog file timeout")
+}
+
 func main() {
 	log.InitializeLogging("virt-handler")
 	libvirt.EventRegisterDefaultImpl()
-	libvirtUri := flag.String("libvirt-uri", "qemu:///system", "Libvirt connection string.")
-	host := flag.String("listen", "0.0.0.0", "Address where to listen on")
-	port := flag.Int("port", 8185, "Port to listen on")
-	hostOverride := flag.String("hostname-override", "", "Kubernetes Pod to monitor for changes")
-	virtShareDir := flag.String("kubevirt-share-dir", "/var/run/kubevirt", "Shared directory between virt-handler and virt-launcher")
-	ephemeralDiskDir := flag.String("ephemeral-disk-dir", "/var/run/libvirt/kubevirt-ephemeral-disk", "Base directory for ephemeral disk data")
-	watchdogTimeoutDuration := flag.Duration("watchdog-timeout", defaultWatchdogTimeout, "Watchdog file timeout.")
-	pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
-	pflag.Parse()
-
-	app := newVirtHandlerApp(host, port, hostOverride, libvirtUri, virtShareDir, ephemeralDiskDir, watchdogTimeoutDuration)
+	app := &virtHandlerApp{}
+	service.Setup(app)
 	app.Run()
 }

cmd/virt-manifest/virt-manifest.go

@@ -20,31 +20,35 @@
 package main
 
 import (
-	"flag"
 	"fmt"
 	"net/http"
 	"time"
 
 	"github.com/emicklei/go-restful"
-	"github.com/spf13/pflag"
 
 	"kubevirt.io/kubevirt/pkg/log"
 	"kubevirt.io/kubevirt/pkg/service"
 	"kubevirt.io/kubevirt/pkg/virt-handler/virtwrap/cli"
 	"kubevirt.io/kubevirt/pkg/virt-manifest/rest"
 )
 
+const (
+	// Default port that virt-manifest listens on.
+	defaultPort = 8186
+
+	// Default address that virt-manifest listens on.
+	defaultHost = "0.0.0.0"
+
+	libvirtUri = "qemu:///system"
+)
+
 type virtManifestApp struct {
-	Service    *service.Service
+	service.ServiceListen
+	service.ServiceLibvirt
 	LibvirtUri string
 }
 
-func newVirtManifestApp(host *string, port *int, libvirtUri *string) *virtManifestApp {
-	return &virtManifestApp{
-		Service:    service.NewService("virt-manifest", *host, *port),
-		LibvirtUri: *libvirtUri,
-	}
-}
+var _ service.Service = &virtManifestApp{}
 
 func (app *virtManifestApp) Run() {
 	logger := log.Log
@@ -67,22 +71,28 @@ func (app *virtManifestApp) Run() {
 	}
 
 	restful.DefaultContainer.Add(ws)
-	server := &http.Server{Addr: app.Service.Address(), Handler: restful.DefaultContainer}
+	server := &http.Server{Addr: app.Address(), Handler: restful.DefaultContainer}
 	logger.Info("Listening for client connections")
 
 	if err := server.ListenAndServe(); err != nil {
 		logger.Reason(err).Error("Unable to start web server.")
 	}
 }
 
+func (app *virtManifestApp) AddFlags() {
+	app.InitFlags()
+
+	app.BindAddress = defaultHost
+	app.Port = defaultPort
+	app.LibvirtUri = libvirtUri
+
+	app.AddCommonFlags()
+	app.AddLibvirtFlags()
+}
+
 func main() {
 	log.InitializeLogging("virt-manifest")
-	libvirtUri := flag.String("libvirt-uri", "qemu:///system", "Libvirt connection string.")
-	listen := flag.String("listen", "0.0.0.0", "Address where to listen on")
-	port := flag.Int("port", 8186, "Port to listen on")
-	pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
-	pflag.Parse()
-
-	app := newVirtManifestApp(listen, port, libvirtUri)
+	app := virtManifestApp{}
+	service.Setup(&app)
 	app.Run()
 }

pkg/service/service.go

@@ -20,24 +20,47 @@
 package service
 
 import (
+	goflag "flag"
 	"fmt"
 	"strconv"
+
+	flag "github.com/spf13/pflag"
 )
 
-type Service struct {
-	Name string
-	Host string
-	Port string
+type Service interface {
+	Run()
+	AddFlags()
+}
+
+type ServiceListen struct {
+	Name        string
+	BindAddress string
+	Port        int
+}
+
+type ServiceLibvirt struct {
+	LibvirtUri string
+}
+
+func (service *ServiceListen) Address() string {
+	return fmt.Sprintf("%s:%s", service.BindAddress, strconv.Itoa(service.Port))
+}
+
+func (service *ServiceListen) InitFlags() {
+	flag.CommandLine.AddGoFlagSet(goflag.CommandLine)
 }
 
-func NewService(name string, host string, port int) *Service {
-	return &Service{
-		Name: name,
-		Host: host,
-		Port: strconv.Itoa(port),
-	}
+func (service *ServiceListen) AddCommonFlags() {
+	flag.StringVar(&service.BindAddress, "listen", service.BindAddress, "Address where to listen on")
+	flag.IntVar(&service.Port, "port", service.Port, "Port to listen on")
+}
+
+func (service *ServiceLibvirt) AddLibvirtFlags() {
+	flag.StringVar(&service.LibvirtUri, "libvirt-uri", service.LibvirtUri, "Libvirt connection string")
+
 }
 
-func (service *Service) Address() string {
-	return fmt.Sprintf("%s:%s", service.Host, service.Port)
+func Setup(service Service) {
+	service.AddFlags()
+	flag.Parse()
 }

pkg/virt-api/api.go

@@ -8,6 +8,7 @@ import (
 	"github.com/emicklei/go-restful-openapi"
 	kithttp "github.com/go-kit/kit/transport/http"
 	openapispec "github.com/go-openapi/spec"
+	flag "github.com/spf13/pflag"
 	"golang.org/x/net/context"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
@@ -21,19 +22,22 @@ import (
 	"kubevirt.io/kubevirt/pkg/virt-api/rest"
 )
 
-type virtAPIApp struct {
-	Service   *service.Service
+const (
+	// Default port that virt-api listens on.
+	defaultPort = 8183
+
+	// Default address that virt-api listens on.
+	defaultHost = "0.0.0.0"
+)
+
+type VirtAPIApp struct {
+	service.ServiceListen
 	SwaggerUI string
 }
 
-func NewVirtAPIApp(host string, port int, swaggerUI string) *virtAPIApp {
-	return &virtAPIApp{
-		Service:   service.NewService("virt-api", host, port),
-		SwaggerUI: swaggerUI,
-	}
-}
+var _ service.Service = &VirtAPIApp{}
 
-func (app *virtAPIApp) Compose() {
+func (app *VirtAPIApp) Compose() {
 	ctx := context.Background()
 	vmGVR := schema.GroupVersionResource{Group: v1.GroupVersion.Group, Version: v1.GroupVersion.Version, Resource: "virtualmachines"}
 	migrationGVR := schema.GroupVersionResource{Group: v1.GroupVersion.Group, Version: v1.GroupVersion.Version, Resource: "migrations"}
@@ -112,7 +116,7 @@ func (app *virtAPIApp) Compose() {
 	restful.Filter(restful.OPTIONSFilter())
 }
 
-func (app *virtAPIApp) ConfigureOpenAPIService() {
+func (app *VirtAPIApp) ConfigureOpenAPIService() {
 	restful.DefaultContainer.Add(restfulspec.NewOpenAPIService(CreateOpenAPIConfig()))
 	http.Handle("/swagger-ui/", http.StripPrefix("/swagger-ui/", http.FileServer(http.Dir(app.SwaggerUI))))
 }
@@ -144,6 +148,18 @@ func addInfoToSwaggerObject(swo *openapispec.Swagger) {
 	}
 }
 
-func (app *virtAPIApp) Run() {
-	log.Fatal(http.ListenAndServe(app.Service.Address(), nil))
+func (app *VirtAPIApp) Run() {
+	log.Fatal(http.ListenAndServe(app.Address(), nil))
+}
+
+func (app *VirtAPIApp) AddFlags() {
+	app.InitFlags()
+
+	app.BindAddress = defaultHost
+	app.Port = defaultPort
+
+	app.AddCommonFlags()
+
+	flag.StringVar(&app.SwaggerUI, "swagger-ui", "third_party/swagger-ui",
+		"swagger-ui location")
 }