Skip to content

Commit ac5e546

Browse files
authored
Merge pull request kubernetes-sigs#9471 from adityabhatia/addFinalizerTests
🌱 Ensure finalizers are resilient on reconciliation
2 parents a71f31b + 5126180 commit ac5e546

File tree

4 files changed

+207
-3
lines changed

4 files changed

+207
-3
lines changed

cmd/clusterctl/client/cluster/ownergraph.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func GetOwnerGraph(ctx context.Context, namespace, kubeconfigPath string) (Owner
5656
}
5757

5858
// graph.Discovery can not be used here as it will use the latest APIVersion for ownerReferences - not those
59-
// present in the object 'metadata.ownerReferences`.
59+
// present in the object `metadata.ownerReferences`.
6060
owners, err := discoverOwnerGraph(ctx, namespace, graph)
6161
if err != nil {
6262
return OwnerGraph{}, errors.Wrap(err, "failed to discovery ownerGraph types")
@@ -107,7 +107,7 @@ func discoverOwnerGraph(ctx context.Context, namespace string, o *objectGraph) (
107107
continue
108108
}
109109
// Exclude the default service account from the owner graph.
110-
// This Secret is not longer generated by default in Kubernetes 1.24+.
110+
// This Secret is no longer generated by default in Kubernetes 1.24+.
111111
// This is not a CAPI related Secret, so it can be ignored.
112112
if obj.GetKind() == "Secret" && strings.Contains(obj.GetName(), "default-token") {
113113
continue

test/e2e/quick_start_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,3 +181,38 @@ var _ = Describe("When following the Cluster API quick-start with dualstack and
181181
}
182182
})
183183
})
184+
185+
var _ = Describe("When following the Cluster API quick-start check finalizers resilience after deletion", func() {
186+
QuickStartSpec(ctx, func() QuickStartSpecInput {
187+
return QuickStartSpecInput{
188+
E2EConfig: e2eConfig,
189+
ClusterctlConfigPath: clusterctlConfigPath,
190+
BootstrapClusterProxy: bootstrapClusterProxy,
191+
ArtifactFolder: artifactFolder,
192+
SkipCleanup: skipCleanup,
193+
InfrastructureProvider: pointer.String("docker"),
194+
PostMachinesProvisioned: func(proxy framework.ClusterProxy, namespace, clusterName string) {
195+
// This check ensures that finalizers are resilient - i.e. correctly re-reconciled - when removed.
196+
framework.ValidateFinalizersResilience(ctx, proxy, namespace, clusterName)
197+
},
198+
}
199+
})
200+
})
201+
202+
var _ = Describe("When following the Cluster API quick-start with ClusterClass check finalizers resilience after deletion [ClusterClass]", func() {
203+
QuickStartSpec(ctx, func() QuickStartSpecInput {
204+
return QuickStartSpecInput{
205+
E2EConfig: e2eConfig,
206+
ClusterctlConfigPath: clusterctlConfigPath,
207+
BootstrapClusterProxy: bootstrapClusterProxy,
208+
ArtifactFolder: artifactFolder,
209+
SkipCleanup: skipCleanup,
210+
Flavor: pointer.String("topology"),
211+
InfrastructureProvider: pointer.String("docker"),
212+
PostMachinesProvisioned: func(proxy framework.ClusterProxy, namespace, clusterName string) {
213+
// This check ensures that finalizers are resilient - i.e. correctly re-reconciled - when removed.
214+
framework.ValidateFinalizersResilience(ctx, proxy, namespace, clusterName)
215+
},
216+
}
217+
})
218+
})

test/framework/finalizers_helpers.go

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
/*
2+
Copyright 2023 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package framework
18+
19+
import (
20+
"context"
21+
"fmt"
22+
"reflect"
23+
"time"
24+
25+
. "github.com/onsi/gomega"
26+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
27+
"k8s.io/apimachinery/pkg/types"
28+
kerrors "k8s.io/apimachinery/pkg/util/errors"
29+
"sigs.k8s.io/controller-runtime/pkg/client"
30+
31+
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
32+
clusterctlcluster "sigs.k8s.io/cluster-api/cmd/clusterctl/client/cluster"
33+
controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
34+
addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1"
35+
infrav1 "sigs.k8s.io/cluster-api/test/infrastructure/docker/api/v1beta1"
36+
"sigs.k8s.io/cluster-api/util/patch"
37+
)
38+
39+
// finalizerAssertion contains a list of expected Finalizers corresponding to a resource Kind.
40+
var finalizerAssertion = map[string][]string{
41+
"Cluster": {clusterv1.ClusterFinalizer},
42+
"Machine": {clusterv1.MachineFinalizer},
43+
"MachineSet": {clusterv1.MachineSetTopologyFinalizer},
44+
"MachineDeployment": {clusterv1.MachineDeploymentTopologyFinalizer},
45+
"ClusterResourceSet": {addonsv1.ClusterResourceSetFinalizer},
46+
"DockerMachine": {infrav1.MachineFinalizer},
47+
"DockerCluster": {infrav1.ClusterFinalizer},
48+
"KubeadmControlPlane": {controlplanev1.KubeadmControlPlaneFinalizer},
49+
}
50+
51+
// ValidateFinalizersResilience checks that expected finalizers are in place, deletes them, and verifies that expected finalizers are properly added again.
52+
func ValidateFinalizersResilience(ctx context.Context, proxy ClusterProxy, namespace, clusterName string) {
53+
clusterKey := client.ObjectKey{Namespace: namespace, Name: clusterName}
54+
55+
// Collect all objects where finalizers were initially set
56+
objectsWithFinalizers := getObjectsWithFinalizers(ctx, proxy, namespace)
57+
58+
// Setting the paused property on the Cluster resource will pause reconciliations, thereby having no effect on Finalizers.
59+
// This also makes debugging easier.
60+
setClusterPause(ctx, proxy.GetClient(), clusterKey, true)
61+
62+
// We are testing the worst-case scenario, i.e. all finalizers are deleted.
63+
// Once all Clusters are paused remove all the Finalizers from all objects in the graph.
64+
// The reconciliation loop should be able to recover from this, by adding the required Finalizers back.
65+
removeFinalizers(ctx, proxy, namespace)
66+
67+
// Unpause the cluster.
68+
setClusterPause(ctx, proxy.GetClient(), clusterKey, false)
69+
70+
// Annotate the MachineDeployment to speed up reconciliation. This ensures MachineDeployment topology Finalizers are re-reconciled.
71+
// TODO: Remove this as part of https://github.com/kubernetes-sigs/cluster-api/issues/9532
72+
forceMachineDeploymentTopologyReconcile(ctx, proxy.GetClient(), clusterKey)
73+
74+
// Check that the Finalizers are as expected after further reconciliations.
75+
assertFinalizersExist(ctx, proxy, namespace, objectsWithFinalizers)
76+
}
77+
78+
// removeFinalizers removes all Finalizers from objects in the owner graph.
79+
func removeFinalizers(ctx context.Context, proxy ClusterProxy, namespace string) {
80+
graph, err := clusterctlcluster.GetOwnerGraph(ctx, namespace, proxy.GetKubeconfigPath())
81+
Expect(err).ToNot(HaveOccurred())
82+
for _, object := range graph {
83+
ref := object.Object
84+
obj := new(unstructured.Unstructured)
85+
obj.SetAPIVersion(ref.APIVersion)
86+
obj.SetKind(ref.Kind)
87+
obj.SetName(ref.Name)
88+
89+
Expect(proxy.GetClient().Get(ctx, client.ObjectKey{Namespace: namespace, Name: object.Object.Name}, obj)).To(Succeed())
90+
helper, err := patch.NewHelper(obj, proxy.GetClient())
91+
Expect(err).ToNot(HaveOccurred())
92+
obj.SetFinalizers([]string{})
93+
Expect(helper.Patch(ctx, obj)).To(Succeed())
94+
}
95+
}
96+
97+
func getObjectsWithFinalizers(ctx context.Context, proxy ClusterProxy, namespace string) map[string]*unstructured.Unstructured {
98+
graph, err := clusterctlcluster.GetOwnerGraph(ctx, namespace, proxy.GetKubeconfigPath())
99+
Expect(err).ToNot(HaveOccurred())
100+
101+
objsWithFinalizers := map[string]*unstructured.Unstructured{}
102+
103+
for _, node := range graph {
104+
nodeNamespacedName := client.ObjectKey{Namespace: node.Object.Namespace, Name: node.Object.Name}
105+
obj := &unstructured.Unstructured{}
106+
obj.SetAPIVersion(node.Object.APIVersion)
107+
obj.SetKind(node.Object.Kind)
108+
err = proxy.GetClient().Get(ctx, nodeNamespacedName, obj)
109+
Expect(err).ToNot(HaveOccurred())
110+
111+
setFinalizers := obj.GetFinalizers()
112+
113+
if len(setFinalizers) > 0 {
114+
objsWithFinalizers[client.ObjectKey{Namespace: node.Object.Namespace, Name: node.Object.Name}.String()] = obj
115+
}
116+
}
117+
118+
return objsWithFinalizers
119+
}
120+
121+
// assertFinalizersExist ensures that current Finalizers match those in the initialObjectsWithFinalizers.
122+
func assertFinalizersExist(ctx context.Context, proxy ClusterProxy, namespace string, initialObjsWithFinalizers map[string]*unstructured.Unstructured) {
123+
Eventually(func() error {
124+
var allErrs []error
125+
finalObjsWithFinalizers := getObjectsWithFinalizers(ctx, proxy, namespace)
126+
127+
for objNamespacedName, obj := range initialObjsWithFinalizers {
128+
// verify if finalizers for this resource were set on reconcile
129+
if _, valid := finalObjsWithFinalizers[objNamespacedName]; !valid {
130+
allErrs = append(allErrs, fmt.Errorf("no finalizers set for %s/%s",
131+
obj.GetKind(), objNamespacedName))
132+
continue
133+
}
134+
135+
// verify if this resource has the appropriate Finalizers set
136+
expectedFinalizers, assert := finalizerAssertion[obj.GetKind()]
137+
if !assert {
138+
continue
139+
}
140+
141+
setFinalizers := finalObjsWithFinalizers[objNamespacedName].GetFinalizers()
142+
if !reflect.DeepEqual(expectedFinalizers, setFinalizers) {
143+
allErrs = append(allErrs, fmt.Errorf("expected finalizers do not exist for %s/%s: expected: %v, found: %v",
144+
obj.GetKind(), objNamespacedName, expectedFinalizers, setFinalizers))
145+
}
146+
}
147+
148+
return kerrors.NewAggregate(allErrs)
149+
}).WithTimeout(1 * time.Minute).WithPolling(2 * time.Second).Should(Succeed())
150+
}
151+
152+
// forceMachineDeploymentTopologyReconcile forces reconciliation of the MachineDeployment.
153+
func forceMachineDeploymentTopologyReconcile(ctx context.Context, cli client.Client, clusterKey types.NamespacedName) {
154+
mdList := &clusterv1.MachineDeploymentList{}
155+
clientOptions := (&client.ListOptions{}).ApplyOptions([]client.ListOption{
156+
client.MatchingLabels{clusterv1.ClusterNameLabel: clusterKey.Name},
157+
})
158+
Expect(cli.List(ctx, mdList, clientOptions)).To(Succeed())
159+
160+
for i := range mdList.Items {
161+
if _, ok := mdList.Items[i].GetLabels()[clusterv1.ClusterTopologyOwnedLabel]; ok {
162+
annotationPatch := client.RawPatch(types.MergePatchType, []byte(fmt.Sprintf("{\"metadata\":{\"annotations\":{\"cluster.x-k8s.io/modifiedAt\":\"%v\"}}}", time.Now().Format(time.RFC3339))))
163+
Expect(cli.Patch(ctx, &mdList.Items[i], annotationPatch)).To(Succeed())
164+
}
165+
}
166+
}

test/framework/ownerreference_helpers.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ func ValidateOwnerReferencesResilience(ctx context.Context, proxy ClusterProxy,
8080
// edge case where an external system intentionally nukes all the owner references in a single operation.
8181
// The assumption is that if the system can recover from this edge case, it can also handle use cases where an owner
8282
// reference is deleted by mistake.
83+
84+
// Setting the paused property on the Cluster resource will pause reconciliations, thereby having no effect on OwnerReferences.
85+
// This also makes debugging easier.
8386
setClusterPause(ctx, proxy.GetClient(), clusterKey, true)
8487

8588
// Once all Clusters are paused remove the OwnerReference from all objects in the graph.
@@ -88,7 +91,7 @@ func ValidateOwnerReferencesResilience(ctx context.Context, proxy ClusterProxy,
8891
// Unpause the cluster.
8992
setClusterPause(ctx, proxy.GetClient(), clusterKey, false)
9093

91-
// Annotate the clusterClass, if one is in use, to speed up reconciliation. This ensures ClusterClass ownerReferences
94+
// Annotate the ClusterClass, if one is in use, to speed up reconciliation. This ensures ClusterClass ownerReferences
9295
// are re-reconciled before asserting the owner reference graph.
9396
forceClusterClassReconcile(ctx, proxy.GetClient(), clusterKey)
9497

0 commit comments

Comments
 (0)