Skip to content

Commit

Permalink
Add checks for not topology owned templates to never reconcile.
Browse files Browse the repository at this point in the history
  • Loading branch information
adilGhaffarDev committed Jul 5, 2022
1 parent 6b58c06 commit e8743aa
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 3 deletions.
22 changes: 20 additions & 2 deletions internal/controllers/topology/cluster/current_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func (r *Reconciler) getCurrentInfrastructureClusterState(ctx context.Context, c
// Nb. This is to make sure that a managed topology cluster does not have a reference to an object that is not
// owned by the topology.
if !labels.IsTopologyOwned(infra) {
return nil, fmt.Errorf("referenced infra cluster object %s is not topology owned", tlog.KObj{Obj: infra})
return nil, fmt.Errorf("infra cluster object %s referenced from cluster %s is not topology owned", tlog.KObj{Obj: infra}, tlog.KObj{Obj: cluster})
}
return infra, nil
}
Expand All @@ -102,7 +102,7 @@ func (r *Reconciler) getCurrentControlPlaneState(ctx context.Context, cluster *c
// Nb. This is to make sure that a managed topology cluster does not have a reference to an object that is not
// owned by the topology.
if !labels.IsTopologyOwned(res.Object) {
return nil, fmt.Errorf("referenced control plane object %s is not topology owned", tlog.KObj{Obj: res.Object})
return nil, fmt.Errorf("control plane object %s referenced from cluster %s is not topology owned", tlog.KObj{Obj: res.Object}, tlog.KObj{Obj: cluster})
}

// If the clusterClass does not mandate the controlPlane has infrastructureMachines, return.
Expand All @@ -119,6 +119,12 @@ func (r *Reconciler) getCurrentControlPlaneState(ctx context.Context, cluster *c
if err != nil {
return nil, errors.Wrapf(err, "failed to get InfrastructureMachineTemplate for %s", tlog.KObj{Obj: res.Object})
}
// check that the referenced object has the ClusterTopologyOwnedLabel label.
// Nb. This is to make sure that a managed topology cluster does not have a reference to an object that is not
// owned by the topology.
if !labels.IsTopologyOwned(res.InfrastructureMachineTemplate) {
return nil, fmt.Errorf("control plane InfrastructureMachineTemplate object %s referenced from cluster %s is not topology owned", tlog.KObj{Obj: res.InfrastructureMachineTemplate}, tlog.KObj{Obj: cluster})
}

mhc := &clusterv1.MachineHealthCheck{}
// MachineHealthCheck always has the same name and namespace as the ControlPlane object it belongs to.
Expand Down Expand Up @@ -181,6 +187,12 @@ func (r *Reconciler) getCurrentMachineDeploymentState(ctx context.Context, clust
if err != nil {
return nil, errors.Wrap(err, fmt.Sprintf("%s Bootstrap reference could not be retrieved", tlog.KObj{Obj: m}))
}
// check that the referenced object has the ClusterTopologyOwnedLabel label.
// Nb. This is to make sure that a managed topology cluster does not have a reference to an object that is not
// owned by the topology.
if !labels.IsTopologyOwned(b) {
return nil, fmt.Errorf("BootstrapTemplate object %s referenced from MD %s is not topology owned", tlog.KObj{Obj: b}, tlog.KObj{Obj: m})
}

// Gets the InfrastructureMachineTemplate
infraRef := m.Spec.Template.Spec.InfrastructureRef
Expand All @@ -191,6 +203,12 @@ func (r *Reconciler) getCurrentMachineDeploymentState(ctx context.Context, clust
if err != nil {
return nil, errors.Wrap(err, fmt.Sprintf("%s Infrastructure reference could not be retrieved", tlog.KObj{Obj: m}))
}
// check that the referenced object has the ClusterTopologyOwnedLabel label.
// Nb. This is to make sure that a managed topology cluster does not have a reference to an object that is not
// owned by the topology.
if !labels.IsTopologyOwned(infra) {
return nil, fmt.Errorf("InfrastructureMachineTemplate object %s referenced from MD %s is not topology owned", tlog.KObj{Obj: infra}, tlog.KObj{Obj: m})
}

// Gets the MachineHealthCheck.
mhc := &clusterv1.MachineHealthCheck{}
Expand Down
29 changes: 28 additions & 1 deletion internal/controllers/topology/cluster/current_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,25 @@ func TestGetCurrentState(t *testing.T) {
// ControlPlane and ControlPlaneInfrastructureMachineTemplate objects.
controlPlaneInfrastructureMachineTemplate := builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "cpInfraTemplate").
Build()
controlPlaneInfrastructureMachineTemplate.SetLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""})
controlPlaneInfrastructureMachineTemplateNotTopologyOwned := builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "cpInfraTemplate").
Build()
controlPlaneTemplateWithInfrastructureMachine := builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cpTemplateWithInfra1").
WithInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplate).
Build()
controlPlaneTemplateWithInfrastructureMachineNotTopologyOwned := builder.ControlPlaneTemplate(metav1.NamespaceDefault, "cpTemplateWithInfra1").
WithInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplateNotTopologyOwned).
Build()
controlPlane := builder.ControlPlane(metav1.NamespaceDefault, "cp1").
Build()
controlPlane.SetLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""})
controlPlaneWithInfra := builder.ControlPlane(metav1.NamespaceDefault, "cp1").
WithInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplate).
Build()
controlPlaneWithInfra.SetLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""})

controlPlaneWithInfraNotTopologyOwned := builder.ControlPlane(metav1.NamespaceDefault, "cp1").
WithInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplateNotTopologyOwned).
Build()
controlPlaneNotTopologyOwned := builder.ControlPlane(metav1.NamespaceDefault, "cp1").
Build()

Expand All @@ -76,6 +84,10 @@ func TestGetCurrentState(t *testing.T) {
WithControlPlaneTemplate(controlPlaneTemplateWithInfrastructureMachine).
WithControlPlaneInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplate).
Build()
clusterClassWithControlPlaneInfraNotTopologyOwned := builder.ClusterClass(metav1.NamespaceDefault, "class1").
WithControlPlaneTemplate(controlPlaneTemplateWithInfrastructureMachineNotTopologyOwned).
WithControlPlaneInfrastructureMachineTemplate(controlPlaneInfrastructureMachineTemplateNotTopologyOwned).
Build()
clusterClassWithNoControlPlaneInfra := builder.ClusterClass(metav1.NamespaceDefault, "class2").
Build()

Expand All @@ -84,8 +96,10 @@ func TestGetCurrentState(t *testing.T) {

machineDeploymentInfrastructure := builder.InfrastructureMachineTemplate(metav1.NamespaceDefault, "infra1").
Build()
machineDeploymentInfrastructure.SetLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""})
machineDeploymentBootstrap := builder.BootstrapTemplate(metav1.NamespaceDefault, "bootstrap1").
Build()
machineDeploymentBootstrap.SetLabels(map[string]string{clusterv1.ClusterTopologyOwnedLabel: ""})

machineDeployment := builder.MachineDeployment(metav1.NamespaceDefault, "md1").
WithLabels(map[string]string{
Expand Down Expand Up @@ -312,6 +326,19 @@ func TestGetCurrentState(t *testing.T) {
"md1": {Object: machineDeployment, BootstrapTemplate: machineDeploymentBootstrap, InfrastructureMachineTemplate: machineDeploymentInfrastructure}},
},
},
{
name: "Fails if the ControlPlane references an InfrastructureMachineTemplate that is not topology owned",
cluster: builder.Cluster(metav1.NamespaceDefault, "cluster1").
// No InfrastructureCluster!
WithControlPlane(controlPlaneWithInfraNotTopologyOwned).
Build(),
class: clusterClassWithControlPlaneInfraNotTopologyOwned,
objects: []client.Object{
controlPlaneWithInfraNotTopologyOwned,
controlPlaneInfrastructureMachineTemplateNotTopologyOwned,
},
wantErr: true,
},
{
name: "Fails if the ControlPlane references an InfrastructureMachineTemplate that does not exist",
cluster: builder.Cluster(metav1.NamespaceDefault, "cluster1").
Expand Down

0 comments on commit e8743aa

Please sign in to comment.