Skip to content

Commit

Permalink
Add full compatibility for podman (#288)
Browse files Browse the repository at this point in the history
This change fills the gap previously seen for podman's inability execute
commands on containers.
A large part of this was handling the returned output from podman's
exec_run, which is a tuple containing the return code and a bytes string
containing both the stdout and stderr.
Due to this, we needed to implement a demuxer.

Also made some minor tweaks to the functional tests
  • Loading branch information
JacobCallahan committed May 13, 2024
1 parent cf80a87 commit 0c7c455
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 12 deletions.
47 changes: 47 additions & 0 deletions broker/binds/containers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,52 @@
"""A collection of classes to ease interaction with Docker and Podman libraries."""

HEADER_SIZE = 8
STDOUT = 1
STDERR = 2


def demux_output(data_bytes):
"""Demuxes the output of a container stream into stdout and stderr streams.
Stream data is expected to be in the following format:
- 1 byte: stream type (1=stdout, 2=stderr)
- 3 bytes: padding
- 4 bytes: payload size (big-endian)
- N bytes: payload data
ref: https://docs.podman.io/en/latest/_static/api.html?version=v5.0#tag/containers/operation/ContainerAttachLibpod
Args:
data_bytes: Bytes object containing the combined stream data.
Returns:
A tuple containing two bytes objects: (stdout, stderr).
"""
stdout = b""
stderr = b""
while len(data_bytes) >= HEADER_SIZE:
# Extract header information
header, data_bytes = data_bytes[:HEADER_SIZE], data_bytes[HEADER_SIZE:]
stream_type = header[0]
payload_size = int.from_bytes(header[4:HEADER_SIZE], "big")
# Check if data is sufficient for payload
if len(data_bytes) < payload_size:
break # Incomplete frame, wait for more data

# Extract and process payload
payload = data_bytes[:payload_size]
if stream_type == STDOUT:
stdout += payload
elif stream_type == STDERR:
stderr += payload
else:
# todo: Handle unexpected stream types
pass

# Update data for next frame
data_bytes = data_bytes[payload_size:]

return stdout, stderr


class ContainerBind:
"""A base class that provides common functionality for Docker and Podman containers."""
Expand Down
12 changes: 10 additions & 2 deletions broker/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import yaml

from broker import exceptions, logger as b_log, settings
from broker.binds.containers import demux_output

FilterTest = namedtuple("FilterTest", "haystack needle test")
INVENTORY_LOCK = threading.Lock()
Expand Down Expand Up @@ -512,8 +513,15 @@ def from_ssh(cls, stdout, channel):
)

@classmethod
def from_duplexed_exec(cls, duplex_exec):
"""Create a Result object from a duplexed exec object from the docker library."""
def from_duplexed_exec(cls, duplex_exec, runtime=None):
"""Create a Result object from a duplexed exec object from podman or docker."""
if runtime == "podman":
stdout, stderr = demux_output(duplex_exec[1])
return cls(
status=duplex_exec[0],
stdout=stdout.decode("utf-8"),
stderr=stderr.decode("utf-8"),
)
if duplex_exec.output[0]:
stdout = duplex_exec.output[0].decode("utf-8")
else:
Expand Down
3 changes: 2 additions & 1 deletion broker/hosts.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ def session(self):
if not isinstance(getattr(self, "_session", None), Session):
# Check to see if we're a non-ssh-enabled Container Host
if hasattr(self, "_cont_inst") and not self._cont_inst.ports.get(22):
self._session = ContainerSession(self)
runtime = "podman" if "podman" in str(self._cont_inst.client) else "docker"
self._session = ContainerSession(self, runtime=runtime)
else:
self.connect()
return self._session
Expand Down
4 changes: 3 additions & 1 deletion broker/providers/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ def _find_ssh_port(port_map, ssh_port=22):
elif isinstance(port_map, dict):
# {'22/tcp': [{'HostIp': '', 'HostPort': '1337'}],
for key, val in port_map.items():
if key.startswith("22"):
if key.startswith("22") and isinstance(val, list):
return val[0]["HostPort"]

def _set_attributes(self, host_inst, broker_args=None, cont_inst=None):
Expand Down Expand Up @@ -297,6 +297,8 @@ def run_container(self, container_host, **kwargs):
@Provider.register_action("container_app")
def execute(self, container_app, **kwargs):
"""Run a container and return the raw results."""
if not kwargs.get("name"):
kwargs["name"] = self._gen_name()
return self.runtime.execute(container_app, **kwargs)

def run_wait_container(self, image_name, **kwargs):
Expand Down
7 changes: 5 additions & 2 deletions broker/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,11 @@ def __init__(self, **kwargs):
class ContainerSession:
"""An approximation of ssh-based functionality from the Session class."""

def __init__(self, cont_inst):
def __init__(self, cont_inst, runtime=None):
self._cont_inst = cont_inst
if not runtime:
runtime = settings.CONTAINER.runtime
self.runtime = runtime

def run(self, command, demux=True, **kwargs):
"""Container approximation of Session.run."""
Expand All @@ -77,7 +80,7 @@ def run(self, command, demux=True, **kwargs):
command = f"/bin/bash -c '{command}'"
result = self._cont_inst._cont_inst.exec_run(command, **kwargs)
if demux:
result = helpers.Result.from_duplexed_exec(result)
result = helpers.Result.from_duplexed_exec(result, self.runtime)
else:
result = helpers.Result.from_nonduplexed_exec(result)
return result
Expand Down
3 changes: 2 additions & 1 deletion broker_settings.yaml.example
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,13 @@ Container:
host_username: "<username>"
host_password: "<plain text password>"
host_port: None
runtime: docker
default: True
- remote:
host: "<remote hostname>"
host_username: "<username>"
host_password: "<plain text password>"
runtime: 'docker'
runtime: podman
# name used to prefix container names, used to distinguish yourself
# if not set, then your local username will be used
# name_prefix: test
Expand Down
2 changes: 1 addition & 1 deletion tests/data/cli_scenarios/containers/execute_ch-d_ubi8.yaml
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
container_app: ubi8:latest
command: "ls -lah"
command: pwd
2 changes: 1 addition & 1 deletion tests/functional/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Do not attempt to use Broker while running these functional tests or you may end
Setup:
- Ensure either Docker or Podman are installed and configured either locally or on a remote host.
- Ensure Broker's Container provider is configured with the details of the previous step.
- Clone the [content-host-d](https://github.com/JacobCallahan/content-host-d) repository and build the UBI8 image, tagging it as `ch-d:ubi8`.
- Clone the [content-host-d](https://github.com/JacobCallahan/content-host-d) repository and build the UBI[7-9] images, tagging them as `ubi[7-9]` respectively.

**SatLab Tests**

Expand Down
6 changes: 3 additions & 3 deletions tests/functional/test_containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ def test_container_e2e_mp():

def test_broker_multi_manager():
with Broker.multi_manager(
ubi7={"container_host": "ubi7:latest"},
ubi8={"container_host": "ubi8:latest", "_count": 2},
ubi9={"container_host": "ubi9:latest"},
ubi7={"container_host": "localhost/ubi7:latest"},
ubi8={"container_host": "localhost/ubi8:latest", "_count": 2},
ubi9={"container_host": "localhost/ubi9:latest"},
) as multi_hosts:
assert "ubi7" in multi_hosts
assert "ubi8" in multi_hosts
Expand Down

0 comments on commit 0c7c455

Please sign in to comment.