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

detect ipv6 stack and apply correct listen address #180

Merged
merged 1 commit into from
Jan 23, 2024

Conversation

zzzeek
Copy link
Contributor

@zzzeek zzzeek commented Jan 16, 2024

on a dual-stack host (or an ipv6 only host), detect the ipv6 and ipv4 addresses individually from /etc/hosts and apply '[::]' as the gmcast_listen address if ipv6 is present.

Fixes: https://issues.redhat.com/browse/OSPRH-3330

@zzzeek zzzeek requested a review from dciabrin January 16, 2024 17:43
@openshift-ci openshift-ci bot requested a review from dprince January 16, 2024 17:43
@hjensas
Copy link

hjensas commented Jan 17, 2024

/test mariadb-operator-build-deploy-kuttl

error: build error: Failed to push image: trying to reuse blob sha256:07a64a71e01156f8f99039bc246149925c6d1480d3957de78510bbec6ec68f7a at destination: pinging container registry quay.rdoproject.org: Get "https://quay.rdoproject.org/v2/": net/http: TLS handshake timeout

PODIP="${PODIPV4}"
IPSTACK="IPV4"
else
PODIP="[::]"
Copy link

@hjensas hjensas Jan 17, 2024

Choose a reason for hiding this comment

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

Do we need all this logic?
If we are defaulting to listen on all interfaces in case IPv6 or Dual-Stack IPv6-and-Ipv4 - Can we now simply configure it with "[::]" for all cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it has to work on an ipv4-only stack as well, IIUC galera needs the specific IP number to bind onto in that case cc @dciabrin

Copy link
Contributor

Choose a reason for hiding this comment

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

We started using [::] for IPv6 as per openstack-archive/puppet-tripleo@84ba7c3.
Galera is probably able to cope with other direct IPv6 addresses now, but that is to be verified. Until then, I think we are good enough with that as it's not a regression from what we used to have.

@hjensas
Copy link

hjensas commented Jan 17, 2024

/test mariadb-operator-build-deploy-kuttl

error: build error: Failed to push image: trying to reuse blob sha256:07a64a71e01156f8f99039bc246149925c6d1480d3957de78510bbec6ec68f7a at destination: pinging container registry quay.rdoproject.org: Get "https://quay.rdoproject.org/v2/": net/http: TLS handshake timeout

@zzzeek
Copy link
Contributor Author

zzzeek commented Jan 22, 2024

/retest

1 similar comment
@zzzeek
Copy link
Contributor Author

zzzeek commented Jan 22, 2024

/retest

on a dual-stack host (or an ipv6 only host), detect the
ipv6 and ipv4 addresses individually from /etc/hosts and
apply '[::]' as the gmcast_listen address if ipv6 is present.

Fixes: https://issues.redhat.com/browse/OSPRH-3330
@zzzeek
Copy link
Contributor Author

zzzeek commented Jan 23, 2024

/retest

@zzzeek
Copy link
Contributor Author

zzzeek commented Jan 23, 2024

now it passes after i put a node hold in. faceslap

@zzzeek
Copy link
Contributor Author

zzzeek commented Jan 23, 2024

/retest

1 similar comment
@zzzeek
Copy link
Contributor Author

zzzeek commented Jan 23, 2024

/retest

Copy link

@hjensas hjensas left a comment

Choose a reason for hiding this comment

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

I'm ok with this.
But I do find it weird that we choose to specify a "listen" address to bind to when using IPv4 - but then if it is IPv6 or IPv4+IPv6 (dual stack) we accept a config that make it listen to any interface.

It seems to me we can use either '0.0.0.0' or '[::]' for all cases instead.

@zzzeek
Copy link
Contributor Author

zzzeek commented Jan 23, 2024

I'm ok with this. But I do find it weird that we choose to specify a "listen" address to bind to when using IPv4 - but then if it is IPv6 or IPv4+IPv6 (dual stack) we accept a config that make it listen to any interface.

It seems to me we can use either '0.0.0.0' or '[::]' for all cases instead.

@dciabrin can you comment on this?

@dciabrin
Copy link
Contributor

I'm ok with this. But I do find it weird that we choose to specify a "listen" address to bind to when using IPv4 - but then if it is IPv6 or IPv4+IPv6 (dual stack) we accept a config that make it listen to any interface.
It seems to me we can use either '0.0.0.0' or '[::]' for all cases instead.

@dciabrin can you comment on this?

I'm still unclear about the general consensus as to listen in container: should we explicitly listen on the interface we expect to receive traffic from, or is listening on all interface and assume k8s plumbing is sufficient to not expose unnecessary interfaces. As I said earlier this IPv6 patch is what we used back in the days because we had issues specifying IPv6 addresses with galera.

So I think this is good enough to fix the original reported issue, and we should probably revisit it to either specify a full IPv6 address is recent galera allows it, or listen to all interfaces if that is a general pattern in pods networking.

Copy link
Contributor

openshift-ci bot commented Jan 23, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dciabrin, hjensas, zzzeek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 1a86db2 into openstack-k8s-operators:main Jan 23, 2024
6 checks passed
@zzzeek zzzeek deleted the fix_ipv6 branch January 25, 2024 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants