Skip to content

Delete pod disruption budget logic#376

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
Miciah:delete-pod-disruption-budget-logic
Mar 27, 2020
Merged

Delete pod disruption budget logic#376
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
Miciah:delete-pod-disruption-budget-logic

Conversation

@Miciah
Copy link
Contributor

@Miciah Miciah commented Mar 14, 2020

#272 in 4.2 added logic to create pod disruption budgets. We subsequently decided to remove pod disruption budgets because they caused problems for draining and for small clusters, and so #323 in 4.3 changed the logic to remove any existing pod disruption budget. The logic is consequently unnecessary in 4.4 and later.

  • manifests/00-cluster-role.yaml: Delete rule to allow the operator to manage the poddisruptionbudgets resource.
  • pkg/manifests/bindata.go: Regenerate.
  • pkg/operator/controller/ingress/controller.go (ensureIngressController): Delete call to ensureRouterPodDisruptionBudget.
  • pkg/operator/controller/ingress/poddisruptionbudget.go: Delete file.
    (ensureRouterPodDisruptionBudget, desiredRouterPodDisruptionBudget, currentRouterPodDisruptionBudget, createRouterPodDisruptionBudget, deleteRouterPodDisruptionBudget, updateRouterPodDisruptionBudget, podDisruptionBudgetChanged): Delete.
  • pkg/operator/controller/names.go (RouterPodDisruptionBudgetName): Delete function.
  • test/e2e/operator_test.go (TestPodDisruptionBudgetDoesNotExist): Delete test.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 14, 2020
Commit 35d4f4f in 4.2 added logic to
create pod disruption budgets.  We subsequently decided to remove pod
disruption budgets because they caused problems for draining and for small
clusters, and so commit f48a2f9 in 4.3
changed the logic to remove any existing pod disruption budget.  The logic
is consequently unnecessary in 4.4 and later.

* manifests/00-cluster-role.yaml: Delete rule to allow the operator to
manage the poddisruptionbudgets resource.
* pkg/manifests/bindata.go: Regenerate.
* pkg/operator/controller/ingress/controller.go (ensureIngressController):
Delete call to ensureRouterPodDisruptionBudget.
* pkg/operator/controller/ingress/poddisruptionbudget.go: Delete file.
(ensureRouterPodDisruptionBudget, desiredRouterPodDisruptionBudget)
(currentRouterPodDisruptionBudget, createRouterPodDisruptionBudget)
(deleteRouterPodDisruptionBudget, updateRouterPodDisruptionBudget)
(podDisruptionBudgetChanged): Delete.
* pkg/operator/controller/names.go (RouterPodDisruptionBudgetName): Delete
function.
* test/e2e/operator_test.go (TestPodDisruptionBudgetDoesNotExist): Delete
test.
@Miciah Miciah force-pushed the delete-pod-disruption-budget-logic branch from 3250ef7 to 0241c7b Compare March 14, 2020 04:36
- poddisruptionbudgets
verbs:
- "*"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will there be some window where, once this is applied, existing operator process reconciliations could fail to apply the PDB due to RBAC denials? Even if so I'm not sure it matters

@ironcladlou
Copy link
Contributor

/lgtm
/retest

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ironcladlou, Miciah

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 27, 2020
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 05b0a5e into openshift:master Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants