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.

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

filter terraform output routerIP for v4 addresses only #936

Closed
wants to merge 2 commits into from

Conversation

lotharbach
Copy link
Contributor

How to categorize this PR?

/area networking
/kind bug
/platform openstack

What this PR does / why we need it:
As described in the issue, openstack routers expose a list of external IPs, and those could be v6 even if the rest of the networking stack is v4 only. This will filter terraform output routerIP for v4 addresses only and return only one of them just in case there could still be multiple, since the rest of the code currently expects a single v4 IP

Which issue(s) this PR fixes:
Fixes #897

Special notes for your reviewer:
I purposefully avoided changing the API and properly handling and forwarding lists of IPs for now. I expect there is a proper v6/dualstack PR coming anyway that will need to handle this cleanly. This minimal change unblocks us for further gardener upgrades, so I wanted to share it. I can see why you would not want to merge it in favor of a proper solution. I'm happy to give this PR a little bit more love with feedback, but I don't want to waste anybodies time if this is not the right direction long-term, which I could totally understand.

Release note:


and return only one of them just in case there could be multiples,
since the rest of the code currently expects a single v4 IP
@lotharbach lotharbach requested review from a team as code owners December 17, 2024 12:15
@gardener-robot gardener-robot added needs/review Needs review area/networking Networking related kind/bug Bug platform/openstack OpenStack platform/infrastructure labels Dec 17, 2024
@gardener-robot
Copy link

@lotharbach Thank you for your contribution.

@gardener-robot gardener-robot added the size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) label Dec 17, 2024
@gardener-robot-ci-1
Copy link
Contributor

Thank you @lotharbach for your contribution. Before I can start building your PR, a member of the organization must set the required label(s) {'reviewed/ok-to-test'}. Once started, you can check the build status in the PR checks section below.

@hebelsan
Copy link
Contributor

Could you please add a comment in the template?

@kon-angelo
Copy link
Contributor

Hi @lotharbach. I created #958 to handle the multiple-ips/ipv6 case in a more similar way that we do in other extensions. Please have a look.

In the meantime I will put this PR on hold. I am sorry for the trouble/delay. I would also appreciate if you could shortly test it, since I our testing infrastructure does not have ipv6 capabilities and it would be nice to know if it actually solves your issue.

/hold

@kon-angelo kon-angelo added the status/on-hold Issue on hold (e.g. because work was suspended) label Jan 16, 2025
@lotharbach lotharbach closed this Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/networking Networking related kind/bug Bug needs/review Needs review platform/openstack OpenStack platform/infrastructure size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) status/on-hold Issue on hold (e.g. because work was suspended)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

status.networking.egressCIDRs[0] is invalid: must be valid canonical CIDR
5 participants