✨ e2e Test: Ensure OwnerRefs are consistently reconciled #7606

killianmuldoon · 2022-11-23T18:17:49Z

Add a new e2e test and set of helper functions to test that ownerReferences are correctly reconciled by Cluster API.

This test is designed to capture how Cluster API creates ownerReferences with a newly created Topology based Cluster using KCP + KubeadmBooststrap + Docker infrastructure.

It also tests how ownerReferences are reconciled after they've been removed and re-reconciled.

Fixes #7575

killianmuldoon · 2022-11-23T18:19:43Z

/test pull-cluster-api-e2e-full-main

killianmuldoon · 2022-11-23T18:21:03Z

/hold

This currently contains all of the commits required to make the test pass. Let's merge it once it's green against main.

killianmuldoon · 2022-11-24T11:49:16Z

/test pull-cluster-api-e2e-full-main

killianmuldoon · 2022-11-24T16:10:22Z

/test pull-cluster-api-e2e-full-main

killianmuldoon · 2022-11-28T12:34:45Z

/test pull-cluster-api-e2e-full-main

killianmuldoon · 2022-11-28T12:34:53Z

/retest

killianmuldoon · 2023-01-18T15:07:10Z

/remove-hold

fabriziopandini · 2023-01-19T22:21:03Z

great work!
only one last note about being really specific about the target use case of GetOwnerGraph, otherwise lgtm

sbueringer

Very nice work!!

cmd/clusterctl/client/cluster/ownergraph.go

sbueringer · 2023-01-20T13:29:09Z

test/framework/ownerreference_helpers.go

+	}
+	Eventually(func() error {
+		graph, err := clusterctlcluster.GetOwnerGraph(namespace, kubeconfigPath)
+		Expect(err).To(BeNil())


I assume we intentionally want to fail here if we get an error once? (and not retry with eventually)

Yeah - actually this is running retries within the func so this is unnecessary.

Looks like the test is flaky if Eventually isn't included because of different order / speed of the reconcilers.

test/framework/ownerreference_helpers.go

cmd/clusterctl/client/cluster/ownergraph.go

test/e2e/quick_start_test.go

sbueringer · 2023-01-20T14:25:01Z

Thx!

/lgtm
/pull-cluster-api-e2e-full-main
(a final one after the last changes)

k8s-ci-robot · 2023-01-20T14:25:07Z

LGTM label has been added.

Git tree hash: ced47f3dcd788c9c2f47c340d62eb20348adbbca

killianmuldoon · 2023-01-20T14:28:43Z

/test pull-cluster-api-e2e-full-main

Signed-off-by: killianmuldoon <kmuldoon@vmware.com>

sbueringer · 2023-01-20T15:05:47Z

@killianmuldoon Maybe dropping the eventually caused the errors?

killianmuldoon · 2023-01-20T15:13:31Z

Yup - #7606 (comment)

Re-running with Eventually added back in now.

killianmuldoon · 2023-01-20T15:13:53Z

/test pull-cluster-api-e2e-full-main

killianmuldoon · 2023-01-20T16:51:43Z

@sbueringer all green 🙂

sbueringer · 2023-01-20T16:55:30Z

Perfect! Thx

/lgtm
/approve

k8s-ci-robot · 2023-01-20T16:55:36Z

LGTM label has been added.

Git tree hash: 118bf208054057fab42efb592da5b435277899f7

fabriziopandini · 2023-01-20T17:01:52Z

/lgtm
/approve

Yay! 🥳

k8s-ci-robot · 2023-01-20T17:02:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini, 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:

~~OWNERS~~ [fabriziopandini,sbueringer]

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

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 23, 2022

k8s-ci-robot requested review from enxebre and JoelSpeed November 23, 2022 18:18

killianmuldoon mentioned this pull request Nov 23, 2022

[WIP] 🌱 add e2e test for ownerRef reconciliation #7569

Closed

killianmuldoon force-pushed the ownerRef-test branch from 7c60f4f to 91bfd4e Compare November 23, 2022 18:20

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 23, 2022

killianmuldoon changed the title ~~✨ e2e Test: Ensure OwnerRefs are consistently reconciled~~ ✨ [WIP] e2e Test: Ensure OwnerRefs are consistently reconciled Nov 23, 2022

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 23, 2022

killianmuldoon mentioned this pull request Nov 24, 2022

🐛 Fix Machine adoption for KCP/MachineSet-owned Machines #7591

Merged

15 tasks

killianmuldoon force-pushed the ownerRef-test branch from 91bfd4e to 6541a82 Compare November 24, 2022 11:42

killianmuldoon force-pushed the ownerRef-test branch from 6541a82 to 4a0c675 Compare November 24, 2022 16:08

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 25, 2022

killianmuldoon force-pushed the ownerRef-test branch from 4a0c675 to 3dab970 Compare November 28, 2022 11:31

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 28, 2022

killianmuldoon force-pushed the ownerRef-test branch 2 times, most recently from 2ccc869 to 4e7458a Compare November 28, 2022 11:46

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 28, 2022

killianmuldoon force-pushed the ownerRef-test branch from cb5ec04 to 2b18923 Compare November 28, 2022 12:34

killianmuldoon force-pushed the ownerRef-test branch from 2b18923 to 70d70aa Compare November 28, 2022 13:52

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 18, 2023

killianmuldoon force-pushed the ownerRef-test branch from 008ea62 to 884d30b Compare January 20, 2023 10:48

sbueringer reviewed Jan 20, 2023

View reviewed changes

cmd/clusterctl/client/cluster/ownergraph.go Outdated Show resolved Hide resolved

test/e2e/quick_start_test.go Show resolved Hide resolved

killianmuldoon force-pushed the ownerRef-test branch 2 times, most recently from c676f9d to 319e024 Compare January 20, 2023 13:57

k8s-ci-robot assigned sbueringer Jan 20, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 20, 2023

add initial test for ownerRefs

fef6d5c

Signed-off-by: killianmuldoon <kmuldoon@vmware.com>

killianmuldoon force-pushed the ownerRef-test branch from 319e024 to fef6d5c Compare January 20, 2023 14:52

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 20, 2023

k8s-ci-robot requested a review from sbueringer January 20, 2023 14:52

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 20, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 20, 2023

k8s-ci-robot assigned fabriziopandini Jan 20, 2023

k8s-ci-robot merged commit 42063b7 into kubernetes-sigs:main Jan 20, 2023

k8s-ci-robot added this to the v1.4 milestone Jan 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ e2e Test: Ensure OwnerRefs are consistently reconciled #7606

✨ e2e Test: Ensure OwnerRefs are consistently reconciled #7606

killianmuldoon commented Nov 23, 2022

killianmuldoon commented Nov 23, 2022

killianmuldoon commented Nov 23, 2022

killianmuldoon commented Nov 24, 2022

killianmuldoon commented Nov 24, 2022

killianmuldoon commented Nov 28, 2022

killianmuldoon commented Nov 28, 2022

killianmuldoon commented Jan 18, 2023

fabriziopandini commented Jan 19, 2023

sbueringer left a comment

sbueringer Jan 20, 2023

killianmuldoon Jan 20, 2023

killianmuldoon Jan 20, 2023

sbueringer commented Jan 20, 2023

k8s-ci-robot commented Jan 20, 2023

killianmuldoon commented Jan 20, 2023

sbueringer commented Jan 20, 2023 •

edited

killianmuldoon commented Jan 20, 2023

killianmuldoon commented Jan 20, 2023

killianmuldoon commented Jan 20, 2023

sbueringer commented Jan 20, 2023

k8s-ci-robot commented Jan 20, 2023

fabriziopandini commented Jan 20, 2023

k8s-ci-robot commented Jan 20, 2023

✨ e2e Test: Ensure OwnerRefs are consistently reconciled #7606

✨ e2e Test: Ensure OwnerRefs are consistently reconciled #7606

Conversation

killianmuldoon commented Nov 23, 2022

killianmuldoon commented Nov 23, 2022

killianmuldoon commented Nov 23, 2022

killianmuldoon commented Nov 24, 2022

killianmuldoon commented Nov 24, 2022

killianmuldoon commented Nov 28, 2022

killianmuldoon commented Nov 28, 2022

killianmuldoon commented Jan 18, 2023

fabriziopandini commented Jan 19, 2023

sbueringer left a comment

Choose a reason for hiding this comment

sbueringer Jan 20, 2023

Choose a reason for hiding this comment

killianmuldoon Jan 20, 2023

Choose a reason for hiding this comment

killianmuldoon Jan 20, 2023

Choose a reason for hiding this comment

sbueringer commented Jan 20, 2023

k8s-ci-robot commented Jan 20, 2023

killianmuldoon commented Jan 20, 2023

sbueringer commented Jan 20, 2023 • edited

killianmuldoon commented Jan 20, 2023

killianmuldoon commented Jan 20, 2023

killianmuldoon commented Jan 20, 2023

sbueringer commented Jan 20, 2023

k8s-ci-robot commented Jan 20, 2023

fabriziopandini commented Jan 20, 2023

k8s-ci-robot commented Jan 20, 2023

sbueringer commented Jan 20, 2023 •

edited