From 9b550f02eb81ae40db3ea65558eb3c09ad69a58e Mon Sep 17 00:00:00 2001 From: jiuker Date: Mon, 5 Aug 2024 11:32:17 +0800 Subject: [PATCH 1/9] fix: avoid system cfg changed pod can't restart fix: avoid system cfg changed pod can't restart --- pkg/controller/main-controller.go | 19 ++++++++++++++++++ pkg/controller/pods.go | 27 +++++++++++++++++++++++++ pkg/controller/pools.go | 33 +++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+) diff --git a/pkg/controller/main-controller.go b/pkg/controller/main-controller.go index 68cdf988392..37c0fea2d6d 100644 --- a/pkg/controller/main-controller.go +++ b/pkg/controller/main-controller.go @@ -19,6 +19,7 @@ import ( "encoding/json" "errors" "fmt" + "maps" "net/http" "os" "os/signal" @@ -1316,6 +1317,24 @@ func (c *Controller) syncHandler(key string) (Result, error) { // return nil so we don't re-queue this work item, this error won't get fixed by reprocessing return WrapResult(Result{}, nil) } + + // check if the system config has changed + // if changed, minio request the systemCfg must be the same to restart. + expectSystemCfg, err := c.getSystemCfgFromStatefulSet(ctx, expectedStatefulSet) + if err != nil { + return WrapResult(Result{}, err) + } + existSystemCfg, err := c.getSystemCfgFromStatefulSet(ctx, existingStatefulSet) + if err != nil { + return WrapResult(Result{}, err) + } + if !maps.Equal(expectSystemCfg, existSystemCfg) { + // found all existing statefulSet pods and delete them + err = c.DeletePodsByStatefulSet(ctx, existingStatefulSet) + if err != nil { + return WrapResult(Result{}, err) + } + } } // Handle PVC expansion diff --git a/pkg/controller/pods.go b/pkg/controller/pods.go index 9eeeef4b578..3f1b125a3bd 100644 --- a/pkg/controller/pods.go +++ b/pkg/controller/pods.go @@ -17,12 +17,16 @@ package controller import ( + "context" "fmt" "time" miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2" "github.com/minio/operator/pkg/utils" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" ) // handlePodChange will handle changes in pods and queue it for processing, pods are already filtered by PodInformer @@ -43,3 +47,26 @@ func (c *Controller) handlePodChange(obj interface{}) { key := fmt.Sprintf("%s/%s", object.GetNamespace(), instanceName) c.healthCheckQueue.AddAfter(key, 1*time.Second) } + +// DeletePodsByStatefulSet deletes all pods associated with a statefulset +func (c *Controller) DeletePodsByStatefulSet(ctx context.Context, sts *appsv1.StatefulSet) (err error) { + listOpt := &client.ListOptions{ + Namespace: sts.Namespace, + } + client.MatchingLabels(sts.Spec.Template.Labels).ApplyToList(listOpt) + podList := &corev1.PodList{} + err = c.k8sClient.List(ctx, podList, listOpt) + if err != nil { + return err + } + for _, item := range podList.Items { + err = c.k8sClient.Delete(ctx, &item) + if err != nil { + // Ignore Not Found + if client.IgnoreNotFound(err) != nil { + return err + } + } + } + return +} diff --git a/pkg/controller/pools.go b/pkg/controller/pools.go index 1b366f07dc0..43ed1b1acaf 100644 --- a/pkg/controller/pools.go +++ b/pkg/controller/pools.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "strings" corev1 "k8s.io/api/core/v1" @@ -92,6 +93,38 @@ func poolSSMatchesSpec(expectedStatefulSet, existingStatefulSet *appsv1.Stateful return true, nil } +// getSystemCfgFromStatefulSet gets the MinIO environment variables from a statefulset +// set getServerSystemCfg at minio +func (c *Controller) getSystemCfgFromStatefulSet(ctx context.Context, sts *appsv1.StatefulSet) (systemCfg map[string]string, err error) { + for _, container := range sts.Spec.Template.Spec.Containers { + if container.Name == miniov2.MinIOServerName { + for _, e := range container.Env { + if strings.HasPrefix(e.Name, "MINIO_") { + switch { + case e.Value != "": + systemCfg[e.Name] = e.Value + case e.ValueFrom != nil && e.ValueFrom.SecretKeyRef != nil: + secret, err := c.kubeClientSet.CoreV1().Secrets(sts.Namespace).Get(ctx, e.ValueFrom.SecretKeyRef.Name, metav1.GetOptions{}) + if err != nil { + return nil, err + } + systemCfg[e.Name] = string(secret.Data[e.ValueFrom.SecretKeyRef.Key]) + case e.ValueFrom != nil && e.ValueFrom.ConfigMapKeyRef != nil: + configMap, err := c.kubeClientSet.CoreV1().ConfigMaps(sts.Namespace).Get(ctx, e.ValueFrom.ConfigMapKeyRef.Name, metav1.GetOptions{}) + if err != nil { + return nil, err + } + systemCfg[e.Name] = string(configMap.Data[e.ValueFrom.ConfigMapKeyRef.Key]) + default: + return nil, fmt.Errorf("unsupported env var %s", e.Name) + } + } + } + } + } + return +} + // restartInitializedPool restarts a pool that is assumed to have been initialized func (c *Controller) restartInitializedPool(ctx context.Context, tenant *miniov2.Tenant, pool miniov2.Pool, tenantConfiguration map[string][]byte) error { // get a new admin client that points to a pod of an already initialized pool (ie: pool-0) From 73ede36b79be90718c9dae96944dc0bc8dce9802 Mon Sep 17 00:00:00 2001 From: jiuker Date: Mon, 5 Aug 2024 11:34:28 +0800 Subject: [PATCH 2/9] delete pdb delete pdb --- pkg/controller/main-controller.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/controller/main-controller.go b/pkg/controller/main-controller.go index 37c0fea2d6d..96f75720200 100644 --- a/pkg/controller/main-controller.go +++ b/pkg/controller/main-controller.go @@ -1329,6 +1329,10 @@ func (c *Controller) syncHandler(key string) (Result, error) { return WrapResult(Result{}, err) } if !maps.Equal(expectSystemCfg, existSystemCfg) { + err = c.DeletePDB(ctx, tenant) + if err != nil { + return Result{}, err + } // found all existing statefulSet pods and delete them err = c.DeletePodsByStatefulSet(ctx, existingStatefulSet) if err != nil { From c2e31d33f565a7190219530bc96bd41245ae5db8 Mon Sep 17 00:00:00 2001 From: jiuker Date: Mon, 5 Aug 2024 11:35:18 +0800 Subject: [PATCH 3/9] comment comment --- pkg/controller/main-controller.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/controller/main-controller.go b/pkg/controller/main-controller.go index 96f75720200..16c4c4d9941 100644 --- a/pkg/controller/main-controller.go +++ b/pkg/controller/main-controller.go @@ -1329,6 +1329,7 @@ func (c *Controller) syncHandler(key string) (Result, error) { return WrapResult(Result{}, err) } if !maps.Equal(expectSystemCfg, existSystemCfg) { + // delete pdb to let deleted all the statefulSet pods err = c.DeletePDB(ctx, tenant) if err != nil { return Result{}, err From 1fd320b41656f3973afc3de390112bcdcbbbd1da Mon Sep 17 00:00:00 2001 From: jiuker Date: Mon, 5 Aug 2024 11:42:15 +0800 Subject: [PATCH 4/9] lint lint --- pkg/controller/pools.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/pools.go b/pkg/controller/pools.go index 43ed1b1acaf..0d4150b19b1 100644 --- a/pkg/controller/pools.go +++ b/pkg/controller/pools.go @@ -114,7 +114,7 @@ func (c *Controller) getSystemCfgFromStatefulSet(ctx context.Context, sts *appsv if err != nil { return nil, err } - systemCfg[e.Name] = string(configMap.Data[e.ValueFrom.ConfigMapKeyRef.Key]) + systemCfg[e.Name] = configMap.Data[e.ValueFrom.ConfigMapKeyRef.Key] default: return nil, fmt.Errorf("unsupported env var %s", e.Name) } From 152f90a18c62046db7ff0fda1c8e04a2ec25ab79 Mon Sep 17 00:00:00 2001 From: jiuker Date: Tue, 6 Aug 2024 10:31:36 +0800 Subject: [PATCH 5/9] init init --- pkg/controller/pools.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/controller/pools.go b/pkg/controller/pools.go index 0d4150b19b1..ef457cee736 100644 --- a/pkg/controller/pools.go +++ b/pkg/controller/pools.go @@ -96,6 +96,7 @@ func poolSSMatchesSpec(expectedStatefulSet, existingStatefulSet *appsv1.Stateful // getSystemCfgFromStatefulSet gets the MinIO environment variables from a statefulset // set getServerSystemCfg at minio func (c *Controller) getSystemCfgFromStatefulSet(ctx context.Context, sts *appsv1.StatefulSet) (systemCfg map[string]string, err error) { + systemCfg = make(map[string]string) for _, container := range sts.Spec.Template.Spec.Containers { if container.Name == miniov2.MinIOServerName { for _, e := range container.Env { From 2749a27791fc367e78e5fe3cc831105942066892 Mon Sep 17 00:00:00 2001 From: jiuker <2818723467@qq.com> Date: Mon, 12 Aug 2024 11:41:02 +0800 Subject: [PATCH 6/9] Update pkg/controller/pods.go Co-authored-by: Ramon de Klein --- pkg/controller/pods.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/controller/pods.go b/pkg/controller/pods.go index 3f1b125a3bd..dac53373e76 100644 --- a/pkg/controller/pods.go +++ b/pkg/controller/pods.go @@ -61,11 +61,9 @@ func (c *Controller) DeletePodsByStatefulSet(ctx context.Context, sts *appsv1.St } for _, item := range podList.Items { err = c.k8sClient.Delete(ctx, &item) - if err != nil { - // Ignore Not Found - if client.IgnoreNotFound(err) != nil { - return err - } + // Ignore Not Found + if client.IgnoreNotFound(err) != nil { + log.Printf("unable to restart %s/%s (ignored): %s", item.Namespace, item.Name, err) } } return From 7fd774335166b3cfb07344d64c97f5fe5739bff0 Mon Sep 17 00:00:00 2001 From: jiuker Date: Mon, 12 Aug 2024 11:51:55 +0800 Subject: [PATCH 7/9] apply suggestion apply suggestion --- pkg/controller/pods.go | 11 +++++++---- pkg/controller/pools.go | 12 ++++++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/pkg/controller/pods.go b/pkg/controller/pods.go index dac53373e76..e1424e0d747 100644 --- a/pkg/controller/pods.go +++ b/pkg/controller/pods.go @@ -19,6 +19,7 @@ package controller import ( "context" "fmt" + "k8s.io/klog/v2" "time" miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2" @@ -60,10 +61,12 @@ func (c *Controller) DeletePodsByStatefulSet(ctx context.Context, sts *appsv1.St return err } for _, item := range podList.Items { - err = c.k8sClient.Delete(ctx, &item) - // Ignore Not Found - if client.IgnoreNotFound(err) != nil { - log.Printf("unable to restart %s/%s (ignored): %s", item.Namespace, item.Name, err) + if item.DeletionTimestamp == nil { + err = c.k8sClient.Delete(ctx, &item) + // Ignore Not Found + if client.IgnoreNotFound(err) != nil { + klog.Infof("unable to restart %s/%s (ignored): %s", item.Namespace, item.Name, err) + } } } return diff --git a/pkg/controller/pools.go b/pkg/controller/pools.go index ef457cee736..f41c19807f7 100644 --- a/pkg/controller/pools.go +++ b/pkg/controller/pools.go @@ -109,13 +109,21 @@ func (c *Controller) getSystemCfgFromStatefulSet(ctx context.Context, sts *appsv if err != nil { return nil, err } - systemCfg[e.Name] = string(secret.Data[e.ValueFrom.SecretKeyRef.Key]) + value, ok := secret.Data[e.ValueFrom.SecretKeyRef.Key] + if !ok { + return nil, fmt.Errorf("secret %s does not have key %s", e.ValueFrom.SecretKeyRef.Name, e.ValueFrom.SecretKeyRef.Key) + } + systemCfg[e.Name] = string(value) case e.ValueFrom != nil && e.ValueFrom.ConfigMapKeyRef != nil: configMap, err := c.kubeClientSet.CoreV1().ConfigMaps(sts.Namespace).Get(ctx, e.ValueFrom.ConfigMapKeyRef.Name, metav1.GetOptions{}) if err != nil { return nil, err } - systemCfg[e.Name] = configMap.Data[e.ValueFrom.ConfigMapKeyRef.Key] + value, ok := configMap.Data[e.ValueFrom.ConfigMapKeyRef.Key] + if !ok { + return nil, fmt.Errorf("configmap %s does not have key %s", e.ValueFrom.ConfigMapKeyRef.Name, e.ValueFrom.ConfigMapKeyRef.Key) + } + systemCfg[e.Name] = value default: return nil, fmt.Errorf("unsupported env var %s", e.Name) } From ac797d598fbcd642280a3b01cc3d79599abff8ef Mon Sep 17 00:00:00 2001 From: jiuker Date: Mon, 12 Aug 2024 12:22:57 +0800 Subject: [PATCH 8/9] lint lint --- pkg/controller/pods.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/pods.go b/pkg/controller/pods.go index e1424e0d747..480113dbf77 100644 --- a/pkg/controller/pods.go +++ b/pkg/controller/pods.go @@ -19,7 +19,6 @@ package controller import ( "context" "fmt" - "k8s.io/klog/v2" "time" miniov2 "github.com/minio/operator/pkg/apis/minio.min.io/v2" @@ -27,6 +26,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" ) From fbd4d22a4effab3e81258de440bbdf409d3b923d Mon Sep 17 00:00:00 2001 From: jiuker Date: Mon, 26 Aug 2024 12:32:03 +0800 Subject: [PATCH 9/9] apply suggestoin apply suggestoin --- pkg/controller/main-controller.go | 10 +++++----- pkg/controller/pods.go | 3 +-- pkg/controller/pools.go | 1 - 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/pkg/controller/main-controller.go b/pkg/controller/main-controller.go index 16c4c4d9941..431e5d0c842 100644 --- a/pkg/controller/main-controller.go +++ b/pkg/controller/main-controller.go @@ -1320,21 +1320,21 @@ func (c *Controller) syncHandler(key string) (Result, error) { // check if the system config has changed // if changed, minio request the systemCfg must be the same to restart. - expectSystemCfg, err := c.getSystemCfgFromStatefulSet(ctx, expectedStatefulSet) + expectedSystemCfg, err := c.getSystemCfgFromStatefulSet(ctx, expectedStatefulSet) if err != nil { return WrapResult(Result{}, err) } - existSystemCfg, err := c.getSystemCfgFromStatefulSet(ctx, existingStatefulSet) + existingSystemCfg, err := c.getSystemCfgFromStatefulSet(ctx, existingStatefulSet) if err != nil { return WrapResult(Result{}, err) } - if !maps.Equal(expectSystemCfg, existSystemCfg) { - // delete pdb to let deleted all the statefulSet pods + if !maps.Equal(expectedSystemCfg, existingSystemCfg) { + // delete pdb to let all the statefulSet pods get deleted err = c.DeletePDB(ctx, tenant) if err != nil { return Result{}, err } - // found all existing statefulSet pods and delete them + // find all existing statefulSet pods and delete them err = c.DeletePodsByStatefulSet(ctx, existingStatefulSet) if err != nil { return WrapResult(Result{}, err) diff --git a/pkg/controller/pods.go b/pkg/controller/pods.go index 480113dbf77..32089be3312 100644 --- a/pkg/controller/pods.go +++ b/pkg/controller/pods.go @@ -56,8 +56,7 @@ func (c *Controller) DeletePodsByStatefulSet(ctx context.Context, sts *appsv1.St } client.MatchingLabels(sts.Spec.Template.Labels).ApplyToList(listOpt) podList := &corev1.PodList{} - err = c.k8sClient.List(ctx, podList, listOpt) - if err != nil { + if err := c.k8sClient.List(ctx, podList, listOpt); err != nil { return err } for _, item := range podList.Items { diff --git a/pkg/controller/pools.go b/pkg/controller/pools.go index f41c19807f7..460ecb06b1b 100644 --- a/pkg/controller/pools.go +++ b/pkg/controller/pools.go @@ -94,7 +94,6 @@ func poolSSMatchesSpec(expectedStatefulSet, existingStatefulSet *appsv1.Stateful } // getSystemCfgFromStatefulSet gets the MinIO environment variables from a statefulset -// set getServerSystemCfg at minio func (c *Controller) getSystemCfgFromStatefulSet(ctx context.Context, sts *appsv1.StatefulSet) (systemCfg map[string]string, err error) { systemCfg = make(map[string]string) for _, container := range sts.Spec.Template.Spec.Containers {