Skip to content

Commit

Permalink
review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
chrischdi committed May 7, 2024
1 parent e183cb2 commit 73cbfb8
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 31 deletions.
21 changes: 19 additions & 2 deletions cmd/clusterctl/client/cluster/ownergraph.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,27 @@ type OwnerGraphNode struct {
// GetOwnerGraphFilterFunction allows filtering the objects returned by GetOwnerGraph.
// The function has to return true for objects which should be kept.
// NOTE: this function signature is exposed to allow implementation of E2E tests; there is
// no guarantee about the stability of this API. Using this test with providers may require
// a custom implementation of this function, or the OwnerGraph it returns.
// no guarantee about the stability of this API.
type GetOwnerGraphFilterFunction func(u unstructured.Unstructured) bool

// FilterClusterObjectsWithNameFilter is used in e2e tests where the owner graph
// gets queried to filter out cluster-wide objects which don't have the s in their
// object name. This avoids assertions on objects which are part of in-parallel
// running tests like ExtensionConfig.
// NOTE: this function signature is exposed to allow implementation of E2E tests; there is
// no guarantee about the stability of this API.
func FilterClusterObjectsWithNameFilter(s string) func(u unstructured.Unstructured) bool {
return func(u unstructured.Unstructured) bool {
// Ignore cluster-wide objects which don't have the clusterName in their object
// name to avoid asserting on cluster-wide objects which get created or deleted
// by tests which run in-parallel (e.g. ExtensionConfig).
if u.GetNamespace() == "" && !strings.Contains(u.GetName(), s) {
return false
}
return true
}
}

// GetOwnerGraph returns a graph with all the objects considered by clusterctl move as nodes and the OwnerReference relationship between those objects as edges.
// NOTE: this data structure is exposed to allow implementation of E2E tests verifying that CAPI can properly rebuild its
// own owner references; there is no guarantee about the stability of this API. Using this test with providers may require
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/cluster_upgrade_runtimesdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ type clusterUpgradeWithRuntimeSDKSpecInput struct {

// Allows to inject a function to be run after the cluster is upgraded.
// If not specified, this is a no-op.
PostMachinesProvisioned func(managementClusterProxy framework.ClusterProxy, workloadClusterNamespace, workloadClusterName string)
PostUpgrade func(managementClusterProxy framework.ClusterProxy, workloadClusterNamespace, workloadClusterName string)
}

// clusterUpgradeWithRuntimeSDKSpec implements a spec that upgrades a cluster and runs the Kubernetes conformance suite.
Expand Down Expand Up @@ -264,9 +264,9 @@ func clusterUpgradeWithRuntimeSDKSpec(ctx context.Context, inputGetter func() cl
WaitForNodesReady: input.E2EConfig.GetIntervals(specName, "wait-nodes-ready"),
})

if input.PostMachinesProvisioned != nil {
if input.PostUpgrade != nil {
log.Logf("Calling PostMachinesProvisioned for cluster %s", klog.KRef(namespace.Name, clusterResources.Cluster.Name))
input.PostMachinesProvisioned(input.BootstrapClusterProxy, namespace.Name, clusterName)
input.PostUpgrade(input.BootstrapClusterProxy, namespace.Name, clusterResources.Cluster.Name)
}

By("Dumping resources and deleting the workload cluster; deletion waits for BeforeClusterDeleteHook to gate the operation")
Expand Down
5 changes: 3 additions & 2 deletions test/e2e/cluster_upgrade_runtimesdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
. "github.com/onsi/gomega"
"k8s.io/utils/ptr"

clusterctlcluster "sigs.k8s.io/cluster-api/cmd/clusterctl/client/cluster"
"sigs.k8s.io/cluster-api/test/framework"
)

Expand All @@ -43,10 +44,10 @@ var _ = Describe("When upgrading a workload cluster using ClusterClass with Runt
ArtifactFolder: artifactFolder,
SkipCleanup: skipCleanup,
InfrastructureProvider: ptr.To("docker"),
PostMachinesProvisioned: func(proxy framework.ClusterProxy, namespace, clusterName string) {
PostUpgrade: func(proxy framework.ClusterProxy, namespace, clusterName string) {
// This check ensures that the resourceVersions are stable, i.e. it verifies there are no
// continuous reconciles when everything should be stable.
framework.ValidateResourceVersionStable(ctx, proxy, namespace, framework.SkipClusterObjectsWithoutNameContains(clusterName))
framework.ValidateResourceVersionStable(ctx, proxy, namespace, clusterctlcluster.FilterClusterObjectsWithNameFilter(clusterName))
},
// "upgrades" is the same as the "topology" flavor but with an additional MachinePool.
Flavor: ptr.To("upgrades-runtimesdk"),
Expand Down
17 changes: 9 additions & 8 deletions test/e2e/quick_start_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
. "github.com/onsi/gomega"
"k8s.io/utils/ptr"

clusterctlcluster "sigs.k8s.io/cluster-api/cmd/clusterctl/client/cluster"
"sigs.k8s.io/cluster-api/test/framework"
"sigs.k8s.io/cluster-api/test/framework/kubetest"
)
Expand All @@ -39,7 +40,7 @@ var _ = Describe("When following the Cluster API quick-start", func() {
InfrastructureProvider: ptr.To("docker"),
PostMachinesProvisioned: func(proxy framework.ClusterProxy, namespace, clusterName string) {
// This check ensures that owner references are resilient - i.e. correctly re-reconciled - when removed.
framework.ValidateOwnerReferencesResilience(ctx, proxy, namespace, clusterName, framework.SkipClusterObjectsWithoutNameContains(clusterName),
framework.ValidateOwnerReferencesResilience(ctx, proxy, namespace, clusterName, clusterctlcluster.FilterClusterObjectsWithNameFilter(clusterName),
framework.CoreOwnerReferenceAssertion,
framework.ExpOwnerReferenceAssertions,
framework.DockerInfraOwnerReferenceAssertions,
Expand All @@ -48,7 +49,7 @@ var _ = Describe("When following the Cluster API quick-start", func() {
framework.KubernetesReferenceAssertions,
)
// This check ensures that owner references are correctly updated to the correct apiVersion.
framework.ValidateOwnerReferencesOnUpdate(ctx, proxy, namespace, clusterName, framework.SkipClusterObjectsWithoutNameContains(clusterName),
framework.ValidateOwnerReferencesOnUpdate(ctx, proxy, namespace, clusterName, clusterctlcluster.FilterClusterObjectsWithNameFilter(clusterName),
framework.CoreOwnerReferenceAssertion,
framework.ExpOwnerReferenceAssertions,
framework.DockerInfraOwnerReferenceAssertions,
Expand All @@ -57,15 +58,15 @@ var _ = Describe("When following the Cluster API quick-start", func() {
framework.KubernetesReferenceAssertions,
)
// This check ensures that finalizers are resilient - i.e. correctly re-reconciled - when removed.
framework.ValidateFinalizersResilience(ctx, proxy, namespace, clusterName, framework.SkipClusterObjectsWithoutNameContains(clusterName),
framework.ValidateFinalizersResilience(ctx, proxy, namespace, clusterName, clusterctlcluster.FilterClusterObjectsWithNameFilter(clusterName),
framework.CoreFinalizersAssertion,
framework.KubeadmControlPlaneFinalizersAssertion,
framework.ExpFinalizersAssertion,
framework.DockerInfraFinalizersAssertion,
)
// This check ensures that the resourceVersions are stable, i.e. it verifies there are no
// continuous reconciles when everything should be stable.
framework.ValidateResourceVersionStable(ctx, proxy, namespace, framework.SkipClusterObjectsWithoutNameContains(clusterName))
framework.ValidateResourceVersionStable(ctx, proxy, namespace, clusterctlcluster.FilterClusterObjectsWithNameFilter(clusterName))
},
}
})
Expand All @@ -83,7 +84,7 @@ var _ = Describe("When following the Cluster API quick-start with ClusterClass [
InfrastructureProvider: ptr.To("docker"),
// This check ensures that owner references are resilient - i.e. correctly re-reconciled - when removed.
PostMachinesProvisioned: func(proxy framework.ClusterProxy, namespace, clusterName string) {
framework.ValidateOwnerReferencesResilience(ctx, proxy, namespace, clusterName, framework.SkipClusterObjectsWithoutNameContains(clusterName),
framework.ValidateOwnerReferencesResilience(ctx, proxy, namespace, clusterName, clusterctlcluster.FilterClusterObjectsWithNameFilter(clusterName),
framework.CoreOwnerReferenceAssertion,
framework.ExpOwnerReferenceAssertions,
framework.DockerInfraOwnerReferenceAssertions,
Expand All @@ -92,7 +93,7 @@ var _ = Describe("When following the Cluster API quick-start with ClusterClass [
framework.KubernetesReferenceAssertions,
)
// This check ensures that owner references are correctly updated to the correct apiVersion.
framework.ValidateOwnerReferencesOnUpdate(ctx, proxy, namespace, clusterName, framework.SkipClusterObjectsWithoutNameContains(clusterName),
framework.ValidateOwnerReferencesOnUpdate(ctx, proxy, namespace, clusterName, clusterctlcluster.FilterClusterObjectsWithNameFilter(clusterName),
framework.CoreOwnerReferenceAssertion,
framework.ExpOwnerReferenceAssertions,
framework.DockerInfraOwnerReferenceAssertions,
Expand All @@ -101,15 +102,15 @@ var _ = Describe("When following the Cluster API quick-start with ClusterClass [
framework.KubernetesReferenceAssertions,
)
// This check ensures that finalizers are resilient - i.e. correctly re-reconciled - when removed.
framework.ValidateFinalizersResilience(ctx, proxy, namespace, clusterName, framework.SkipClusterObjectsWithoutNameContains(clusterName),
framework.ValidateFinalizersResilience(ctx, proxy, namespace, clusterName, clusterctlcluster.FilterClusterObjectsWithNameFilter(clusterName),
framework.CoreFinalizersAssertion,
framework.KubeadmControlPlaneFinalizersAssertion,
framework.ExpFinalizersAssertion,
framework.DockerInfraFinalizersAssertion,
)
// This check ensures that the resourceVersions are stable, i.e. it verifies there are no
// continuous reconciles when everything should be stable.
framework.ValidateResourceVersionStable(ctx, proxy, namespace, framework.SkipClusterObjectsWithoutNameContains(clusterName))
framework.ValidateResourceVersionStable(ctx, proxy, namespace, clusterctlcluster.FilterClusterObjectsWithNameFilter(clusterName))
},
}
})
Expand Down
16 changes: 0 additions & 16 deletions test/framework/ownerreference_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -382,22 +382,6 @@ func setClusterPause(ctx context.Context, cli client.Client, clusterKey types.Na
Expect(cli.Patch(ctx, cluster, pausePatch)).To(Succeed())
}

// SkipClusterObjectsWithoutNameContains is used in e2e tests where the owner graph
// gets queried to filter out cluster-wide objects which don't have the clusterName
// in their object name. This avoids assertions on objects which are part of in-parallel
// running tests like ExtensionConfig.
func SkipClusterObjectsWithoutNameContains(clusterName string) func(u unstructured.Unstructured) bool {
return func(u unstructured.Unstructured) bool {
// Ignore cluster-wide objects which don't have the clusterName in their object
// name to avoid asserting on cluster-wide objects which get created or deleted
// by tests which run in-parallel (e.g. ExtensionConfig).
if u.GetNamespace() == "" && !strings.Contains(u.GetName(), clusterName) {
return false
}
return true
}
}

// forceClusterClassReconcile force reconciliation of the ClusterClass associated with the Cluster if one exists. If the
// Cluster has no ClusterClass this is a no-op.
func forceClusterClassReconcile(ctx context.Context, cli client.Client, clusterKey types.NamespacedName) {
Expand Down

0 comments on commit 73cbfb8

Please sign in to comment.