diff --git a/drivers/storage/portworx/component/disruption_budget.go b/drivers/storage/portworx/component/disruption_budget.go index 24d4924765..5236be1210 100644 --- a/drivers/storage/portworx/component/disruption_budget.go +++ b/drivers/storage/portworx/component/disruption_budget.go @@ -100,23 +100,34 @@ func (c *disruptionBudget) createPortworxPodDisruptionBudget( cluster *corev1.StorageCluster, ownerRef *metav1.OwnerReference, ) error { - var err error - c.sdkConn, err = pxutil.GetPortworxConn(c.sdkConn, c.k8sClient, cluster.Namespace) + userProvidedMinValue, err := pxutil.MinAvailableForStoragePDB(cluster) if err != nil { - return err + logrus.Warnf("Invalid value for annotation %s: %v", pxutil.AnnotationStoragePodDisruptionBudget, err) + userProvidedMinValue = -1 } - storageNodesCount, err := pxutil.CountStorageNodes(cluster, c.sdkConn, c.k8sClient) - if err != nil { - c.closeSdkConn() - return err + var minAvailable int + if err != nil || userProvidedMinValue < 0 { + c.sdkConn, err = pxutil.GetPortworxConn(c.sdkConn, c.k8sClient, cluster.Namespace) + if err != nil { + return err + } + + storageNodesCount, err := pxutil.CountStorageNodes(cluster, c.sdkConn, c.k8sClient) + if err != nil { + c.closeSdkConn() + return err + } + minAvailable = storageNodesCount - 1 + } else { + minAvailable = userProvidedMinValue } - // Create PDB only if there are at least 3 nodes. With 2 nodes are less, if 1 + // Create PDB only if there are at least 3 nodes. With 2 nodes or less, if 1 // node goes down Portworx will lose quorum anyway. Such clusters would be // non-prod clusters and there is no point in blocking the evictions. - if storageNodesCount > 2 { - minAvailable := intstr.FromInt(storageNodesCount - 1) + if minAvailable > 1 { + minAvailableIntStr := intstr.FromInt(minAvailable) pdb := &policyv1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ Name: StoragePodDisruptionBudgetName, @@ -124,7 +135,7 @@ func (c *disruptionBudget) createPortworxPodDisruptionBudget( OwnerReferences: []metav1.OwnerReference{*ownerRef}, }, Spec: policyv1.PodDisruptionBudgetSpec{ - MinAvailable: &minAvailable, + MinAvailable: &minAvailableIntStr, Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ constants.LabelKeyClusterName: cluster.Name, diff --git a/drivers/storage/portworx/components_test.go b/drivers/storage/portworx/components_test.go index 1fce68db77..f872cecffe 100644 --- a/drivers/storage/portworx/components_test.go +++ b/drivers/storage/portworx/components_test.go @@ -13,29 +13,17 @@ import ( "testing" "time" - ocpconfig "github.com/openshift/api/config/v1" - "github.com/golang-jwt/jwt/v4" "github.com/golang/mock/gomock" - osdapi "github.com/libopenstorage/openstorage/api" - "github.com/libopenstorage/operator/drivers/storage/portworx/component" - "github.com/libopenstorage/operator/drivers/storage/portworx/manifest" - pxutil "github.com/libopenstorage/operator/drivers/storage/portworx/util" - corev1 "github.com/libopenstorage/operator/pkg/apis/core/v1" - "github.com/libopenstorage/operator/pkg/constants" - "github.com/libopenstorage/operator/pkg/mock" - "github.com/libopenstorage/operator/pkg/util" - k8sutil "github.com/libopenstorage/operator/pkg/util/k8s" - testutil "github.com/libopenstorage/operator/pkg/util/test" + ocpconfig "github.com/openshift/api/config/v1" ocp_secv1 "github.com/openshift/api/security/v1" - "github.com/sirupsen/logrus" - "github.com/stretchr/testify/assert" - "golang.org/x/sys/unix" - apiextensionsops "github.com/portworx/sched-ops/k8s/apiextensions" coreops "github.com/portworx/sched-ops/k8s/core" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" policyv1 "k8s.io/api/policy/v1" @@ -56,6 +44,17 @@ import ( "k8s.io/client-go/tools/record" api "k8s.io/kubernetes/pkg/apis/core" "sigs.k8s.io/controller-runtime/pkg/client" + + osdapi "github.com/libopenstorage/openstorage/api" + "github.com/libopenstorage/operator/drivers/storage/portworx/component" + "github.com/libopenstorage/operator/drivers/storage/portworx/manifest" + pxutil "github.com/libopenstorage/operator/drivers/storage/portworx/util" + corev1 "github.com/libopenstorage/operator/pkg/apis/core/v1" + "github.com/libopenstorage/operator/pkg/constants" + "github.com/libopenstorage/operator/pkg/mock" + "github.com/libopenstorage/operator/pkg/util" + k8sutil "github.com/libopenstorage/operator/pkg/util/k8s" + testutil "github.com/libopenstorage/operator/pkg/util/test" ) const ( @@ -11471,7 +11470,21 @@ func TestPodDisruptionBudgetEnabled(t *testing.T) { err = testutil.Get(k8sClient, storagePDB, component.StoragePodDisruptionBudgetName, cluster.Namespace) require.True(t, errors.IsNotFound(err)) - // TestCase: Create storage PDB if total nodes with storage is at least 3 + // TestCase: Do not create storage PDB if storage pod annotation value is less than 2 + cluster.Annotations = map[string]string{ + pxutil.AnnotationStoragePodDisruptionBudget: "1", + } + + err = driver.PreInstall(cluster) + require.NoError(t, err) + + storagePDB = &policyv1.PodDisruptionBudget{} + err = testutil.Get(k8sClient, storagePDB, component.StoragePodDisruptionBudgetName, cluster.Namespace) + require.True(t, errors.IsNotFound(err)) + + // TestCase: Create storage PDB if total nodes with storage is at least 3. + // Also, ignore the annotation if the value is an invalid integer + cluster.Annotations[pxutil.AnnotationStoragePodDisruptionBudget] = "invalid" expectedNodeEnumerateResp.Nodes = []*osdapi.StorageNode{ {Pools: []*osdapi.StoragePool{{ID: 1}}, SchedulerNodeName: "node1"}, {Pools: []*osdapi.StoragePool{{ID: 2}}, SchedulerNodeName: "node2"}, @@ -11492,7 +11505,9 @@ func TestPodDisruptionBudgetEnabled(t *testing.T) { require.Equal(t, cluster.Name, storagePDB.Spec.Selector.MatchLabels[constants.LabelKeyClusterName]) require.Equal(t, constants.LabelValueTrue, storagePDB.Spec.Selector.MatchLabels[constants.LabelKeyStoragePod]) - // TestCase: Update storage PDB if count of nodes with storage changes + // TestCase: Update storage PDB if count of nodes with storage changes. + // Also, ignore the annotation if the value is an invalid integer + cluster.Annotations[pxutil.AnnotationStoragePodDisruptionBudget] = "still_invalid" expectedNodeEnumerateResp.Nodes = []*osdapi.StorageNode{ {Pools: []*osdapi.StoragePool{{ID: 1}}, SchedulerNodeName: "node1"}, {Pools: []*osdapi.StoragePool{{ID: 2}}, SchedulerNodeName: "node2"}, diff --git a/drivers/storage/portworx/util/util.go b/drivers/storage/portworx/util/util.go index d8fa3a2830..acf8e3c4c7 100644 --- a/drivers/storage/portworx/util/util.go +++ b/drivers/storage/portworx/util/util.go @@ -123,6 +123,9 @@ const ( AnnotationRunOnMaster = pxAnnotationPrefix + "/run-on-master" // AnnotationPodDisruptionBudget annotation indicating whether to create pod disruption budgets AnnotationPodDisruptionBudget = pxAnnotationPrefix + "/pod-disruption-budget" + // AnnotationStoragePodDisruptionBudget annotation to specify the min available value of the px-storage + // pod disruption budget + AnnotationStoragePodDisruptionBudget = pxAnnotationPrefix + "/storage-pdb-min-available" // AnnotationPodSecurityPolicy annotation indicating whether to enable creation // of pod security policies AnnotationPodSecurityPolicy = pxAnnotationPrefix + "/pod-security-policy" @@ -1062,6 +1065,13 @@ func GetClusterID(cluster *corev1.StorageCluster) string { return cluster.Name } +func MinAvailableForStoragePDB(cluster *corev1.StorageCluster) (int, error) { + if cluster.Annotations[AnnotationStoragePodDisruptionBudget] != "" { + return strconv.Atoi(cluster.Annotations[AnnotationStoragePodDisruptionBudget]) + } + return -1, nil +} + // CountStorageNodes counts how many px storage node are there on given k8s cluster, // use this to count number of storage pods as well func CountStorageNodes(