From befabe5c6fd5776b5dfca0e1b717591224e5e469 Mon Sep 17 00:00:00 2001 From: Mike Rostermund Date: Mon, 6 Jan 2025 11:05:13 +0100 Subject: [PATCH] Revert "Implemented PodDisruptionBudget" --- api/v1alpha1/humiocluster_types.go | 14 -- api/v1alpha1/zz_generated.deepcopy.go | 30 ---- .../crds/core.humio.com_humioclusters.yaml | 19 --- .../bases/core.humio.com_humioclusters.yaml | 19 --- config/rbac/role.yaml | 36 ---- controllers/humiocluster_controller.go | 91 ---------- .../clusters/humiocluster_controller_test.go | 157 ------------------ docs/api.md | 41 ----- 8 files changed, 407 deletions(-) diff --git a/api/v1alpha1/humiocluster_types.go b/api/v1alpha1/humiocluster_types.go index a793314c..8d1a0ece 100644 --- a/api/v1alpha1/humiocluster_types.go +++ b/api/v1alpha1/humiocluster_types.go @@ -101,9 +101,6 @@ type HumioClusterSpec struct { // NodePools can be used to define additional groups of Humio cluster pods that share a set of configuration. NodePools []HumioNodePoolSpec `json:"nodePools,omitempty"` - - // PodDisruptionBudget defines the configuration for the PodDisruptionBudget - PodDisruptionBudget *PodDisruptionBudgetSpec `json:"podDisruptionBudget,omitempty"` } type HumioNodeSpec struct { @@ -454,17 +451,6 @@ type HumioClusterList struct { Items []HumioCluster `json:"items"` } -// PodDisruptionBudgetSpec defines the configuration for the PodDisruptionBudget -type PodDisruptionBudgetSpec struct { - // MinAvailable specifies the minimum number of pods that must be available - // +optional - MinAvailable *intstr.IntOrString `json:"minAvailable,omitempty"` - - // MaxUnavailable specifies the maximum number of pods that can be unavailable - // +optional - MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty"` -} - // Len is the number of elements in the collection func (l HumioPodStatusList) Len() int { return len(l) diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 2396f88f..140c1df8 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -800,11 +800,6 @@ func (in *HumioClusterSpec) DeepCopyInto(out *HumioClusterSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.PodDisruptionBudget != nil { - in, out := &in.PodDisruptionBudget, &out.PodDisruptionBudget - *out = new(PodDisruptionBudgetSpec) - (*in).DeepCopyInto(*out) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HumioClusterSpec. @@ -2077,31 +2072,6 @@ func (in *HumioViewStatus) DeepCopy() *HumioViewStatus { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PodDisruptionBudgetSpec) DeepCopyInto(out *PodDisruptionBudgetSpec) { - *out = *in - if in.MinAvailable != nil { - in, out := &in.MinAvailable, &out.MinAvailable - *out = new(intstr.IntOrString) - **out = **in - } - if in.MaxUnavailable != nil { - in, out := &in.MaxUnavailable, &out.MaxUnavailable - *out = new(intstr.IntOrString) - **out = **in - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PodDisruptionBudgetSpec. -func (in *PodDisruptionBudgetSpec) DeepCopy() *PodDisruptionBudgetSpec { - if in == nil { - return nil - } - out := new(PodDisruptionBudgetSpec) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VarSource) DeepCopyInto(out *VarSource) { *out = *in diff --git a/charts/humio-operator/crds/core.humio.com_humioclusters.yaml b/charts/humio-operator/crds/core.humio.com_humioclusters.yaml index f6cec3ba..036d2432 100644 --- a/charts/humio-operator/crds/core.humio.com_humioclusters.yaml +++ b/charts/humio-operator/crds/core.humio.com_humioclusters.yaml @@ -13154,25 +13154,6 @@ spec: description: PodAnnotations can be used to specify annotations that will be added to the Humio pods type: object - podDisruptionBudget: - description: PodDisruptionBudget defines the configuration for the - PodDisruptionBudget - properties: - maxUnavailable: - anyOf: - - type: integer - - type: string - description: MaxUnavailable specifies the maximum number of pods - that can be unavailable - x-kubernetes-int-or-string: true - minAvailable: - anyOf: - - type: integer - - type: string - description: MinAvailable specifies the minimum number of pods - that must be available - x-kubernetes-int-or-string: true - type: object podLabels: additionalProperties: type: string diff --git a/config/crd/bases/core.humio.com_humioclusters.yaml b/config/crd/bases/core.humio.com_humioclusters.yaml index f6cec3ba..036d2432 100644 --- a/config/crd/bases/core.humio.com_humioclusters.yaml +++ b/config/crd/bases/core.humio.com_humioclusters.yaml @@ -13154,25 +13154,6 @@ spec: description: PodAnnotations can be used to specify annotations that will be added to the Humio pods type: object - podDisruptionBudget: - description: PodDisruptionBudget defines the configuration for the - PodDisruptionBudget - properties: - maxUnavailable: - anyOf: - - type: integer - - type: string - description: MaxUnavailable specifies the maximum number of pods - that can be unavailable - x-kubernetes-int-or-string: true - minAvailable: - anyOf: - - type: integer - - type: string - description: MinAvailable specifies the minimum number of pods - that must be available - x-kubernetes-int-or-string: true - type: object podLabels: additionalProperties: type: string diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 89e8be02..a538a200 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -4,18 +4,6 @@ kind: ClusterRole metadata: name: manager-role rules: -- apiGroups: - - apps - resources: - - statefulsets - verbs: - - create - - delete - - get - - list - - patch - - update - - watch - apiGroups: - "" resources: @@ -448,27 +436,3 @@ rules: - patch - update - watch -- apiGroups: - - networking.k8s.io - resources: - - ingresses - verbs: - - create - - delete - - get - - list - - patch - - update - - watch -- apiGroups: - - policy - resources: - - poddisruptionbudgets - verbs: - - create - - delete - - get - - list - - patch - - update - - watch diff --git a/controllers/humiocluster_controller.go b/controllers/humiocluster_controller.go index 5db638ac..54b68021 100644 --- a/controllers/humiocluster_controller.go +++ b/controllers/humiocluster_controller.go @@ -32,11 +32,8 @@ import ( "github.com/humio/humio-operator/internal/kubernetes" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" - policyv1 "k8s.io/api/policy/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/strings/slices" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -82,9 +79,6 @@ const ( //+kubebuilder:rbac:groups=core,resources=secrets,verbs=create;delete;get;list;patch;update;watch //+kubebuilder:rbac:groups=core,resources=serviceaccounts,verbs=create;delete;get;list;patch;update;watch //+kubebuilder:rbac:groups=networking.k8s.io,resources=ingress,verbs=create;delete;get;list;patch;update;watch -//+kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;list;watch;create;update;patch;delete -//+kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=get;list;watch;create;update;patch;delete func (r *HumioClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { // when running tests, ignore resources that are not in the correct namespace @@ -312,13 +306,6 @@ func (r *HumioClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request return result, err } - // Set podDisruptionBudget - if err = r.reconcilePodDisruptionBudget(ctx, hc); err != nil { - return r.updateStatus(ctx, r.Client.Status(), hc, statusOptions(). - withMessage(r.logErrorAndReturn(err, "unable to set pod disruption budget").Error()), - ) - } - r.Log.Info("done reconciling") return r.updateStatus(ctx, r.Client.Status(), hc, statusOptions().withState(hc.Status.State).withMessage("")) } @@ -334,7 +321,6 @@ func (r *HumioClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { Owns(&corev1.PersistentVolumeClaim{}). Owns(&corev1.ConfigMap{}). Owns(&networkingv1.Ingress{}). - Owns(&policyv1.PodDisruptionBudget{}). Complete(r) } @@ -2363,80 +2349,3 @@ func getHumioNodePoolManagers(hc *humiov1alpha1.HumioCluster) HumioNodePoolList } return humioNodePools } - -// podLabelsForHumio returns the labels for selecting the resources -// belonging to the given humioCluster CR name. -func (r *HumioClusterReconciler) podLabelsForHumio(name string) map[string]string { - return map[string]string{"app": "humio", "humio_cr": name} -} - -func (r *HumioClusterReconciler) reconcilePodDisruptionBudget(ctx context.Context, humioCluster *humiov1alpha1.HumioCluster) error { - // Define the desired PodDisruptionBudget object - pdb := &policyv1.PodDisruptionBudget{ - ObjectMeta: metav1.ObjectMeta{ - Name: humioCluster.Name + "-pdb", // Or a more suitable name - Namespace: humioCluster.Namespace, - Labels: r.podLabelsForHumio(humioCluster.Name), // Make sure labels are correct - }, - Spec: policyv1.PodDisruptionBudgetSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: r.podLabelsForHumio(humioCluster.Name), - }, - }, - } - - // Set the MinAvailable or MaxUnavailable value - if humioCluster.Spec.PodDisruptionBudget != nil { - if humioCluster.Spec.PodDisruptionBudget.MinAvailable != nil { - pdb.Spec.MinAvailable = humioCluster.Spec.PodDisruptionBudget.MinAvailable - } else if humioCluster.Spec.PodDisruptionBudget.MaxUnavailable != nil { - pdb.Spec.MaxUnavailable = humioCluster.Spec.PodDisruptionBudget.MaxUnavailable - } - } else { - // Set default values if not specified in the CR - defaultMinAvailable := intstr.FromInt32(2) // Example default: at least 2 pods available - pdb.Spec.MinAvailable = &defaultMinAvailable - } - - // Check if the PodDisruptionBudget already exists - foundPdb := &policyv1.PodDisruptionBudget{} - err := r.Client.Get(ctx, types.NamespacedName{Name: pdb.Name, Namespace: pdb.Namespace}, foundPdb) - if err != nil && k8serrors.IsNotFound(err) { - // Create the PodDisruptionBudget - r.Log.Info("Creating a new PodDisruptionBudget", "PDB.Namespace", pdb.Namespace, "PDB.Name", pdb.Name) - err = r.Client.Create(ctx, pdb) - if err != nil { - return err - } - return nil - } else if err != nil { - return err - } - - // Update the PodDisruptionBudget if it exists and needs updating - if humioCluster.Spec.PodDisruptionBudget != nil { - if needsPDBUpdate(foundPdb, pdb) { - foundPdb.Spec = pdb.Spec - r.Log.Info("Updating PodDisruptionBudget", "PDB.Namespace", foundPdb.Namespace, "PDB.Name", foundPdb.Name) - err = r.Client.Update(ctx, foundPdb) - if err != nil { - return err - } - } - } - return nil -} - -func needsPDBUpdate(current, desired *policyv1.PodDisruptionBudget) bool { - if current.Spec.MinAvailable != nil && desired.Spec.MinAvailable != nil { - if current.Spec.MinAvailable.String() != desired.Spec.MinAvailable.String() { - return true - } - } - if current.Spec.MaxUnavailable != nil && desired.Spec.MaxUnavailable != nil { - if current.Spec.MaxUnavailable.String() != desired.Spec.MaxUnavailable.String() { - return true - } - } - return false -} diff --git a/controllers/suite/clusters/humiocluster_controller_test.go b/controllers/suite/clusters/humiocluster_controller_test.go index 8615039a..0bdf530f 100644 --- a/controllers/suite/clusters/humiocluster_controller_test.go +++ b/controllers/suite/clusters/humiocluster_controller_test.go @@ -35,7 +35,6 @@ import ( . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" - policyv1 "k8s.io/api/policy/v1" schedulingv1 "k8s.io/api/scheduling/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" @@ -6042,162 +6041,6 @@ var _ = Describe("HumioCluster Controller", func() { Expect(mostSeenUnavailable).To(BeNumerically("==", toCreate.Spec.NodeCount)) }) }) - - Context("HumioCluster PodDisruptionBudget", Label("envtest", "dummy", "real"), func() { - var ( - key types.NamespacedName - toCreate *humiov1alpha1.HumioCluster - ctx context.Context - - cancel context.CancelFunc - cleanupHelper func() - ) - - BeforeEach(func() { - ctx, cancel = context.WithTimeout(context.Background(), testTimeout) - key = types.NamespacedName{ - Name: "humiocluster-pdb-test", - Namespace: testProcessNamespace, - } - - cleanupHelper = func() { - resourcesToDelete := []client.Object{ - &policyv1.PodDisruptionBudget{ObjectMeta: metav1.ObjectMeta{Name: key.Name + "-pdb", Namespace: key.Namespace}}, - &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: key.Name + "-admin-token", Namespace: key.Namespace}}, - &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: key.Name + "-bootstrap-token", Namespace: key.Namespace}}, - &humiov1alpha1.HumioBootstrapToken{ObjectMeta: metav1.ObjectMeta{Name: key.Name, Namespace: key.Namespace}}, - &humiov1alpha1.HumioCluster{ObjectMeta: metav1.ObjectMeta{Name: key.Name, Namespace: key.Namespace}}, - } - - for _, obj := range resourcesToDelete { - err := k8sClient.Delete(ctx, obj) - if err != nil && !k8serrors.IsNotFound(err) { - Expect(err).NotTo(HaveOccurred()) - } - } - } - - // Create basic cluster configuration - toCreate = suite.ConstructBasicSingleNodeHumioCluster(key, true) - toCreate.Spec.NodeCount = 3 - }) - - AfterEach(func() { - cleanupHelper() - cancel() - }) - - It("Should create PDB with user-specified minAvailable", func() { - minAvailable := intstr.FromInt32(1) - toCreate.Spec.PodDisruptionBudget = &humiov1alpha1.PodDisruptionBudgetSpec{ - MinAvailable: &minAvailable, - } - - // Create the HumioCluster - suite.CreateAndBootstrapCluster(ctx, k8sClient, testHumioClient, toCreate, true, humiov1alpha1.HumioClusterStateRunning, testTimeout) - defer suite.CleanupCluster(ctx, k8sClient, toCreate) - - foundPdb := &policyv1.PodDisruptionBudget{} - Eventually(func() error { - return k8sClient.Get(ctx, types.NamespacedName{Name: toCreate.Name + "-pdb", Namespace: toCreate.Namespace}, foundPdb) - }, testTimeout, suite.TestInterval).Should(Succeed()) - - Expect(foundPdb.Spec.MinAvailable.IntValue()).To(Equal(1)) - Expect(foundPdb.Spec.MaxUnavailable).To(BeNil()) - }) - - It("Should create PDB with user-specified maxUnavailable", func() { - maxUnavailable := intstr.FromInt32(1) - toCreate.Spec.PodDisruptionBudget = &humiov1alpha1.PodDisruptionBudgetSpec{ - MaxUnavailable: &maxUnavailable, - } - - // Create the HumioCluster - suite.CreateAndBootstrapCluster(ctx, k8sClient, testHumioClient, toCreate, true, humiov1alpha1.HumioClusterStateRunning, testTimeout) - defer suite.CleanupCluster(ctx, k8sClient, toCreate) - - foundPdb := &policyv1.PodDisruptionBudget{} - Eventually(func() error { - return k8sClient.Get(ctx, types.NamespacedName{Name: toCreate.Name + "-pdb", Namespace: toCreate.Namespace}, foundPdb) - }, testTimeout, suite.TestInterval).Should(Succeed()) - - Expect(foundPdb.Spec.MaxUnavailable.IntValue()).To(Equal(1)) - Expect(foundPdb.Spec.MinAvailable).To(BeNil()) - }) - - It("Should update PDB if spec changes", func() { - // Create the HumioCluster - suite.CreateAndBootstrapCluster(ctx, k8sClient, testHumioClient, toCreate, true, humiov1alpha1.HumioClusterStateRunning, testTimeout) - defer suite.CleanupCluster(ctx, k8sClient, toCreate) - - // Get the HumioCluster - updatedHumioCluster := &humiov1alpha1.HumioCluster{} - Expect(k8sClient.Get(ctx, key, updatedHumioCluster)).Should(Succeed()) - - // Update HumioCluster with new PDB spec - minAvailable := intstr.FromInt32(1) - Eventually(func() error { - updatedHumioCluster = &humiov1alpha1.HumioCluster{} - err := k8sClient.Get(ctx, key, updatedHumioCluster) - if err != nil { - return err - } - updatedHumioCluster.Spec.PodDisruptionBudget = &humiov1alpha1.PodDisruptionBudgetSpec{ - MinAvailable: &minAvailable, - } - return k8sClient.Update(ctx, updatedHumioCluster) - }, testTimeout, suite.TestInterval).Should(Succeed()) - - // Check if PDB is updated - foundPdb := &policyv1.PodDisruptionBudget{} - Eventually(func() error { - err := k8sClient.Get(ctx, types.NamespacedName{Name: toCreate.Name + "-pdb", Namespace: toCreate.Namespace}, foundPdb) - if err != nil { - return err - } - if foundPdb.Spec.MinAvailable == nil { - return fmt.Errorf("minAvailable not set on PDB") - } - if foundPdb.Spec.MinAvailable.IntValue() != 1 { - return fmt.Errorf("minAvailable value is not 1 on PDB") - } - return nil - }, testTimeout, suite.TestInterval).Should(Succeed()) - }) - - It("Should not update PDB if spec does not change", func() { - // Create the HumioCluster - suite.CreateAndBootstrapCluster(ctx, k8sClient, testHumioClient, toCreate, true, humiov1alpha1.HumioClusterStateRunning, testTimeout) - defer suite.CleanupCluster(ctx, k8sClient, toCreate) - - // Get the PDB - var initialResourceVersion string - foundPdb := &policyv1.PodDisruptionBudget{} - Eventually(func() error { - err := k8sClient.Get(ctx, types.NamespacedName{Name: toCreate.Name + "-pdb", Namespace: toCreate.Namespace}, foundPdb) - if err == nil { - initialResourceVersion = foundPdb.ResourceVersion - } - return err - }, testTimeout, suite.TestInterval).Should(Succeed()) - - // Reconcile again without changes, this is done by fetching a new HumioCluster - updatedHumioCluster := &humiov1alpha1.HumioCluster{} - Expect(k8sClient.Get(ctx, key, updatedHumioCluster)).Should(Succeed()) - - // Check if PDB's resource version is the same - Consistently(func() string { - // Get the updated PDB - var updatedPdb policyv1.PodDisruptionBudget - err := k8sClient.Get(ctx, types.NamespacedName{Name: foundPdb.Name, Namespace: foundPdb.Namespace}, &updatedPdb) - if err != nil { - return "" - } - return updatedPdb.ResourceVersion - }, "2s", suite.TestInterval).Should(Equal(initialResourceVersion)) - }) - }) - }) // TODO: Consider refactoring goroutine to a "watcher". https://book-v1.book.kubebuilder.io/beyond_basics/controller_watches diff --git a/docs/api.md b/docs/api.md index c1c13eb5..e352d695 100644 --- a/docs/api.md +++ b/docs/api.md @@ -4309,13 +4309,6 @@ Deprecated: LogScale 1.70.0 deprecated this option, and was later removed in Log PodAnnotations can be used to specify annotations that will be added to the Humio pods
false - - podDisruptionBudget - object - - PodDisruptionBudget defines the configuration for the PodDisruptionBudget
- - false podLabels map[string]string @@ -30962,40 +30955,6 @@ Humio pods can be updated in a rolling fashion or if they must be replaced at th -### HumioCluster.spec.podDisruptionBudget -[↩ Parent](#humioclusterspec) - - - -PodDisruptionBudget defines the configuration for the PodDisruptionBudget - - - - - - - - - - - - - - - - - - - - - -
NameTypeDescriptionRequired
maxUnavailableint or string - MaxUnavailable specifies the maximum number of pods that can be unavailable
- 		false
minAvailableint or string - MinAvailable specifies the minimum number of pods that must be available
- 		false
- - ### HumioCluster.spec.podSecurityContext [↩ Parent](#humioclusterspec)