[NET-4144] add gateway pdb #2985

kkavish · 2023-09-21T12:00:03Z

Changes proposed in this PR:

Add PDB for all gateways

How I've tested this PR:
manually

How I expect reviewers to test this PR:

Deploy consul k8s cluster using kind with gateways, check kubectl get poddisruptionbudget -n for PDBs
Unit tests

Checklist:

Tests added
CHANGELOG entry added

- PDB for gateways

- CHANGELOG.md

hashicorp-cla · 2023-09-21T12:00:08Z

All committers have signed the CLA.

- added entry in .changelog folder

Ganeshrockz

Skimmed through the changes and have some basic questions. Will go through the PR again in detail once those queries get resolved.

CHANGELOG.md

.changelog/2985.txt

charts/consul/values.yaml

charts/consul/templates/crd-gateways-disruptionbudget.yaml

- removed entry in .changelog folder - reverted changes in values.yaml done for local testing

Ganeshrockz

Looks good mostly. Deferring the approval to the service-mesh/gateways team who owns the codebase.

.changelog/2985.txt

- correction in .changelog

mr-miles · 2023-09-28T11:31:51Z

Sorry to drop in out of nowhere! But any chance to add a pdb for the ingress gateways too? I know ingress gateways themselves are deprecated for replacement by api-gateways, but they are still supported. Its confusing if the gateways are inconsistent in their config, unless they're going to be removed from the chart completely.

kkavish · 2023-09-28T11:48:20Z

Sorry to drop in out of nowhere! But any chance to add a pdb for the ingress gateways too? I know ingress gateways themselves are deprecated for replacement by api-gateways, but they are still supported. Its confusing if the gateways are inconsistent in their config, unless they're going to be removed from the chart completely.

thanks for suggesting it, makes sense, we'd add it to ingress as well.

- added pdb to ingress gateways - correction in .changelog

nathancoleman · 2023-10-04T14:44:41Z

.changelog/2985.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+pdb: adding pod disruption budget for all the gateways


Suggested change

pdb: adding pod disruption budget for all the gateways

gateways: create PodDisruptionBudget for all gateways

charts/consul/templates/crd-gateways-disruptionbudget.yaml

charts/consul/values.yaml

- modified all PDB configs to be homogenous. - added poddisruptionbudget.go for custom gateways. - correction in .changelog

control-plane/api/v1alpha1/api_gateway_types.go

nathancoleman · 2023-10-13T18:12:19Z

control-plane/api/v1alpha1/api_gateway_types.go

+}
+
+type PodDisruptionBudgetSpec struct {
+	Enabled        bool                `json:"enabled,omitempty"`


I wouldn't expect an Enabled field here. If the field is non-nil then we can assume that it's enabled

my thought process was:

The configs remain same for resources that we create and the resources created through templates. i.e. they all have - enable, minAvailable and maxUnavailable thus making them homogenous.

For the resources we create, if the end-user/dev needs to disable PDB she/he could just go ahead and make enabled = false instead of deleting the whole stanza and adding it back again when they require it.

Shall I go ahead and remove the enabled flag anyways?

control-plane/api-gateway/gatekeeper/gatekeeper.go

control-plane/api-gateway/gatekeeper/gatekeeper_test.go

control-plane/api-gateway/gatekeeper/poddisruptionbudget.go

- updating PDB to follow the pattern in role.go

david-yu · 2023-10-24T17:03:26Z

@nathancoleman I chatted with @kkavish he said this might need re-review? However I'm not sure if it's ready given that it looks like most of the tests are still failing?

nathancoleman · 2023-10-24T22:06:02Z

Re-reviewing is on my To Do list; however, it's lower in priority than work related to Consul 1.17.0 releasing next week. I confirmed with @kkavish that we're not trying to get this change into 1.17.0.

david-yu · 2023-10-24T22:08:44Z

Sure not a rush. Let's revisit after 1.17, I also asked @kkavish to brush up this PR to get the tests to pass as well.

david-yu · 2023-11-08T07:51:51Z

@kkavish Could you try re-basing to get the unit tests on control plane to pass?

- disabling pdbs in values.yaml

- data plane image

- running tests

# Conflicts: # charts/consul/values.yaml

- running tests

david-yu · 2023-12-01T19:47:18Z

I'm guess the PR is still failing to pass any acceptance tests?

kkavish · 2023-12-04T10:00:10Z

I'm guess the PR is still failing to pass any acceptance tests?

@david-yu sorry, I've been on a vacation, I'll try to get this fixed this week.

- running tests

# Conflicts: # charts/consul/templates/crd-gatewayclassconfigs.yaml

- running tests

kkavish added 3 commits September 20, 2023 12:08

# NET-4144

fcf1462

- PDB for gateways

# NET-4144

598cb7c

- PDB for gateways

# NET-4144

b32c35c

- CHANGELOG.md

kkavish added the pr/no-backport signals that a PR will not contain a backport label label Sep 21, 2023

# NET-4144

c89c398

- added entry in .changelog folder

kkavish changed the title ~~Net 4144 gateway pdb~~ [NET-4144] add gateway pdb Sep 21, 2023

kkavish added the consul-india PRs/Issues assigned to Consul India team label Sep 21, 2023

kkavish requested review from curtbushko, nathancoleman and wilkermichael September 21, 2023 12:19

Ganeshrockz reviewed Sep 22, 2023

View reviewed changes

# NET-4144

b136d83

- removed entry in .changelog folder - reverted changes in values.yaml done for local testing

Ganeshrockz reviewed Sep 22, 2023

View reviewed changes

.changelog/2985.txt Outdated Show resolved Hide resolved

# NET-4144

d894409

- correction in .changelog

# NET-4144

1b97827

- added pdb to ingress gateways - correction in .changelog

nathancoleman requested changes Oct 4, 2023

View reviewed changes

# NET-4144

9669b7c

- modified all PDB configs to be homogenous. - added poddisruptionbudget.go for custom gateways. - correction in .changelog

nathancoleman requested changes Oct 13, 2023

View reviewed changes

kkavish added 2 commits October 15, 2023 23:29

# NET-4144

7f85323

- updating PDB to follow the pattern in role.go

# NET-4144

41aa3c1

- updating PDB to follow the pattern in role.go

curtbushko removed request for curtbushko and wilkermichael October 27, 2023 19:01

Merge branch 'main' into net-4144-gateway-pdb

c309d18

kkavish added 5 commits November 8, 2023 15:46

# NET-4144

e6cb2d4

- disabling pdbs in values.yaml

# NET-4144

c4eae25

- data plane image

# NET-4144

ed84571

- running tests

Merge branch 'main' into net-4144-gateway-pdb

82efbfd

# Conflicts: # charts/consul/values.yaml

# NET-4144

0a9259f

- running tests

david-yu linked an issue Nov 14, 2023 that may be closed by this pull request

No way to add pdb to gateways #2162

Open

kkavish added 6 commits November 14, 2023 23:24

# NET-4144

0bbb085

- running tests

# NET-4144

5e64768

- running tests

# NET-4144

0dd6470

- running tests

# NET-4144

57459c8

- running tests

# NET-4144

fb32ec4

- running tests

# NET-4144

c50f91e

- running tests

kkavish and others added 4 commits December 4, 2023 15:54

Merge branch 'main' into net-4144-gateway-pdb

f75df38

# NET-4144

586d11d

- running tests

Merge branch 'main' into net-4144-gateway-pdb

3630396

# Conflicts: # charts/consul/templates/crd-gatewayclassconfigs.yaml

# NET-4144

458df74

- running tests

kkavish closed this Dec 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NET-4144] add gateway pdb #2985

[NET-4144] add gateway pdb #2985

kkavish commented Sep 21, 2023 •

edited

Loading

hashicorp-cla commented Sep 21, 2023 •

edited

Loading

Ganeshrockz left a comment

Ganeshrockz left a comment •

edited

Loading

mr-miles commented Sep 28, 2023

kkavish commented Sep 28, 2023

nathancoleman Oct 4, 2023

nathancoleman Oct 13, 2023

kkavish Oct 15, 2023

david-yu commented Oct 24, 2023

nathancoleman commented Oct 24, 2023

david-yu commented Oct 24, 2023

david-yu commented Nov 8, 2023

david-yu commented Dec 1, 2023

kkavish commented Dec 4, 2023

		@@ -0,0 +1,3 @@
		```release-note:improvement
		pdb: adding pod disruption budget for all the gateways

	pdb: adding pod disruption budget for all the gateways
	gateways: create PodDisruptionBudget for all gateways

[NET-4144] add gateway pdb #2985

[NET-4144] add gateway pdb #2985

Conversation

kkavish commented Sep 21, 2023 • edited Loading

hashicorp-cla commented Sep 21, 2023 • edited Loading

Ganeshrockz left a comment

Choose a reason for hiding this comment

Ganeshrockz left a comment • edited Loading

Choose a reason for hiding this comment

mr-miles commented Sep 28, 2023

kkavish commented Sep 28, 2023

nathancoleman Oct 4, 2023

Choose a reason for hiding this comment

nathancoleman Oct 13, 2023

Choose a reason for hiding this comment

kkavish Oct 15, 2023

Choose a reason for hiding this comment

david-yu commented Oct 24, 2023

nathancoleman commented Oct 24, 2023

david-yu commented Oct 24, 2023

david-yu commented Nov 8, 2023

david-yu commented Dec 1, 2023

kkavish commented Dec 4, 2023

kkavish commented Sep 21, 2023 •

edited

Loading

hashicorp-cla commented Sep 21, 2023 •

edited

Loading

Ganeshrockz left a comment •

edited

Loading