faradayio / cage

Develop and deploy complex Docker applications
http://cage.faraday.io
Apache License 2.0
307 stars 26 forks source link

Unable to port map UDP ports #75

Closed nsriram closed 6 years ago

nsriram commented 6 years ago

Hi, We use cage extensively in our project and have a need to expose UDP port (for using COAP). Declaring the following however does not start and fails with invalid ports '5683/udp'.

ports:

Thanks, Sriram

emk commented 6 years ago

Ah, good catch! I don't have time to fix this today, but if anybody would like to try to fix this, I can walk them through what needs to happen. If not, it may need to wait until next week.

The actual fix would need to be made in compose_yml. First, the code would need to have a protocol added here:

/// Specify how to map container ports to the host.
#[derive(Debug, Clone, PartialEq, Eq)]
#[allow(missing_copy_implementations)]
pub struct PortMapping {
    /// An optional host address on which to listen.  Defaults to all host
    /// addresses.  If this field is specified, then `host_ports` must also
    /// be specified.
    pub host_address: Option<IpAddr>,
    /// The host port(s) on which to listen.  Must contain the same number
    /// of ports as `container_ports`.  Defaults to an
    /// automatically-assigned port number.
    pub host_ports: Option<Ports>,
    /// The container port(s) to export.
    pub container_ports: Ports,

    /// PRIVATE.  Mark this struct as having unknown fields for future
    /// compatibility.  This prevents direct construction and exhaustive
    /// matching.  This needs to be be public because of
    /// http://stackoverflow.com/q/39277157/12089
    #[doc(hidden)]
    pub _hidden: (),
}

We're want to add:

    pub protocol: Protocol

...which could be defined as:

/// An IP protocol.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Protocol {
    /// Transmission Control Protocol, the default.
    Tcp,
    /// User Datagram Protocol.
    Udp,
}

impl Default for Protocol {
    fn default() -> Self {
        Protocol::Tcp
    }
}

From there, it would be necessary to implement FromStr and fmt::Display. With any luck, all the necessary changes should be local to that file. I'm happy to provide help to anybody who wants to do this!

I will probably have time to work on this myself earlier next, if nobody beats me to it. Feel free to ping me if I don't get to it. Thank you for the bug report!

camjackson commented 6 years ago

I've started work on this, WIP PR is here: https://github.com/emk/compose_yml/pull/8.

Not sure exactly what the next step is but I'll keep digging.

emk commented 6 years ago

I think we're really close: We just need to get that patch merged, and then I can release a new cage ASAP. Thank you very much for digging into this!

emk commented 6 years ago

@camjackson has fixed this in #77, and it will be released in v0.2.3. Thank you for the bug report, and to @camjackson for the fix!

nsriram commented 6 years ago

Thanks Cam & Eric for closing this.

emk commented 6 years ago

No problem! @camjackson did all the work, and I was very happy to merge the changes. :-)