Skip to content

Commit

Permalink
Use remote NodeRef in Machine and MachineSet controllers
Browse files Browse the repository at this point in the history
Signed-off-by: Vince Prignano <vincepri@vmware.com>
  • Loading branch information
vincepri committed Jun 20, 2019
1 parent 1c10a3f commit f41b137
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 21 deletions.
1 change: 1 addition & 0 deletions pkg/controller/machine/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ go_library(
deps = [
"//pkg/apis/cluster/v1alpha1:go_default_library",
"//pkg/controller/error:go_default_library",
"//pkg/controller/remote:go_default_library",
"//pkg/util:go_default_library",
"//vendor/github.com/pkg/errors:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
Expand Down
44 changes: 33 additions & 11 deletions pkg/controller/machine/machine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"k8s.io/klog"
clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"
controllerError "sigs.k8s.io/cluster-api/pkg/controller/error"
"sigs.k8s.io/cluster-api/pkg/controller/remote"
"sigs.k8s.io/cluster-api/pkg/util"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller"
Expand Down Expand Up @@ -181,8 +182,8 @@ func (r *ReconcileMachine) Reconcile(request reconcile.Request) (reconcile.Resul

if m.Status.NodeRef != nil {
klog.Infof("Deleting node %q for machine %q", m.Status.NodeRef.Name, m.Name)
if err := r.deleteNode(ctx, m.Status.NodeRef.Name); err != nil {
klog.Errorf("Error deleting node %q for machine %q", name, err)
if err := r.deleteNode(ctx, cluster, m.Status.NodeRef.Name); err != nil && !apierrors.IsNotFound(err) {
klog.Errorf("Error deleting node %q for machine %q: %v", m.Status.NodeRef.Name, name, err)
return reconcile.Result{}, err
}
}
Expand Down Expand Up @@ -253,6 +254,9 @@ func (r *ReconcileMachine) getCluster(ctx context.Context, machine *clusterv1.Ma
return cluster, nil
}

// isDeletedAllowed returns false if the Machine we're trying to delete is the
// Machine hosting this controller. It only works when the controllers are
// running in the workload cluster and does not support remote references.
func (r *ReconcileMachine) isDeleteAllowed(machine *clusterv1.Machine) bool {
if r.nodeName == "" || machine.Status.NodeRef == nil {
return true
Expand All @@ -274,15 +278,33 @@ func (r *ReconcileMachine) isDeleteAllowed(machine *clusterv1.Machine) bool {
return node.UID != machine.Status.NodeRef.UID
}

func (r *ReconcileMachine) deleteNode(ctx context.Context, name string) error {
var node corev1.Node
if err := r.Client.Get(ctx, client.ObjectKey{Name: name}, &node); err != nil {
if apierrors.IsNotFound(err) {
klog.V(2).Infof("Node %q not found", name)
return nil
func (r *ReconcileMachine) deleteNode(ctx context.Context, cluster *clusterv1.Cluster, name string) error {
if cluster == nil {
var node corev1.Node
if err := r.Client.Get(ctx, client.ObjectKey{Name: name}, &node); err != nil {
if apierrors.IsNotFound(err) {
klog.V(2).Infof("Node %q not found", name)
return nil
}
klog.Errorf("Failed to get node %q: %v", name, err)
return err
}
klog.Errorf("Failed to get node %q: %v", name, err)
return err
return r.Client.Delete(ctx, &node)
}

remoteClient, err := remote.NewClusterClient(r.Client, cluster)
if err != nil {
klog.Errorf("Error creating a remote client for cluster %q while deleting Machine %q, won't retry: %v",
cluster.Name, name, err)
return nil
}
return r.Client.Delete(ctx, &node)

corev1Remote, err := remoteClient.CoreV1()
if err != nil {
klog.Errorf("Error creating a remote client for cluster %q while deleting Machine %q, won't retry: %v",
cluster.Name, name, err)
return nil
}

return corev1Remote.Nodes().Delete(name, &metav1.DeleteOptions{})
}
1 change: 1 addition & 0 deletions pkg/controller/machineset/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ go_library(
deps = [
"//pkg/apis/cluster/v1alpha1:go_default_library",
"//pkg/controller/noderefutil:go_default_library",
"//pkg/controller/remote:go_default_library",
"//pkg/util:go_default_library",
"//vendor/github.com/pkg/errors:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
Expand Down
45 changes: 35 additions & 10 deletions pkg/controller/machineset/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ import (
"context"
"fmt"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/klog"
"sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"
"sigs.k8s.io/cluster-api/pkg/controller/noderefutil"
"sigs.k8s.io/cluster-api/pkg/controller/remote"
"sigs.k8s.io/controller-runtime/pkg/client"
)

Expand All @@ -37,6 +37,7 @@ const (

func (c *ReconcileMachineSet) calculateStatus(ms *v1alpha1.MachineSet, filteredMachines []*v1alpha1.Machine) v1alpha1.MachineSetStatus {
newStatus := ms.Status

// Count the number of machines that have labels matching the labels of the machine
// template of the replica set, the matching machines may have more
// labels than are in the template. Because the label of machineTemplateSpec is
Expand All @@ -46,15 +47,28 @@ func (c *ReconcileMachineSet) calculateStatus(ms *v1alpha1.MachineSet, filteredM
readyReplicasCount := 0
availableReplicasCount := 0
templateLabel := labels.Set(ms.Spec.Template.Labels).AsSelectorPreValidated()

// Retrieve Cluster, if any.
cluster, _ := c.getCluster(ms)

for _, machine := range filteredMachines {
if templateLabel.Matches(labels.Set(machine.Labels)) {
fullyLabeledReplicasCount++
}
node, err := c.getMachineNode(machine)

if machine.Status.NodeRef == nil {
klog.Warningf("Unable to retrieve Node status for Machine %q in namespace %q: missing NodeRef",
machine.Name, machine.Namespace)
continue
}

node, err := c.getMachineNode(cluster, machine)
if err != nil {
klog.V(4).Infof("Unable to get node for machine %v, %v", machine.Name, err)
klog.Warningf("Unable to retrieve Node status for Machine %q in namespace %q: %v",
machine.Name, machine.Namespace, err)
continue
}

if noderefutil.IsNodeReady(node) {
readyReplicasCount++
if noderefutil.IsNodeAvailable(node, ms.Spec.MinReadySeconds, metav1.Now()) {
Expand Down Expand Up @@ -122,13 +136,24 @@ func updateMachineSetStatus(c client.Client, ms *v1alpha1.MachineSet, newStatus
return nil, updateErr
}

func (c *ReconcileMachineSet) getMachineNode(machine *v1alpha1.Machine) (*corev1.Node, error) {
nodeRef := machine.Status.NodeRef
if nodeRef == nil {
return nil, errors.New("machine has no node ref")
func (c *ReconcileMachineSet) getMachineNode(cluster *v1alpha1.Cluster, machine *v1alpha1.Machine) (*corev1.Node, error) {
if cluster == nil {
// Try to retrieve the Node from the local cluster, if no Cluster reference is found.
node := &corev1.Node{}
err := c.Client.Get(context.Background(), client.ObjectKey{Name: machine.Status.NodeRef.Name}, node)
return node, err
}

// Otherwise, proceed to get the remote cluster client and get the Node.
remoteClient, err := remote.NewClusterClient(c.Client, cluster)
if err != nil {
return nil, err
}

corev1Remote, err := remoteClient.CoreV1()
if err != nil {
return nil, err
}

node := &corev1.Node{}
err := c.Client.Get(context.Background(), client.ObjectKey{Name: nodeRef.Name}, node)
return node, err
return corev1Remote.Nodes().Get(machine.Status.NodeRef.Name, metav1.GetOptions{})
}

0 comments on commit f41b137

Please sign in to comment.