Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support PDB for egress #275

Merged
merged 5 commits into from
Apr 10, 2024
Merged

Support PDB for egress #275

merged 5 commits into from
Apr 10, 2024

Conversation

tkna
Copy link
Contributor

@tkna tkna commented Mar 6, 2024

Fixes: #219

This PR adds PDB support for Egress pods.
PDB for egress NAT pods is created by specifying spec.podDisruptionBudget like below.

apiVersion: coil.cybozu.com/v2
kind: Egress
metadata:
  namespace: other-network
  name: egress
spec: 
  podDisruptionBudget:
    maxUnavailable: 1

@tkna tkna self-assigned this Mar 6, 2024
@tkna tkna requested review from terassyi, yokaze and chez-shanpu March 6, 2024 06:21
chez-shanpu
chez-shanpu reviewed Mar 7, 2024
View reviewed changes
v2/controllers/egress_controller.go Outdated Show resolved Hide resolved
v2/api/v2/egress_types.go Outdated Show resolved Hide resolved
v2/controllers/egress_controller.go Outdated Show resolved Hide resolved
v2/controllers/egress_controller.go Show resolved Hide resolved
v2/api/v2/egress_types.go Outdated Show resolved Hide resolved
yokaze
yokaze requested changes Mar 28, 2024
View reviewed changes
v2/config/rbac/coil-controller_role.yaml Show resolved Hide resolved
v2/controllers/egress_controller_test.go Show resolved Hide resolved
v2/api/v2/egress_types.go Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
v2/api/v2/egress_types.go Outdated Show resolved Hide resolved
v2/controllers/egress_controller.go Show resolved Hide resolved
chez-shanpu
chez-shanpu reviewed Apr 3, 2024
View reviewed changes
v2/controllers/egress_controller.go Outdated Show resolved Hide resolved
v2/controllers/egress_controller.go Show resolved Hide resolved
v2/controllers/egress_controller.go Show resolved Hide resolved
yokaze
yokaze requested changes Apr 3, 2024
View reviewed changes
docs/usage.md Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
v2/api/v2/egress_types.go Outdated Show resolved Hide resolved
v2/api/v2/egress_types.go Show resolved Hide resolved
v2/e2e/coil_test.go Outdated
@@ -272,9 +280,25 @@ var _ = Describe("Coil", func() {
}
return int(depl.Status.ReadyReplicas)
}).Should(Equal(2))

By("deleting Egress")
kubectlSafe(nil, "delete", "-f", "manifests/egress.yaml")
Copy link
Contributor

Choose a reason for hiding this comment

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

Now manifests/egress.yaml includes internet Namespace, so deleting the namespace should trigger the PDB deletion.
I would like to confirm the PDB to be deleted without deleting the entire Namespace. Is it possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

0e44d8d

tkna added 4 commits April 4, 2024 07:09
@tkna
Support PDB for egress
a661931 
Signed-off-by: naoki-take <[email protected]>
@tkna
Reflect comments
6883ead 
Signed-off-by: naoki-take <[email protected]>
@tkna
Add PDB deletion test
2c86454 
Signed-off-by: naoki-take <[email protected]>
@tkna
Reflect comments
ba71ceb 
Signed-off-by: naoki-take <[email protected]>
@tkna
Update go.mod and go.sum
ad8fa7c 
Signed-off-by: naoki-take <[email protected]>
@chez-shanpu chez-shanpu self-requested a review April 5, 2024 07:37
chez-shanpu
chez-shanpu approved these changes Apr 5, 2024
View reviewed changes
Copy link
Contributor

@chez-shanpu chez-shanpu left a comment

Choose a reason for hiding this comment

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

LGTM

@chez-shanpu chez-shanpu requested a review from yokaze April 5, 2024 07:40
terassyi
terassyi approved these changes Apr 8, 2024
View reviewed changes
Copy link
Contributor

@terassyi terassyi left a comment

Choose a reason for hiding this comment

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

LGTM

yokaze
yokaze approved these changes Apr 9, 2024
View reviewed changes
Copy link
Contributor

@yokaze yokaze left a comment

Choose a reason for hiding this comment

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

LGTM

@tkna tkna merged commit f70825d into main Apr 10, 2024
7 checks passed
@tkna tkna deleted the add-pdb branch April 10, 2024 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@terassyi terassyi terassyi approved these changes

@chez-shanpu chez-shanpu chez-shanpu approved these changes

@yokaze yokaze yokaze approved these changes

Assignees

@tkna tkna

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create PDB for Egress NAT pods
4 participants
@tkna @yokaze @chez-shanpu @terassyi