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

Podman based local host #1240

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Podman based local host #1240

wants to merge 7 commits into from

Conversation

pducolin
Copy link
Contributor

@pducolin pducolin commented Nov 8, 2024

What does this PR do?

Add a docker podman based host scenario, to be used locally to run e2e tests with the agent installed on a local docker container.

Which scenarios this will impact?

VM scenarios are impacted by some refactoring commits

Motivation

This PR takes over #829 work by @carlosroman and adds support to use a local Ubuntu instance running on a docker container as it was a host.

The instance is configured to talk through SSH, with the user's public key uploaded at setup up time.

Why podman? Podman is a container management tool that supports running systemd on containers. For more details see https://developers.redhat.com/blog/2019/04/24/how-to-run-systemd-in-a-container

Additional Notes

I plan to follow up with a refactoring to move Runner to an interface with a RemoteRunner and LocalRunner implementation, to allow re-using all the cross-platform helpers we have for files management.

Once that is done, I will write the dockerfile within pulumi, so that we clean it up at stack destroy

Performance

From local tests, it takes ~1 minute to create an environment with fakeintake and agent installed on the host and 2 seconds to destroy it.


RUN service ssh start

EXPOSE 22

Choose a reason for hiding this comment

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

🟠 Code Vulnerability

do not expose ssh service from your container (...read more)

Never expose admin ports such as the SSH port 22 in your container. It increases the surface of attack of your containers.

View in Datadog  Leave us feedback  Documentation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This docker file is meant only for local development

@pducolin pducolin marked this pull request as ready for review November 15, 2024 16:09
@pducolin pducolin requested review from a team as code owners November 15, 2024 16:09
@pducolin pducolin changed the title wip Podman based local host Nov 18, 2024

RUN service ssh start

EXPOSE 22

Choose a reason for hiding this comment

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

🟠 Code Vulnerability

do not expose ssh service from your container (...read more)

Never expose admin ports such as the SSH port 22 in your container. It increases the surface of attack of your containers.

View in Datadog  Leave us feedback  Documentation

@@ -285,7 +285,7 @@ func WithIntakeHostname(hostname string) func(*Params) error {
func WithFakeintake(fakeintake *fakeintake.Fakeintake) func(*Params) error {
return func(p *Params) error {
p.ResourceOptions = append(p.ResourceOptions, utils.PulumiDependsOn(fakeintake))
return withIntakeHostname(fakeintake.Host)(p)
return withIntakeHostname(fakeintake.Host, fakeintake.Port)(p)
Copy link
Member

Choose a reason for hiding this comment

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

It will change the behavior because currently we always use port 80. We might end up with the config using port 443 when the fakeintake is deployed in ECS. Are we sure it still works as expected?

return e.InfraConfig.Get(DDInfraDefaultPublicKeyPath)
}

func (e *Environment) GetCommonEnvironment() *config.CommonEnvironment {
Copy link
Member

Choose a reason for hiding this comment

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

Do you really need to access the underlying common environment? I do not expect it to be used since we introduced the config.Env interface

Comment on lines +14 to +15
// NewVM creates an localpodman Ubuntu VM Instance and returns a Remote component.
// Without any parameter it creates an Ubuntu VM on AMD64 architecture.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could make it more clear that it does not really create a VM but rather a container that is supposed to behave like a VM.
The second line is also a bit strange because we cannot pass any parameter. We can just indicate that it creates a Ubuntu container with AMD64 architecture

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants