Merge pull request #654 from ginglis13/metrics-bug-fixes

minor demux snapshotter metrics bug fixes

Author: ginglis13

snapshotter/app/service.go

@@ -63,8 +63,9 @@ func Run(config config.Config) error {
 	if config.Snapshotter.Metrics.Enable {
 		sdHost := config.Snapshotter.Metrics.Host
 		sdPort := config.Snapshotter.Metrics.ServiceDiscoveryPort
-		serviceDiscovery := discovery.NewServiceDiscovery(sdHost, sdPort, cache)
-		monitor, err := initMetricsProxyMonitor(config.Snapshotter.Metrics.PortRange)
+		serviceDiscovery = discovery.NewServiceDiscovery(sdHost, sdPort, cache)
+		var err error
+		monitor, err = initMetricsProxyMonitor(config.Snapshotter.Metrics.PortRange)
 		if err != nil {
 			log.G(ctx).WithError(err).Fatal("failed creating metrics proxy monitor")
 			return err
@@ -113,19 +114,20 @@ func Run(config config.Config) error {
 			if err := snapshotter.Close(); err != nil {
 				log.G(ctx).WithError(err).Error("failed to close snapshotter")
 			}
-			if config.Snapshotter.Metrics.Enable {
-				if err := serviceDiscovery.Shutdown(ctx); err != nil {
-					log.G(ctx).WithError(err).Error("failed to shutdown service discovery server")
-				}
-				// Senders to this channel would panic if it is closed. However snapshotter.Close() will
-				// shutdown all metrics proxies and ensure there are no more senders over the channel.
-				monitor.Stop()
-			}
 		}()
 
 		for {
 			select {
 			case <-stop:
+				// cancelling context will cause shutdown to fail; shutdown before cancel
+				if config.Snapshotter.Metrics.Enable {
+					if err := serviceDiscovery.Shutdown(ctx); err != nil {
+						log.G(ctx).WithError(err).Error("failed to shutdown service discovery server")
+					}
+					// Senders to this channel would panic if it is closed. However snapshotter.Close() will
+					// shutdown all metrics proxies and ensure there are no more senders over the channel.
+					monitor.Stop()
+				}
 				cancel()
 				return nil
 			case <-ctx.Done():

snapshotter/demux/metrics/proxy.go

@@ -129,10 +129,11 @@ func NewProxy(host string, monitor *Monitor, labels map[string]string, dialer fu
 				DialContext: dialer,
 			},
 		},
-		server: nil,
-		host:   host,
-		Port:   port,
-		Labels: labels,
+		server:  nil,
+		host:    host,
+		Port:    port,
+		Labels:  labels,
+		monitor: monitor,
 	}, nil
 }
 

snapshotter/internal/http_address_resolver.go

@@ -36,6 +36,7 @@ import (
 var (
 	port               int
 	remotePort         int
+	metricsRemotePort  int
 	containerdSockPath string
 	logger             *logrus.Logger
 )
@@ -44,6 +45,7 @@ func init() {
 	flag.IntVar(&port, "port", 10001, "service port for address resolver")
 	flag.StringVar(&containerdSockPath, "containerdSocket", "/run/firecracker-containerd/containerd.sock", "filepath to the containerd socket")
 	flag.IntVar(&remotePort, "remotePort", 10000, "the remote port on which the remote snapshotter is listening")
+	flag.IntVar(&metricsRemotePort, "metricsRemotePort", 10002, "the remote port on which the remote snapshotter metrics server is listening")
 	logger = logrus.New()
 }
 
@@ -135,6 +137,10 @@ func queryAddress(writ http.ResponseWriter, req *http.Request) {
 		Network:         "unix",
 		Address:         vmInfo.VSockPath,
 		SnapshotterPort: strconv.Itoa(remotePort),
+		MetricsPort:     strconv.Itoa(metricsRemotePort),
+		Labels: map[string]string{
+			"VMID": namespace,
+		},
 	})
 	if err != nil {
 		http.Error(writ, "Internal server error", http.StatusInternalServerError)

tools/image-builder/files_stargz/etc/systemd/system/metrics-socat.service

@@ -19,4 +19,4 @@ SuccessAction=reboot
 
 [Service]
 Type=simple
-ExecStart=socat VSOCK-LISTEN:10001,reuseaddr,fork TCP:localhost:8234
+ExecStart=socat VSOCK-LISTEN:10002,reuseaddr,fork TCP:localhost:8234