diff --git a/pkg/config/config.go b/pkg/config/config.go index 59f63ba3..61a69933 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -93,6 +93,10 @@ var ( dashboardTLSKeyPathKey = newKey("dashboardTLSKeyPath", DefaultTLSKeyFile) dashboardTLSCertPathKey = newKey("dashboardTLSCertPath", DefaultTLSCrtFile) skipTLSWarningKey = newBoolOrStringKey("skipTLSWarning", false) + pdbKey = "pdb" + pdbDisabledSubKey = newBoolOrStringKey("disabled", false) + pdbMaxUnavailableSubKey = newStringKey("maxUnavailable") + pdbMinAvailableSubKey = newStringKey("minAvailable") ) // Warning is an issue with configuration that we will report as undesirable @@ -185,6 +189,14 @@ type SpiceConfig struct { ProjectLabels bool ProjectAnnotations bool Passthrough map[string]string + PDB PDBConfig +} + +// PDBConfig holds the configuration for the PodDisruptionBudget. +type PDBConfig struct { + Disabled bool + MaxUnavailable *intstr.IntOrString + MinAvailable *intstr.IntOrString } // NewConfig checks that the values in the config + the secrets are sane @@ -427,6 +439,39 @@ func NewConfig(cluster *v1alpha1.SpiceDBCluster, globalConfig *OperatorConfig, s warnings = append(warnings, saAnnotationWarnings...) } + if pdbRaw, ok := config[pdbKey]; ok { + delete(config, pdbKey) + pdbMap, ok := pdbRaw.(map[string]any) + if !ok { + errs = append(errs, fmt.Errorf("expected object for key %q", pdbKey)) + } else { + pdbConfig := RawConfig(pdbMap) + + spiceConfig.PDB.Disabled, err = pdbDisabledSubKey.pop(pdbConfig) + if err != nil { + errs = append(errs, err) + } + + if s := pdbMaxUnavailableSubKey.pop(pdbConfig); s != "" { + val := parseIntOrStringValue(s) + spiceConfig.PDB.MaxUnavailable = &val + } + + if s := pdbMinAvailableSubKey.pop(pdbConfig); s != "" { + val := parseIntOrStringValue(s) + spiceConfig.PDB.MinAvailable = &val + } + + if spiceConfig.PDB.MaxUnavailable != nil && spiceConfig.PDB.MinAvailable != nil { + errs = append(errs, fmt.Errorf("only one of pdb.maxUnavailable or pdb.minAvailable can be set")) + } + + for k := range pdbConfig { + warnings = append(warnings, fmt.Errorf("unknown key %q in pdb config", k)) + } + } + } + // generate secret refs for tls if specified if len(spiceConfig.TLSSecretName) > 0 { passthroughKeys := []*key[string]{ @@ -981,15 +1026,23 @@ func (c *Config) PodDisruptionBudget() *applypolicyv1.PodDisruptionBudgetApplyCo } func (c *Config) unpatchedPDB() *applypolicyv1.PodDisruptionBudgetApplyConfiguration { + spec := applypolicyv1.PodDisruptionBudgetSpec(). + WithSelector(applymetav1.LabelSelector().WithMatchLabels( + map[string]string{metadata.KubernetesInstanceLabelKey: deploymentName(c.Name)}, + )) + + switch { + case c.PDB.MaxUnavailable != nil: + spec = spec.WithMaxUnavailable(*c.PDB.MaxUnavailable) + case c.PDB.MinAvailable != nil: + spec = spec.WithMinAvailable(*c.PDB.MinAvailable) + default: + spec = spec.WithMaxUnavailable(intstr.FromInt32(1)) + } + return applypolicyv1.PodDisruptionBudget(pdbName(c.Name), c.Namespace). WithLabels(metadata.LabelsForComponent(c.Name, metadata.ComponentPDBLabel)). - WithSpec(applypolicyv1.PodDisruptionBudgetSpec(). - WithSelector(applymetav1.LabelSelector().WithMatchLabels( - map[string]string{metadata.KubernetesInstanceLabelKey: deploymentName(c.Name)}, - )). - // only allow one pod to be unavailable at a time - WithMaxUnavailable(intstr.FromInt32(1)), - ) + WithSpec(spec) } func (c *Config) commonLabels(name string) map[string]string { @@ -1107,3 +1160,12 @@ func deploymentName(name string) string { func pdbName(name string) string { return fmt.Sprintf("%s-spicedb", name) } + +// parseIntOrStringValue parses a string as an integer or, if that fails, +// returns it as a string-typed IntOrString (e.g. for percentage values like "50%"). +func parseIntOrStringValue(s string) intstr.IntOrString { + if v, err := strconv.ParseInt(s, 10, 32); err == nil { + return intstr.FromInt32(int32(v)) + } + return intstr.FromString(s) +} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 74e3f3ab..a11c1e9f 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -3183,9 +3183,11 @@ metadata: func TestPDB(t *testing.T) { resources := newFakeResources() tests := []struct { - name string - cluster v1alpha1.ClusterSpec - wantPDB *applypolicyv1.PodDisruptionBudgetApplyConfiguration + name string + cluster v1alpha1.ClusterSpec + wantPDB *applypolicyv1.PodDisruptionBudgetApplyConfiguration + wantPDBDisabled bool + wantErr error }{ { name: "pdb sets maxUnavailable to 1", @@ -3257,6 +3259,136 @@ metadata: }), ).WithMaxUnavailable(intstr.FromInt32(1))), }, + { + name: "pdb disabled", + cluster: v1alpha1.ClusterSpec{ + SecretRef: "test-secret", + Config: json.RawMessage(` + { + "datastoreEngine": "cockroachdb", + "pdb": { + "disabled": true + } + } + `), + }, + wantPDBDisabled: true, + }, + { + name: "pdb with custom maxUnavailable integer", + cluster: v1alpha1.ClusterSpec{ + SecretRef: "test-secret", + Config: json.RawMessage(` + { + "datastoreEngine": "cockroachdb", + "pdb": { + "maxUnavailable": "2" + } + } + `), + }, + wantPDB: applypolicyv1.PodDisruptionBudget("test-spicedb", "test"). + WithLabels(metadata.LabelsForComponent("test", metadata.ComponentPDBLabel)). + WithLabels(map[string]string{ + metadata.KubernetesInstanceLabelKey: "test-spicedb", + metadata.KubernetesNameLabelKey: "test-spicedb", + metadata.KubernetesComponentLabelKey: metadata.ComponentSpiceDBLabelValue, + metadata.KubernetesVersionLabelKey: "v1", + }). + WithOwnerReferences(applymetav1.OwnerReference(). + WithName("test"). + WithKind(v1alpha1.SpiceDBClusterKind). + WithAPIVersion(v1alpha1.SchemeGroupVersion.String()). + WithUID("1")). + WithSpec( + applypolicyv1.PodDisruptionBudgetSpec().WithSelector( + applymetav1.LabelSelector().WithMatchLabels(map[string]string{ + metadata.KubernetesInstanceLabelKey: "test-spicedb", + }), + ).WithMaxUnavailable(intstr.FromInt32(2))), + }, + { + name: "pdb with custom maxUnavailable percentage", + cluster: v1alpha1.ClusterSpec{ + SecretRef: "test-secret", + Config: json.RawMessage(` + { + "datastoreEngine": "cockroachdb", + "pdb": { + "maxUnavailable": "50%" + } + } + `), + }, + wantPDB: applypolicyv1.PodDisruptionBudget("test-spicedb", "test"). + WithLabels(metadata.LabelsForComponent("test", metadata.ComponentPDBLabel)). + WithLabels(map[string]string{ + metadata.KubernetesInstanceLabelKey: "test-spicedb", + metadata.KubernetesNameLabelKey: "test-spicedb", + metadata.KubernetesComponentLabelKey: metadata.ComponentSpiceDBLabelValue, + metadata.KubernetesVersionLabelKey: "v1", + }). + WithOwnerReferences(applymetav1.OwnerReference(). + WithName("test"). + WithKind(v1alpha1.SpiceDBClusterKind). + WithAPIVersion(v1alpha1.SchemeGroupVersion.String()). + WithUID("1")). + WithSpec( + applypolicyv1.PodDisruptionBudgetSpec().WithSelector( + applymetav1.LabelSelector().WithMatchLabels(map[string]string{ + metadata.KubernetesInstanceLabelKey: "test-spicedb", + }), + ).WithMaxUnavailable(intstr.FromString("50%"))), + }, + { + name: "pdb with custom minAvailable", + cluster: v1alpha1.ClusterSpec{ + SecretRef: "test-secret", + Config: json.RawMessage(` + { + "datastoreEngine": "cockroachdb", + "pdb": { + "minAvailable": "3" + } + } + `), + }, + wantPDB: applypolicyv1.PodDisruptionBudget("test-spicedb", "test"). + WithLabels(metadata.LabelsForComponent("test", metadata.ComponentPDBLabel)). + WithLabels(map[string]string{ + metadata.KubernetesInstanceLabelKey: "test-spicedb", + metadata.KubernetesNameLabelKey: "test-spicedb", + metadata.KubernetesComponentLabelKey: metadata.ComponentSpiceDBLabelValue, + metadata.KubernetesVersionLabelKey: "v1", + }). + WithOwnerReferences(applymetav1.OwnerReference(). + WithName("test"). + WithKind(v1alpha1.SpiceDBClusterKind). + WithAPIVersion(v1alpha1.SchemeGroupVersion.String()). + WithUID("1")). + WithSpec( + applypolicyv1.PodDisruptionBudgetSpec().WithSelector( + applymetav1.LabelSelector().WithMatchLabels(map[string]string{ + metadata.KubernetesInstanceLabelKey: "test-spicedb", + }), + ).WithMinAvailable(intstr.FromInt32(3))), + }, + { + name: "pdb with both maxUnavailable and minAvailable returns error", + cluster: v1alpha1.ClusterSpec{ + SecretRef: "test-secret", + Config: json.RawMessage(` + { + "datastoreEngine": "cockroachdb", + "pdb": { + "maxUnavailable": "1", + "minAvailable": "1" + } + } + `), + }, + wantErr: fmt.Errorf("only one of pdb.maxUnavailable or pdb.minAvailable can be set"), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -3273,14 +3405,21 @@ metadata: Spec: tt.cluster, } got, _, err := NewConfig(cluster, ptr.To(testGlobalConfig.Copy()), singleSecretMap("test-secret", secret), resources) + if tt.wantErr != nil { + require.ErrorContains(t, err, tt.wantErr.Error()) + return + } require.NoError(t, err) - wantPDB, err := json.Marshal(tt.wantPDB) - require.NoError(t, err) - gotPDB, err := json.Marshal(got.PodDisruptionBudget()) - require.NoError(t, err) + require.Equal(t, tt.wantPDBDisabled, got.PDB.Disabled) - require.JSONEq(t, string(wantPDB), string(gotPDB)) + if tt.wantPDB != nil { + wantPDB, err := json.Marshal(tt.wantPDB) + require.NoError(t, err) + gotPDB, err := json.Marshal(got.PodDisruptionBudget()) + require.NoError(t, err) + require.JSONEq(t, string(wantPDB), string(gotPDB)) + } }) } } diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 2a15c1cc..fc3a3f8f 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -557,9 +557,28 @@ func (c *Controller) ensurePDB(next ...handler.Handler) handler.Handler { }, ) + cfg := CtxConfig.MustValue(ctx) + + // If PDB is disabled, delete any existing PDB and skip creation + if cfg.PDB.Disabled { + clusterName := CtxClusterNN.MustValue(ctx).Name + expectedLabels := metadata.LabelsForComponent(clusterName, metadata.ComponentPDBLabel) + for _, pdb := range pdbComponent.List(ctx, CtxClusterNN.MustValue(ctx)) { + if !hasAllLabels(pdb.Labels, expectedLabels) { + logr.FromContextOrDiscard(ctx).V(4).Info("skipping pdb deletion: missing operator labels", "namespace", pdb.Namespace, "name", pdb.Name) + continue + } + nn := types.NamespacedName{Name: pdb.Name, Namespace: pdb.Namespace} + if err := deletePDB(ctx, nn); err != nil && !apierrors.IsNotFound(err) { + logr.FromContextOrDiscard(ctx).Error(err, "error deleting pdb") + } + } + handler.Handlers(next).MustOne().Handle(ctx) + return + } + // build the handler that ensures the PDB with the proper definition // exists and run it - cfg := CtxConfig.MustValue(ctx) component.NewEnsureComponentByHash( component.NewHashableComponent(pdbComponent, hash.NewObjectHash(), "authzed.com/controller-component-hash"), CtxClusterNN, @@ -954,3 +973,12 @@ func (c *Controller) ensureService(next ...handler.Handler) handler.Handler { handler.Handlers(next).MustOne().Handle(ctx) }, "ensureService") } + +func hasAllLabels(actual, expected map[string]string) bool { + for k, v := range expected { + if actual[k] != v { + return false + } + } + return true +}