fix(operator): ipv6 support
Hi,
First thanks, I was tired of redis-sentinel tilt issue. 😸
I saw a recent commit https://github.com/dragonflydb/dragonfly-operator/commit/8cc255a26da90e4781c8610d003251500e026630 about ipv6 fix but turn out it was not enought. The root issue was that the label with ipv6 ip cannot be write.
This goal of this PR is to fix IPV6 support, I did it on an ipv6 only cluster. I battle all the evening, but it's now working.
This is a breaking change for now, as I have to switch from label for the masterIP to annotations as we can't store []:, so an ipv6 in label. Ipv6 or not, store the ip as annotation looks the good way to do.
I'm not sure about the "migration" process:
- I can then add a compatibility mode keeping and testing the masterIP label, but for the next release we will need to remove it.
- Or we just keep the code simple and bump a major with breaking change alert.
Here a sample I use:
apiVersion: dragonflydb.io/v1alpha1
kind: Dragonfly
metadata:
labels:
app.kubernetes.io/created-by: dragonfly-operator
app.kubernetes.io/instance: dragonfly-test
app.kubernetes.io/managed-by: kustomize
app.kubernetes.io/name: dragonfly
app.kubernetes.io/part-of: dragonfly-operator
name: dragonfly-test
namespace: default
spec:
args:
- '--bind=::'
- '--admin_bind=::'
image: docker.dragonflydb.io/dragonflydb/dragonfly:v1.25.4
replicas: 2
resources:
limits:
cpu: 600m
memory: 384Mi
requests:
cpu: 500m
memory: 256Mi
I would be happy to see this merge. Is it ok for you ?
Thanks for all the work!
The masterIP label is used to match if the dragonfly replica state matches with what was configured by the Operator. There should not be any issue in moving this to an annotation (and it makes sense).
The compatibility story is important for sure, My preference would be to do them both for now i.e set as label (if ipv4 only) and annotation (for both ipv4 and ipv6) for a version and remove the label part permanently in future versions. Wdyt?
Also, You should be setting/and using the masterIP here at
https://github.com/dragonflydb/dragonfly-operator/blob/2afc70fe7c24e9285bc57521e46dfdded4cd22a6/internal/controller/dragonfly_instance.go#L248
This is OK for me, I will do this :-)
I'm ready for another review @Pothulapati 😃
I add another commit to bind by default on dualstack, this should not hurt.
@Pothulapati should I be worried about the CI state ? Can you please re-run it for me or have idea about what happen ? Or all is good ? 😄
For info this PR is running or my ipv6 only production since few days without issues 👍🏻
Hi @Pothulapati can you help me with this PR please ? 🙏🏻
Hi, anybody can help me passing this PR ? Its running without issue in my prod since weeks.
@Pothulapati @Abhra303 Up
I will check
Up
@Abhra303 @Pothulapati The operator does not work in IPv6 clusters, maybe it's time to look at this PR?
Or make it clear that the project has been abandoned
Hi here,
@Abhra303 @Pothulapati can I somehow help to get this PR merged ? Do you need something (but time) ?
Hi, To let you know I just finally rebase, and I use it, with dragonfly 1.32 on ipv6. All good. Can you help me getting merged this one ? Thank you
Hi, sorry for not responding to the PR. Taking a look now.
Hi, what's the status on this PR? It's over 9 months old now and the operator still doesn't properly support IPv6.
Hi, I just rebase my PR, anybody can help get this merged ?