Skip to content
This repository was archived by the owner on Nov 27, 2023. It is now read-only.

ACI: Allow setting protocol when publishing ports#1053

Merged
ndeloof merged 1 commit intomainfrom
aci_udp_ports
Dec 14, 2020
Merged

ACI: Allow setting protocol when publishing ports#1053
ndeloof merged 1 commit intomainfrom
aci_udp_ports

Conversation

@gtardif
Copy link
Copy Markdown
Contributor

@gtardif gtardif commented Dec 11, 2020

Signed-off-by: Guillaume Tardif guillaume.tardif@gmail.com

What I did

  • Add protocol conversion when creating ACI container group for tcp/udp
  • deploy and check runtime docker inspect also returns the correct port protocol.
  • added an error for unknown protocols

Related issue
Fixes #985

/test-aci

(not mandatory) A picture of a cute animal, if possible in relation with what you did

@gtardif gtardif requested review from ndeloof and rumpl December 11, 2020 15:35
@github-actions github-actions Bot added the aci label Dec 11, 2020
Comment thread aci/convert/ports.go Outdated
groupProtocol = containerinstance.UDP
containerProtocol = containerinstance.ContainerNetworkProtocolUDP
default:
return nil, nil, nil, fmt.Errorf("Unknown protocol %q in exposed port for service %q", portConfig.Protocol, service.Name)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return nil, nil, nil, fmt.Errorf("Unknown protocol %q in exposed port for service %q", portConfig.Protocol, service.Name)
return nil, nil, nil, fmt.Errorf("unknown protocol %q in exposed port for service %q", portConfig.Protocol, service.Name)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... can't put this in my head ! :) fixed

Signed-off-by: Guillaume Tardif <guillaume.tardif@gmail.com>
@karolz-ms
Copy link
Copy Markdown
Contributor

☺️ thank you @gtardif

@ndeloof ndeloof merged commit 99de963 into main Dec 14, 2020
@ndeloof ndeloof deleted the aci_udp_ports branch December 14, 2020 08:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ACI: 'run' command fails to create UDP protocol binding for public IP address

4 participants