diff --git a/controllers/vspherevm_controller.go b/controllers/vspherevm_controller.go index 0746e2c0a9..80be445c10 100644 --- a/controllers/vspherevm_controller.go +++ b/controllers/vspherevm_controller.go @@ -81,15 +81,9 @@ func AddVMControllerToManager(ctx context.Context, controllerManagerCtx *capvcon controllerNameLong = fmt.Sprintf("%s/%s/%s", controllerManagerCtx.Namespace, controllerManagerCtx.Name, controllerNameShort) ) - // Build the controller context. - controllerContext := &capvcontext.ControllerContext{ - ControllerManagerContext: controllerManagerCtx, - Name: controllerNameShort, - Recorder: record.New(mgr.GetEventRecorderFor(controllerNameLong)), - Logger: controllerManagerCtx.Logger.WithName(controllerNameShort), - } r := vmReconciler{ - ControllerContext: controllerContext, + ControllerManagerContext: controllerManagerCtx, + Recorder: record.New(mgr.GetEventRecorderFor(controllerNameLong)), VMService: &govmomi.VMService{}, remoteClusterCacheTracker: tracker, } @@ -148,20 +142,22 @@ func AddVMControllerToManager(ctx context.Context, controllerManagerCtx *capvcon } type vmReconciler struct { - *capvcontext.ControllerContext - + record.Recorder + *capvcontext.ControllerManagerContext VMService services.VirtualMachineService remoteClusterCacheTracker *remote.ClusterCacheTracker } // Reconcile ensures the back-end state reflects the Kubernetes resource state intent. func (r vmReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) { - r.Logger.V(4).Info("Starting Reconcile", "key", req.NamespacedName) + log := ctrl.LoggerFrom(ctx) + + log.V(4).Info("Starting Reconcile", "key", req.NamespacedName) // Get the VSphereVM resource for this request. vsphereVM := &infrav1.VSphereVM{} if err := r.Client.Get(ctx, req.NamespacedName, vsphereVM); err != nil { if apierrors.IsNotFound(err) { - r.Logger.Info("VSphereVM not found, won't reconcile", "key", req.NamespacedName) + log.Info("VSphereVM not found, won't reconcile", "key", req.NamespacedName) return reconcile.Result{}, nil } return reconcile.Result{}, err @@ -192,26 +188,34 @@ func (r vmReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.R // in that case nil vsphereMachine can cause panic and CrashLoopBackOff the pod // preventing vspheremachine_controller from setting the ownerref if err != nil || vsphereMachine == nil { - r.Logger.Info("Owner VSphereMachine not found, won't reconcile", "key", req.NamespacedName) + log.Info("Owner VSphereMachine not found, won't reconcile", "key", req.NamespacedName) return reconcile.Result{}, nil } + log = log.WithValues("VSphereMachine", klog.KObj(vsphereMachine), "Cluster", klog.KRef(vsphereMachine.Namespace, vsphereMachine.Labels[clusterv1.ClusterNameLabel])) + ctx = ctrl.LoggerInto(ctx, log) vsphereCluster, err := util.GetVSphereClusterFromVSphereMachine(ctx, r.Client, vsphereMachine) if err != nil || vsphereCluster == nil { - r.Logger.Info("VSphereCluster not found, won't reconcile", "key", ctrlclient.ObjectKeyFromObject(vsphereMachine)) + log.Info("VSphereCluster not found, won't reconcile", "key", ctrlclient.ObjectKeyFromObject(vsphereMachine)) return reconcile.Result{}, nil } + log = log.WithValues("VSphereCluster", klog.KObj(vsphereCluster)) + ctx = ctrl.LoggerInto(ctx, log) + // Fetch the CAPI Machine. machine, err := clusterutilv1.GetOwnerMachine(ctx, r.Client, vsphereMachine.ObjectMeta) if err != nil { return reconcile.Result{}, err } if machine == nil { - r.Logger.Info("Waiting for OwnerRef to be set on VSphereMachine", "key", vsphereMachine.Name) + log.Info("Waiting for OwnerRef to be set on VSphereMachine", "key", vsphereMachine.Name) return reconcile.Result{}, nil } + log = log.WithValues("Machine", klog.KObj(machine)) + ctx = ctrl.LoggerInto(ctx, log) + var vsphereFailureDomain *infrav1.VSphereFailureDomain if failureDomain := machine.Spec.FailureDomain; failureDomain != nil { vsphereDeploymentZone := &infrav1.VSphereDeploymentZone{} @@ -227,20 +231,19 @@ func (r vmReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.R // Create the VM context for this request. vmContext := &capvcontext.VMContext{ - ControllerContext: r.ControllerContext, - VSphereVM: vsphereVM, - VSphereFailureDomain: vsphereFailureDomain, - Session: authSession, - Logger: r.Logger.WithName(req.Namespace).WithName(req.Name), - PatchHelper: patchHelper, + ControllerManagerContext: r.ControllerManagerContext, + VSphereVM: vsphereVM, + VSphereFailureDomain: vsphereFailureDomain, + Session: authSession, + PatchHelper: patchHelper, } // Print the task-ref upon entry and upon exit. - vmContext.Logger.V(4).Info( + log.V(4).Info( "VSphereVM.Status.TaskRef OnEntry", "task-ref", vmContext.VSphereVM.Status.TaskRef) defer func() { - vmContext.Logger.V(4).Info( + log.V(4).Info( "VSphereVM.Status.TaskRef OnExit", "task-ref", vmContext.VSphereVM.Status.TaskRef) }() @@ -266,7 +269,7 @@ func (r vmReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.R cluster, err := clusterutilv1.GetClusterFromMetadata(ctx, r.Client, vsphereVM.ObjectMeta) if err == nil { if annotations.IsPaused(cluster, vsphereVM) { - r.Logger.V(4).Info("VSphereVM %s/%s linked to a cluster that is paused", + log.V(4).Info("VSphereVM %s/%s linked to a cluster that is paused", vsphereVM.Namespace, vsphereVM.Name) return reconcile.Result{}, nil } @@ -290,7 +293,7 @@ func (r vmReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.R // reconcile encases the behavior of the controller around cluster module information // retrieval depending upon inputs passed. // -// This logic was moved to a smaller function outside of the main Reconcile() loop +// This logic was moved to a smaller function outside the main Reconcile() loop // for the ease of testing. func (r vmReconciler) reconcile(ctx context.Context, vmCtx *capvcontext.VMContext, input fetchClusterModuleInput) (reconcile.Result, error) { if feature.Gates.Enabled(feature.NodeAntiAffinity) { @@ -314,7 +317,8 @@ func (r vmReconciler) reconcile(ctx context.Context, vmCtx *capvcontext.VMContex } func (r vmReconciler) reconcileDelete(ctx context.Context, vmCtx *capvcontext.VMContext) (reconcile.Result, error) { - vmCtx.Logger.Info("Handling deleted VSphereVM") + log := ctrl.LoggerFrom(ctx) + log.Info("Handling deleted VSphereVM") conditions.MarkFalse(vmCtx.VSphereVM, infrav1.VMProvisionedCondition, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, "") result, vm, err := r.VMService.DestroyVM(ctx, vmCtx) @@ -330,14 +334,14 @@ func (r vmReconciler) reconcileDelete(ctx context.Context, vmCtx *capvcontext.VM // Requeue the operation until the VM is "notfound". if vm.State != infrav1.VirtualMachineStateNotFound { - vmCtx.Logger.Info("vm state is not reconciled", "expected-vm-state", infrav1.VirtualMachineStateNotFound, "actual-vm-state", vm.State) + log.Info("vm state is not reconciled", "expected-vm-state", infrav1.VirtualMachineStateNotFound, "actual-vm-state", vm.State) return reconcile.Result{}, nil } // Attempt to delete the node corresponding to the vsphere VM result, err = r.deleteNode(ctx, vmCtx, vm.Name) if err != nil { - r.Logger.V(6).Info("unable to delete node", "err", err) + log.V(6).Info("unable to delete node", "err", err) } if !result.IsZero() { // a non-zero value means we need to requeue the request before proceed. @@ -359,6 +363,7 @@ func (r vmReconciler) reconcileDelete(ctx context.Context, vmCtx *capvcontext.VM // until the node moves to Ready state. Hence, on Machine deletion it is unable to delete // the kubernetes node corresponding to the VM. func (r vmReconciler) deleteNode(ctx context.Context, vmCtx *capvcontext.VMContext, name string) (reconcile.Result, error) { + log := ctrl.LoggerFrom(ctx) // Fetching the cluster object from the VSphereVM object to create a remote client to the cluster cluster, err := clusterutilv1.GetClusterFromMetadata(ctx, r.Client, vmCtx.VSphereVM.ObjectMeta) if err != nil { @@ -367,7 +372,7 @@ func (r vmReconciler) deleteNode(ctx context.Context, vmCtx *capvcontext.VMConte clusterClient, err := r.remoteClusterCacheTracker.GetClient(ctx, ctrlclient.ObjectKeyFromObject(cluster)) if err != nil { if errors.Is(err, remote.ErrClusterLocked) { - r.Logger.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker") + log.V(5).Info("Requeuing because another worker has the lock on the ClusterCacheTracker") return ctrl.Result{Requeue: true}, nil } return ctrl.Result{}, err @@ -383,14 +388,16 @@ func (r vmReconciler) deleteNode(ctx context.Context, vmCtx *capvcontext.VMConte } func (r vmReconciler) reconcileNormal(ctx context.Context, vmCtx *capvcontext.VMContext) (reconcile.Result, error) { + log := ctrl.LoggerFrom(ctx) + if vmCtx.VSphereVM.Status.FailureReason != nil || vmCtx.VSphereVM.Status.FailureMessage != nil { - r.Logger.Info("VM is failed, won't reconcile", "namespace", vmCtx.VSphereVM.Namespace, "name", vmCtx.VSphereVM.Name) + log.Info("VM is failed, won't reconcile", "namespace", vmCtx.VSphereVM.Namespace, "name", vmCtx.VSphereVM.Name) return reconcile.Result{}, nil } if r.isWaitingForStaticIPAllocation(vmCtx) { conditions.MarkFalse(vmCtx.VSphereVM, infrav1.VMProvisionedCondition, infrav1.WaitingForStaticIPAllocationReason, clusterv1.ConditionSeverityInfo, "") - vmCtx.Logger.Info("vm is waiting for static ip to be available") + log.Info("vm is waiting for static ip to be available") return reconcile.Result{}, nil } @@ -401,13 +408,13 @@ func (r vmReconciler) reconcileNormal(ctx context.Context, vmCtx *capvcontext.VM // Get or create the VM. vm, err := r.VMService.ReconcileVM(ctx, vmCtx) if err != nil { - vmCtx.Logger.Error(err, "error reconciling VM") + log.Error(err, "error reconciling VM") return reconcile.Result{}, errors.Wrapf(err, "failed to reconcile VM") } // Do not proceed until the backend VM is marked ready. if vm.State != infrav1.VirtualMachineStateReady { - vmCtx.Logger.Info( + log.Info( "VM state is not reconciled", "expected-vm-state", infrav1.VirtualMachineStateReady, "actual-vm-state", vm.State) @@ -415,7 +422,7 @@ func (r vmReconciler) reconcileNormal(ctx context.Context, vmCtx *capvcontext.VM } // Update the VSphereVM's BIOS UUID. - vmCtx.Logger.Info("vm bios-uuid", "biosuuid", vm.BiosUUID) + log.Info("vm bios-uuid", "biosuuid", vm.BiosUUID) // defensive check to ensure we are not removing the biosUUID if vm.BiosUUID != "" { @@ -441,7 +448,7 @@ func (r vmReconciler) reconcileNormal(ctx context.Context, vmCtx *capvcontext.VM // Once the network is online the VM is considered ready. vmCtx.VSphereVM.Status.Ready = true conditions.MarkTrue(vmCtx.VSphereVM, infrav1.VMProvisionedCondition) - vmCtx.Logger.Info("VSphereVM is ready") + log.Info("VSphereVM is ready") return reconcile.Result{}, nil } @@ -549,20 +556,21 @@ func (r vmReconciler) ipAddressClaimToVSphereVM(_ context.Context, a ctrlclient. } func (r vmReconciler) retrieveVcenterSession(ctx context.Context, vsphereVM *infrav1.VSphereVM) (*session.Session, error) { + log := ctrl.LoggerFrom(ctx) // Get cluster object and then get VSphereCluster object params := session.NewParams(). WithServer(vsphereVM.Spec.Server). WithDatacenter(vsphereVM.Spec.Datacenter). - WithUserInfo(r.ControllerContext.Username, r.ControllerContext.Password). + WithUserInfo(r.ControllerManagerContext.Username, r.ControllerManagerContext.Password). WithThumbprint(vsphereVM.Spec.Thumbprint). WithFeatures(session.Feature{ - EnableKeepAlive: r.EnableKeepAlive, - KeepAliveDuration: r.KeepAliveDuration, + EnableKeepAlive: r.ControllerManagerContext.EnableKeepAlive, + KeepAliveDuration: r.ControllerManagerContext.KeepAliveDuration, }) cluster, err := clusterutilv1.GetClusterFromMetadata(ctx, r.Client, vsphereVM.ObjectMeta) if err != nil { - r.Logger.Info("VsphereVM is missing cluster label or cluster does not exist") + log.Info("VsphereVM is missing cluster label or cluster does not exist") return session.GetOrCreate(ctx, params) } @@ -577,13 +585,13 @@ func (r vmReconciler) retrieveVcenterSession(ctx context.Context, vsphereVM *inf vsphereCluster := &infrav1.VSphereCluster{} err = r.Client.Get(ctx, key, vsphereCluster) if err != nil { - r.Logger.Info("VSphereCluster couldn't be retrieved") + log.Info("VSphereCluster couldn't be retrieved") return session.GetOrCreate(ctx, params) } if vsphereCluster.Spec.IdentityRef != nil { - creds, err := identity.GetCredentials(ctx, r.Client, vsphereCluster, r.Namespace) + creds, err := identity.GetCredentials(ctx, r.Client, vsphereCluster, r.ControllerManagerContext.Namespace) if err != nil { return nil, errors.Wrap(err, "failed to retrieve credentials from IdentityRef") } @@ -602,8 +610,8 @@ func (r vmReconciler) fetchClusterModuleInfo(ctx context.Context, clusterModInpu owner ctrlclient.Object err error ) + log := ctrl.LoggerFrom(ctx) machine := clusterModInput.Machine - logger := r.Logger.WithName(machine.Namespace).WithName(machine.Name) input := util.FetchObjectInput{ Client: r.Client, @@ -626,11 +634,11 @@ func (r vmReconciler) fetchClusterModuleInfo(ctx context.Context, clusterModInpu for _, mod := range clusterModInput.VSphereCluster.Spec.ClusterModules { if mod.TargetObjectName == owner.GetName() { - logger.Info("cluster module with UUID found", "moduleUUID", mod.ModuleUUID) + log.Info("cluster module with UUID found", "moduleUUID", mod.ModuleUUID) return pointer.String(mod.ModuleUUID), nil } } - logger.V(4).Info("no cluster module found") + log.V(4).Info("no cluster module found") return nil, nil } diff --git a/controllers/vspherevm_controller_test.go b/controllers/vspherevm_controller_test.go index b0d8c8ccd5..6a873461d7 100644 --- a/controllers/vspherevm_controller_test.go +++ b/controllers/vspherevm_controller_test.go @@ -37,7 +37,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" @@ -203,13 +202,8 @@ func TestReconcileNormal_WaitingForIPAddrAllocation(t *testing.T) { controllerMgrContext.Password = password controllerMgrContext.Username = simr.ServerURL().User.Username() - controllerContext := &capvcontext.ControllerContext{ - ControllerManagerContext: controllerMgrContext, - Recorder: record.New(apirecord.NewFakeRecorder(100)), - Logger: log.Log, - } return vmReconciler{ - ControllerContext: controllerContext, + ControllerManagerContext: controllerMgrContext, VMService: vmService, remoteClusterCacheTracker: tracker, } @@ -369,9 +363,9 @@ func TestVmReconciler_WaitingForStaticIPAllocation(t *testing.T) { }, } - controllerCtx := fake.NewControllerContext(fake.NewControllerManagerContext()) - vmContext := fake.NewVMContext(context.Background(), controllerCtx) - r := vmReconciler{ControllerContext: controllerCtx} + controllerManagerCtx := fake.NewControllerManagerContext() + vmContext := fake.NewVMContext(context.Background(), controllerManagerCtx) + r := vmReconciler{ControllerManagerContext: controllerManagerCtx} for _, tt := range tests { // Need to explicitly reinitialize test variable, looks odd, but needed @@ -483,12 +477,10 @@ func TestRetrievingVCenterCredentialsFromCluster(t *testing.T) { initObjs = append(initObjs, secret, vsphereVM, vsphereMachine, machine, cluster, vsphereCluster) controllerMgrContext := fake.NewControllerManagerContext(initObjs...) - controllerContext := &capvcontext.ControllerContext{ - ControllerManagerContext: controllerMgrContext, + r := vmReconciler{ Recorder: record.New(apirecord.NewFakeRecorder(100)), - Logger: log.Log, + ControllerManagerContext: controllerMgrContext, } - r := vmReconciler{ControllerContext: controllerContext} _, err = r.Reconcile(context.Background(), ctrl.Request{NamespacedName: util.ObjectKey(vsphereVM)}) g := NewWithT(t) @@ -519,12 +511,10 @@ func TestRetrievingVCenterCredentialsFromCluster(t *testing.T) { initObjs = append(initObjs, secret, vsphereVM, vsphereMachine, machine, cluster, vsphereCluster) controllerMgrContext := fake.NewControllerManagerContext(initObjs...) - controllerContext := &capvcontext.ControllerContext{ - ControllerManagerContext: controllerMgrContext, + r := vmReconciler{ Recorder: record.New(apirecord.NewFakeRecorder(100)), - Logger: log.Log, + ControllerManagerContext: controllerMgrContext, } - r := vmReconciler{ControllerContext: controllerContext} _, err = r.Reconcile(context.Background(), ctrl.Request{NamespacedName: util.ObjectKey(vsphereVM)}) g := NewWithT(t) @@ -570,12 +560,9 @@ func Test_reconcile(t *testing.T) { setupReconciler := func(vmService services.VirtualMachineService, initObjs ...client.Object) vmReconciler { return vmReconciler{ - ControllerContext: &capvcontext.ControllerContext{ - ControllerManagerContext: fake.NewControllerManagerContext(initObjs...), - Recorder: record.New(apirecord.NewFakeRecorder(100)), - Logger: log.Log, - }, - VMService: vmService, + Recorder: record.New(apirecord.NewFakeRecorder(100)), + ControllerManagerContext: fake.NewControllerManagerContext(initObjs...), + VMService: vmService, } } @@ -591,9 +578,8 @@ func Test_reconcile(t *testing.T) { }, nil) r := setupReconciler(fakeVMSvc, initObjs...) _, err := r.reconcile(ctx, &capvcontext.VMContext{ - ControllerContext: r.ControllerContext, - VSphereVM: vsphereVM, - Logger: r.Logger, + ControllerManagerContext: r.ControllerManagerContext, + VSphereVM: vsphereVM, }, fetchClusterModuleInput{ VSphereCluster: vsphereCluster, Machine: machine, @@ -607,9 +593,8 @@ func Test_reconcile(t *testing.T) { _ = feature.MutableGates.Set("NodeAntiAffinity=true") r := setupReconciler(new(fake_svc.VMService), initObjs...) _, err := r.reconcile(ctx, &capvcontext.VMContext{ - ControllerContext: r.ControllerContext, - VSphereVM: vsphereVM, - Logger: r.Logger, + ControllerManagerContext: r.ControllerManagerContext, + VSphereVM: vsphereVM, }, fetchClusterModuleInput{ VSphereCluster: vsphereCluster, Machine: machine, @@ -633,9 +618,8 @@ func Test_reconcile(t *testing.T) { r := setupReconciler(fakeVMSvc, objsWithHierarchy...) _, err := r.reconcile(ctx, &capvcontext.VMContext{ - ControllerContext: r.ControllerContext, - VSphereVM: vsphereVM, - Logger: r.Logger, + ControllerManagerContext: r.ControllerManagerContext, + VSphereVM: vsphereVM, }, fetchClusterModuleInput{ VSphereCluster: vsphereCluster, Machine: machine, @@ -666,9 +650,8 @@ func Test_reconcile(t *testing.T) { r := setupReconciler(fakeVMSvc, objsWithHierarchy...) _, err := r.reconcile(ctx, &capvcontext.VMContext{ - ControllerContext: r.ControllerContext, - VSphereVM: deletedVM, - Logger: r.Logger, + ControllerManagerContext: r.ControllerManagerContext, + VSphereVM: deletedVM, }, fetchClusterModuleInput{ VSphereCluster: vsphereCluster, Machine: machine, @@ -681,9 +664,8 @@ func Test_reconcile(t *testing.T) { t.Run("when info cannot be fetched", func(t *testing.T) { r := setupReconciler(fakeVMSvc, initObjs...) _, err := r.reconcile(ctx, &capvcontext.VMContext{ - ControllerContext: r.ControllerContext, - VSphereVM: deletedVM, - Logger: r.Logger, + ControllerManagerContext: r.ControllerManagerContext, + VSphereVM: deletedVM, }, fetchClusterModuleInput{ VSphereCluster: vsphereCluster, Machine: machine, diff --git a/controllers/vspherevm_ipaddress_reconciler.go b/controllers/vspherevm_ipaddress_reconciler.go index 2705ca3686..a71f89166b 100644 --- a/controllers/vspherevm_ipaddress_reconciler.go +++ b/controllers/vspherevm_ipaddress_reconciler.go @@ -30,6 +30,7 @@ import ( ipamv1 "sigs.k8s.io/cluster-api/exp/ipam/api/v1alpha1" clusterutilv1 "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -46,6 +47,7 @@ import ( func (r vmReconciler) reconcileIPAddressClaims(ctx context.Context, vmCtx *capvcontext.VMContext) error { totalClaims, claimsCreated := 0, 0 claimsFulfilled := 0 + log := ctrl.LoggerFrom(ctx) var ( claims []conditions.Getter @@ -63,12 +65,12 @@ func (r vmReconciler) reconcileIPAddressClaims(ctx context.Context, vmCtx *capvc } err := vmCtx.Client.Get(ctx, ipAddrClaimKey, ipAddrClaim) if err != nil && !apierrors.IsNotFound(err) { - vmCtx.Logger.Error(err, "fetching IPAddressClaim failed", "name", ipAddrClaimName) + log.Error(err, "fetching IPAddressClaim failed", "name", ipAddrClaimName) return err } ipAddrClaim, created, err := createOrPatchIPAddressClaim(ctx, vmCtx, ipAddrClaimName, poolRef) if err != nil { - vmCtx.Logger.Error(err, "createOrPatchIPAddressClaim failed", "name", ipAddrClaimName) + log.Error(err, "createOrPatchIPAddressClaim failed", "name", ipAddrClaimName) errList = append(errList, err) continue } @@ -159,10 +161,11 @@ func createOrPatchIPAddressClaim(ctx context.Context, vmCtx *capvcontext.VMConte claim.Spec.PoolRef.Name = poolRef.Name return nil } + log := ctrl.LoggerFrom(ctx) result, err := ctrlutil.CreateOrPatch(ctx, vmCtx.Client, claim, mutateFn) if err != nil { - vmCtx.Logger.Error( + log.Error( err, "failed to CreateOrPatch IPAddressClaim", "namespace", @@ -178,20 +181,20 @@ func createOrPatchIPAddressClaim(ctx context.Context, vmCtx *capvcontext.VMConte } switch result { case ctrlutil.OperationResultCreated: - vmCtx.Logger.Info( + log.Info( "created claim", "claim", key, ) return claim, true, nil case ctrlutil.OperationResultUpdated: - vmCtx.Logger.Info( + log.Info( "updated claim", "claim", key, ) case ctrlutil.OperationResultNone, ctrlutil.OperationResultUpdatedStatus, ctrlutil.OperationResultUpdatedStatusOnly: - vmCtx.Logger.V(5).Info( + log.V(5).Info( "no change required for claim", "claim", key, "operation", result, @@ -203,12 +206,13 @@ func createOrPatchIPAddressClaim(ctx context.Context, vmCtx *capvcontext.VMConte // deleteIPAddressClaims removes the finalizers from the IPAddressClaim objects // thus freeing them up for garbage collection. func (r vmReconciler) deleteIPAddressClaims(ctx context.Context, vmCtx *capvcontext.VMContext) error { + log := ctrl.LoggerFrom(ctx) for devIdx, device := range vmCtx.VSphereVM.Spec.Network.Devices { for poolRefIdx := range device.AddressesFromPools { // check if claim exists ipAddrClaim := &ipamv1.IPAddressClaim{} ipAddrClaimName := util.IPAddressClaimName(vmCtx.VSphereVM.Name, devIdx, poolRefIdx) - vmCtx.Logger.Info("removing finalizer", "IPAddressClaim", ipAddrClaimName) + log.Info("removing finalizer", "IPAddressClaim", ipAddrClaimName) ipAddrClaimKey := client.ObjectKey{ Namespace: vmCtx.VSphereVM.Namespace, Name: ipAddrClaimName, diff --git a/controllers/vspherevm_ipaddress_reconciler_test.go b/controllers/vspherevm_ipaddress_reconciler_test.go index 063df2a9f8..67a902f0cb 100644 --- a/controllers/vspherevm_ipaddress_reconciler_test.go +++ b/controllers/vspherevm_ipaddress_reconciler_test.go @@ -40,11 +40,10 @@ import ( func Test_vmReconciler_reconcileIPAddressClaims(t *testing.T) { name, namespace := "test-vm", "my-namespace" setup := func(vsphereVM *infrav1.VSphereVM, initObjects ...client.Object) *capvcontext.VMContext { - controllerCtx := fake.NewControllerContext(fake.NewControllerManagerContext(initObjects...)) return &capvcontext.VMContext{ - ControllerContext: controllerCtx, - VSphereVM: vsphereVM, - Logger: logr.Discard(), + ControllerManagerContext: fake.NewControllerManagerContext(initObjects...), + VSphereVM: vsphereVM, + Logger: logr.Discard(), } } ctx := context.Background() diff --git a/pkg/context/fake/fake_vm_context.go b/pkg/context/fake/fake_vm_context.go index f4074e770a..0f775282a2 100644 --- a/pkg/context/fake/fake_vm_context.go +++ b/pkg/context/fake/fake_vm_context.go @@ -28,25 +28,24 @@ import ( // NewVMContext returns a fake VMContext for unit testing // reconcilers with a fake client. -func NewVMContext(ctx context.Context, controllerCtx *capvcontext.ControllerContext) *capvcontext.VMContext { +func NewVMContext(ctx context.Context, controllerManagerCtx *capvcontext.ControllerManagerContext) *capvcontext.VMContext { // Create the resources. vsphereVM := newVSphereVM() // Add the resources to the fake client. - if err := controllerCtx.Client.Create(ctx, &vsphereVM); err != nil { + if err := controllerManagerCtx.Client.Create(ctx, &vsphereVM); err != nil { panic(err) } - helper, err := patch.NewHelper(&vsphereVM, controllerCtx.Client) + helper, err := patch.NewHelper(&vsphereVM, controllerManagerCtx.Client) if err != nil { panic(err) } return &capvcontext.VMContext{ - ControllerContext: controllerCtx, - VSphereVM: &vsphereVM, - Logger: controllerCtx.Logger.WithName(vsphereVM.Name), - PatchHelper: helper, + ControllerManagerContext: controllerManagerCtx, + VSphereVM: &vsphereVM, + PatchHelper: helper, } } diff --git a/pkg/context/vm_context.go b/pkg/context/vm_context.go index c384153ad5..f90c3d1180 100644 --- a/pkg/context/vm_context.go +++ b/pkg/context/vm_context.go @@ -30,7 +30,7 @@ import ( // VMContext is a Go context used with a VSphereVM. type VMContext struct { - *ControllerContext + *ControllerManagerContext ClusterModuleInfo *string VSphereVM *infrav1.VSphereVM PatchHelper *patch.Helper diff --git a/pkg/services/govmomi/create_test.go b/pkg/services/govmomi/create_test.go index 2ef46e5a0b..5c5218be5e 100644 --- a/pkg/services/govmomi/create_test.go +++ b/pkg/services/govmomi/create_test.go @@ -41,7 +41,7 @@ func TestCreate(t *testing.T) { defer simr.Destroy() ctx := context.Background() - vmContext := fake.NewVMContext(ctx, fake.NewControllerContext(fake.NewControllerManagerContext())) + vmContext := fake.NewVMContext(ctx, fake.NewControllerManagerContext()) vmContext.VSphereVM.Spec.Server = simr.ServerURL().Host authSession, err := session.GetOrCreate( diff --git a/pkg/services/govmomi/ipam/status_test.go b/pkg/services/govmomi/ipam/status_test.go index 9aeec95b1a..74d460db6f 100644 --- a/pkg/services/govmomi/ipam/status_test.go +++ b/pkg/services/govmomi/ipam/status_test.go @@ -50,7 +50,7 @@ func Test_buildIPAMDeviceConfigs(t *testing.T) { before := func() { ctx = context.Background() - vmCtx = *fake.NewVMContext(ctx, fake.NewControllerContext(fake.NewControllerManagerContext())) + vmCtx = *fake.NewVMContext(ctx, fake.NewControllerManagerContext()) networkStatus = []infrav1.NetworkStatus{ {Connected: true, MACAddr: devMAC}, } @@ -251,7 +251,7 @@ func Test_BuildState(t *testing.T) { before := func() { ctx = context.Background() - vmCtx = *fake.NewVMContext(ctx, fake.NewControllerContext(fake.NewControllerManagerContext())) + vmCtx = *fake.NewVMContext(ctx, fake.NewControllerManagerContext()) networkStatus = []infrav1.NetworkStatus{ {Connected: true, MACAddr: devMAC}, } diff --git a/pkg/services/govmomi/metadata/metadata_suite_test.go b/pkg/services/govmomi/metadata/metadata_suite_test.go index 5c20e3a27a..a9d640a032 100644 --- a/pkg/services/govmomi/metadata/metadata_suite_test.go +++ b/pkg/services/govmomi/metadata/metadata_suite_test.go @@ -141,7 +141,7 @@ func configureSimulatorAndContext(ctx context.Context) (err error) { return } - vmCtx = fake.NewVMContext(ctx, fake.NewControllerContext(fake.NewControllerManagerContext())) + vmCtx = fake.NewVMContext(ctx, fake.NewControllerManagerContext()) vmCtx.VSphereVM.Spec.Server = sim.ServerURL().Host authSession, err := session.GetOrCreate( diff --git a/pkg/services/govmomi/service_test.go b/pkg/services/govmomi/service_test.go index a0867e3823..088d296cb0 100644 --- a/pkg/services/govmomi/service_test.go +++ b/pkg/services/govmomi/service_test.go @@ -45,10 +45,8 @@ const ( func emptyVirtualMachineContext() *virtualMachineContext { return &virtualMachineContext{ VMContext: capvcontext.VMContext{ - Logger: logr.Discard(), - ControllerContext: &capvcontext.ControllerContext{ - ControllerManagerContext: &capvcontext.ControllerManagerContext{}, - }, + Logger: logr.Discard(), + ControllerManagerContext: &capvcontext.ControllerManagerContext{}, }, } } diff --git a/pkg/services/govmomi/vcenter/clone.go b/pkg/services/govmomi/vcenter/clone.go index 16e3bc41d9..6822c90224 100644 --- a/pkg/services/govmomi/vcenter/clone.go +++ b/pkg/services/govmomi/vcenter/clone.go @@ -48,11 +48,11 @@ const ( // in VMContext.VSphereVM.Status.TaskRef. func Clone(ctx context.Context, vmCtx *capvcontext.VMContext, bootstrapData []byte, format bootstrapv1.Format) error { vmCtx = &capvcontext.VMContext{ - ControllerContext: vmCtx.ControllerContext, - VSphereVM: vmCtx.VSphereVM, - Session: vmCtx.Session, - Logger: vmCtx.Logger.WithName("vcenter"), - PatchHelper: vmCtx.PatchHelper, + ControllerManagerContext: vmCtx.ControllerManagerContext, + VSphereVM: vmCtx.VSphereVM, + Session: vmCtx.Session, + Logger: vmCtx.Logger.WithName("vcenter"), + PatchHelper: vmCtx.PatchHelper, } vmCtx.Logger.Info("starting clone process") diff --git a/test/e2e/finalisers_test.go b/test/e2e/finalisers_test.go new file mode 100644 index 0000000000..e69de29bb2