Broker changes for remote podman (tcp and ipv6) #302
Conversation
| """Return the first matching network that matches all attr_dict keys and values.""" | ||
| for network in self.networks: | ||
| if all(network.attrs.get(k) == v for k, v in attr_dict.items()): | ||
| return network |
There was a problem hiding this comment.
in our case I have disabled the ipv4 interfaces even from the podman network, if we go though this logic it will work gets the same network. But in case someone creates another network then it will have ipv4 and ipv6 interface as well. @JacobCallahan Do you think that it will be good to pass the network name/id from the broker_settings file ? it will also eliminate to have iteration as well.
There was a problem hiding this comment.
great suggestion. i've now added a new network setting under the container config that can accept a specific network name.
JacobCallahan
force-pushed
the
master
branch
3 times, most recently
from
f37fd90 to
3585b4f
Compare
|
@omkarkhatavkar i've tested changing the network locally and things look good, but can you validate it works on your end? You can check the network a container is associated with by this attribute chain. container_host._cont_inst.attrs["NetworkSettings"]["Networks"] |
We have encountered a couple of issues that need to be solved in order to use Broker with remote podman hosts. First is an issue with the ssh\-based connection to remote podman that was investigated in a previous issue. The solution to this from Broker's side is largely changing the connection string based on the host\_port given. Second is to give Broker the ability to automatically find and pass along the correct network information when spinning up an ipv6 container. The solution for this is to list networks, search for one that is ipv6 enabled, then modify the container creation arguments to pass along that network information
| @@ -144,8 +165,10 @@ def __init__(self, **kwargs): | |||
| self._ClientClass = PodmanClient | |||
| if self.host == "localhost": | |||
| self.uri = "unix:///run/user/1000/podman/podman.sock" | |||
| else: | |||
| elif kwargs.get("port") == SSH_PORT: | |||
There was a problem hiding this comment.
wouldn't it be better to check for existing schema in the {host} bit, and only apply this default in case it is not explicitly specified? That would also support e.g. ssh on a non-standard port.
Alternatively, you might introduce a new config key, called "schema", but I agree this might be a bit overengineered solution
There was a problem hiding this comment.
for now, this will be fine. if we run into issues with non-standard ssh ports in the future, then we can re-evaluate for sure.
broker/broker.py
Outdated
| if provider.__name__ in self._kwargs: | ||
| self._kwargs[provider.__name__] = self._kwargs.pop(provider.__name__) |
There was a problem hiding this comment.
Can you please clarify why this code is needed here?
[ins] In [30]: mydict = {'foo': 'bar'}
[ins] In [31]: mydict['foo'] = mydict.pop('foo')
[ins] In [32]: mydict
Out[32]: {'foo': 'bar'}This seems like an unnecessary step. Isn't it missing something?
Having a comment here would be nice.
There was a problem hiding this comment.
great catch! this was originally implemented with a different argument provider_instance that I ultimately decided didn't fit into the previously established way of specifying an alternate instance. This section looked like this before an apparently too-late-in-the-day refactor!
if "provider_instance" in self._kwargs:
self._kwargs[provider.__name__] = self._kwargs.pop("provider_instance")This change allows users to use a comma-separated list in the container network settings. This will then expand to pass each network on container creation. If any of the networks aren't found, then an exception will be raised. I also threw in the ability to specify an alternate provider instance to load when doing most broker actions, like checkout.
We have encountered a couple of issues that need to be solved in order to use Broker with remote podman hosts.
First is an issue with the ssh-based connection to remote podman that was investigated in a previous issue. The
solution to this from Broker's side is largely changing the connection string based on the host_port given.
Second is to give Broker the ability to automatically find and pass along the correct network information when
spinning up an ipv6 container. The solution for this is to list networks, search for one that is ipv6 enabled, then
modify the container creation arguments to pass along that network information