From 554c663b5ad2bd0c0fa4c58559ec2a9b19ed399f Mon Sep 17 00:00:00 2001 From: Jhon Honce Date: Thu, 16 Apr 2020 16:42:33 -0700 Subject: Fix bug where two configurations had been created * registry.PodmanConfig() new returns a pointer to the source of truth Signed-off-by: Jhon Honce --- cmd/podman/common/create.go | 2 +- cmd/podman/containers/stop.go | 3 ++- cmd/podman/containers/wait.go | 2 +- cmd/podman/main.go | 9 ++------- cmd/podman/registry/config.go | 16 ++++++++++++---- cmd/podman/registry/registry.go | 10 +++++----- cmd/podman/registry/remote.go | 2 +- cmd/podman/root.go | 25 ++++++++++++------------- pkg/domain/infra/runtime_abi.go | 4 ++-- pkg/domain/infra/runtime_image_proxy.go | 2 +- pkg/domain/infra/runtime_libpod.go | 14 +++++++------- pkg/domain/infra/runtime_proxy.go | 2 +- pkg/domain/infra/runtime_tunnel.go | 4 ++-- 13 files changed, 49 insertions(+), 46 deletions(-) diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go index a7c8435c9..49a40dfa0 100644 --- a/cmd/podman/common/create.go +++ b/cmd/podman/common/create.go @@ -10,7 +10,7 @@ import ( const sizeWithUnitFormat = "(format: `[]`, where unit = b (bytes), k (kilobytes), m (megabytes), or g (gigabytes))" -var containerConfig = registry.NewPodmanConfig() +var containerConfig = registry.PodmanConfig() func GetCreateFlags(cf *ContainerCLIOpts) *pflag.FlagSet { createFlags := pflag.FlagSet{} diff --git a/cmd/podman/containers/stop.go b/cmd/podman/containers/stop.go index 1ee9186a7..c1560be08 100644 --- a/cmd/podman/containers/stop.go +++ b/cmd/podman/containers/stop.go @@ -45,7 +45,8 @@ func init() { flags.StringArrayVarP(&stopOptions.CIDFiles, "cidfile", "", nil, "Read the container ID from the file") flags.BoolVarP(&stopOptions.Latest, "latest", "l", false, "Act on the latest container podman is aware of") flags.UintVarP(&stopTimeout, "time", "t", defaultContainerConfig.Engine.StopTimeout, "Seconds to wait for stop before killing the container") - if registry.PodmanOptions.EngineMode == entities.ABIMode { + + if registry.IsRemote() { _ = flags.MarkHidden("latest") _ = flags.MarkHidden("cidfile") _ = flags.MarkHidden("ignore") diff --git a/cmd/podman/containers/wait.go b/cmd/podman/containers/wait.go index 83c164e16..47f28f4c6 100644 --- a/cmd/podman/containers/wait.go +++ b/cmd/podman/containers/wait.go @@ -43,7 +43,7 @@ func init() { flags.DurationVarP(&waitOptions.Interval, "interval", "i", time.Duration(250), "Milliseconds to wait before polling for completion") flags.BoolVarP(&waitOptions.Latest, "latest", "l", false, "Act on the latest container podman is aware of") flags.StringVar(&waitCondition, "condition", "stopped", "Condition to wait on") - if registry.PodmanOptions.EngineMode == entities.ABIMode { + if registry.IsRemote() { // TODO: This is the same as V1. We could skip creating the flag altogether in V2... _ = flags.MarkHidden("latest") } diff --git a/cmd/podman/main.go b/cmd/podman/main.go index 5f7bfe3c9..2d9e45177 100644 --- a/cmd/podman/main.go +++ b/cmd/podman/main.go @@ -14,12 +14,6 @@ import ( "github.com/containers/storage/pkg/reexec" ) -func init() { - // This is the bootstrap configuration, if user gives - // CLI flags parts of this configuration may be overwritten - registry.PodmanOptions = registry.NewPodmanConfig() -} - func main() { if reexec.Init() { // We were invoked with a different argv[0] indicating that we @@ -27,9 +21,10 @@ func main() { return } + cfg := registry.PodmanConfig() for _, c := range registry.Commands { for _, m := range c.Mode { - if registry.PodmanOptions.EngineMode == m { + if cfg.EngineMode == m { parent := rootCmd if c.Parent != nil { parent = c.Parent diff --git a/cmd/podman/registry/config.go b/cmd/podman/registry/config.go index 358f9172e..fc6eb538e 100644 --- a/cmd/podman/registry/config.go +++ b/cmd/podman/registry/config.go @@ -6,6 +6,7 @@ import ( "path/filepath" "runtime" "strings" + "sync" "github.com/containers/common/pkg/config" "github.com/containers/libpod/pkg/domain/entities" @@ -19,11 +20,18 @@ const ( ) var ( - PodmanOptions entities.PodmanConfig + podmanOptions entities.PodmanConfig + podmanSync sync.Once ) -// NewPodmanConfig creates a PodmanConfig from the environment -func NewPodmanConfig() entities.PodmanConfig { +// PodmanConfig returns an entities.PodmanConfig built up from +// environment and CLI +func PodmanConfig() *entities.PodmanConfig { + podmanSync.Do(newPodmanConfig) + return &podmanOptions +} + +func newPodmanConfig() { if err := setXdgDirs(); err != nil { fmt.Fprintf(os.Stderr, err.Error()) os.Exit(1) @@ -63,7 +71,7 @@ func NewPodmanConfig() entities.PodmanConfig { cfg.Network.NetworkConfigDir = "" } - return entities.PodmanConfig{Config: cfg, EngineMode: mode} + podmanOptions = entities.PodmanConfig{Config: cfg, EngineMode: mode} } // SetXdgDirs ensures the XDG_RUNTIME_DIR env and XDG_CONFIG_HOME variables are set. diff --git a/cmd/podman/registry/registry.go b/cmd/podman/registry/registry.go index 1c5e5d21b..2e9d59d10 100644 --- a/cmd/podman/registry/registry.go +++ b/cmd/podman/registry/registry.go @@ -49,8 +49,8 @@ func ImageEngine() entities.ImageEngine { // NewImageEngine is a wrapper for building an ImageEngine to be used for PreRunE functions func NewImageEngine(cmd *cobra.Command, args []string) (entities.ImageEngine, error) { if imageEngine == nil { - PodmanOptions.FlagSet = cmd.Flags() - engine, err := infra.NewImageEngine(PodmanOptions) + podmanOptions.FlagSet = cmd.Flags() + engine, err := infra.NewImageEngine(&podmanOptions) if err != nil { return nil, err } @@ -66,8 +66,8 @@ func ContainerEngine() entities.ContainerEngine { // NewContainerEngine is a wrapper for building an ContainerEngine to be used for PreRunE functions func NewContainerEngine(cmd *cobra.Command, args []string) (entities.ContainerEngine, error) { if containerEngine == nil { - PodmanOptions.FlagSet = cmd.Flags() - engine, err := infra.NewContainerEngine(PodmanOptions) + podmanOptions.FlagSet = cmd.Flags() + engine, err := infra.NewContainerEngine(&podmanOptions) if err != nil { return nil, err } @@ -101,7 +101,7 @@ func Context() context.Context { } func ContextWithOptions(ctx context.Context) context.Context { - cliCtx = context.WithValue(ctx, PodmanOptionsKey{}, PodmanOptions) + cliCtx = context.WithValue(ctx, PodmanOptionsKey{}, podmanOptions) return cliCtx } diff --git a/cmd/podman/registry/remote.go b/cmd/podman/registry/remote.go index 5378701e7..95870750e 100644 --- a/cmd/podman/registry/remote.go +++ b/cmd/podman/registry/remote.go @@ -5,5 +5,5 @@ import ( ) func IsRemote() bool { - return PodmanOptions.EngineMode == entities.TunnelMode + return podmanOptions.EngineMode == entities.TunnelMode } diff --git a/cmd/podman/root.go b/cmd/podman/root.go index 259e10c55..729ca6aa7 100644 --- a/cmd/podman/root.go +++ b/cmd/podman/root.go @@ -77,7 +77,7 @@ func init() { syslogHook, ) - rootFlags(registry.PodmanOptions, rootCmd.PersistentFlags()) + rootFlags(registry.PodmanConfig(), rootCmd.PersistentFlags()) } func Execute() { @@ -98,9 +98,7 @@ func persistentPreRunE(cmd *cobra.Command, args []string) error { // TODO: Remove trace statement in podman V2.1 logrus.Debugf("Called %s.PersistentPreRunE()", cmd.Name()) - // Update PodmanOptions now that we "know" more - // TODO: pass in path overriding configuration file - registry.PodmanOptions = registry.NewPodmanConfig() + cfg := registry.PodmanConfig() // Prep the engines if _, err := registry.NewImageEngine(cmd, args); err != nil { @@ -111,10 +109,10 @@ func persistentPreRunE(cmd *cobra.Command, args []string) error { } if cmd.Flag("cpu-profile").Changed { - f, err := os.Create(registry.PodmanOptions.CpuProfile) + f, err := os.Create(cfg.CpuProfile) if err != nil { return errors.Wrapf(err, "unable to create cpu profiling file %s", - registry.PodmanOptions.CpuProfile) + cfg.CpuProfile) } if err := pprof.StartCPUProfile(f); err != nil { return err @@ -124,11 +122,11 @@ func persistentPreRunE(cmd *cobra.Command, args []string) error { if cmd.Flag("trace").Changed { tracer, closer := tracing.Init("podman") opentracing.SetGlobalTracer(tracer) - registry.PodmanOptions.SpanCloser = closer + cfg.SpanCloser = closer - registry.PodmanOptions.Span = tracer.StartSpan("before-context") - registry.PodmanOptions.SpanCtx = opentracing.ContextWithSpan(registry.Context(), registry.PodmanOptions.Span) - opentracing.StartSpanFromContext(registry.PodmanOptions.SpanCtx, cmd.Name()) + cfg.Span = tracer.StartSpan("before-context") + cfg.SpanCtx = opentracing.ContextWithSpan(registry.Context(), cfg.Span) + opentracing.StartSpanFromContext(cfg.SpanCtx, cmd.Name()) } // Setup Rootless environment, IFF: @@ -149,12 +147,13 @@ func persistentPostRunE(cmd *cobra.Command, args []string) error { // TODO: Remove trace statement in podman V2.1 logrus.Debugf("Called %s.PersistentPostRunE()", cmd.Name()) + cfg := registry.PodmanConfig() if cmd.Flag("cpu-profile").Changed { pprof.StopCPUProfile() } if cmd.Flag("trace").Changed { - registry.PodmanOptions.Span.Finish() - registry.PodmanOptions.SpanCloser.Close() + cfg.Span.Finish() + cfg.SpanCloser.Close() } return nil } @@ -199,7 +198,7 @@ func syslogHook() { } } -func rootFlags(opts entities.PodmanConfig, flags *pflag.FlagSet) { +func rootFlags(opts *entities.PodmanConfig, flags *pflag.FlagSet) { // V2 flags flags.StringVarP(&opts.Uri, "remote", "r", "", "URL to access Podman service") flags.StringSliceVar(&opts.Identities, "identity", []string{}, "path to SSH identity file") diff --git a/pkg/domain/infra/runtime_abi.go b/pkg/domain/infra/runtime_abi.go index 0dbcf2ad2..7aa6986a7 100644 --- a/pkg/domain/infra/runtime_abi.go +++ b/pkg/domain/infra/runtime_abi.go @@ -12,7 +12,7 @@ import ( ) // NewContainerEngine factory provides a libpod runtime for container-related operations -func NewContainerEngine(facts entities.PodmanConfig) (entities.ContainerEngine, error) { +func NewContainerEngine(facts *entities.PodmanConfig) (entities.ContainerEngine, error) { switch facts.EngineMode { case entities.ABIMode: r, err := NewLibpodRuntime(facts.FlagSet, facts) @@ -25,7 +25,7 @@ func NewContainerEngine(facts entities.PodmanConfig) (entities.ContainerEngine, } // NewContainerEngine factory provides a libpod runtime for image-related operations -func NewImageEngine(facts entities.PodmanConfig) (entities.ImageEngine, error) { +func NewImageEngine(facts *entities.PodmanConfig) (entities.ImageEngine, error) { switch facts.EngineMode { case entities.ABIMode: r, err := NewLibpodImageRuntime(facts.FlagSet, facts) diff --git a/pkg/domain/infra/runtime_image_proxy.go b/pkg/domain/infra/runtime_image_proxy.go index 535fba858..ea5d0e6f2 100644 --- a/pkg/domain/infra/runtime_image_proxy.go +++ b/pkg/domain/infra/runtime_image_proxy.go @@ -12,7 +12,7 @@ import ( // ContainerEngine Image Proxy will be EOL'ed after podman is separated from libpod repo -func NewLibpodImageRuntime(flags *pflag.FlagSet, opts entities.PodmanConfig) (entities.ImageEngine, error) { +func NewLibpodImageRuntime(flags *pflag.FlagSet, opts *entities.PodmanConfig) (entities.ImageEngine, error) { r, err := GetRuntime(context.Background(), flags, opts) if err != nil { return nil, err diff --git a/pkg/domain/infra/runtime_libpod.go b/pkg/domain/infra/runtime_libpod.go index e335dd560..dc59fec3d 100644 --- a/pkg/domain/infra/runtime_libpod.go +++ b/pkg/domain/infra/runtime_libpod.go @@ -24,11 +24,11 @@ type engineOpts struct { migrate bool noStore bool withFDS bool - config entities.PodmanConfig + config *entities.PodmanConfig } // GetRuntimeMigrate gets a libpod runtime that will perform a migration of existing containers -func GetRuntimeMigrate(ctx context.Context, fs *flag.FlagSet, cfg entities.PodmanConfig, newRuntime string) (*libpod.Runtime, error) { +func GetRuntimeMigrate(ctx context.Context, fs *flag.FlagSet, cfg *entities.PodmanConfig, newRuntime string) (*libpod.Runtime, error) { return getRuntime(ctx, fs, &engineOpts{ name: newRuntime, renumber: false, @@ -40,7 +40,7 @@ func GetRuntimeMigrate(ctx context.Context, fs *flag.FlagSet, cfg entities.Podma } // GetRuntimeDisableFDs gets a libpod runtime that will disable sd notify -func GetRuntimeDisableFDs(ctx context.Context, fs *flag.FlagSet, cfg entities.PodmanConfig) (*libpod.Runtime, error) { +func GetRuntimeDisableFDs(ctx context.Context, fs *flag.FlagSet, cfg *entities.PodmanConfig) (*libpod.Runtime, error) { return getRuntime(ctx, fs, &engineOpts{ renumber: false, migrate: false, @@ -51,7 +51,7 @@ func GetRuntimeDisableFDs(ctx context.Context, fs *flag.FlagSet, cfg entities.Po } // GetRuntimeRenumber gets a libpod runtime that will perform a lock renumber -func GetRuntimeRenumber(ctx context.Context, fs *flag.FlagSet, cfg entities.PodmanConfig) (*libpod.Runtime, error) { +func GetRuntimeRenumber(ctx context.Context, fs *flag.FlagSet, cfg *entities.PodmanConfig) (*libpod.Runtime, error) { return getRuntime(ctx, fs, &engineOpts{ renumber: true, migrate: false, @@ -62,7 +62,7 @@ func GetRuntimeRenumber(ctx context.Context, fs *flag.FlagSet, cfg entities.Podm } // GetRuntime generates a new libpod runtime configured by command line options -func GetRuntime(ctx context.Context, flags *flag.FlagSet, cfg entities.PodmanConfig) (*libpod.Runtime, error) { +func GetRuntime(ctx context.Context, flags *flag.FlagSet, cfg *entities.PodmanConfig) (*libpod.Runtime, error) { return getRuntime(ctx, flags, &engineOpts{ renumber: false, migrate: false, @@ -73,7 +73,7 @@ func GetRuntime(ctx context.Context, flags *flag.FlagSet, cfg entities.PodmanCon } // GetRuntimeNoStore generates a new libpod runtime configured by command line options -func GetRuntimeNoStore(ctx context.Context, fs *flag.FlagSet, cfg entities.PodmanConfig) (*libpod.Runtime, error) { +func GetRuntimeNoStore(ctx context.Context, fs *flag.FlagSet, cfg *entities.PodmanConfig) (*libpod.Runtime, error) { return getRuntime(ctx, fs, &engineOpts{ renumber: false, migrate: false, @@ -160,7 +160,7 @@ func getRuntime(ctx context.Context, fs *flag.FlagSet, opts *engineOpts) (*libpo } if fs.Changed("runtime") { - options = append(options, libpod.WithOCIRuntime(cfg.Engine.OCIRuntime)) + options = append(options, libpod.WithOCIRuntime(cfg.RuntimePath)) } if fs.Changed("conmon") { diff --git a/pkg/domain/infra/runtime_proxy.go b/pkg/domain/infra/runtime_proxy.go index fbe45ea8f..41193fd89 100644 --- a/pkg/domain/infra/runtime_proxy.go +++ b/pkg/domain/infra/runtime_proxy.go @@ -12,7 +12,7 @@ import ( // ContainerEngine Proxy will be EOL'ed after podman is separated from libpod repo -func NewLibpodRuntime(flags *flag.FlagSet, opts entities.PodmanConfig) (entities.ContainerEngine, error) { +func NewLibpodRuntime(flags *flag.FlagSet, opts *entities.PodmanConfig) (entities.ContainerEngine, error) { r, err := GetRuntime(context.Background(), flags, opts) if err != nil { return nil, err diff --git a/pkg/domain/infra/runtime_tunnel.go b/pkg/domain/infra/runtime_tunnel.go index 129fdeb2c..752218aaf 100644 --- a/pkg/domain/infra/runtime_tunnel.go +++ b/pkg/domain/infra/runtime_tunnel.go @@ -11,7 +11,7 @@ import ( "github.com/containers/libpod/pkg/domain/infra/tunnel" ) -func NewContainerEngine(facts entities.PodmanConfig) (entities.ContainerEngine, error) { +func NewContainerEngine(facts *entities.PodmanConfig) (entities.ContainerEngine, error) { switch facts.EngineMode { case entities.ABIMode: return nil, fmt.Errorf("direct runtime not supported") @@ -23,7 +23,7 @@ func NewContainerEngine(facts entities.PodmanConfig) (entities.ContainerEngine, } // NewImageEngine factory provides a libpod runtime for image-related operations -func NewImageEngine(facts entities.PodmanConfig) (entities.ImageEngine, error) { +func NewImageEngine(facts *entities.PodmanConfig) (entities.ImageEngine, error) { switch facts.EngineMode { case entities.ABIMode: return nil, fmt.Errorf("direct image runtime not supported") -- cgit v1.2.3-54-g00ecf