Enhance the code to support to use as container engine podman for e2e

[cmoulliard]

• Enhanced the code to support to use as container engine podman and not only docker for 2e2
• Issue: Support to run the e2e test cases using podman #472

You can test this PR locally and override the defaults value: docker and /var/run/docker.sock to test a different container engine

❯ make e2e CONTAINER_ENGINE=podman DOCKER_SOCKET=unix:///var/folders/28/g86pgjxj0wl1nkd_85c2krjw0000gn/T/podman/podman-machine-default-api.sock
go test -v -p 1 -timeout 15m --tags=e2e ./tests/e2e/podman/...
=== RUN   Test_CreateCluster
    e2e.go:110: running idpbuilder create
    e2e.go:250: checking argocd app argocd in argocd ns
    e2e.go:257: app argocd ready
    e2e.go:250: checking argocd app nginx in argocd ns
    e2e.go:257: app nginx ready
    e2e.go:250: checking argocd app gitea in argocd ns
    e2e.go:257: app gitea ready
    e2e.go:267: all argocd apps healthy
    e2e.go:273: testing argocd endpoints
    e2e.go:293: testing gitea endpoints
    e2e.go:214: testing gitea container registry
    e2e.go:83: cleaning up environment
    e2e.go:101: finished cleaning up container engine environment
    e2e.go:132: running idpbuilder create --use-path-routing
    e2e.go:267: all argocd apps healthy
    e2e.go:273: testing argocd endpoints
    e2e.go:293: testing gitea endpoints
    e2e.go:214: testing gitea container registry
    e2e.go:83: cleaning up environment
    e2e.go:101: finished cleaning up container engine environment
    e2e.go:154: running idpbuilder create --port 2443
    e2e.go:267: all argocd apps healthy
    e2e.go:273: testing argocd endpoints
    e2e.go:293: testing gitea endpoints
    e2e.go:83: cleaning up environment
    e2e.go:101: finished cleaning up container engine environment
    e2e.go:175: running create --package ../../../pkg/controllers/custompackage/test/resources/customPackages/testDir
    e2e.go:267: all argocd apps healthy
    e2e.go:250: checking argocd app my-app in argocd ns
    e2e.go:257: app my-app ready
    e2e.go:250: checking argocd app my-app2 in argocd ns
    e2e.go:257: app my-app2 ready
    e2e.go:267: all argocd apps healthy
    e2e.go:83: cleaning up environment
    e2e.go:101: finished cleaning up container engine environment
--- PASS: Test_CreateCluster (651.25s)
PASS
ok      github.com/cnoe-io/idpbuilder/tests/e2e/podman  651.951s

[cmoulliard]

/e2e

[nabuskey]

If we actually run this in GHA, we are doubling the time required to run all tests. We just need to be mindful when enabling it in the workflows.

[nabuskey]

Can we create a new package just for podman instead of mixing docker and podman? Commands are very similar so you can abstract and re-use them in podman. Every time there's a slight differences between docker and podman, we have to do things like this if os.Getenv("CONTAINER_ENGINE") == "podman" {. I'd rather branch earlier.

[cmoulliard]

If we actually run this in GHA, we are doubling the time required to run all tests.

Why do you want to do that ? This change only allows to run the tests using as engine docker or podman and should not require to run both engines.

[cmoulliard]

/e2e

[cmoulliard]

/e2e

[cmoulliard]

/e2e

[cmoulliard]

/e2e

[cmoulliard]

/e2e

[cmoulliard]

e2e tests succeeded using either podman or docker

[cmoulliard]

/e2e

[cmoulliard]

/e2e

[cmoulliard]

What do you think if we do this small improvement ?

// Interface containing commands to be executed (aka exec.command()). Until now we just need idpbuilder executable but additional methods could be added
type CommandExecutor interface {
	RunIdp(ctx context.Context, cmd string, timeout time.Duration) ([]byte, error)
}

// Container engine and methods able to return the engine cli to be used or to run a docker podman command
type Engine interface {
	RunCommand(ctx context.Context, cmd string, timeout time.Duration) ([]byte, error)
	GetClient() string
}

// Type name to be used to access the different methods of the interface
type ToolBox struct {
	Client string
}

// Implementation of the method RunIdp of the interface: CommandExecutor
func (t *ToolBox) RunIdp(ctx context.Context, cmd string, timeout time.Duration) ([]byte, error) {
	return shared.RunIdp(ctx, cmd, timeout)
}

// Implementation of the method Getclient of the interface: Engine
func (t *ToolBox) GetClient() string {
	return t.Client
}

// Implementation of the method RunCommand of the interface: Engine
func (t *ToolBox) RunCommand(ctx context.Context, command string, timeout time.Duration) ([]byte, error) {
	cmdCtx, cancel := context.WithTimeout(ctx, timeout)
	defer cancel()

	cmds := strings.Split(command, " ")
	if len(cmds) == 0 {
		return nil, fmt.Errorf("supply at least one command")
	}

	binary := cmds[0]
	args := make([]string, 0, len(cmds)-1)
	if len(cmds) > 1 {
		args = append(args, cmds[1:]...)
	}

	if cmds[1] == "login" || cmds[1] == "push" || cmds[1] == "pull" {
		args = append(args, "--tls-verify=false")
	}

	c := exec.CommandContext(cmdCtx, binary, args...)

	// DOCKER_HOST = unix:///var/run/docker.sock is needed for podman running in rootless mode
	c.Env = append(os.Environ(), "DOCKER_HOST="+os.Getenv("DOCKER_HOST"))

	b, err := c.CombinedOutput()
	if err != nil {
		return nil, fmt.Errorf("error while running %s: %s, %s", command, err, b)
	}

	return b, nil
}

// dummy main code
func main() {
	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
	defer cancel()

        // create an intance of the toolbox
	toolBox := &ToolBox{ client: "podman"}

	// Execute the idpbuilder create command
	output, err := toolBox.RunIdp(ctx, "create", 3*time.Second)
	...

@nabuskey

[nabuskey]

Do we need to worry about running e2e tests in parallel? By default tests run parallel across multiple packages.

We might end up in a situation where you are running podman and docker tests at the same time using the same port. This will result in failures.

[nabuskey]

Signature is exactly the same. Why can't we use RunCommand? If you need env vars to be set, you can update the function to take a map that defines env vars.

If we do that, we don't need this interface because we can just use the function as-is.

[cmoulliard]

Good to know. By the way there is a difference as RunCommand is used to rung the container engine cmd (podman pusll, podman push, etc) why RunIdpCommand is used to run IDP command: idp create, etc

Note: See my comment where I proposed to move RunIdpCommand

[nabuskey]

Personally, introducing interfaces is not necessary for two engines. Makes it a bit more difficult to read for beginners. Function calls are sufficient.

[cmoulliard]

If we don't use an interface then how would you like that we accommodate within the e2e go file and this function

func TestCreateCluster(t *testing.T, containerEngine container.Engine) {
...

the access to the method runIdpCommand or runCommand no matter which engine is used: podman or docker

...
	t.Log("running idpbuilder create")
	b, err := containerEngine.RunIdpCommand(ctx, fmt.Sprintf("%s create --recreate", IdpbuilderBinaryLocation), 0)
	assert.NoError(t, err, fmt.Sprintf("error while running create: %s, %s", err, b))

[nabuskey]

What's are the differences between RunIdpCommand and RunCommand supposed to be? In implementation, the differences I see are env variable handling and context handling. We should be able to combine the two? Maybe something like:

type Engine interface {
	GetEnv() map[string][string]
	RunCommand(ctx context.Context, cmd string, timeout time.Duration) ([]byte, error)
	GetClient() string
}

Then in here, call GetEnv, set the env var accordingly.

[cmoulliard]

We might end up in a situation where you are running podman and docker tests at the same time using the same port. This will result in failures.

This is not something that we are doing now. We can address such a need in a separate future PR

[nabuskey]

This is not something that we are doing now. We can address such a need in a separate future PR

You are right that we are not doing this right now because we only have one package. With this change we have two packages. By default, tests in different packages run in parallel. If we have two packages that need to bind to the same port, it's not good. So all I am saying is we need to make sure we test this well and make sure we don't break E2E.

[cmoulliard]

We might end up in a situation where you are running podman and docker tests at the same time using the same port. This will result in failures.

What do you then suggest that we do as configuring a machine able to run podman and docker engines at the same time for // tests is quite difficult and will never happen. until ow on github workflow as podman cannot yet run there ?

[nabuskey]

We might end up in a situation where you are running podman and docker tests at the same time using the same port. This will result in failures.

What do you then suggest that we do as configuring a machine able to run podman and docker engines at the same time for // tests is quite difficult and will never happen. until ow on github workflow as podman cannot yet run there ?

The GHA image we use come with both podman and docker. They both work out of the box, as far as I know. I have docker and podman installed on my workstation and they work fine too.

https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2404-Readme.md

What we need to do is make sure we don't run these tests in parallel to avoid host port contentions. Looking at the Makefile, we might be ok but we need to test.

[cmoulliard]

What we need to do is make sure we don't run these tests in parallel to avoid host port contentions. Looking at the Makefile, we might be ok but we need to test.

/e2e which have been executed a couple of times on this PR don't run tests in // as the makefile checks the environment var setting the container engine to be used: docker or podman. So I propose that we merge @nabuskey @punkwalker

[cmoulliard]

I updated the github workflow to test using podman and docker @nabuskey
Should I do the same change when we pass as github comment /e2e

[nabuskey]

What's are the differences between RunIdpCommand and RunCommand supposed to be? In implementation, the differences I see are env variable handling and context handling. We should be able to combine the two? Maybe something like:

type Engine interface {
	GetEnv() map[string][string]
	RunCommand(ctx context.Context, cmd string, timeout time.Duration) ([]byte, error)
	GetClient() string
}

Then in here, call GetEnv, set the env var accordingly.

[cmoulliard]

What's are the differences between RunIdpCommand and RunCommand supposed to be?
They looks similar but the command RunIdpCommand will use as executable idpbuilder while RunCommand will perform kubectl <CMD>

Note: As the code of this PR is working. I suggest to merge it and to improve it in a new PR as I don't have the time at the moment to refactor the code as that will be needed. WDYT ? @nabuskey