Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[release/0.9] Fix for port conflict with docker daemon #1373

Merged
merged 1 commit into from
May 2, 2022
Merged

[release/0.9] Fix for port conflict with docker daemon #1373

merged 1 commit into from
May 2, 2022

Conversation

ameyag
Copy link
Contributor

@ameyag ameyag commented Apr 25, 2022

Since moby master is vendoring 0.9.x hcsshim, backporting #1370 to release/0.9 branch

@ameyag ameyag requested a review from a team as a code owner April 25, 2022 21:01
@dcantah
Copy link
Contributor

dcantah commented Apr 25, 2022

@ameyag Does this mean we don't need the other backport pr?

@ameyag
Copy link
Contributor Author

ameyag commented Apr 25, 2022

@dcantah - We need both backports.

This one to get the change into moby master which is currently on 0.9.2 (https://github.com/moby/moby/blob/master/vendor.mod#L14)

#1371 for moby's 20.10 branch that uses hcsshim's moby branch (https://github.com/moby/moby/blob/20.10/vendor.conf#L2)

@dcantah
Copy link
Contributor

dcantah commented Apr 25, 2022

@ameyag Gotcha 👍 Looks like there's a linter issue also on this branch (not related to your change). Let me look into it

@ameyag ameyag force-pushed the rel-0.9-external-port branch from 8dd80e3 to 9242615 Compare April 25, 2022 23:03
@ameyag
Copy link
Contributor Author

ameyag commented May 2, 2022

@dcantah - Let me know if a15940f would be a right way to fix linter issue?

@dcantah
Copy link
Contributor

dcantah commented May 2, 2022

@ameyag Sorry, I never got back to testing the linter issue 😥. We skip the go install on master also due to some issues with 1.18 so that's fine, and it seems like the same is happening here. I guess, can we pin the version to 1.17.8 like on master as well? I think this is fine then

@ameyag ameyag mentioned this pull request May 2, 2022
Merged
@dcantah
Copy link
Contributor

dcantah commented May 2, 2022

@ameyag Your linter change was checked in so a rebase/get rid of the fix here and I think we're good to go!

@ameyag @kestratt
Adding ExternalPortReserved flag to NatPolicy for HNS API V1. THis is…
5aba212 
… a flag exposed for docker to avoid port reservation conflict with external port (microsoft#1370)

HNS API V2 will use NatFlags to check and see if ExternalPortReserved is set

(cherry picked from commit b85f3fd)
Signed-off-by: Ameya Gawde <agawde@mirantis.com>

Co-authored-by: Kendall Stratton <kestratt@microsoft.com>
(cherry picked from commit dbb347e)
Signed-off-by: Ameya Gawde <agawde@mirantis.com>
@ameyag ameyag force-pushed the rel-0.9-external-port branch from a15940f to 5aba212 Compare May 2, 2022 21:41
@ameyag
Copy link
Contributor Author

ameyag commented May 2, 2022

@dcantah - Thanks. Cleaned up and rebased.

@dcantah dcantah merged commit 83e1852 into microsoft:release/0.9 May 2, 2022
@dcantah
Copy link
Contributor

dcantah commented May 3, 2022

@ameyag We'll get out a new 0.9 release once this is checked in and backported as well #1362. Thanks for being patient 😅

@ameyag ameyag mentioned this pull request May 23, 2022
Merged
@ameyag ameyag deleted the rel-0.9-external-port branch November 7, 2022 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anmaxvl anmaxvl anmaxvl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ameyag @dcantah @anmaxvl