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.

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

test/e2e/systemd_activate_test.go: simplify test #18056

Merged
merged 1 commit into from
Apr 12, 2023
Merged
Changes from all commits
Commits
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
57 changes: 19 additions & 38 deletions test/e2e/systemd_activate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
. "github.com/onsi/gomega/gexec"
"github.com/opencontainers/selinux/go-selinux"
)

var _ = Describe("Systemd activate", func() {
Expand All @@ -27,6 +26,8 @@ var _ = Describe("Systemd activate", func() {
var activate string

BeforeEach(func() {
SkipIfRemote("Testing stopped service requires both podman and podman-remote binaries")

tempDir, err = testUtils.CreateTempDirInTempDir()
if err != nil {
fmt.Fprintf(os.Stderr, "%v\n", err)
Expand All @@ -36,8 +37,6 @@ var _ = Describe("Systemd activate", func() {
podmanTest = PodmanTestCreate(tempDir)
podmanTest.Setup()

SkipIfRemote("Testing stopped service requires both podman and podman-remote binaries")

activate, err = exec.LookPath("systemd-socket-activate")
if err != nil {
activate = "/usr/bin/systemd-socket-activate"
Expand Down Expand Up @@ -65,60 +64,42 @@ var _ = Describe("Systemd activate", func() {
Expect(err).ToNot(HaveOccurred())
addr := net.JoinHostPort(host, strconv.Itoa(port))

// Make a temporary root directory
tmpRootDir := filepath.Join(tempDir, "server_root")
err = os.Mkdir(tmpRootDir, 0755)
Expect(err).ToNot(HaveOccurred())
defer os.RemoveAll(tmpRootDir)

// When SELinux is enabled, a storage root directory should be
// labeled with a specific value
if selinux.GetEnabled() {
rootDir := "/var/lib/containers"
label := "container_var_lib_t"
if isRootless() {
rootDir = filepath.Join(os.Getenv("HOME"), ".local/share/containers")
label = "data_home_t"
}

args := []string{"--reference", rootDir, tmpRootDir}
// If rootDir doesn't exist, use "chcon -t" to label tmpRootDir
// instead of "chcon --reference"
if _, err := os.Stat(rootDir); err != nil {
args = []string{"-t", label, tmpRootDir}
}

chcon := testUtils.SystemExec("chcon", args)
Expect(chcon).Should(Exit(0))
}
podmanOptions := podmanTest.makeOptions(nil, false, false)

activateSession := testUtils.StartSystemExec(activate, []string{
systemdArgs := []string{
"-E", "http_proxy", "-E", "https_proxy", "-E", "no_proxy",
"-E", "HTTP_PROXY", "-E", "HTTPS_PROXY", "-E", "NO_PROXY",
"-E", "XDG_RUNTIME_DIR",
"--listen", addr,
podmanTest.PodmanBinary,
"--root", tmpRootDir,
"system", "service",
"--time=0",
})
podmanTest.PodmanBinary}
systemdArgs = append(systemdArgs, podmanOptions...)
systemdArgs = append(systemdArgs, "system", "service", "--time=0")

activateSession := testUtils.StartSystemExec(activate, systemdArgs)
Expect(activateSession.Exited).ShouldNot(Receive(), "Failed to start podman service")
defer activateSession.Signal(syscall.SIGTERM)

// Curried functions for specialized podman calls
// Create custom functions for running podman and
// podman-remote. This test is a rare exception where both
// binaries need to be run in parallel. Usually, the remote
// and non-remote details are hidden. Yet we use the
// `podmanOptions` above to make sure all settings (root,
// runroot, events, tmpdir, etc.) are used as in other e2e
// tests.
podmanRemote := func(args ...string) *testUtils.PodmanSession {
args = append([]string{"--url", "tcp://" + addr}, args...)
return testUtils.SystemExec(podmanTest.RemotePodmanBinary, args)
}

podman := func(args ...string) *testUtils.PodmanSession {
Copy link
Member

Choose a reason for hiding this comment

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

Should you need to repush for any reason, I think future maintainers might welcome a comment here explaining that we can't use the usual podmanTest.Podman() because we need to ensure that we invoke local-podman.

Or, on further thought, WDYT of using podmanTest.Podman(), and adding SkipIfRemote()?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice idea(s)!

Copy link
Member

Choose a reason for hiding this comment

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

@vrothberg, #18083 has merged, so it's safe to proceed with this. I have a moderate preference for following up on my suggestion above (use Podman, add SkipIfRemote) but will not block on it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Note that the test already skips remote. I moved it up a bit to make it easier to spot.

args = append([]string{"--root", tmpRootDir}, args...)
args = append(podmanOptions, args...)
return testUtils.SystemExec(podmanTest.PodmanBinary, args)
}

containerName := "top_" + testUtils.RandomString(8)
apiSession := podmanRemote(
"create", "--tty", "--name", containerName, "--entrypoint", "top",
"quay.io/libpod/alpine_labels:latest",
ALPINE,
)
Expect(apiSession).Should(Exit(0))
defer podman("rm", "-f", containerName)
Expand Down