From 025c96776273ba97e2daac8d197615b2e0cda8a2 Mon Sep 17 00:00:00 2001 From: Eugene Yakubovich Date: Tue, 7 Apr 2015 19:52:02 -0700 Subject: [PATCH] rkt: remove --spawn-metadata-svc option from run cmd Also remove --no-idle from metadata-service since it is no longer required. It was really only useful with --spawn-metadata-svc but also racy. Fixes #724 --- Documentation/app-container.md | 3 ++- Documentation/metadata-service.md | 7 ------ rkt/metadata_service.go | 25 ++++--------------- rkt/metadata_service_test.go | 7 ++---- rkt/run.go | 31 +++++++++++------------- rkt/run_prepared.go | 10 ++++---- stage0/run.go | 40 ++++--------------------------- 7 files changed, 30 insertions(+), 93 deletions(-) diff --git a/Documentation/app-container.md b/Documentation/app-container.md index 443bc17..251b184 100644 --- a/Documentation/app-container.md +++ b/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 diff --git a/Documentation/metadata-service.md b/Documentation/metadata-service.md index e81f9e6..25a905e 100644 --- a/Documentation/metadata-service.md +++ b/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. diff --git a/rkt/metadata_service.go b/rkt/metadata_service.go index 48f5328..7462a45 100644 --- a/rkt/metadata_service.go +++ b/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) { diff --git a/rkt/metadata_service_test.go b/rkt/metadata_service_test.go index 056f6b7..1689aaf 100644 --- a/rkt/metadata_service_test.go +++ b/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) } } diff --git a/rkt/run.go b/rkt/run.go index d86bd6d..302570c 100644 --- a/rkt/run.go +++ b/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 diff --git a/rkt/run_prepared.go b/rkt/run_prepared.go index d14404e..dce8497 100644 --- a/rkt/run_prepared.go +++ b/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 diff --git a/stage0/run.go b/stage0/run.go index 4552ede..61aa907 100644 --- a/stage0/run.go +++ b/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 @@ -258,13 +256,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) @@ -447,26 +438,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() -}