Fix PDB implementation

api/cluster/pdb.go

@@ -20,7 +20,6 @@ type PodDisruptionBudget struct {
 	Labels                   map[string]string
 	MaxUnavailablePercentage *int
 	MinAvailablePercentage   *int
-	Selector                 *metav1.LabelSelector
 }
 
 func NewPodDisruptionBudget(modelService *models.Service, componentType string, pdbConfig config.PodDisruptionBudgetConfig) *PodDisruptionBudget {
@@ -47,10 +46,10 @@ func (cfg PodDisruptionBudget) BuildPDBSpec() (*policyv1cfg.PodDisruptionBudgetS
 	// Since we can specify only one of maxUnavailable and minAvailable, minAvailable takes precedence
 	// https://kubernetes.io/docs/tasks/run-application/configure-pdb/#specifying-a-poddisruptionbudget
 	if cfg.MinAvailablePercentage != nil {
-		minAvailable := intstr.FromInt(*cfg.MinAvailablePercentage)
+		minAvailable := intstr.FromString(fmt.Sprintf("%d%%", *cfg.MinAvailablePercentage))
 		pdbSpec.MinAvailable = &minAvailable
 	} else if cfg.MaxUnavailablePercentage != nil {
-		maxUnavailable := intstr.FromInt(*cfg.MaxUnavailablePercentage)
+		maxUnavailable := intstr.FromString(fmt.Sprintf("%d%%", *cfg.MaxUnavailablePercentage))
 		pdbSpec.MaxUnavailable = &maxUnavailable
 	}
 
@@ -90,7 +89,7 @@ func (c *controller) deployPodDisruptionBudget(ctx context.Context, pdb *PodDisr
 	pdbCfg.WithLabels(pdb.Labels)
 	pdbCfg.WithSpec(pdbSpec)
 
-	_, err = c.policyClient.PodDisruptionBudgets(pdb.Namespace).Apply(ctx, pdbCfg, metav1.ApplyOptions{})
+	_, err = c.policyClient.PodDisruptionBudgets(pdb.Namespace).Apply(ctx, pdbCfg, metav1.ApplyOptions{FieldManager: "application/apply-patch"})
 	if err != nil {
 		return err
 	}

api/cluster/pdb_test.go

@@ -0,0 +1,100 @@
+package cluster
+
+import (
+	"reflect"
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+	metav1cfg "k8s.io/client-go/applyconfigurations/meta/v1"
+	policyv1cfg "k8s.io/client-go/applyconfigurations/policy/v1"
+)
+
+func TestPodDisruptionBudget_BuildPDBSpec(t *testing.T) {
+	defaultInt := 20
+	defaultIntOrString := intstr.FromString("20%")
+	defaultLabels := map[string]string{
+		"gojek.com/app": "sklearn-sample-s",
+	}
+
+	type fields struct {
+		Name                     string
+		Namespace                string
+		Labels                   map[string]string
+		MaxUnavailablePercentage *int
+		MinAvailablePercentage   *int
+		Selector                 *metav1.LabelSelector
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		want    *policyv1cfg.PodDisruptionBudgetSpecApplyConfiguration
+		wantErr bool
+	}{
+		{
+			name: "valid: enabled with min_available",
+			fields: fields{
+				Name:                     "sklearn-sample-s-1-model-pdb",
+				Namespace:                "pdb-test",
+				Labels:                   defaultLabels,
+				MaxUnavailablePercentage: nil,
+				MinAvailablePercentage:   &defaultInt,
+			},
+			want: &policyv1cfg.PodDisruptionBudgetSpecApplyConfiguration{
+				MinAvailable: &defaultIntOrString,
+				Selector: &metav1cfg.LabelSelectorApplyConfiguration{
+					MatchLabels: defaultLabels,
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "valid: enabled but both max_unavailable and min_available specified. will use min available",
+			fields: fields{
+				Name:                     "sklearn-sample-s-1-model-pdb",
+				Namespace:                "pdb-test",
+				Labels:                   defaultLabels,
+				MaxUnavailablePercentage: &defaultInt,
+				MinAvailablePercentage:   &defaultInt,
+			},
+			want: &policyv1cfg.PodDisruptionBudgetSpecApplyConfiguration{
+				MinAvailable: &defaultIntOrString,
+				Selector: &metav1cfg.LabelSelectorApplyConfiguration{
+					MatchLabels: defaultLabels,
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "invalid: enabled but no max_unavailable and min_available",
+			fields: fields{
+				Name:                     "sklearn-sample-s-1-model-pdb",
+				Namespace:                "pdb-test",
+				Labels:                   map[string]string{},
+				MaxUnavailablePercentage: nil,
+				MinAvailablePercentage:   nil,
+			},
+			want:    nil,
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := PodDisruptionBudget{
+				Name:                     tt.fields.Name,
+				Namespace:                tt.fields.Namespace,
+				Labels:                   tt.fields.Labels,
+				MaxUnavailablePercentage: tt.fields.MaxUnavailablePercentage,
+				MinAvailablePercentage:   tt.fields.MinAvailablePercentage,
+			}
+			got, err := cfg.BuildPDBSpec()
+			if (err != nil) != tt.wantErr {
+				t.Errorf("PodDisruptionBudget.BuildPDBSpec() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("PodDisruptionBudget.BuildPDBSpec() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

api/config/environment_test.go

@@ -98,7 +98,7 @@ func TestUnmarshalTopologySpreadConstraints(t *testing.T) {
 }
 
 func TestPDBConfig(t *testing.T) {
-	defaultMaxUnavailablePercentage := 20
+	defaultMinAvailablePercentage := 20
 
 	testCases := []struct {
 		desc              string
@@ -110,8 +110,8 @@ func TestPDBConfig(t *testing.T) {
 			desc:          "Success: valid pdb config",
 			envConfigPath: "./testdata/valid-environment-1.yaml",
 			expectedPDBConfig: PodDisruptionBudgetConfig{
-				Enabled:                  true,
-				MaxUnavailablePercentage: &defaultMaxUnavailablePercentage,
+				Enabled:                true,
+				MinAvailablePercentage: &defaultMinAvailablePercentage,
 			},
 		},
 		{

api/config/testdata/valid-environment-1.yaml

@@ -11,7 +11,7 @@
   max_memory: "8Gi"
   pod_disruption_budget:
     enabled: true
-    max_unavailable_percentage: 20
+    min_available_percentage: 20
   queue_resource_percentage: "20"
   default_prediction_job_config:
     executor_replica: 3