dragonfly-operator icon indicating copy to clipboard operation
dragonfly-operator copied to clipboard

fix(operator): ipv6 support

Open cyrinux opened this issue 1 year ago • 15 comments

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 ?

cyrinux avatar Dec 05 '24 18:12 cyrinux

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 :-)

cyrinux avatar Dec 06 '24 07:12 cyrinux

I'm ready for another review @Pothulapati 😃

cyrinux avatar Dec 06 '24 08:12 cyrinux

I add another commit to bind by default on dualstack, this should not hurt.

cyrinux avatar Dec 06 '24 10:12 cyrinux

@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 👍🏻

cyrinux avatar Dec 11 '24 08:12 cyrinux

Hi @Pothulapati can you help me with this PR please ? 🙏🏻

cyrinux avatar Feb 11 '25 15:02 cyrinux

Hi, anybody can help me passing this PR ? Its running without issue in my prod since weeks.

cyrinux avatar Mar 25 '25 14:03 cyrinux

@Pothulapati @Abhra303 Up

Ais8Ooz8 avatar Apr 04 '25 15:04 Ais8Ooz8

I will check

Abhra303 avatar Apr 04 '25 17:04 Abhra303

Up

Ais8Ooz8 avatar Apr 29 '25 11:04 Ais8Ooz8

@Abhra303 @Pothulapati The operator does not work in IPv6 clusters, maybe it's time to look at this PR?

Ais8Ooz8 avatar May 23 '25 07:05 Ais8Ooz8

Or make it clear that the project has been abandoned

Ais8Ooz8 avatar May 23 '25 07:05 Ais8Ooz8

Hi here,

@Abhra303 @Pothulapati can I somehow help to get this PR merged ? Do you need something (but time) ?

cyrinux avatar Jun 17 '25 06:06 cyrinux

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

cyrinux avatar Aug 08 '25 19:08 cyrinux

Hi, sorry for not responding to the PR. Taking a look now.

Abhra303 avatar Aug 13 '25 05:08 Abhra303

Hi, what's the status on this PR? It's over 9 months old now and the operator still doesn't properly support IPv6.

dhess avatar Sep 25 '25 01:09 dhess

Hi, I just rebase my PR, anybody can help get this merged ?

cyrinux avatar Nov 25 '25 10:11 cyrinux