From 2afc70fe7c24e9285bc57521e46dfdded4cd22a6 Mon Sep 17 00:00:00 2001 From: Cyril Levis Date: Thu, 5 Dec 2024 18:36:19 +0100 Subject: [PATCH 1/5] fix(operator): ipv6 support --- internal/controller/dragonfly_instance.go | 47 +++++++++++++++++------ internal/controller/util.go | 37 ++++++++++++------ internal/resources/const.go | 2 +- 3 files changed, 62 insertions(+), 24 deletions(-) diff --git a/internal/controller/dragonfly_instance.go b/internal/controller/dragonfly_instance.go index 0e2b143c..3f64e97c 100644 --- a/internal/controller/dragonfly_instance.go +++ b/internal/controller/dragonfly_instance.go @@ -168,15 +168,25 @@ func (dfi *DragonflyInstance) masterExists(ctx context.Context) (bool, error) { } func (dfi *DragonflyInstance) getMasterIp(ctx context.Context) (string, error) { - dfi.log.Info("retrieving ip of the master") + dfi.log.Info("retrieving IP of the master") pods, err := dfi.getPods(ctx) if err != nil { return "", err } for _, pod := range pods.Items { - if pod.Status.Phase == corev1.PodRunning && pod.Status.ContainerStatuses[0].Ready && pod.Labels[resources.Role] == resources.Master { - return pod.Status.PodIP, nil + if pod.Status.Phase == corev1.PodRunning && + pod.Status.ContainerStatuses[0].Ready && + pod.Labels[resources.Role] == resources.Master { + + masterIp, hasMasterIp := pod.Annotations[resources.MasterIp] + if hasMasterIp { + dfi.log.Info("Retrieved Master IP from annotation", "masterIp", masterIp) + return masterIp, nil + } + + masterIp = pod.Status.PodIP + return masterIp, nil } } @@ -207,7 +217,7 @@ func (dfi *DragonflyInstance) configureReplica(ctx context.Context, pod *corev1. // connected to the right master func (dfi *DragonflyInstance) checkReplicaRole(ctx context.Context, pod *corev1.Pod, masterIp string) (bool, error) { redisClient := redis.NewClient(&redis.Options{ - Addr: fmt.Sprintf("%s:%d", pod.Status.PodIP, resources.DragonflyAdminPort), + Addr: net.JoinHostPort(pod.Status.PodIP, strconv.Itoa(resources.DragonflyAdminPort)), }) defer redisClient.Close() @@ -262,7 +272,8 @@ func (dfi *DragonflyInstance) checkAndConfigureReplication(ctx context.Context) // check for one master and all replicas podRoles := make(map[string][]string) for _, pod := range pods.Items { - podRoles[pod.Labels[resources.Role]] = append(podRoles[pod.Labels[resources.Role]], pod.Name) + role := pod.Labels[resources.Role] + podRoles[role] = append(podRoles[role], pod.Name) } if len(podRoles[resources.Master]) != 1 { @@ -299,7 +310,7 @@ func (dfi *DragonflyInstance) checkAndConfigureReplication(ctx context.Context) return err } - // configuring to the right master + // Configure to the right master if not correct if !ok { dfi.log.Info("configuring pod as replica to the right master", "pod", pod.Name) if err := dfi.configureReplica(ctx, &pod); err != nil { @@ -309,7 +320,7 @@ func (dfi *DragonflyInstance) checkAndConfigureReplication(ctx context.Context) } } - dfi.log.Info("all pods are configured correctly", "dfi", dfi.df.Name) + dfi.log.Info("All pods are configured correctly", "dfi", dfi.df.Name) return nil } @@ -335,8 +346,11 @@ func (dfi *DragonflyInstance) replicaOf(ctx context.Context, pod *corev1.Pod, ma }) defer redisClient.Close() + // Sanitize masterIp in case ipv6 + masterIp = sanitizeIp(masterIp) + dfi.log.Info("Trying to invoke SLAVE OF command", "pod", pod.Name, "master", masterIp, "addr", redisClient.Options().Addr) - resp, err := redisClient.SlaveOf(ctx, masterIp, fmt.Sprint(resources.DragonflyAdminPort)).Result() + resp, err := redisClient.SlaveOf(ctx, masterIp, strconv.Itoa(resources.DragonflyAdminPort)).Result() if err != nil { return fmt.Errorf("error running SLAVE OF command: %s", err) } @@ -345,11 +359,14 @@ func (dfi *DragonflyInstance) replicaOf(ctx context.Context, pod *corev1.Pod, ma return fmt.Errorf("response of `SLAVE OF` on replica is not OK: %s", resp) } - dfi.log.Info("Marking pod role as replica", "pod", pod.Name) + dfi.log.Info("Marking pod role as replica", "pod", pod.Name, "masterIp", masterIp) pod.Labels[resources.Role] = resources.Replica - pod.Labels[resources.MasterIp] = masterIp + if pod.Annotations == nil { + pod.Annotations = make(map[string]string) + } + pod.Annotations[resources.MasterIp] = masterIp if err := dfi.client.Update(ctx, pod); err != nil { - return fmt.Errorf("could not update replica label") + return fmt.Errorf("could not update replica annotation: %w", err) } return nil @@ -373,8 +390,14 @@ func (dfi *DragonflyInstance) replicaOfNoOne(ctx context.Context, pod *corev1.Po return fmt.Errorf("response of `SLAVE OF NO ONE` on master is not OK: %s", resp) } - dfi.log.Info("Marking pod role as master", "pod", pod.Name) + masterIp := pod.Status.PodIP + + dfi.log.Info("Marking pod role as master", "pod", pod.Name, "masterIp", masterIp) pod.Labels[resources.Role] = resources.Master + if pod.Annotations == nil { + pod.Annotations = make(map[string]string) + } + pod.Annotations[resources.MasterIp] = masterIp if err := dfi.client.Update(ctx, pod); err != nil { return err } diff --git a/internal/controller/util.go b/internal/controller/util.go index 562be1cb..69c33746 100644 --- a/internal/controller/util.go +++ b/internal/controller/util.go @@ -20,6 +20,8 @@ import ( "context" "errors" "fmt" + "net" + "strconv" "strings" "github.com/dragonflydb/dragonfly-operator/internal/resources" @@ -89,7 +91,7 @@ func getLatestReplica(ctx context.Context, c client.Client, statefulSet *appsv1. // replTakeover runs the replTakeOver on the given replica pod func replTakeover(ctx context.Context, c client.Client, newMaster *corev1.Pod) error { redisClient := redis.NewClient(&redis.Options{ - Addr: fmt.Sprintf("%s:%d", newMaster.Status.PodIP, resources.DragonflyAdminPort), + Addr: net.JoinHostPort(newMaster.Status.PodIP, strconv.Itoa(resources.DragonflyPort)), }) defer redisClient.Close() @@ -111,13 +113,13 @@ func replTakeover(ctx context.Context, c client.Client, newMaster *corev1.Pod) e } func isStableState(ctx context.Context, pod *corev1.Pod) (bool, error) { - // wait until pod IP is ready + // Ensure PodIP and Pod Phase are ready if pod.Status.PodIP == "" || pod.Status.Phase != corev1.PodRunning { return false, nil } redisClient := redis.NewClient(&redis.Options{ - Addr: fmt.Sprintf("%s:%d", pod.Status.PodIP, resources.DragonflyAdminPort), + Addr: net.JoinHostPort(pod.Status.PodIP, strconv.Itoa(resources.DragonflyAdminPort)), }) defer redisClient.Close() @@ -135,14 +137,7 @@ func isStableState(ctx context.Context, pod *corev1.Pod) (bool, error) { return false, errors.New("empty info") } - data := map[string]string{} - for _, line := range strings.Split(info, "\n") { - if line == "" || strings.HasPrefix(line, "#") { - continue - } - kv := strings.Split(line, ":") - data[kv[0]] = strings.TrimSuffix(kv[1], "\r") - } + data := parseRedisInfo(info) if data["master_sync_in_progress"] == "1" { return false, nil @@ -158,3 +153,23 @@ func isStableState(ctx context.Context, pod *corev1.Pod) (bool, error) { return true, nil } + +// Helper function to parse Redis INFO data +func parseRedisInfo(info string) map[string]string { + data := map[string]string{} + for _, line := range strings.Split(info, "\n") { + if line == "" || strings.HasPrefix(line, "#") { + continue + } + kv := strings.Split(line, ":") + if len(kv) == 2 { + data[kv[0]] = strings.TrimSuffix(kv[1], "\r") + } + } + return data +} + +// sanitizeIp Ipv6 +func sanitizeIp(masterIp string) string { + return strings.Trim(masterIp, "[]") +} diff --git a/internal/resources/const.go b/internal/resources/const.go index ec6a3b5b..e4c8ca6c 100644 --- a/internal/resources/const.go +++ b/internal/resources/const.go @@ -56,7 +56,7 @@ const ( // KubernetesPartOfLabel is the name of a higher level application this one is part of KubernetesPartOfLabelKey = "app.kubernetes.io/part-of" - MasterIp string = "master-ip" + MasterIp string = "operator.dragonflydb.io/masterIP" Role string = "role" From 5a4b4f0c56d3fcd8d189d1329acc91784013d3ae Mon Sep 17 00:00:00 2001 From: Cyril Levis Date: Fri, 6 Dec 2024 09:08:02 +0100 Subject: [PATCH 2/5] chore: useless change --- internal/controller/dragonfly_instance.go | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/internal/controller/dragonfly_instance.go b/internal/controller/dragonfly_instance.go index 3f64e97c..b56986fa 100644 --- a/internal/controller/dragonfly_instance.go +++ b/internal/controller/dragonfly_instance.go @@ -168,25 +168,15 @@ func (dfi *DragonflyInstance) masterExists(ctx context.Context) (bool, error) { } func (dfi *DragonflyInstance) getMasterIp(ctx context.Context) (string, error) { - dfi.log.Info("retrieving IP of the master") + dfi.log.Info("retrieving ip of the master") pods, err := dfi.getPods(ctx) if err != nil { return "", err } for _, pod := range pods.Items { - if pod.Status.Phase == corev1.PodRunning && - pod.Status.ContainerStatuses[0].Ready && - pod.Labels[resources.Role] == resources.Master { - - masterIp, hasMasterIp := pod.Annotations[resources.MasterIp] - if hasMasterIp { - dfi.log.Info("Retrieved Master IP from annotation", "masterIp", masterIp) - return masterIp, nil - } - - masterIp = pod.Status.PodIP - return masterIp, nil + if pod.Status.Phase == corev1.PodRunning && pod.Status.ContainerStatuses[0].Ready && pod.Labels[resources.Role] == resources.Master { + return pod.Status.PodIP, nil } } @@ -272,8 +262,7 @@ func (dfi *DragonflyInstance) checkAndConfigureReplication(ctx context.Context) // check for one master and all replicas podRoles := make(map[string][]string) for _, pod := range pods.Items { - role := pod.Labels[resources.Role] - podRoles[role] = append(podRoles[role], pod.Name) + podRoles[pod.Labels[resources.Role]] = append(podRoles[pod.Labels[resources.Role]], pod.Name) } if len(podRoles[resources.Master]) != 1 { From cf485c98e7e61b6cd164751dc5c864e1cb70a6e2 Mon Sep 17 00:00:00 2001 From: Cyril Levis Date: Fri, 6 Dec 2024 09:43:14 +0100 Subject: [PATCH 3/5] chore: retrocompatibility --- internal/controller/dragonfly_instance.go | 20 ++++++++++++++++++-- internal/resources/const.go | 3 ++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/internal/controller/dragonfly_instance.go b/internal/controller/dragonfly_instance.go index b56986fa..2142425f 100644 --- a/internal/controller/dragonfly_instance.go +++ b/internal/controller/dragonfly_instance.go @@ -235,8 +235,12 @@ func (dfi *DragonflyInstance) checkReplicaRole(ctx context.Context, pod *corev1. } } - if masterIp != redisMasterIp && masterIp != pod.Labels[resources.MasterIp] { - return false, nil + // for compatibily, to be remove in futur version + // check if the masterIp matches either the label (for compatibility) or the annotation + if masterIp != redisMasterIp { + if masterIp != pod.Labels[resources.MasterIpLabel] && masterIp != pod.Annotations[resources.MasterIp] { + return false, nil + } } return true, nil @@ -354,6 +358,12 @@ func (dfi *DragonflyInstance) replicaOf(ctx context.Context, pod *corev1.Pod, ma pod.Annotations = make(map[string]string) } pod.Annotations[resources.MasterIp] = masterIp + + // for compatibily, to be remove in futur version + if !strings.Contains(masterIp, ":") { + pod.Labels[resources.MasterIpLabel] = masterIp + } + if err := dfi.client.Update(ctx, pod); err != nil { return fmt.Errorf("could not update replica annotation: %w", err) } @@ -383,10 +393,16 @@ func (dfi *DragonflyInstance) replicaOfNoOne(ctx context.Context, pod *corev1.Po dfi.log.Info("Marking pod role as master", "pod", pod.Name, "masterIp", masterIp) pod.Labels[resources.Role] = resources.Master + // for compatibily, to be remove in futur version + if !strings.Contains(masterIp, ":") { + pod.Labels[resources.MasterIpLabel] = masterIp + } + if pod.Annotations == nil { pod.Annotations = make(map[string]string) } pod.Annotations[resources.MasterIp] = masterIp + if err := dfi.client.Update(ctx, pod); err != nil { return err } diff --git a/internal/resources/const.go b/internal/resources/const.go index e4c8ca6c..79b5c113 100644 --- a/internal/resources/const.go +++ b/internal/resources/const.go @@ -56,7 +56,8 @@ const ( // KubernetesPartOfLabel is the name of a higher level application this one is part of KubernetesPartOfLabelKey = "app.kubernetes.io/part-of" - MasterIp string = "operator.dragonflydb.io/masterIP" + MasterIpLabel string = "master-ip" + MasterIp string = "operator.dragonflydb.io/masterIP" Role string = "role" From 040d2efe2e1b57dd7509d0264a711ec30634c5c9 Mon Sep 17 00:00:00 2001 From: Cyril Levis Date: Fri, 6 Dec 2024 10:01:13 +0100 Subject: [PATCH 4/5] chore: wording --- internal/controller/dragonfly_instance.go | 6 +++--- internal/controller/util.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/controller/dragonfly_instance.go b/internal/controller/dragonfly_instance.go index 2142425f..671e9f2a 100644 --- a/internal/controller/dragonfly_instance.go +++ b/internal/controller/dragonfly_instance.go @@ -303,7 +303,7 @@ func (dfi *DragonflyInstance) checkAndConfigureReplication(ctx context.Context) return err } - // Configure to the right master if not correct + // configuring to the right master if !ok { dfi.log.Info("configuring pod as replica to the right master", "pod", pod.Name) if err := dfi.configureReplica(ctx, &pod); err != nil { @@ -313,7 +313,7 @@ func (dfi *DragonflyInstance) checkAndConfigureReplication(ctx context.Context) } } - dfi.log.Info("All pods are configured correctly", "dfi", dfi.df.Name) + dfi.log.Info("all pods are configured correctly", "dfi", dfi.df.Name) return nil } @@ -365,7 +365,7 @@ func (dfi *DragonflyInstance) replicaOf(ctx context.Context, pod *corev1.Pod, ma } if err := dfi.client.Update(ctx, pod); err != nil { - return fmt.Errorf("could not update replica annotation: %w", err) + return fmt.Errorf("could not update replica metadatas: %w", err) } return nil diff --git a/internal/controller/util.go b/internal/controller/util.go index 69c33746..0bfb977c 100644 --- a/internal/controller/util.go +++ b/internal/controller/util.go @@ -113,7 +113,7 @@ func replTakeover(ctx context.Context, c client.Client, newMaster *corev1.Pod) e } func isStableState(ctx context.Context, pod *corev1.Pod) (bool, error) { - // Ensure PodIP and Pod Phase are ready + // wait until pod IP is ready if pod.Status.PodIP == "" || pod.Status.Phase != corev1.PodRunning { return false, nil } From cf69781a2fd71332bc6972a51aaa15b6f8d828c7 Mon Sep 17 00:00:00 2001 From: Cyril Levis Date: Fri, 6 Dec 2024 10:58:04 +0100 Subject: [PATCH 5/5] fix: bind on dualstack by default both operator and dragonfly, should not hurt. --- charts/dragonfly-operator/templates/deployment.yaml | 6 +++--- config/default/manager_auth_proxy_patch.yaml | 6 +++--- internal/resources/const.go | 2 ++ manifests/dragonfly-operator.yaml | 6 +++--- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/charts/dragonfly-operator/templates/deployment.yaml b/charts/dragonfly-operator/templates/deployment.yaml index 0c24699f..ec600c23 100644 --- a/charts/dragonfly-operator/templates/deployment.yaml +++ b/charts/dragonfly-operator/templates/deployment.yaml @@ -36,8 +36,8 @@ spec: {{- toYaml .Values.podSecurityContext | nindent 8 }} containers: - args: - - --secure-listen-address=0.0.0.0:8443 - - --upstream=http://127.0.0.1:8080/ + - --secure-listen-address=[::]:8443 + - --upstream=http://[::1]:8080/ - --logtostderr=true - --v=0 image: "{{ .Values.rbacProxy.image.repository }}:{{ .Values.rbacProxy.image.tag }}" @@ -55,7 +55,7 @@ spec: - name: manager args: - --health-probe-bind-address=:8081 - - --metrics-bind-address=127.0.0.1:8080 + - --metrics-bind-address=[::1]:8080 - --leader-elect command: - /manager diff --git a/config/default/manager_auth_proxy_patch.yaml b/config/default/manager_auth_proxy_patch.yaml index 801e9bb2..b2c1c4d3 100644 --- a/config/default/manager_auth_proxy_patch.yaml +++ b/config/default/manager_auth_proxy_patch.yaml @@ -33,8 +33,8 @@ spec: - "ALL" image: gcr.io/kubebuilder/kube-rbac-proxy:v0.16.0 args: - - "--secure-listen-address=0.0.0.0:8443" - - "--upstream=http://127.0.0.1:8080/" + - "--secure-listen-address=[::]:8443" + - "--upstream=http://[::1]:8080/" - "--logtostderr=true" - "--v=0" ports: @@ -51,5 +51,5 @@ spec: - name: manager args: - "--health-probe-bind-address=:8081" - - "--metrics-bind-address=127.0.0.1:8080" + - "--metrics-bind-address=[::1]:8080" - "--leader-elect" diff --git a/internal/resources/const.go b/internal/resources/const.go index 79b5c113..8f9d10fd 100644 --- a/internal/resources/const.go +++ b/internal/resources/const.go @@ -69,6 +69,8 @@ const ( var DefaultDragonflyArgs = []string{ "--alsologtostderr", "--primary_port_http_enabled=false", + "--bind=::", + "--admin_bind=::", fmt.Sprintf("--admin_port=%d", DragonflyAdminPort), "--admin_nopass", } diff --git a/manifests/dragonfly-operator.yaml b/manifests/dragonfly-operator.yaml index 6cf779aa..b11c2198 100644 --- a/manifests/dragonfly-operator.yaml +++ b/manifests/dragonfly-operator.yaml @@ -2072,8 +2072,8 @@ spec: - linux containers: - args: - - --secure-listen-address=0.0.0.0:8443 - - --upstream=http://127.0.0.1:8080/ + - --secure-listen-address=[::]:8443 + - --upstream=http://[::1]:8080/ - --logtostderr=true - --v=0 image: gcr.io/kubebuilder/kube-rbac-proxy:v0.16.0 @@ -2096,7 +2096,7 @@ spec: - ALL - args: - --health-probe-bind-address=:8081 - - --metrics-bind-address=127.0.0.1:8080 + - --metrics-bind-address=[::1]:8080 - --leader-elect command: - /manager