Skip to content

Commit

Permalink
Fix reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
@tahsinrahman
tahsinrahman committed Sep 4, 2019
1 parent 2a55573 commit a7a9129
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 41 deletions.
71 changes: 47 additions & 24 deletions controllers/cluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (r *ClusterReconciler) SetupWithManager(mgr ctrl.Manager, options controlle
For(&clusterv1.Cluster{}).
Watches(
&source.Kind{Type: &clusterv1.Machine{}},
&handler.EnqueueRequestsFromMapFunc{ToRequests: handler.ToRequestsFunc(r.MachineToCluster)},
&handler.EnqueueRequestsFromMapFunc{ToRequests: handler.ToRequestsFunc(r.machineToCluster)},
).
WithOptions(options).
Build(r)
Expand Down Expand Up @@ -137,7 +137,7 @@ func (r *ClusterReconciler) reconcile(ctx context.Context, cluster *clusterv1.Cl
reconciliationErrors := []error{
r.reconcileInfrastructure(ctx, cluster),
r.reconcileKubeconfig(ctx, cluster),
r.reconcileClusterStatus(ctx, cluster),
r.reconcileControlPlaneInitialized(ctx, cluster),
}

// Parse the errors, making sure we record if there is a RequeueAfterError.
Expand Down Expand Up @@ -297,19 +297,19 @@ func splitMachineList(list *clusterv1.MachineList) (*clusterv1.MachineList, *clu
return controlplanes, nodes
}

func (r *ClusterReconciler) reconcileClusterStatus(ctx context.Context, cluster *clusterv1.Cluster) error {
func (r *ClusterReconciler) reconcileControlPlaneInitialized(ctx context.Context, cluster *clusterv1.Cluster) error {
if cluster.Status.ControlPlaneInitialized {
return nil
}

machines := &clusterv1.MachineList{}
err := r.Client.List(ctx, machines, client.InNamespace(cluster.Namespace))
machines, err := r.getMachinesInCluster(ctx, cluster.Namespace, cluster.Name)
if err != nil {
r.Log.Error(err, "error getting machines in cluster", "cluster", cluster.Name)
return err
}

for _, m := range machines.Items {
if util.IsControlPlaneMachine(&m) && m.Status.NodeRef != nil {
for _, m := range machines {
if util.IsControlPlaneMachine(m) && m.Status.NodeRef != nil {
cluster.Status.ControlPlaneInitialized = true
return nil
}
Expand All @@ -318,35 +318,58 @@ func (r *ClusterReconciler) reconcileClusterStatus(ctx context.Context, cluster
return nil
}

// MachineToMachineSets is a handler.ToRequestsFunc to be used to enqeue requests for reconciliation
// for Cluster to update its status.controlplaneInitialized field
func (r *ClusterReconciler) MachineToCluster(o handler.MapObject) []ctrl.Request {
result := []ctrl.Request{}
// getMachinesInCluster returns all of the Machine objects that belong to the cluster
func (r *ClusterReconciler) getMachinesInCluster(ctx context.Context, namespace, name string) ([]*clusterv1.Machine, error) {
if name == "" {
return nil, nil
}

machineList := &clusterv1.MachineList{}
labels := map[string]string{clusterv1.MachineClusterLabelName: name}

if err := r.Client.List(ctx, machineList, client.InNamespace(namespace), client.MatchingLabels(labels)); err != nil {
return nil, errors.Wrap(err, "failed to list machines")
}

machines := []*clusterv1.Machine{}
for i := range machineList.Items {
m := &machineList.Items[i]
if m.DeletionTimestamp.IsZero() {
machines = append(machines, m)
}
}
return machines, nil
}

// machineToCluster is a handler.ToRequestsFunc to be used to enqueue requests for reconciliation
// for Cluster to update its status.controlPlaneInitialized field
func (r *ClusterReconciler) machineToCluster(o handler.MapObject) []ctrl.Request {
m, ok := o.Object.(*clusterv1.Machine)
if !ok {
r.Log.Error(errors.New("did not get machine"), "got", fmt.Sprintf("%T", o.Object))
return result
return nil
}
if !util.IsControlPlaneMachine(m) {
return nil
}
if m.Status.NodeRef == nil {
return nil
}

cluster, err := util.GetClusterFromMetadata(context.Background(), r.Client, m.ObjectMeta)
if err != nil {
r.Log.Error(err, "failed to get cluster", "machine", m.Name, "cluster", m.ClusterName)
return result
return nil
}

if cluster.Status.ControlPlaneInitialized {
return result
}

if util.IsControlPlaneMachine(m) && m.Status.NodeRef != nil {
result = append(result, ctrl.Request{
NamespacedName: types.NamespacedName{
Namespace: cluster.Namespace,
Name: cluster.Name,
},
})
return nil
}

return result
return []ctrl.Request{{
NamespacedName: types.NamespacedName{
Namespace: cluster.Namespace,
Name: cluster.Name,
},
}}
}
37 changes: 20 additions & 17 deletions controllers/cluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ var _ = Describe("Cluster Reconciler", func() {
patchHelper, err := patch.NewHelper(machine, k8sClient)
Expect(err).NotTo(HaveOccurred())

machine.Status.NodeRef = &v1.ObjectReference{}
machine.Status.NodeRef = &v1.ObjectReference{Kind: "Node", Name: "test-node"}
Expect(patchHelper.Patch(ctx, machine)).Should(BeNil())

// Assertion
Expand All @@ -287,7 +287,7 @@ var _ = Describe("Cluster Reconciler", func() {
})
})

func TestClusterReconciler_MachineToCluster(t *testing.T) {
func TestClusterReconciler_machineToCluster(t *testing.T) {
cluster := &clusterv1.Cluster{
TypeMeta: metav1.TypeMeta{
Kind: "Cluster",
Expand All @@ -300,7 +300,7 @@ func TestClusterReconciler_MachineToCluster(t *testing.T) {
Status: clusterv1.ClusterStatus{},
}

m1 := &clusterv1.Machine{
controlPlaneWithNoderef := &clusterv1.Machine{
TypeMeta: metav1.TypeMeta{
Kind: "Machine",
},
Expand All @@ -312,10 +312,13 @@ func TestClusterReconciler_MachineToCluster(t *testing.T) {
},
},
Status: clusterv1.MachineStatus{
NodeRef: &v1.ObjectReference{},
NodeRef: &v1.ObjectReference{
Kind: "Node",
Namespace: "test-node",
},
},
}
m2 := &clusterv1.Machine{
controlPlaneWithoutNoderef := &clusterv1.Machine{
TypeMeta: metav1.TypeMeta{
Kind: "Machine",
},
Expand All @@ -327,7 +330,7 @@ func TestClusterReconciler_MachineToCluster(t *testing.T) {
},
},
}
m3 := &clusterv1.Machine{
nonControlPlane := &clusterv1.Machine{
TypeMeta: metav1.TypeMeta{
Kind: "Machine",
},
Expand All @@ -347,8 +350,8 @@ func TestClusterReconciler_MachineToCluster(t *testing.T) {
{
name: "controlplane machine, noderef is set, should return cluster",
o: handler.MapObject{
Meta: m1.GetObjectMeta(),
Object: m1,
Meta: controlPlaneWithNoderef.GetObjectMeta(),
Object: controlPlaneWithNoderef,
},
want: []ctrl.Request{
{NamespacedName: client.ObjectKey{
Expand All @@ -360,28 +363,28 @@ func TestClusterReconciler_MachineToCluster(t *testing.T) {
{
name: "controlplane machine, noderef is not set",
o: handler.MapObject{
Meta: m2.GetObjectMeta(),
Object: m2,
Meta: controlPlaneWithoutNoderef.GetObjectMeta(),
Object: controlPlaneWithoutNoderef,
},
want: []ctrl.Request{},
want: nil,
},
{
name: "not controlplane machine",
o: handler.MapObject{
Meta: m3.GetObjectMeta(),
Object: m3,
Meta: nonControlPlane.GetObjectMeta(),
Object: nonControlPlane,
},
want: []ctrl.Request{},
want: nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := &ClusterReconciler{
Client: fake.NewFakeClient(cluster, m1, m2, m3),
Client: fake.NewFakeClient(cluster, controlPlaneWithNoderef, controlPlaneWithoutNoderef, nonControlPlane),
Log: log.Log,
}
if got := r.MachineToCluster(tt.o); !reflect.DeepEqual(got, tt.want) {
t.Errorf("MachineToCluster() = %v, want %v", got, tt.want)
if got := r.machineToCluster(tt.o); !reflect.DeepEqual(got, tt.want) {
t.Errorf("machineToCluster() = %v, want %v", got, tt.want)
}
})
}
Expand Down

0 comments on commit a7a9129

Please sign in to comment.