Skip to content

Commit

Permalink
Merge pull request #8318 from fabriziopandini/add-soft-ownership-from…
Browse files Browse the repository at this point in the history 
…-clusters-to-clusterresourcesetbinding

🐛 Add soft ownership from clusters to ClusterResourceSetBinding
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Mar 21, 2023
2 parents eb8a970 + 4847063 commit c6cac04
Show file tree
Hide file tree
Showing 3 changed files with 177 additions and 37 deletions.
40 changes: 39 additions & 1 deletion cmd/clusterctl/client/cluster/objectgraph.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
)

const clusterTopologyNameKey = "cluster.spec.topology.class"
const clusterResourceSetBindingClusterNameKey = "clusterresourcesetbinding.spec.clustername"

type empty struct{}

Expand Down Expand Up @@ -146,6 +147,17 @@ func (n *node) captureAdditionalInformation(obj *unstructured.Unstructured) erro
}
}

// If the node is a ClusterResourceSetBinding capture the name of the cluster it is referencing to.
if n.identity.GroupVersionKind().GroupKind() == addonsv1.GroupVersion.WithKind("ClusterResourceSetBinding").GroupKind() {
binding := &addonsv1.ClusterResourceSetBinding{}
if err := localScheme.Convert(obj, binding, nil); err != nil {
return errors.Wrapf(err, "failed to convert object %s to ClusterResourceSetBinding", n.identityStr())
}
if n.additionalInfo == nil {
n.additionalInfo = map[string]interface{}{}
}
n.additionalInfo[clusterResourceSetBindingClusterNameKey] = binding.Spec.ClusterName
}
return nil
}

Expand Down Expand Up @@ -504,6 +516,17 @@ func (o *objectGraph) getClusterClasses() []*node {
return clusterClasses
}

// getClusterResourceSetBinding returns the list of ClusterResourceSetBinding existing in the object graph.
func (o *objectGraph) getClusterResourceSetBinding() []*node {
crs := []*node{}
for _, node := range o.uidToNode {
if node.identity.GroupVersionKind().GroupKind() == addonsv1.GroupVersion.WithKind("ClusterResourceSetBinding").GroupKind() {
crs = append(crs, node)
}
}
return crs
}

// getClusters returns the list of Secrets existing in the object graph.
func (o *objectGraph) getSecrets() []*node {
secrets := []*node{}
Expand Down Expand Up @@ -588,7 +611,7 @@ func (o *objectGraph) setSoftOwnership() {
// Cluster that uses a ClusterClass are soft owned by that ClusterClass.
for _, clusterClass := range clusterClasses {
for _, cluster := range clusters {
// if the cluster uses a managed topoloy and uses the clusterclass
// if the cluster uses a managed topology and uses the clusterclass
// set the clusterclass as a soft owner of the cluster.
if className, ok := cluster.additionalInfo[clusterTopologyNameKey]; ok {
if className == clusterClass.identity.Name && clusterClass.identity.Namespace == cluster.identity.Namespace {
Expand All @@ -597,6 +620,21 @@ func (o *objectGraph) setSoftOwnership() {
}
}
}

crsBindings := o.getClusterResourceSetBinding()
// ClusterResourceSetBinding that refers to a Cluster are soft owned by that Cluster.
for _, binding := range crsBindings {
clusterName, ok := binding.additionalInfo[clusterResourceSetBindingClusterNameKey]
if !ok {
continue
}

for _, cluster := range clusters {
if clusterName == cluster.identity.Name && binding.identity.Namespace == cluster.identity.Namespace {
binding.addSoftOwner(cluster)
}
}
}
}

// setTenants identifies all the nodes linked to a parent with forceMoveHierarchy = true (e.g. Clusters or ClusterResourceSet)
Expand Down
165 changes: 137 additions & 28 deletions cmd/clusterctl/client/cluster/objectgraph_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1120,6 +1120,8 @@ var objectGraphsTests = []struct {
"addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSetBinding, ns1/cluster1": {
owners: []string{
"addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSet, ns1/crs1",
},
softOwners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1",
},
},
Expand Down Expand Up @@ -1201,12 +1203,16 @@ var objectGraphsTests = []struct {
"addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSetBinding, ns1/cluster1": {
owners: []string{
"addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSet, ns1/crs1",
},
softOwners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1",
},
},
"addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSetBinding, ns1/cluster2": {
owners: []string{
"addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSet, ns1/crs1",
},
softOwners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster2",
},
},
Expand Down Expand Up @@ -1887,32 +1893,148 @@ func Test_objectGraph_setSoftOwnership(t *testing.T) {
objs []client.Object
}
tests := []struct {
name string
fields fields
wantSecrets map[string][]string
name string
fields fields
want wantGraph
}{
{
name: "A cluster with a soft owned secret",
fields: fields{
objs: test.NewFakeCluster("ns1", "foo").Objs(),
objs: test.NewFakeCluster("ns1", "cluster1").Objs(),
},
wantSecrets: map[string][]string{ // wantSecrets is a map[node UID] --> list of soft owner UIDs
"/v1, Kind=Secret, ns1/foo-ca": { // the ca secret has no explicit OwnerRef to the cluster, so it should be identified as a soft ownership
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/foo",
want: wantGraph{
nodes: map[string]wantGraphItem{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1": {
forceMove: true,
forceMoveHierarchy: true,
},
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureCluster, ns1/cluster1": {
owners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1",
},
},
"/v1, Kind=Secret, ns1/cluster1-ca": { // the ca secret has no explicit OwnerRef to the cluster, so it should be identified as a soft ownership
softOwners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1",
},
},
"/v1, Kind=Secret, ns1/cluster1-kubeconfig": { // the kubeconfig secret has explicit OwnerRef to the cluster, so it should NOT be identified as a soft ownership
owners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1",
},
},
},
"/v1, Kind=Secret, ns1/foo-kubeconfig": {}, // the kubeconfig secret has explicit OwnerRef to the cluster, so it should NOT be identified as a soft ownership
},
},
{
name: "A cluster with a soft owned secret (cluster name with - in the middle)",
name: "A ClusterClass with a soft owned Cluster",
fields: fields{
objs: test.NewFakeCluster("ns1", "foo-bar").Objs(),
objs: func() []client.Object {
objs := test.NewFakeClusterClass("ns1", "class1").Objs()
objs = append(objs, test.NewFakeCluster("ns1", "cluster1").WithTopologyClass("class1").Objs()...)

return objs
}(),
},
wantSecrets: map[string][]string{ // wantSecrets is a map[node UID] --> list of soft owner UIDs
"/v1, Kind=Secret, ns1/foo-bar-ca": { // the ca secret has no explicit OwnerRef to the cluster, so it should be identified as a soft ownership
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/foo-bar",
want: wantGraph{
nodes: map[string]wantGraphItem{
"cluster.x-k8s.io/v1beta1, Kind=ClusterClass, ns1/class1": {
forceMove: true,
forceMoveHierarchy: true,
},
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureClusterTemplate, ns1/class1": {
owners: []string{
"cluster.x-k8s.io/v1beta1, Kind=ClusterClass, ns1/class1",
},
},
"controlplane.cluster.x-k8s.io/v1beta1, Kind=GenericControlPlaneTemplate, ns1/class1": {
owners: []string{
"cluster.x-k8s.io/v1beta1, Kind=ClusterClass, ns1/class1",
},
},
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1": {
forceMove: true,
forceMoveHierarchy: true,
softOwners: []string{
"cluster.x-k8s.io/v1beta1, Kind=ClusterClass, ns1/class1", // NB. this cluster is not linked to the clusterclass through owner ref, but it is detected as soft ownership
},
},
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureCluster, ns1/cluster1": {
owners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1",
},
},
"/v1, Kind=Secret, ns1/cluster1-ca": {
softOwners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", // NB. this secret is not linked to the cluster through owner ref, but it is detected as soft ownership
},
},
"/v1, Kind=Secret, ns1/cluster1-kubeconfig": {
owners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1",
},
},
},
},
},
{
name: "A Cluster with a soft owned ClusterResourceSetBinding",
fields: fields{
objs: func() []client.Object {
objs := test.NewFakeCluster("ns1", "cluster1").Objs()
objs = append(objs, test.NewFakeClusterResourceSet("ns1", "crs1").
WithSecret("resource-s1").
WithConfigMap("resource-c1").
ApplyToCluster(test.SelectClusterObj(objs, "ns1", "cluster1")).
Objs()...)

return objs
}(),
},
want: wantGraph{
nodes: map[string]wantGraphItem{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1": {
forceMove: true,
forceMoveHierarchy: true,
},
"infrastructure.cluster.x-k8s.io/v1beta1, Kind=GenericInfrastructureCluster, ns1/cluster1": {
owners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1",
},
},
"/v1, Kind=Secret, ns1/cluster1-ca": {
softOwners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", // NB. this secret is not linked to the cluster through owner ref, but it is detected as soft ownership
},
},
"/v1, Kind=Secret, ns1/cluster1-kubeconfig": {
owners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1",
},
},
"addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSet, ns1/crs1": {
forceMove: true,
forceMoveHierarchy: true,
},
"addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSetBinding, ns1/cluster1": {
owners: []string{
"addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSet, ns1/crs1",
},
softOwners: []string{
"cluster.x-k8s.io/v1beta1, Kind=Cluster, ns1/cluster1", // NB. this ClusterResourceSetBinding is not linked to the cluster through owner ref, but it is detected as soft ownership
},
},
"/v1, Kind=Secret, ns1/resource-s1": {
owners: []string{
"addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSet, ns1/crs1",
},
},
"/v1, Kind=ConfigMap, ns1/resource-c1": {
owners: []string{
"addons.cluster.x-k8s.io/v1beta1, Kind=ClusterResourceSet, ns1/crs1",
},
},
},
"/v1, Kind=Secret, ns1/foo-bar-kubeconfig": {}, // the kubeconfig secret has explicit OwnerRef to the cluster, so it should NOT be identified as a soft ownership
},
},
}
Expand All @@ -1925,20 +2047,7 @@ func Test_objectGraph_setSoftOwnership(t *testing.T) {

graph.setSoftOwnership()

gotSecrets := graph.getSecrets()
g.Expect(gotSecrets).To(HaveLen(len(tt.wantSecrets)))

for _, secret := range gotSecrets {
wantObjects, ok := tt.wantSecrets[string(secret.identity.UID)]
g.Expect(ok).To(BeTrue())

gotObjects := []string{}
for softOwners := range secret.softOwners {
gotObjects = append(gotObjects, string(softOwners.identity.UID))
}

g.Expect(gotObjects).To(ConsistOf(wantObjects))
}
assertGraph(t, graph, tt.want)
})
}
}
Expand Down
9 changes: 1 addition & 8 deletions cmd/clusterctl/internal/test/fake_objects.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,7 @@ func (f *FakeClusterResourceSet) Objs() []client.Object {
Namespace: cluster.Namespace,
},
Spec: addonsv1.ClusterResourceSetBindingSpec{
ClusterName: cluster.Name,
Bindings: []*addonsv1.ResourceSetBinding{
{
ClusterResourceSetName: crs.Name,
Expand All @@ -1119,14 +1120,6 @@ func (f *FakeClusterResourceSet) Objs() []client.Object {

objs = append(objs, binding)

// binding are owned by the Cluster / ownership set by the ClusterResourceSet controller
binding.SetOwnerReferences(append(binding.OwnerReferences, metav1.OwnerReference{
APIVersion: cluster.APIVersion,
Kind: cluster.Kind,
Name: cluster.Name,
UID: cluster.UID,
}))

resourceSetBinding := addonsv1.ResourceSetBinding{
ClusterResourceSetName: crs.Name,
Resources: []addonsv1.ResourceBinding{},
Expand Down

0 comments on commit c6cac04

Please sign in to comment.