Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix pod creation with "new:" syntax #7152

Merged
merged 1 commit into from
Jul 31, 2020
+16 −9
Merged

fix pod creation with "new:" syntax #7152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions cmd/podman/containers/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func create(cmd *cobra.Command, args []string) error {
return err
}

if _, err := createPodIfNecessary(s); err != nil {
if _, err := createPodIfNecessary(s, cliVals.Net); err != nil {
return err
}

Expand Down Expand Up @@ -283,7 +283,7 @@ func openCidFile(cidfile string) (*os.File, error) {
// createPodIfNecessary automatically creates a pod when requested. if the pod name
// has the form new:ID, the pod ID is created and the name in the spec generator is replaced
// with ID.
func createPodIfNecessary(s *specgen.SpecGenerator) (*entities.PodCreateReport, error) {
func createPodIfNecessary(s *specgen.SpecGenerator, netOpts *entities.NetOptions) (*entities.PodCreateReport, error) {
if !strings.HasPrefix(s.Pod, "new:") {
return nil, nil
}
Expand All @@ -292,11 +292,10 @@ func createPodIfNecessary(s *specgen.SpecGenerator) (*entities.PodCreateReport,
return nil, errors.Errorf("new pod name must be at least one character")
}
createOptions := entities.PodCreateOptions{
Name: podName,
Infra: true,
Net: &entities.NetOptions{
PublishPorts: s.PortMappings,
},
Name: podName,
Infra: true,
Net: netOpts,
CreateCommand: os.Args,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably best to leave this empty - it's mainly used for podman generate systemd --new, and it could have misleading results there

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it is correct to set here. I checked podman generate systemd --new and it throws an error that you have to use podman pod create. This can later be used to implement this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vrothberg Thoughts here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me. As @Luap99 mentioned, generate systemd requires a pod to be generated via pod create and will error out otherwise (see https://github.com/containers/podman/blob/master/pkg/systemd/generate/pods.go#L265).

Setting it here will be more accurate as generate systemd will otherwise error out with "no create command found" which would be less informative than "pod does not appear to be created via podman pod create"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright. We should be good to merge after a rebase, then.

}
s.Pod = podName
return registry.ContainerEngine().PodCreate(context.Background(), createOptions)
Expand Down
2 changes: 1 addition & 1 deletion cmd/podman/containers/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func run(cmd *cobra.Command, args []string) error {
}
runOpts.Spec = s

if _, err := createPodIfNecessary(s); err != nil {
if _, err := createPodIfNecessary(s, cliVals.Net); err != nil {
return err
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/specgen/generate/namespaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,10 @@ func specConfigureNamespaces(s *specgen.SpecGenerator, g *generate.Generator, rt
func GetNamespaceOptions(ns []string) ([]libpod.PodCreateOption, error) {
var options []libpod.PodCreateOption
var erroredOptions []libpod.PodCreateOption
if ns == nil {
//set the default namespaces
ns = strings.Split(specgen.DefaultKernelNamespaces, ",")
}
for _, toShare := range ns {
switch toShare {
case "cgroup":
Expand Down
6 changes: 5 additions & 1 deletion test/e2e/run_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,11 @@ USER mail`
})

It("podman run --pod automatically", func() {
session := podmanTest.Podman([]string{"run", "--pod", "new:foobar", ALPINE, "ls"})
session := podmanTest.Podman([]string{"run", "-d", "--pod", "new:foobar", ALPINE, "nc", "-l", "-p", "8080"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))

session = podmanTest.Podman([]string{"run", "--pod", "foobar", ALPINE, "/bin/sh", "-c", "echo test | nc -w 1 127.0.0.1 8080"})
session.WaitWithDefaultTimeout()
Expect(session.ExitCode()).To(Equal(0))

Expand Down