Skip to content

Commit a63407f

Browse files
committed
fix: Do not create PodDisruptionBudget in single replica topology mode
The PodDisruptionBudget does not make sense in single replica topology mode. It would have "spec.minAvailable" set to 1, but there would always be one pod. It would cause a warning to be always present: "The pod disruption budget is at the minimum disruptions allowed level. The number of current healthy pods is equal to the desired healthy pods." Assisted-by: Claude <noreply@anthropic.com> Signed-off-by: Andrej Krejcir <akrejcir@redhat.com>
1 parent 0b178fe commit a63407f

4 files changed

Lines changed: 55 additions & 8 deletions

File tree

internal/operands/template-validator/reconcile.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ func (t *templateValidator) Reconcile(request *common.Request) ([]common.Reconci
7272
reconcileConfigMap,
7373
reconcileDeployment,
7474
reconcileValidatingWebhook,
75-
reconcilePodDisruptionBudget,
75+
}
76+
if !request.IsSingleReplicaTopologyMode() {
77+
funcs = append(funcs, reconcilePodDisruptionBudget)
7678
}
7779
funcs = append(funcs, reconcileNetworkPolicies(request)...)
7880
return common.CollectResourceStatus(request, funcs...)

internal/operands/template-validator/reconcile_test.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ import (
66

77
. "github.com/onsi/ginkgo/v2"
88
. "github.com/onsi/gomega"
9-
securityv1 "github.com/openshift/api/security/v1"
109

10+
osconfv1 "github.com/openshift/api/config/v1"
11+
securityv1 "github.com/openshift/api/security/v1"
1112
admission "k8s.io/api/admissionregistration/v1"
1213
apps "k8s.io/api/apps/v1"
1314
core "k8s.io/api/core/v1"
@@ -95,6 +96,15 @@ var _ = Describe("Template validator operand", func() {
9596
}
9697
})
9798

99+
It("should not create PodDisruptionBudget when SingleReplicaTopologyMode is used", func() {
100+
request.TopologyMode = osconfv1.SingleReplicaTopologyMode
101+
102+
_, err := operand.Reconcile(&request)
103+
Expect(err).ToNot(HaveOccurred())
104+
105+
ExpectResourceNotExists(newPodDisruptionBudget(namespace), request)
106+
})
107+
98108
It("should not update webhook CA bundle", func() {
99109
_, err := operand.Reconcile(&request)
100110
Expect(err).ToNot(HaveOccurred())

tests/single_node_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,13 @@ import (
44
. "github.com/onsi/ginkgo/v2"
55
. "github.com/onsi/gomega"
66

7+
policy "k8s.io/api/policy/v1"
8+
"k8s.io/apimachinery/pkg/api/errors"
79
"kubevirt.io/controller-lifecycle-operator-sdk/api"
10+
"sigs.k8s.io/controller-runtime/pkg/client"
811

912
ssp "kubevirt.io/ssp-operator/api/v1beta3"
13+
validator "kubevirt.io/ssp-operator/internal/operands/template-validator"
1014
"kubevirt.io/ssp-operator/tests/env"
1115
)
1216

@@ -81,4 +85,10 @@ var _ = Describe("Single Node Topology", func() {
8185
deployment := getTemplateValidatorDeployment()
8286
Expect(int(deployment.Status.Replicas)).Should(Equal(0), "In Single Mode Topology the number of replicas is at most 1")
8387
})
88+
89+
It("[test_id:TODO] PodDisruptionBudget should not be created", func() {
90+
key := client.ObjectKey{Name: validator.DeploymentName, Namespace: strategy.GetNamespace()}
91+
pdb := &policy.PodDisruptionBudget{}
92+
Expect(apiClient.Get(ctx, key, pdb)).To(MatchError(errors.IsNotFound, "errors.IsNotFound"))
93+
})
8494
})

tests/validator_test.go

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,14 @@ var _ = Describe("Template validator operand", func() {
214214
Entry("[test_id:4912] deployment", &deploymentRes),
215215
Entry("[test_id:TODO] network policy kube api and dns", &networkPolicyKubeAPIAndDNSRes),
216216
Entry("[test_id:TODO] network policy webhook and metrics", &networkPolicyWebhookAndMetricsRes),
217-
Entry("[test_id:TODO] PodDisruptionBudget", &podDisruptionBudgetRes),
218217
)
219218

219+
It("[test_id:TODO] PodDisruptionBudget", decorators.Conformance, func() {
220+
strategy.SkipUnlessHighlyAvailableTopologyMode()
221+
err := apiClient.Get(ctx, podDisruptionBudgetRes.GetKey(), podDisruptionBudgetRes.NewResource())
222+
Expect(err).ToNot(HaveOccurred())
223+
})
224+
220225
DescribeTable("should set app labels", expectAppLabels,
221226
Entry("[test_id:5824]cluster role", &clusterRoleRes),
222227
Entry("[test_id:5825]cluster role binding", &clusterRoleBindingRes),
@@ -228,8 +233,12 @@ var _ = Describe("Template validator operand", func() {
228233
Entry("[test_id:5828]deployment", &deploymentRes),
229234
Entry("[test_id:TODO]network policy kube api and dns", &networkPolicyKubeAPIAndDNSRes),
230235
Entry("[test_id:TODO]network policy webhook and metrics", &networkPolicyWebhookAndMetricsRes),
231-
Entry("[test_id:TODO] PodDisruptionBudget", &podDisruptionBudgetRes),
232236
)
237+
238+
It("[test_id:TODO] should set app labels on PodDisruptionBudget", func() {
239+
strategy.SkipUnlessHighlyAvailableTopologyMode()
240+
expectAppLabels(&podDisruptionBudgetRes)
241+
})
233242
})
234243

235244
Context("resource deletion", func() {
@@ -244,8 +253,12 @@ var _ = Describe("Template validator operand", func() {
244253
Entry("[test_id:4924] deployment", &deploymentRes),
245254
Entry("[test_id:TODO] network policy kube api and dns", &networkPolicyKubeAPIAndDNSRes),
246255
Entry("[test_id:TODO] network policy webhook and metrics", &networkPolicyWebhookAndMetricsRes),
247-
Entry("[test_id:TODO] PodDisruptionBudget", &podDisruptionBudgetRes),
248256
)
257+
258+
It("[test_id:TODO] PodDisruptionBudget recreate after delete", decorators.Conformance, func() {
259+
strategy.SkipUnlessHighlyAvailableTopologyMode()
260+
expectRecreateAfterDelete(&podDisruptionBudgetRes)
261+
})
249262
})
250263

251264
Context("resource change", func() {
@@ -259,9 +272,13 @@ var _ = Describe("Template validator operand", func() {
259272
Entry("[test_id:4925] deployment", &deploymentRes),
260273
Entry("[test_id:TODO] network policy kube api and dns", &networkPolicyKubeAPIAndDNSRes),
261274
Entry("[test_id:TODO] network policy webhook and metrics", &networkPolicyWebhookAndMetricsRes),
262-
Entry("[test_id:TODO] PodDisruptionBudget", &podDisruptionBudgetRes),
263275
)
264276

277+
It("[test_id:TODO] should restore modified PodDisruptionBudget", decorators.Conformance, func() {
278+
strategy.SkipUnlessHighlyAvailableTopologyMode()
279+
expectRestoreAfterUpdate(&podDisruptionBudgetRes)
280+
})
281+
265282
Context("with pause", func() {
266283
BeforeEach(func() {
267284
strategy.SkipSspUpdateTestsIfNeeded()
@@ -281,8 +298,12 @@ var _ = Describe("Template validator operand", func() {
281298
Entry("[test_id:5539] deployment", &deploymentRes),
282299
Entry("[test_id:TODO] network policy kube api and dns", &networkPolicyKubeAPIAndDNSRes),
283300
Entry("[test_id:TODO] network policy webhook and metrics", &networkPolicyWebhookAndMetricsRes),
284-
Entry("[test_id:TODO] PodDisruptionBudget", &podDisruptionBudgetRes),
285301
)
302+
303+
It("[test_id:TODO] should restore modified PodDisruptionBudget with pause", decorators.Conformance, func() {
304+
strategy.SkipUnlessHighlyAvailableTopologyMode()
305+
expectRestoreAfterUpdateWithPause(&podDisruptionBudgetRes)
306+
})
286307
})
287308

288309
DescribeTable("should restore modified app labels", expectAppLabelsRestoreAfterUpdate,
@@ -295,8 +316,12 @@ var _ = Describe("Template validator operand", func() {
295316
Entry("[test_id:6209] deployment", &deploymentRes),
296317
Entry("[test_id:TODO] network policy kube api and dns", &networkPolicyKubeAPIAndDNSRes),
297318
Entry("[test_id:TODO] network policy webhook and metrics", &networkPolicyWebhookAndMetricsRes),
298-
Entry("[test_id:TODO] PodDisruptionBudget", &podDisruptionBudgetRes),
299319
)
320+
321+
It("[test_id:TODO] should restore modified app labels on PodDisruptionBudget", func() {
322+
strategy.SkipUnlessHighlyAvailableTopologyMode()
323+
expectAppLabelsRestoreAfterUpdate(&podDisruptionBudgetRes)
324+
})
300325
})
301326

302327
It("[test_id:4913] should successfully start template-validator pod", decorators.Conformance, func() {

0 commit comments

Comments
 (0)