Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Refactor context for vspheremachine controller #2376

Merged
merged 1 commit into from
Sep 21, 2023
Merged

✨ Refactor context for vspheremachine controller #2376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion controllers/controllers_suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func setup() {
if err := AddClusterControllerToManager(ctx, testEnv.GetContext(), testEnv.Manager, &infrav1.VSphereCluster{}, controllerOpts); err != nil {
panic(fmt.Sprintf("unable to setup VsphereCluster controller: %v", err))
}
if err := AddMachineControllerToManager(testEnv.GetContext(), testEnv.Manager, &infrav1.VSphereMachine{}, controllerOpts); err != nil {
if err := AddMachineControllerToManager(ctx, testEnv.GetContext(), testEnv.Manager, &infrav1.VSphereMachine{}, controllerOpts); err != nil {
panic(fmt.Sprintf("unable to setup VsphereMachine controller: %v", err))
}
if err := AddVMControllerToManager(testEnv.GetContext(), testEnv.Manager, tracker, controllerOpts); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion controllers/vmware/test/controllers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ func getManager(cfg *rest.Config, networkProvider string) manager.Manager {
return err
}

return controllers.AddMachineControllerToManager(controllerCtx, mgr, &vmwarev1.VSphereMachine{}, controllerOpts)
return controllers.AddMachineControllerToManager(ctx, controllerCtx, mgr, &vmwarev1.VSphereMachine{}, controllerOpts)
}

mgr, err := manager.New(ctx, opts)
Expand Down
67 changes: 36 additions & 31 deletions controllers/vspheremachine_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ const hostInfoErrStr = "host info cannot be used as a label value"
// AddMachineControllerToManager adds the machine controller to the provided
// manager.

func AddMachineControllerToManager(controllerCtx *capvcontext.ControllerManagerContext, mgr manager.Manager, controlledType client.Object, options controller.Options) error {
func AddMachineControllerToManager(ctx context.Context, controllerManagerContext *capvcontext.ControllerManagerContext, mgr manager.Manager, controlledType client.Object, options controller.Options) error {
supervisorBased, err := util.IsSupervisorType(controlledType)
if err != nil {
return err
Expand All @@ -89,21 +89,21 @@ func AddMachineControllerToManager(controllerCtx *capvcontext.ControllerManagerC
controlledTypeName = reflect.TypeOf(controlledType).Elem().Name()
controlledTypeGVK = infrav1.GroupVersion.WithKind(controlledTypeName)
controllerNameShort = fmt.Sprintf("%s-controller", strings.ToLower(controlledTypeName))
controllerNameLong = fmt.Sprintf("%s/%s/%s", controllerCtx.Namespace, controllerCtx.Name, controllerNameShort)
controllerNameLong = fmt.Sprintf("%s/%s/%s", controllerManagerContext.Namespace, controllerManagerContext.Name, controllerNameShort)
)

if supervisorBased {
controlledTypeGVK = vmwarev1.GroupVersion.WithKind(controlledTypeName)
controllerNameShort = fmt.Sprintf("%s-supervisor-controller", strings.ToLower(controlledTypeName))
controllerNameLong = fmt.Sprintf("%s/%s/%s", controllerCtx.Namespace, controllerCtx.Name, controllerNameShort)
controllerNameLong = fmt.Sprintf("%s/%s/%s", controllerManagerContext.Namespace, controllerManagerContext.Name, controllerNameShort)
}

// Build the controller context.
controllerContext := &capvcontext.ControllerContext{
ControllerManagerContext: controllerCtx,
ControllerManagerContext: controllerManagerContext,
Name: controllerNameShort,
Recorder: record.New(mgr.GetEventRecorderFor(controllerNameLong)),
Logger: controllerCtx.Logger.WithName(controllerNameShort),
Logger: controllerManagerContext.Logger.WithName(controllerNameShort),
}

builder := ctrl.NewControllerManagedBy(mgr).
Expand All @@ -121,10 +121,10 @@ func AddMachineControllerToManager(controllerCtx *capvcontext.ControllerManagerC
// should cause a resource to be synchronized, such as a goroutine
// waiting on some asynchronous, external task to complete.
WatchesRawSource(
&source.Channel{Source: controllerCtx.GetGenericEventChannelFor(controlledTypeGVK)},
&source.Channel{Source: controllerManagerContext.GetGenericEventChannelFor(controlledTypeGVK)},
&handler.EnqueueRequestForObject{},
).
WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(ctrl.LoggerFrom(controllerCtx), controllerCtx.WatchFilterValue))
WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(ctrl.LoggerFrom(ctx), controllerManagerContext.WatchFilterValue))

r := &machineReconciler{
ControllerContext: controllerContext,
Expand All @@ -136,7 +136,7 @@ func AddMachineControllerToManager(controllerCtx *capvcontext.ControllerManagerC
// Watch any VirtualMachine resources owned by this VSphereMachine
builder.Owns(&vmoprv1.VirtualMachine{})
r.VMService = &vmoperator.VmopMachineService{}
networkProvider, err := inframanager.GetNetworkProvider(context.TODO(), controllerCtx.Client, controllerCtx.NetworkProvider)
networkProvider, err := inframanager.GetNetworkProvider(ctx, controllerManagerContext.Client, controllerManagerContext.NetworkProvider)
if err != nil {
return errors.Wrap(err, "failed to create a network provider")
}
Expand Down Expand Up @@ -177,9 +177,9 @@ type machineReconciler struct {
}

// Reconcile ensures the back-end state reflects the Kubernetes resource state intent.
func (r *machineReconciler) Reconcile(_ context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
func (r *machineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
var machineContext capvcontext.MachineContext
logger := r.Logger.WithName(req.Namespace).WithName(req.Name)
logger := ctrl.LoggerFrom(ctx)
logger.V(4).Info("Starting Reconcile")

// Fetch VSphereMachine object and populate the machine context
Expand All @@ -201,7 +201,7 @@ func (r *machineReconciler) Reconcile(_ context.Context, req ctrl.Request) (_ ct
return reconcile.Result{}, nil
}

cluster := r.fetchCAPICluster(machine, machineContext.GetVSphereMachine())
cluster := r.fetchCAPICluster(ctx, machine, machineContext.GetVSphereMachine())

// Create the patch helper.
patchHelper, err := patch.NewHelper(machineContext.GetVSphereMachine(), r.Client)
Expand Down Expand Up @@ -235,7 +235,7 @@ func (r *machineReconciler) Reconcile(_ context.Context, req ctrl.Request) (_ ct
}()

if !machineContext.GetObjectMeta().DeletionTimestamp.IsZero() {
return r.reconcileDelete(machineContext)
return r.reconcileDelete(ctx, machineContext)
}

// Checking whether cluster is nil here as we still want to allow delete even if cluster is not found.
Expand All @@ -257,11 +257,12 @@ func (r *machineReconciler) Reconcile(_ context.Context, req ctrl.Request) (_ ct
return reconcile.Result{}, nil
}
// Handle non-deleted machines
return r.reconcileNormal(machineContext)
return r.reconcileNormal(ctx, machineContext)
}

func (r *machineReconciler) reconcileDelete(machineCtx capvcontext.MachineContext) (reconcile.Result, error) {
machineCtx.GetLogger().Info("Handling deleted VSphereMachine")
func (r *machineReconciler) reconcileDelete(ctx context.Context, machineCtx capvcontext.MachineContext) (reconcile.Result, error) {
log := ctrl.LoggerFrom(ctx)
log.Info("Handling deleted VSphereMachine")
conditions.MarkFalse(machineCtx.GetVSphereMachine(), infrav1.VMProvisionedCondition, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, "")

if err := r.VMService.ReconcileDelete(machineCtx); err != nil {
Expand All @@ -278,28 +279,29 @@ func (r *machineReconciler) reconcileDelete(machineCtx capvcontext.MachineContex
return reconcile.Result{RequeueAfter: 10 * time.Second}, nil
}

func (r *machineReconciler) reconcileNormal(machineCtx capvcontext.MachineContext) (reconcile.Result, error) {
func (r *machineReconciler) reconcileNormal(ctx context.Context, machineCtx capvcontext.MachineContext) (reconcile.Result, error) {
log := ctrl.LoggerFrom(ctx)
machineFailed, err := r.VMService.SyncFailureReason(machineCtx)
if err != nil && !apierrors.IsNotFound(err) {
return reconcile.Result{}, err
}

// If the VSphereMachine is in an error state, return early.
if machineFailed {
machineCtx.GetLogger().Info("Error state detected, skipping reconciliation")
log.Info("Error state detected, skipping reconciliation")
return reconcile.Result{}, nil
}

//nolint:gocritic
if r.supervisorBased {
err := r.setVMModifiers(machineCtx)
err := r.setVMModifiers(ctx, machineCtx)
if err != nil {
return reconcile.Result{}, err
}
} else {
// vmwarev1.VSphereCluster doesn't set Cluster.Status.Ready until the API endpoint is available.
if !machineCtx.GetCluster().Status.InfrastructureReady {
machineCtx.GetLogger().Info("Cluster infrastructure is not ready yet")
log.Info("Cluster infrastructure is not ready yet")
conditions.MarkFalse(machineCtx.GetVSphereMachine(), infrav1.VMProvisionedCondition, infrav1.WaitingForClusterInfrastructureReason, clusterv1.ConditionSeverityInfo, "")
return reconcile.Result{}, nil
}
Expand All @@ -308,11 +310,11 @@ func (r *machineReconciler) reconcileNormal(machineCtx capvcontext.MachineContex
// Make sure bootstrap data is available and populated.
if machineCtx.GetMachine().Spec.Bootstrap.DataSecretName == nil {
if !util.IsControlPlaneMachine(machineCtx.GetVSphereMachine()) && !conditions.IsTrue(machineCtx.GetCluster(), clusterv1.ControlPlaneInitializedCondition) {
machineCtx.GetLogger().Info("Waiting for the control plane to be initialized")
log.Info("Waiting for the control plane to be initialized")
conditions.MarkFalse(machineCtx.GetVSphereMachine(), infrav1.VMProvisionedCondition, clusterv1.WaitingForControlPlaneAvailableReason, clusterv1.ConditionSeverityInfo, "")
return ctrl.Result{}, nil
}
machineCtx.GetLogger().Info("Waiting for bootstrap data to be available")
log.Info("Waiting for bootstrap data to be available")
conditions.MarkFalse(machineCtx.GetVSphereMachine(), infrav1.VMProvisionedCondition, infrav1.WaitingForBootstrapDataReason, clusterv1.ConditionSeverityInfo, "")
return reconcile.Result{}, nil
}
Expand All @@ -327,9 +329,9 @@ func (r *machineReconciler) reconcileNormal(machineCtx capvcontext.MachineContex
// The machine is patched at the last stage before marking the VM as provisioned
// This makes sure that the VSphereMachine exists and is in a Running state
// before attempting to patch.
err = r.patchMachineLabelsWithHostInfo(machineCtx)
err = r.patchMachineLabelsWithHostInfo(ctx, machineCtx)
if err != nil {
r.Logger.Error(err, "failed to patch machine with host info label", "machine ", machineCtx.GetMachine().Name)
log.Error(err, "failed to patch machine with host info label", "machine ", machineCtx.GetMachine().Name)
return reconcile.Result{}, err
}

Expand All @@ -340,7 +342,8 @@ func (r *machineReconciler) reconcileNormal(machineCtx capvcontext.MachineContex
// patchMachineLabelsWithHostInfo adds the ESXi host information as a label to the Machine object.
// The ESXi host information is added with the CAPI node label prefix
// which would be added onto the node by the CAPI controllers.
func (r *machineReconciler) patchMachineLabelsWithHostInfo(machineCtx capvcontext.MachineContext) error {
func (r *machineReconciler) patchMachineLabelsWithHostInfo(ctx context.Context, machineCtx capvcontext.MachineContext) error {
log := ctrl.LoggerFrom(ctx)
hostInfo, err := r.VMService.GetHostInfo(machineCtx)
if err != nil {
return err
Expand All @@ -350,7 +353,7 @@ func (r *machineReconciler) patchMachineLabelsWithHostInfo(machineCtx capvcontex
errs := validation.IsValidLabelValue(info)
if len(errs) > 0 {
err := errors.Errorf("%s: %s", hostInfoErrStr, strings.Join(errs, ","))
r.Logger.Error(err, hostInfoErrStr, "info", hostInfo)
log.Error(err, hostInfoErrStr, "info", hostInfo)
return err
}

Expand Down Expand Up @@ -385,22 +388,24 @@ func (r *machineReconciler) clusterToVSphereMachines(ctx context.Context, a clie
return requests
}

func (r *machineReconciler) fetchCAPICluster(machine *clusterv1.Machine, vsphereMachine metav1.Object) *clusterv1.Cluster {
func (r *machineReconciler) fetchCAPICluster(ctx context.Context, machine *clusterv1.Machine, vsphereMachine metav1.Object) *clusterv1.Cluster {
log := ctrl.LoggerFrom(ctx)
cluster, err := clusterutilv1.GetClusterFromMetadata(r, r.Client, machine.ObjectMeta)
if err != nil {
r.Logger.Info("Machine is missing cluster label or cluster does not exist")
log.Info("Machine is missing cluster label or cluster does not exist")
return nil
}
if annotations.IsPaused(cluster, vsphereMachine) {
r.Logger.V(4).Info("VSphereMachine %s/%s linked to a cluster that is paused", vsphereMachine.GetNamespace(), vsphereMachine.GetName())
log.V(4).Info("VSphereMachine %s/%s linked to a cluster that is paused", vsphereMachine.GetNamespace(), vsphereMachine.GetName())
return nil
}

return cluster
}

// Return hooks that will be invoked when a VirtualMachine is created.
func (r *machineReconciler) setVMModifiers(machineCtx capvcontext.MachineContext) error {
func (r *machineReconciler) setVMModifiers(ctx context.Context, machineCtx capvcontext.MachineContext) error {
log := ctrl.LoggerFrom(ctx)
supervisorMachineCtx, ok := machineCtx.(*vmware.SupervisorMachineContext)
if !ok {
return errors.New("received unexpected MachineContext. expecting SupervisorMachineContext type")
Expand All @@ -409,8 +414,8 @@ func (r *machineReconciler) setVMModifiers(machineCtx capvcontext.MachineContext
networkModifier := func(obj runtime.Object) (runtime.Object, error) {
// No need to check the type. We know this will be a VirtualMachine
vm, _ := obj.(*vmoprv1.VirtualMachine)
supervisorMachineCtx.Logger.V(3).Info("Applying network config to VM", "vm-name", vm.Name)
err := r.networkProvider.ConfigureVirtualMachine(context.TODO(), supervisorMachineCtx.GetClusterContext(), vm)
log.V(3).Info("Applying network config to VM", "vm-name", vm.Name)
err := r.networkProvider.ConfigureVirtualMachine(ctx, supervisorMachineCtx.GetClusterContext(), vm)
if err != nil {
return nil, errors.Errorf("failed to configure machine network: %+v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ func setupVAPIControllers(ctx context.Context, controllerCtx *capvcontext.Contro
if err := controllers.AddClusterControllerToManager(ctx, controllerCtx, mgr, &infrav1.VSphereCluster{}, concurrency(vSphereClusterConcurrency)); err != nil {
return err
}
if err := controllers.AddMachineControllerToManager(controllerCtx, mgr, &infrav1.VSphereMachine{}, concurrency(vSphereMachineConcurrency)); err != nil {
if err := controllers.AddMachineControllerToManager(ctx, controllerCtx, mgr, &infrav1.VSphereMachine{}, concurrency(vSphereMachineConcurrency)); err != nil {
return err
}
if err := controllers.AddVMControllerToManager(controllerCtx, mgr, tracker, concurrency(vSphereVMConcurrency)); err != nil {
Expand All @@ -375,7 +375,7 @@ func setupSupervisorControllers(ctx context.Context, controllerCtx *capvcontext.
return err
}

if err := controllers.AddMachineControllerToManager(controllerCtx, mgr, &vmwarev1.VSphereMachine{}, concurrency(vSphereMachineConcurrency)); err != nil {
if err := controllers.AddMachineControllerToManager(ctx, controllerCtx, mgr, &vmwarev1.VSphereMachine{}, concurrency(vSphereMachineConcurrency)); err != nil {
return err
}

Expand Down