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.

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

fix: fix enforcing up to date ControlPlane's ClusterRole and ClusterRoleBinding #11

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented Mar 15, 2024

What this PR does / why we need it:

There was an error in the patching logic were the controller would try to patch using a generated resource and this getting:

2024-03-14T17:43:26Z	ERROR	Reconciler error	{"controller": "controlplane", "controllerGroup": "gateway-operator.konghq.com", "controllerKind": "ControlPlane", "ControlPlane": {"name":"aigateway-test-8cnvx","namespace":"4e1a7586-c08c-4d11-b421-763dad2f6767"}, "namespace": "4e1a7586-c08c-4d11-b421-763dad2f6767", "name": "aigateway-test-8cnvx", "reconcileID": "17f23e6e-b72b-4036-8bf9-4cb45cd00eec", "error": "failed patching ControlPlane's ClusterRoleBinding controlplane-aigateway-test-8cnvx-rgwnj: the name of the object (controlplane-aigateway-test-8cnvx-rgwnj based on URL) was undeterminable: name must be provided"}
failed patching ControlPlane's ClusterRoleBinding controlplane-aigateway-test-8cnvx-rgwnj: the name of the object (controlplane-aigateway-test-8cnvx-rgwnj based on URL) was undeterminable: name must be provided

This PR fixes that both for ClusterRole and ClusterRoleBinding.

This should fix the flakiness in several of the integration tests that deploy and update ControlPlanes (directly or indirectly).

Exemplar failure on CI: https://github.com/Kong/gateway-operator-archive/actions/runs/8285193525/job/22672382974?pr=1592#step:4:3602

To reproduce that locally it's enough to edit either of those resources on the cluster. Either change one of the ClusterRole's policy rules or ClusterRoleBinding's subjects.

The test that's in place

func TestEnsureClusterRoleBinding(t *testing.T) {
did not catch that because fake client doesn't detect this type of errors.

@pmalek pmalek self-assigned this Mar 15, 2024
@pmalek pmalek requested a review from a team March 15, 2024 13:16
@pmalek pmalek marked this pull request as ready for review March 15, 2024 13:16
@pmalek pmalek merged commit 00f85d5 into main Mar 15, 2024
13 checks passed
@pmalek pmalek deleted the fix-enforcing-up-to-date-controlplanes-cluster-role-and-cluster-role-binding branch March 15, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants