Skip to content

Commit

Permalink
remove controllerctx from clusterctx part2
Browse files Browse the repository at this point in the history
  • Loading branch information
sbueringer committed Sep 13, 2023
1 parent 4876c23 commit 645848c
Show file tree
Hide file tree
Showing 22 changed files with 249 additions and 220 deletions.
165 changes: 87 additions & 78 deletions controllers/serviceaccount_controller.go

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions controllers/serviceaccount_controller_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func unitTestsReconcileNormal() {
// a fake vSphereCluster in the test and pass it to during context setup.
reconciler = ServiceAccountReconciler{}
controllerCtx = helpers.NewUnitTestContextForController(namespace, vsphereCluster, false, initObjects, nil)
_, err := reconciler.ReconcileNormal(controllerCtx.GuestClusterContext)
_, err := reconciler.ReconcileNormal(ctx, controllerCtx.GuestClusterContext)
Expect(err).NotTo(HaveOccurred())

// Update the VSphereCluster and its status in the fake client.
Expand Down Expand Up @@ -83,7 +83,7 @@ func unitTestsReconcileNormal() {
}
})
It("should create a service account and a secret", func() {
_, err := reconciler.ReconcileNormal(controllerCtx.GuestClusterContext)
_, err := reconciler.ReconcileNormal(ctx, controllerCtx.GuestClusterContext)
Expect(err).NotTo(HaveOccurred())

svcAccount := &corev1.ServiceAccount{}
Expand Down Expand Up @@ -137,6 +137,6 @@ func unitTestsReconcileNormal() {
// and then re-invokes reconcileNormal.
func updateServiceAccountSecretAndReconcileNormal(ctx context.Context, controllerCtx *helpers.UnitTestContextForController, reconciler ServiceAccountReconciler, object client.Object) {
assertServiceAccountAndUpdateSecret(ctx, controllerCtx.ControllerContext.Client, object.GetNamespace(), object.GetName())
_, err := reconciler.ReconcileNormal(controllerCtx.GuestClusterContext)
_, err := reconciler.ReconcileNormal(ctx, controllerCtx.GuestClusterContext)
Expect(err).NotTo(HaveOccurred())
}
69 changes: 36 additions & 33 deletions controllers/servicediscovery_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func AddServiceDiscoveryControllerToManager(controllerCtx *capvcontext.Controlle
Recorder: record.New(mgr.GetEventRecorderFor(controllerNameLong)),
Logger: controllerCtx.Logger.WithName(controllerNameShort),
}
r := serviceDiscoveryReconciler{
r := &serviceDiscoveryReconciler{
ControllerContext: controllerContext,
remoteClusterCacheTracker: tracker,
}
Expand Down Expand Up @@ -131,15 +131,15 @@ type serviceDiscoveryReconciler struct {
remoteClusterCacheTracker *remote.ClusterCacheTracker
}

func (r serviceDiscoveryReconciler) Reconcile(ctx context.Context, req reconcile.Request) (_ reconcile.Result, reterr error) {
logger := r.Logger.WithName(req.Namespace).WithName(req.Name)
logger.V(4).Info("Starting Reconcile")
func (r *serviceDiscoveryReconciler) Reconcile(ctx context.Context, req reconcile.Request) (_ reconcile.Result, reterr error) {
log := r.Logger.WithName(req.Namespace).WithName(req.Name)
log.V(4).Info("Starting Reconcile")

// Get the vspherecluster for this request.
vsphereCluster := &vmwarev1.VSphereCluster{}
if err := r.Client.Get(r, req.NamespacedName, vsphereCluster); err != nil {
if apierrors.IsNotFound(err) {
logger.Info("Cluster not found, won't reconcile", "key", req.NamespacedName)
log.Info("Cluster not found, won't reconcile", "key", req.NamespacedName)
return reconcile.Result{}, nil
}
return reconcile.Result{}, err
Expand All @@ -158,11 +158,9 @@ func (r serviceDiscoveryReconciler) Reconcile(ctx context.Context, req reconcile

// Create the cluster context for this request.
clusterContext := &vmwarecontext.ClusterContext{
ControllerContext: r.ControllerContext,
VSphereCluster: vsphereCluster,
PatchHelper: patchHelper,
VSphereCluster: vsphereCluster,
PatchHelper: patchHelper,
}
clusterContext.ControllerContext.Logger = logger

// Always issue a patch when exiting this function so changes to the
// resource are patched back to the API server.
Expand All @@ -171,7 +169,7 @@ func (r serviceDiscoveryReconciler) Reconcile(ctx context.Context, req reconcile
if reterr == nil {
reterr = err
} else {
clusterContext.ControllerContext.Logger.Error(err, "patch failed", "cluster", clusterContext.String())
log.Error(err, "patch failed", "cluster", clusterContext.String())
}
}
}()
Expand All @@ -183,32 +181,33 @@ func (r serviceDiscoveryReconciler) Reconcile(ctx context.Context, req reconcile

cluster, err := clusterutilv1.GetClusterFromMetadata(r, r.Client, vsphereCluster.ObjectMeta)
if err != nil {
logger.Info("unable to get capi cluster from vsphereCluster", "err", err)
log.Info("unable to get capi cluster from vsphereCluster", "err", err)
return reconcile.Result{RequeueAfter: clusterNotReadyRequeueTime}, nil
}

// We cannot proceed until we are able to access the target cluster. Until
// then just return a no-op and wait for the next sync.
guestClient, err := r.remoteClusterCacheTracker.GetClient(clusterContext.ControllerContext, client.ObjectKeyFromObject(cluster))
guestClient, err := r.remoteClusterCacheTracker.GetClient(ctx, client.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")
return ctrl.Result{Requeue: true}, nil
}
logger.Info("The control plane is not ready yet", "err", err)
log.Info("The control plane is not ready yet", "err", err)
return reconcile.Result{RequeueAfter: clusterNotReadyRequeueTime}, nil
}

// Defer to the Reconciler for reconciling a non-delete event.
return r.ReconcileNormal(&vmwarecontext.GuestClusterContext{
return r.ReconcileNormal(ctx, &vmwarecontext.GuestClusterContext{
ClusterContext: clusterContext,
GuestClient: guestClient,
})
}

func (r serviceDiscoveryReconciler) ReconcileNormal(guestClusterCtx *vmwarecontext.GuestClusterContext) (reconcile.Result, error) {
guestClusterCtx.ControllerContext.Logger.V(4).Info("Reconciling Service Discovery", "cluster", guestClusterCtx.VSphereCluster.Name)
if err := r.reconcileSupervisorHeadlessService(guestClusterCtx); err != nil {
func (r *serviceDiscoveryReconciler) ReconcileNormal(ctx context.Context, guestClusterCtx *vmwarecontext.GuestClusterContext) (reconcile.Result, error) {
log := ctrl.LoggerFrom(ctx)
log.V(4).Info("Reconciling Service Discovery", "cluster", guestClusterCtx.VSphereCluster.Name)
if err := r.reconcileSupervisorHeadlessService(ctx, guestClusterCtx); err != nil {
conditions.MarkFalse(guestClusterCtx.VSphereCluster, vmwarev1.ServiceDiscoveryReadyCondition, vmwarev1.SupervisorHeadlessServiceSetupFailedReason,
clusterv1.ConditionSeverityWarning, err.Error())
return reconcile.Result{}, errors.Wrapf(err, "failed to configure supervisor headless service for %v", guestClusterCtx.VSphereCluster)
Expand All @@ -219,15 +218,17 @@ func (r serviceDiscoveryReconciler) ReconcileNormal(guestClusterCtx *vmwareconte

// Setup a local k8s service in the target cluster that proxies to the Supervisor Cluster API Server. The add-ons are
// dependent on this local service to connect to the Supervisor Cluster.
func (r serviceDiscoveryReconciler) reconcileSupervisorHeadlessService(guestClusterCtx *vmwarecontext.GuestClusterContext) error {
func (r *serviceDiscoveryReconciler) reconcileSupervisorHeadlessService(ctx context.Context, guestClusterCtx *vmwarecontext.GuestClusterContext) error {
log := ctrl.LoggerFrom(ctx)

// Create the headless service to the supervisor api server on the target cluster.
supervisorPort := vmwarev1.SupervisorAPIServerPort
svc := NewSupervisorHeadlessService(vmwarev1.SupervisorHeadlessSvcPort, supervisorPort)
if err := guestClusterCtx.GuestClient.Create(guestClusterCtx.ControllerContext, svc); err != nil && !apierrors.IsAlreadyExists(err) {
if err := guestClusterCtx.GuestClient.Create(ctx, svc); err != nil && !apierrors.IsAlreadyExists(err) {
return errors.Wrapf(err, "cannot create k8s service %s/%s in ", svc.Namespace, svc.Name)
}

supervisorHost, err := GetSupervisorAPIServerAddress(guestClusterCtx.ClusterContext)
supervisorHost, err := r.getSupervisorAPIServerAddress(ctx)
if err != nil {
// Note: We have watches on the LB Svc (VIP) & the cluster-info configmap (FIP). There is no need to return an error to keep
// re-trying.
Expand All @@ -236,7 +237,7 @@ func (r serviceDiscoveryReconciler) reconcileSupervisorHeadlessService(guestClus
return nil
}

guestClusterCtx.ControllerContext.Logger.Info("Discovered supervisor apiserver address", "host", supervisorHost, "port", supervisorPort)
log.Info("Discovered supervisor apiserver address", "host", supervisorHost, "port", supervisorPort)
// CreateOrPatch the newEndpoints with the discovered supervisor api server address
newEndpoints := NewSupervisorHeadlessServiceEndpoints(
supervisorHost,
Expand All @@ -253,7 +254,7 @@ func (r serviceDiscoveryReconciler) reconcileSupervisorHeadlessService(guestClus
},
}
result, err := controllerutil.CreateOrPatch(
guestClusterCtx.ControllerContext,
ctx,
guestClusterCtx.GuestClient,
endpoints,
func() error {
Expand All @@ -272,31 +273,31 @@ func (r serviceDiscoveryReconciler) reconcileSupervisorHeadlessService(guestClus

switch result {
case controllerutil.OperationResultNone:
guestClusterCtx.ControllerContext.Logger.Info(
log.Info(
"no update required for k8s service endpoints",
"endpointsKey",
endpointsKey,
"endpointsSubsets",
endpointsSubsetsStr,
)
case controllerutil.OperationResultCreated:
guestClusterCtx.ControllerContext.Logger.Info(
log.Info(
"created k8s service endpoints",
"endpointsKey",
endpointsKey,
"endpointsSubsets",
endpointsSubsetsStr,
)
case controllerutil.OperationResultUpdated:
guestClusterCtx.ControllerContext.Logger.Info(
log.Info(
"updated k8s service endpoints",
"endpointsKey",
endpointsKey,
"endpointsSubsets",
endpointsSubsetsStr,
)
default:
guestClusterCtx.ControllerContext.Logger.Error(
log.Error(
fmt.Errorf(
"unexpected result during createOrPatch k8s service endpoints",
),
Expand All @@ -313,18 +314,20 @@ func (r serviceDiscoveryReconciler) reconcileSupervisorHeadlessService(guestClus
return nil
}

func GetSupervisorAPIServerAddress(clusterCtx *vmwarecontext.ClusterContext) (string, error) {
func (r *serviceDiscoveryReconciler) getSupervisorAPIServerAddress(ctx context.Context) (string, error) {
log := ctrl.LoggerFrom(ctx)

// Discover the supervisor api server address
// 1. Check if a k8s service "kube-system/kube-apiserver-lb-svc" is available, if so, fetch the loadbalancer IP.
// 2. If not, get the Supervisor Cluster Management Network Floating IP (FIP) from the cluster-info configmap. This is
// to support non-NSX-T development usecases only. If we are unable to find the cluster-info configmap for some reason,
// we log the error.
supervisorHost, err := GetSupervisorAPIServerVIP(clusterCtx.ControllerContext.Client)
supervisorHost, err := GetSupervisorAPIServerVIP(r.Client)
if err != nil {
clusterCtx.ControllerContext.Logger.Info("Unable to discover supervisor apiserver virtual ip, fallback to floating ip", "reason", err.Error())
supervisorHost, err = GetSupervisorAPIServerFIP(clusterCtx.ControllerContext.Client)
log.Info("Unable to discover supervisor apiserver virtual ip, fallback to floating ip", "reason", err.Error())
supervisorHost, err = GetSupervisorAPIServerFIP(r.Client)
if err != nil {
clusterCtx.ControllerContext.Logger.Error(err, "Unable to discover supervisor apiserver address")
log.Error(err, "Unable to discover supervisor apiserver address")
return "", errors.Wrapf(err, "Unable to discover supervisor apiserver address")
}
}
Expand Down Expand Up @@ -453,7 +456,7 @@ func getClusterFromKubeConfig(config *clientcmdapi.Config) *clientcmdapi.Cluster

// serviceToClusters is a mapper function used to enqueue reconcile.Requests
// It watches for Service objects of type LoadBalancer for the supervisor api-server.
func (r serviceDiscoveryReconciler) serviceToClusters(ctx context.Context, o client.Object) []reconcile.Request {
func (r *serviceDiscoveryReconciler) serviceToClusters(ctx context.Context, o client.Object) []reconcile.Request {
if o.GetNamespace() != vmwarev1.SupervisorLoadBalancerSvcNamespace || o.GetName() != vmwarev1.SupervisorLoadBalancerSvcName {
return nil
}
Expand All @@ -462,7 +465,7 @@ func (r serviceDiscoveryReconciler) serviceToClusters(ctx context.Context, o cli

// configMapToClusters is a mapper function used to enqueue reconcile.Requests
// It watches for cluster-info configmaps for the supervisor api-server.
func (r serviceDiscoveryReconciler) configMapToClusters(ctx context.Context, o client.Object) []reconcile.Request {
func (r *serviceDiscoveryReconciler) configMapToClusters(ctx context.Context, o client.Object) []reconcile.Request {
if o.GetNamespace() != metav1.NamespacePublic || o.GetName() != bootstrapapi.ConfigMapClusterInfo {
return nil
}
Expand Down
7 changes: 5 additions & 2 deletions controllers/svcdiscovery_controller_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func serviceDiscoveryUnitTestsReconcileNormal() {
JustBeforeEach(func() {
vsphereCluster = fake.NewVSphereCluster(namespace)
controllerCtx = helpers.NewUnitTestContextForController(namespace, &vsphereCluster, false, initObjects, nil)
_, err := reconciler.ReconcileNormal(controllerCtx.GuestClusterContext)
_, err := reconciler.ReconcileNormal(ctx, controllerCtx.GuestClusterContext)
Expect(err).NotTo(HaveOccurred())

// Update the VSphereCluster and its status in the fake client.
Expand Down Expand Up @@ -77,7 +77,10 @@ func serviceDiscoveryUnitTestsReconcileNormal() {
assertServiceDiscoveryCondition(controllerCtx.VSphereCluster, corev1.ConditionTrue, "", "", "")
})
It("Should get supervisor master endpoint IP", func() {
supervisorEndpointIP, err := GetSupervisorAPIServerAddress(controllerCtx.ClusterContext)
r := &serviceDiscoveryReconciler{
ControllerContext: controllerCtx.ControllerContext,
}
supervisorEndpointIP, err := r.getSupervisorAPIServerAddress(ctx)
Expect(err).ShouldNot(HaveOccurred())
Expect(supervisorEndpointIP).To(Equal(testSupervisorAPIServerVIP))
})
Expand Down
2 changes: 1 addition & 1 deletion controllers/vmware/vspherecluster_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func (r *ClusterReconciler) reconcileNormal(ctx context.Context, clusterCtx *vmw
clusterCtx.VSphereCluster.Status.ResourcePolicyName = resourcePolicyName

// Configure the cluster for the cluster network
err = r.NetworkProvider.ProvisionClusterNetwork(clusterCtx)
err = r.NetworkProvider.ProvisionClusterNetwork(ctx, clusterCtx)
if err != nil {
return errors.Wrapf(err,
"failed to configure cluster network for vsphereCluster %s/%s",
Expand Down
14 changes: 8 additions & 6 deletions controllers/vmware/vspherecluster_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
ctrl "sigs.k8s.io/controller-runtime"

vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
capvcontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/context/vmware"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/services/network"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/services/vmoperator"
Expand Down Expand Up @@ -57,26 +58,27 @@ var _ = Describe("Cluster Controller Tests", func() {
vsphereMachine *vmwarev1.VSphereMachine
ctx = ctrl.SetupSignalHandler()
clusterCtx *vmware.ClusterContext
controllerCtx *capvcontext.ControllerContext
reconciler *ClusterReconciler
)

BeforeEach(func() {
// Create all necessary dependencies
cluster = util.CreateCluster(clusterName)
vsphereCluster = util.CreateVSphereCluster(clusterName)
clusterCtx = util.CreateClusterContext(cluster, vsphereCluster)
clusterCtx, controllerCtx = util.CreateClusterContext(cluster, vsphereCluster)
vsphereMachine = util.CreateVSphereMachine(machineName, clusterName, className, imageName, storageClass, controlPlaneLabelTrue)

reconciler = &ClusterReconciler{
Client: clusterCtx.ControllerContext.Client,
Client: controllerCtx.Client,
NetworkProvider: network.DummyNetworkProvider(),
ControlPlaneService: &vmoperator.CPService{
Client: clusterCtx.ControllerContext.Client,
Client: controllerCtx.Client,
},
}

Expect(clusterCtx.ControllerContext.Client.Create(ctx, cluster)).To(Succeed())
Expect(clusterCtx.ControllerContext.Client.Create(ctx, vsphereCluster)).To(Succeed())
Expect(controllerCtx.Client.Create(ctx, cluster)).To(Succeed())
Expect(controllerCtx.Client.Create(ctx, vsphereCluster)).To(Succeed())
})

// Ensure that the mechanism for reconciling clusters when a control plane machine gets an IP works
Expand Down Expand Up @@ -148,7 +150,7 @@ var _ = Describe("Cluster Controller Tests", func() {
},
}

Expect(clusterCtx.ControllerContext.Client.Create(ctx, zone)).To(Succeed())
Expect(controllerCtx.Client.Create(ctx, zone)).To(Succeed())
}

fds, err := reconciler.getFailureDomains(ctx)
Expand Down
2 changes: 1 addition & 1 deletion controllers/vspheremachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ func (r *machineReconciler) setVMModifiers(machineCtx capvcontext.MachineContext
// 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(supervisorMachineCtx.GetClusterContext(), vm)
err := r.networkProvider.ConfigureVirtualMachine(context.TODO(), supervisorMachineCtx.GetClusterContext(), vm)
if err != nil {
return nil, errors.Errorf("failed to configure machine network: %+v", err)
}
Expand Down
11 changes: 7 additions & 4 deletions pkg/context/fake/fake_guest_cluster_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,29 @@ limitations under the License.
package fake

import (
"context"

apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/client-go/kubernetes/scheme"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

capvcontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/context/vmware"
)

// NewGuestClusterContext returns a fake GuestClusterContext for unit testing
// guest cluster controllers with a fake client.
func NewGuestClusterContext(ctx *vmware.ClusterContext, prototypeCluster bool, gcInitObjects ...client.Object) *vmware.GuestClusterContext {
func NewGuestClusterContext(ctx context.Context, clusterCtx *vmware.ClusterContext, controllerCtx *capvcontext.ControllerContext, prototypeCluster bool, gcInitObjects ...client.Object) *vmware.GuestClusterContext {
if prototypeCluster {
cluster := newCluster(ctx.VSphereCluster)
if err := ctx.ControllerContext.Client.Create(ctx.ControllerContext, cluster); err != nil {
cluster := newCluster(clusterCtx.VSphereCluster)
if err := controllerCtx.Client.Create(ctx, cluster); err != nil {
panic(err)
}
}

return &vmware.GuestClusterContext{
ClusterContext: ctx,
ClusterContext: clusterCtx,
GuestClient: NewFakeGuestClusterClient(gcInitObjects...),
}
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/context/fake/fake_vmware_cluster_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ func NewVmwareClusterContext(controllerCtx *capvcontext.ControllerContext, names
}

return &vmware.ClusterContext{
ControllerContext: controllerCtx,
VSphereCluster: vsphereCluster,
VSphereCluster: vsphereCluster,
}
}
8 changes: 3 additions & 5 deletions pkg/context/vmware/cluster_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,13 @@ import (
"sigs.k8s.io/cluster-api/util/patch"

vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1"
capvcontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
)

// ClusterContext is a Go context used with a CAPI cluster.
type ClusterContext struct {
ControllerContext *capvcontext.ControllerContext
Cluster *clusterv1.Cluster
VSphereCluster *vmwarev1.VSphereCluster
PatchHelper *patch.Helper
Cluster *clusterv1.Cluster
VSphereCluster *vmwarev1.VSphereCluster
PatchHelper *patch.Helper
}

// String returns ClusterAPIVersion/ClusterNamespace/ClusterName.
Expand Down
Loading

0 comments on commit 645848c

Please sign in to comment.