Merge pull request #737 from eyakubovich/remove-spawn-metadatasvc

rkt: remove --spawn-metadata-svc option from run cmd

Documentation/app-container.md

@@ -11,8 +11,9 @@ It also leverages schema and code from the upstream [appc/spec][appc-spec] repo
 To validate that `rkt` successfully implements the ACE part of the spec, use the App Container [validation ACIs][appc-readme]:
 
 ```
+$ sudo rkt metadata-service &  # Make sure metadata service is running
 $ sudo rkt --insecure-skip-verify run \
-	--private-net --spawn-metadata-svc \
+	--private-net \
 	--volume database,kind=host,source=/tmp \
 	https://github.com/appc/spec/releases/download/v0.5.1/ace-validator-main.aci \
 	https://github.com/appc/spec/releases/download/v0.5.1/ace-validator-sidekick.aci

Documentation/metadata-service.md

@@ -19,12 +19,5 @@ In addition to listening on a Unix socket, the metadata service will also listen
 When contacting the metadata service, the apps utilize this port.
 In order to avoid exposing host IP to the pods, the metadata service installs an iptables rule to redirect traffic destined for 169.254.169.255:80 to itself.
 
-## Launching metadata service with `rkt run`
-
-For non-production use, it is often convenient to launch the metadata service inline with running a pod.
-`rkt run` accepts a `--spawn-metadata-svc` command line argument to launch the metadata service prior to running a pod.
-When started in this fashion, the metadata service will exit when there are no more pods registered with it.
-Please note that it is strongly recommended to only use this mode when experimenting with rkt on command line.
-
 ## Using the metadata service
 See [App Container specification](https://github.com/appc/spec/blob/master/SPEC.md#app-container-metadata-service) for more information about the metadata service including a list of supported endpoints and their usage.

rkt/metadata_service.go

@@ -58,7 +58,6 @@ var (
 	errAppNotFound = errors.New("app not found")
 
 	flagListenPort       int
-	flagNoIdle           bool
 	metadataServiceFlags flag.FlagSet
 
 	exitCh = make(chan os.Signal, 1)
@@ -71,7 +70,6 @@ const (
 func init() {
 	commands = append(commands, cmdMetadataService)
 	metadataServiceFlags.IntVar(&flagListenPort, "listen-port", common.MetadataServicePort, "listen port")
-	metadataServiceFlags.BoolVar(&flagNoIdle, "no-idle", false, "exit when last pod is unregistered")
 }
 
 type mdsPod struct {
@@ -123,19 +121,19 @@ func (ps *podStore) addApp(u *types.UUID, app string, manifest *schema.ImageMani
 	return nil
 }
 
-func (ps *podStore) remove(u *types.UUID) (bool, error) {
+func (ps *podStore) remove(u *types.UUID) error {
 	ps.mutex.Lock()
 	defer ps.mutex.Unlock()
 
 	p, ok := ps.byUUID[*u]
 	if !ok {
-		return false, errPodNotFound
+		return errPodNotFound
 	}
 
 	delete(ps.byUUID, *u)
 	delete(ps.byIP, p.ip)
 
-	return len(ps.byUUID) == 0, nil
+	return nil
 }
 
 func (ps *podStore) getUUID(ip string) (*types.UUID, error) {
@@ -225,28 +223,13 @@ func handleUnregisterPod(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	lastOne, err := pods.remove(uuid)
-	if err != nil {
+	if err := pods.remove(uuid); err != nil {
 		w.WriteHeader(http.StatusNotFound)
 		fmt.Fprint(w, err)
 		return
 	}
 
 	w.WriteHeader(http.StatusOK)
-
-	if flagNoIdle && lastOne {
-		// TODO(eyakubovich): this is very racy
-		// It's possible for last pod to get unregistered
-		// and svc gets flagged to shutdown. Then another pod
-		// starts to launch, sees that port is in use and doesn't
-		// start metadata svc only for this one to exit a moment later.
-		// However, --no-idle is meant for demos and having a single
-		// pod spawn up (via --spawn-metadata-svc). The design
-		// of metadata svc is also likely to change as we convert it
-		// to be backed by persistent storage.
-		// wait for signal and exit
-		close(exitCh)
-	}
 }
 
 func handleRegisterApp(w http.ResponseWriter, r *http.Request) {

rkt/metadata_service_test.go

@@ -107,15 +107,12 @@ func TestPodStoreGetManifests(t *testing.T) {
 func TestPodStoreRemove(t *testing.T) {
 	ps, uuid, _, _ := setupPodStoreTest(t)
 
-	last, err := ps.remove(uuid)
+	err := ps.remove(uuid)
 	if err != nil {
 		t.Errorf("remove failed with %v", err)
 	}
-	if !last {
-		t.Error("remove of last item returned last=false")
-	}
 
-	if _, err := ps.remove(uuid); err != errPodNotFound {
+	if err := ps.remove(uuid); err != errPodNotFound {
 		t.Errorf("remove with unknown pod returned %v", err)
 	}
 }

rkt/run.go

@@ -49,16 +49,15 @@ End the image arguments with a lone "---" to resume argument parsing.`,
 		Flags:                &runFlags,
 		WantsFlagsTerminator: true,
 	}
-	runFlags                 flag.FlagSet
-	flagStage1Image          string
-	flagVolumes              volumeList
-	flagPorts                portList
-	flagPrivateNet           bool
-	flagSpawnMetadataService bool
-	flagInheritEnv           bool
-	flagExplicitEnv          envMap
-	flagInteractive          bool
-	flagNoOverlay            bool
+	runFlags        flag.FlagSet
+	flagStage1Image string
+	flagVolumes     volumeList
+	flagPorts       portList
+	flagPrivateNet  bool
+	flagInheritEnv  bool
+	flagExplicitEnv envMap
+	flagInteractive bool
+	flagNoOverlay   bool
 )
 
 func init() {
@@ -76,7 +75,6 @@ func init() {
 	runFlags.Var(&flagVolumes, "volume", "volumes to mount into the pod")
 	runFlags.Var(&flagPorts, "port", "ports to expose on the host (requires --private-net)")
 	runFlags.BoolVar(&flagPrivateNet, "private-net", false, "give pod a private network")
-	runFlags.BoolVar(&flagSpawnMetadataService, "spawn-metadata-svc", false, "launch metadata svc if not running")
 	runFlags.BoolVar(&flagInheritEnv, "inherit-env", false, "inherit all environment variables not set by apps")
 	runFlags.BoolVar(&flagNoOverlay, "no-overlay", false, "disable overlay filesystem")
 	runFlags.Var(&flagExplicitEnv, "set-env", "an environment variable to set for apps in the form name=value")
@@ -185,12 +183,11 @@ func runRun(args []string) (exit int) {
 	}
 
 	rcfg := stage0.RunConfig{
-		CommonConfig:         cfg,
-		PrivateNet:           flagPrivateNet,
-		SpawnMetadataService: flagSpawnMetadataService,
-		LockFd:               lfd,
-		Interactive:          flagInteractive,
-		Images:               rktApps.GetImageIDs(),
+		CommonConfig: cfg,
+		PrivateNet:   flagPrivateNet,
+		LockFd:       lfd,
+		Interactive:  flagInteractive,
+		Images:       rktApps.GetImageIDs(),
 	}
 	stage0.Run(rcfg, p.path()) // execs, never returns
 

rkt/run_prepared.go

@@ -44,7 +44,6 @@ var (
 func init() {
 	commands = append(commands, cmdRunPrepared)
 	runPreparedFlags.BoolVar(&flagPrivateNet, "private-net", false, "give pod a private network")
-	runPreparedFlags.BoolVar(&flagSpawnMetadataService, "spawn-metadata-svc", false, "launch metadata svc if not running")
 	runPreparedFlags.BoolVar(&flagInteractive, "interactive", false, "the pod is interactive")
 }
 
@@ -129,11 +128,10 @@ func runRunPrepared(args []string) (exit int) {
 			UUID:        p.uuid,
 			Debug:       globalFlags.Debug,
 		},
-		PrivateNet:           flagPrivateNet,
-		SpawnMetadataService: flagSpawnMetadataService,
-		LockFd:               lfd,
-		Interactive:          flagInteractive,
-		Images:               imgs,
+		PrivateNet:  flagPrivateNet,
+		LockFd:      lfd,
+		Interactive: flagInteractive,
+		Images:      imgs,
 	}
 	stage0.Run(rcfg, p.path()) // execs, never returns
 	return 1

stage0/run.go

@@ -29,7 +29,6 @@ import (
 	"io/ioutil"
 	"log"
 	"os"
-	"os/exec"
 	"path"
 	"path/filepath"
 	"runtime"
@@ -61,11 +60,10 @@ type PrepareConfig struct {
 // configuration parameters needed by Run
 type RunConfig struct {
 	CommonConfig
-	PrivateNet           bool         // pod should have its own network stack
-	SpawnMetadataService bool         // launch metadata service
-	LockFd               int          // lock file descriptor
-	Interactive          bool         // whether the pod is interactive or not
-	Images               []types.Hash // application images (prepare gets them via Apps)
+	PrivateNet  bool         // pod should have its own network stack
+	LockFd      int          // lock file descriptor
+	Interactive bool         // whether the pod is interactive or not
+	Images      []types.Hash // application images (prepare gets them via Apps)
 }
 
 // configuration shared by both Run and Prepare
@@ -252,13 +250,6 @@ func Run(cfg RunConfig, dir string) {
 		log.Fatalf("setting lock fd environment: %v", err)
 	}
 
-	if cfg.SpawnMetadataService {
-		log.Print("Launching metadata svc")
-		if err := launchMetadataService(cfg.Debug); err != nil {
-			log.Printf("Failed to launch metadata svc: %v", err)
-		}
-	}
-
 	log.Printf("Pivoting to filesystem %s", dir)
 	if err := os.Chdir(dir); err != nil {
 		log.Fatalf("failed changing to dir: %v", err)
@@ -441,26 +432,3 @@ func overlayRender(cfg RunConfig, img types.Hash, cdir string, dest string) erro
 
 	return nil
 }
-
-func launchMetadataService(debug bool) error {
-	// readlink so arg[0] displays useful info
-	exe, err := os.Readlink("/proc/self/exe")
-	if err != nil {
-		return fmt.Errorf("error reading /proc/self/exe link: %v", err)
-	}
-
-	args := []string{exe}
-	if debug {
-		args = append(args, "--debug")
-	}
-	args = append(args, "metadata-service", "--no-idle")
-
-	cmd := exec.Cmd{
-		Path:   exe,
-		Args:   args,
-		Stdout: os.Stdout,
-		Stderr: os.Stderr,
-	}
-
-	return cmd.Start()
-}