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

set flag for container to container communication #84

Merged
merged 1 commit into from
Mar 9, 2023
Merged

set flag for container to container communication #84

merged 1 commit into from
Mar 9, 2023

Conversation

rwaffen
Copy link
Member

@rwaffen rwaffen commented Mar 1, 2023

under the premisse that the beaker is runnning in a container and the test-container is on the same docker-deamon/on the same docker-host, we can set the nested docker flag and get in the condition that we are both are containers and can connect directly.

tested in our local gitlab with docker-runners

//cc @QueerCodingGirl
Ref: #64 , #46

@codecov
Copy link

codecov bot commented Mar 1, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change

Comparison is base (2285a57) 84.74% compared to head (1695354) 84.74%.

❗ Current head 1695354 differs from pull request most recent head 9b13597. Consider uploading reports for the commit 9b13597 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #84   +/-   ##
=======================================
  Coverage   84.74%   84.74%           
=======================================
  Files           1        1           
  Lines         295      295           
=======================================
  Hits          250      250           
  Misses         45       45           
Impacted Files Coverage Δ
lib/beaker/hypervisor/docker.rb 84.74% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@bastelfreak
Copy link
Member

@rwaffen can you please take one of our modules, for example puppet-collectd, update the Gemfile to use your beaker fork and create a PR? Then we can test if this change breaks existing setups.

@rwaffen
Copy link
Member Author

rwaffen commented Mar 1, 2023

okay, will do it like this. this was also how we tested this in our local setup.

@trevor-vaughan
Copy link
Contributor

I'd like to ask that someone run this on a podman system as well before merging to make sure it doesn't break that stack.

@trevor-vaughan
Copy link
Contributor

Update: The core tests passed so it's probably fine but I haven't checked to see if we do multi-node in that one yet.

@rwaffen
Copy link
Member Author

rwaffen commented Mar 1, 2023

i have no podman system at hand :(

@trevor-vaughan
Copy link
Contributor

It looks like we may want to add a cross-host test to the internal beaker tests so that we don't have to worry about an external check.

@rwaffen
Copy link
Member Author

rwaffen commented Mar 1, 2023

so how do we proceed from here on?

@rwaffen
Copy link
Member Author

rwaffen commented Mar 1, 2023

@jay7x said in the slack, that podman ist also working for him

@trevor-vaughan
Copy link
Contributor

The ideal method would be to add a test to https://github.com/voxpupuli/beaker-docker/blob/master/spec/beaker/hypervisor/docker_spec.rb that does some sort of verifiable communication between the two nodes.

SSH with a subcommand that echos something to a file or anything that we can throw between the two nodes that can be validated.

@rwaffen
Copy link
Member Author

rwaffen commented Mar 1, 2023

i don't know if we have this "node" concept in GHA. if you are talking docker-nodes as in a host with a running docker-daemon. or how do i have to understand this? this is way more complex than i thought it would be 😅

@trevor-vaughan
Copy link
Contributor

Beaker spins up multiple simultaneous containers and they are addressable by hostname (check the /etc/hosts test in that file).

You should be able to add a test at the bottom of the file that loops through the hosts and tries to do something between them.

This is probably an overcomplicated example but I can't think of a more straightforward one that I can link to at the moment. Will update if I think of one.

@bastelfreak bastelfreak added the bug label Mar 3, 2023
@rwaffen
Copy link
Member Author

rwaffen commented Mar 3, 2023

@trevor-vaughan is this okay so? we starting beaker in a docker container and let it spawn the beaker-docker acceptance tests?

@rwaffen
Copy link
Member Author

rwaffen commented Mar 9, 2023

@bastelfreak is it okay to merge this? we tested it together and it works like intended.

@bastelfreak bastelfreak merged commit 52abcfb into voxpupuli:master Mar 9, 2023
@rwaffen rwaffen deleted the container2container branch March 27, 2023 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants