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

Remove dependency on github.com/docker/docker #215

Merged
merged 3 commits into from
Apr 5, 2024

Conversation

andrewkroh
Copy link
Member

@andrewkroh andrewkroh commented Apr 5, 2024

Docker is used to test in environments with different hostnames and domain names. This was done by interacting with the Docker API via a dependency on github.com/docker/docker. But this brings in a lot of dependency baggage.

This change replaces docker API interaction with invocation of docker run. This results in a reduced dependency set for the module.

I recommend to hide whitespace changes when viewing the diff.

Docker is used to test in environments with different hostnames
and domain names. This was done by interacting with the Docker API
via a dependency on github.com/docker/docker. But this brings in
a lot of dependency baggage.

This change replaces docker API interaction with invocation of `docker run`.
This results in a reduced dependency set for the module.
Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

🎉 ✂️ 🎉

providers/linux/host_fqdn_integration_linux_test.go Outdated Show resolved Hide resolved
Comment on lines 125 to 126
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
Copy link
Contributor

Choose a reason for hiding this comment

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

Capture these in a buffer? Then they can be emitted in the t.Logf call.

Copy link
Member Author

@andrewkroh andrewkroh Apr 5, 2024

Choose a reason for hiding this comment

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

SGTM d518340. Here's the new output:

ubuntu@neat-chimpanzee:/go-sysinfo/providers/linux$ go test  -run TestHost_FQDN -tags integration -v .
=== RUN   TestHost_FQDN
=== RUN   TestHost_FQDN/TestHost_FQDN_set_hostname+domainname
    host_fqdn_integration_linux_test.go:84: Running docker container using ["run" "--rm" "-v" "/home/ubuntu/go:/go" "-v" "/go-sysinfo:/go-sysinfo" "-w=/go-sysinfo" "--hostname=hostname" "--domainname=some.domain" "golang:1.22.2" "go" "test" "-v" "-tags" "integration,docker" "-run" "^TestHost_FQDN_set$" "./providers/linux"]
    host_fqdn_integration_linux_test.go:84: Container output:
        === RUN   TestHost_FQDN_set
        --- PASS: TestHost_FQDN_set (0.00s)
        PASS
        ok      github.com/elastic/go-sysinfo/providers/linux   0.005s
    host_fqdn_integration_linux_test.go:84: Exiting container
=== RUN   TestHost_FQDN/TestHost_FQDN_set_hostname_only
    host_fqdn_integration_linux_test.go:84: Running docker container using ["run" "--rm" "-v" "/home/ubuntu/go:/go" "-v" "/go-sysinfo:/go-sysinfo" "-w=/go-sysinfo" "--hostname=hostname.some.domain" "golang:1.22.2" "go" "test" "-v" "-tags" "integration,docker" "-run" "^TestHost_FQDN_set$" "./providers/linux"]
    host_fqdn_integration_linux_test.go:84: Container output:
        === RUN   TestHost_FQDN_set
        --- PASS: TestHost_FQDN_set (0.00s)
        PASS
        ok      github.com/elastic/go-sysinfo/providers/linux   0.006s
    host_fqdn_integration_linux_test.go:84: Exiting container
=== RUN   TestHost_FQDN/TestHost_FQDN_not_set
    host_fqdn_integration_linux_test.go:84: Running docker container using ["run" "--rm" "-v" "/home/ubuntu/go:/go" "-v" "/go-sysinfo:/go-sysinfo" "-w=/go-sysinfo" "golang:1.22.2" "go" "test" "-v" "-count" "1" "-tags" "integration,docker" "-run" "^TestHost_FQDN_not_set$" "./providers/linux"]
    host_fqdn_integration_linux_test.go:84: Container output:
        === RUN   TestHost_FQDN_not_set
        --- PASS: TestHost_FQDN_not_set (0.00s)
        PASS
        ok      github.com/elastic/go-sysinfo/providers/linux   0.007s
    host_fqdn_integration_linux_test.go:84: Exiting container
--- PASS: TestHost_FQDN (54.56s)
    --- PASS: TestHost_FQDN/TestHost_FQDN_set_hostname+domainname (17.82s)
    --- PASS: TestHost_FQDN/TestHost_FQDN_set_hostname_only (18.14s)
    --- PASS: TestHost_FQDN/TestHost_FQDN_not_set (18.60s)
PASS
ok      github.com/elastic/go-sysinfo/providers/linux   54.562s

@andrewkroh andrewkroh requested a review from efd6 April 5, 2024 03:56
@andrewkroh andrewkroh enabled auto-merge (squash) April 5, 2024 03:58
@andrewkroh andrewkroh disabled auto-merge April 5, 2024 03:58
@andrewkroh andrewkroh merged commit 8569714 into elastic:main Apr 5, 2024
18 checks passed
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.

2 participants