Skip to content

Commit

Permalink
Add full compatibility for podman
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 Apr 16, 2024
1 parent f91b4eb commit 874e804
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 11 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
10 changes: 9 additions & 1 deletion 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 @@ -511,8 +512,15 @@ def from_ssh(cls, stdout, channel):
)

@classmethod
def from_duplexed_exec(cls, duplex_exec):
def from_duplexed_exec(cls, duplex_exec, runtime=None):
"""Create a Result object from a duplexed exec object from the docker library."""
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 @@ -282,6 +282,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 @@ -123,9 +123,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 874e804

Please sign in to comment.