Skip to content

Commit

Permalink
Don't use the uid in containerinstall code (#3066)
Browse files Browse the repository at this point in the history
  • Loading branch information
hawkowl authored Jul 28, 2023
1 parent b5ea75e commit 06cfba5
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 46 deletions.
33 changes: 17 additions & 16 deletions pkg/containerinstall/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"runtime"
"time"

"github.com/Azure/go-autorest/autorest/to"
"github.com/containers/podman/v4/pkg/bindings/containers"
"github.com/containers/podman/v4/pkg/bindings/images"
"github.com/containers/podman/v4/pkg/bindings/secrets"
Expand Down Expand Up @@ -43,12 +42,11 @@ var (
func (m *manager) Install(ctx context.Context, sub *api.SubscriptionDocument, doc *api.OpenShiftClusterDocument, version *api.OpenShiftVersion) error {
s := []steps.Step{
steps.Action(func(context.Context) error {
options := &images.PullOptions{
Quiet: to.BoolPtr(true),
Policy: to.StringPtr("always"),
Username: to.StringPtr(m.pullSecret.Username),
Password: to.StringPtr(m.pullSecret.Password),
}
options := (&images.PullOptions{}).
WithQuiet(true).
WithPolicy("always").
WithUsername(m.pullSecret.Username).
WithPassword(m.pullSecret.Password)

_, err := images.Pull(m.conn, version.Properties.InstallerPullspec, options)
return err
Expand All @@ -70,21 +68,16 @@ func (m *manager) Install(ctx context.Context, sub *api.SubscriptionDocument, do
}

func (m *manager) putSecret(secretName string) specgen.Secret {
uid := uint32(os.Getuid())
gid := uint32(os.Getgid())
return specgen.Secret{
Source: m.clusterUUID + "-" + secretName,
Target: "/.azure/" + secretName,
UID: uid,
GID: gid,
Mode: 0o644,
}
}

func (m *manager) startContainer(ctx context.Context, version *api.OpenShiftVersion) error {
s := specgen.NewSpecGenerator(version.Properties.InstallerPullspec, false)
s.Name = "installer-" + m.clusterUUID
s.User = fmt.Sprintf("%d", os.Getuid())

s.Secrets = []specgen.Secret{
m.putSecret("99_aro.json"),
Expand Down Expand Up @@ -140,7 +133,9 @@ func (m *manager) createSecrets(ctx context.Context, doc *api.OpenShiftClusterDo
if err != nil {
return err
}
_, err = secrets.Create(m.conn, bytes.NewBuffer(encCluster), &secrets.CreateOptions{Name: to.StringPtr(m.clusterUUID + "-99_aro.json")})
_, err = secrets.Create(
m.conn, bytes.NewBuffer(encCluster),
(&secrets.CreateOptions{}).WithName(m.clusterUUID+"-99_aro.json"))
if err != nil {
return err
}
Expand All @@ -149,7 +144,9 @@ func (m *manager) createSecrets(ctx context.Context, doc *api.OpenShiftClusterDo
if err != nil {
return err
}
_, err = secrets.Create(m.conn, bytes.NewBuffer(encSub), &secrets.CreateOptions{Name: to.StringPtr(m.clusterUUID + "-99_sub.json")})
_, err = secrets.Create(
m.conn, bytes.NewBuffer(encSub),
(&secrets.CreateOptions{}).WithName(m.clusterUUID+"-99_sub.json"))
if err != nil {
return err
}
Expand Down Expand Up @@ -189,7 +186,9 @@ func (m *manager) secretFromFile(from, name string) error {
return err
}

_, err = secrets.Create(m.conn, f, &secrets.CreateOptions{Name: to.StringPtr(m.clusterUUID + "-" + name)})
_, err = secrets.Create(
m.conn, f,
(&secrets.CreateOptions{}).WithName(m.clusterUUID+"-"+name))
return err
}

Expand All @@ -201,7 +200,9 @@ func (m *manager) cleanupContainers(ctx context.Context) error {
getContainerLogs(m.conn, m.log, containerName)
}

_, err := containers.Remove(m.conn, containerName, &containers.RemoveOptions{Force: to.BoolPtr(true), Ignore: to.BoolPtr(true)})
_, err := containers.Remove(
m.conn, containerName,
(&containers.RemoveOptions{}).WithForce(true).WithIgnore(true))
if err != nil {
m.log.Errorf("unable to remove container: %v", err)
}
Expand Down
91 changes: 63 additions & 28 deletions pkg/containerinstall/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,22 @@ package containerinstall
// Licensed under the Apache License 2.0.

import (
"bytes"
"context"
"testing"
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

"github.com/Azure/go-autorest/autorest/to"
"github.com/containers/podman/v4/libpod/define"
"github.com/containers/podman/v4/pkg/bindings/containers"
"github.com/containers/podman/v4/pkg/bindings/images"
"github.com/containers/podman/v4/pkg/bindings/secrets"
"github.com/containers/podman/v4/pkg/domain/entities"
"github.com/containers/podman/v4/pkg/specgen"
"github.com/onsi/gomega/types"
"github.com/opencontainers/runtime-spec/specs-go"
"github.com/sirupsen/logrus"
"github.com/sirupsen/logrus/hooks/test"

Expand All @@ -26,15 +31,19 @@ const TEST_PULLSPEC = "registry.access.redhat.com/ubi8/go-toolset:1.18.4"

var _ = Describe("Podman", Ordered, func() {
var err error
var cancel context.CancelFunc
var conn context.Context
var hook *test.Hook
var log *logrus.Entry
var containerName string
var containerID string
var secret *entities.SecretCreateReport

BeforeAll(func(ctx context.Context) {
BeforeAll(func() {
var err error
conn, err = getConnection(ctx)
var outerconn context.Context
outerconn, cancel = context.WithCancel(context.Background())
conn, err = getConnection(outerconn)
if err != nil {
Skip("unable to access podman: %v")
}
Expand All @@ -44,53 +53,79 @@ var _ = Describe("Podman", Ordered, func() {
})

It("can pull images", func() {
_, err = images.Pull(conn, TEST_PULLSPEC, &images.PullOptions{Policy: to.StringPtr("missing")})
_, err = images.Pull(conn, TEST_PULLSPEC, (&images.PullOptions{}).WithPolicy("missing"))
Expect(err).To(BeNil())
})

It("can create a secret", func() {

secret, err = secrets.Create(
conn, bytes.NewBufferString("hello\n"),
(&secrets.CreateOptions{}).WithName(containerName))
Expect(err).To(BeNil())

})

It("can start a container", func() {
s := specgen.NewSpecGenerator(TEST_PULLSPEC, false)
s.Name = containerName
s.Entrypoint = []string{"/bin/bash", "-c", "echo 'hello'"}
s.Secrets = []specgen.Secret{
{
Source: containerName,
Target: "/.azure/testfile",
Mode: 0o644,
},
}
s.Mounts = append(s.Mounts, specs.Mount{
Destination: "/.azure",
Type: "tmpfs",
Source: "",
})
s.WorkDir = "/.azure"
s.Entrypoint = []string{"/bin/bash", "-c", "cat testfile"}

containerID, err = runContainer(conn, log, s)
Expect(err).To(BeNil())
})

It("can wait for completion", func() {
exit, err := containers.Wait(conn, containerID, nil)
exit, err := containers.Wait(conn, containerID, (&containers.WaitOptions{}).WithCondition([]define.ContainerStatus{define.ContainerStateExited}))
Expect(err).To(BeNil())
Expect(exit).To(Equal(0), "exit code was %d, not 0", exit)
Expect(exit).To(BeEquivalentTo(0), "exit code was %d, not 0", exit)
})

It("can fetch container logs", func() {
err = getContainerLogs(conn, log, containerID)
Expect(err).To(BeNil())

entries := []map[string]types.GomegaMatcher{
{
"msg": Equal("created container " + containerName + " with ID " + containerID),
"level": Equal(logrus.InfoLevel),
},
{
"msg": Equal("started container " + containerID),
"level": Equal(logrus.InfoLevel),
},
{
"msg": Equal("stdout: hello\n"),
"level": Equal(logrus.InfoLevel),
},
}

err = testlog.AssertLoggingOutput(hook, entries)
Expect(err).To(BeNil())
// sometimes logs take a few seconds to flush to disk, so just retry for 10s
Eventually(func(g Gomega) {
hook.Reset()
err = getContainerLogs(conn, log, containerID)
g.Expect(err).To(BeNil())
entries := []map[string]types.GomegaMatcher{
{
"msg": Equal("stdout: hello\n"),
"level": Equal(logrus.InfoLevel),
},
}

err = testlog.AssertLoggingOutput(hook, entries)
g.Expect(err).To(BeNil())
}).WithTimeout(10 * time.Second).WithPolling(time.Second).Should(Succeed())
})

AfterAll(func() {
if containerID != "" {
_, err = containers.Remove(conn, containerID, &containers.RemoveOptions{Force: to.BoolPtr(true)})
_, err = containers.Remove(conn, containerID, (&containers.RemoveOptions{}).WithForce(true))
Expect(err).To(BeNil())
}

if secret != nil {
err = secrets.Remove(conn, secret.ID)
Expect(err).To(BeNil())
}

if cancel != nil {
cancel()
}
})
})

Expand Down
3 changes: 1 addition & 2 deletions pkg/containerinstall/podman.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"context"
"os"

"github.com/Azure/go-autorest/autorest/to"
"github.com/containers/podman/v4/pkg/bindings"
"github.com/containers/podman/v4/pkg/bindings/containers"
"github.com/containers/podman/v4/pkg/specgen"
Expand Down Expand Up @@ -41,7 +40,7 @@ func getContainerLogs(ctx context.Context, log *logrus.Entry, containerName stri
err := containers.Logs(
ctx,
containerName,
&containers.LogOptions{Stderr: to.BoolPtr(true), Stdout: to.BoolPtr(true)},
(&containers.LogOptions{}).WithStderr(true).WithStdout(true),
stdout,
stderr,
)
Expand Down

0 comments on commit 06cfba5

Please sign in to comment.