🐛 Fix secret selection logic for ownerRef test #7973
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/test ? |
|
@killianmuldoon: The following commands are available to trigger required jobs:
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/test pull-cluster-api-e2e-full-main |
killianmuldoon
force-pushed
the
fix-ownerRef-secrets
branch
3 times, most recently
from
5541888 to
11435e7
Compare
k8s-ci-robot
added
size/M
killianmuldoon
changed the title
killianmuldoon
force-pushed
the
fix-ownerRef-secrets
branch
from
11435e7 to
2eb9432
Compare
| @@ -79,3 +96,12 @@ func GetOwnerGraph(namespace, kubeconfigPath string) (OwnerGraph, error) { | |||
| } | |||
| return owners, nil | |||
| } | |||
|
|
|||
| // isClusterSecret checks whether a Secret is related to a CAPI Cluster by checking if the secret type is ClusterSecretType. | |||
| func isClusterSecret(ref corev1.ObjectReference, c client.Client) (bool, error) { | |||
There was a problem hiding this comment.
I've tested this locally with Kubernetes 1.20.15 and it works. I think this approach makes sense for what we're currently testing, but we might want to be careful of expectations if providers pick up this test.
killianmuldoon
force-pushed
the
fix-ownerRef-secrets
branch
from
2eb9432 to
8abbaf4
Compare
killianmuldoon
force-pushed
the
fix-ownerRef-secrets
branch
from
8abbaf4 to
4f4b730
Compare
|
/retest /lgtm Thx for fixing this so quickly! |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 43780247d54c0d5133e1332a19a4271cadf03e36
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
@killianmuldoon As you mentioned it in Slack, WDYT about adding a |
|
@killianmuldoon sorry for getting late at this, but have you consider to use the concept of "tenant" cluster already embedded inside discovery, so we have this logic implemented in only one place? |
|
@fabriziopandini Sorry I already merged to get CI green again. But let's open a follow-up PR if we want to refactor. |
|
Opened #7976 - thanks @fabriziopandini - it's much cleaner! |
Fixes a failure in the ownerReference e2e test for Kubernetes versions below v1.24. This ensures that the secrets considered for the ownerReference test are only those directly associated with the Cluster.