Skip to content

Commit

Permalink
Pass context.Context directly rather than passing its pointer in func…
Browse files Browse the repository at this point in the history
…tion parameters (#714)

Signed-off-by: Wenqi Qiu <wenqiq@vmware.com>
  • Loading branch information
wenqiq committed Aug 23, 2024
1 parent b2e5395 commit 7331b75
Show file tree
Hide file tree
Showing 22 changed files with 235 additions and 236 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,30 +46,30 @@ type IPAddressAllocationReconciler struct {
Recorder record.EventRecorder
}

func deleteSuccess(r *IPAddressAllocationReconciler, _ *context.Context, o *v1alpha1.IPAddressAllocation) {
func deleteSuccess(r *IPAddressAllocationReconciler, _ context.Context, o *v1alpha1.IPAddressAllocation) {
r.Recorder.Event(o, v1.EventTypeNormal, common.ReasonSuccessfulDelete, "IPAddressAllocation CR has been successfully deleted")
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteSuccessTotal, MetricResType)
}

func deleteFail(r *IPAddressAllocationReconciler, c *context.Context, o *v1alpha1.IPAddressAllocation, e *error) {
func deleteFail(r *IPAddressAllocationReconciler, c context.Context, o *v1alpha1.IPAddressAllocation, e *error) {
r.setReadyStatusFalse(c, o, metav1.Now(), e)
r.Recorder.Event(o, v1.EventTypeWarning, common.ReasonFailDelete, fmt.Sprintf("%v", *e))
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteFailTotal, MetricResType)
}

func updateSuccess(r *IPAddressAllocationReconciler, c *context.Context, o *v1alpha1.IPAddressAllocation) {
func updateSuccess(r *IPAddressAllocationReconciler, c context.Context, o *v1alpha1.IPAddressAllocation) {
r.setReadyStatusTrue(c, o, metav1.Now())
r.Recorder.Event(o, v1.EventTypeNormal, common.ReasonSuccessfulUpdate, "IPAddressAllocation CR has been successfully updated")
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerUpdateSuccessTotal, MetricResType)
}

func updateFail(r *IPAddressAllocationReconciler, c *context.Context, o *v1alpha1.IPAddressAllocation, e *error) {
func updateFail(r *IPAddressAllocationReconciler, c context.Context, o *v1alpha1.IPAddressAllocation, e *error) {
r.setReadyStatusFalse(c, o, metav1.Now(), e)
r.Recorder.Event(o, v1.EventTypeWarning, common.ReasonFailUpdate, fmt.Sprintf("%v", *e))
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerUpdateFailTotal, MetricResType)
}

func (r *IPAddressAllocationReconciler) setReadyStatusFalse(ctx *context.Context, ipaddressallocation *v1alpha1.IPAddressAllocation, transitionTime metav1.Time, err *error) {
func (r *IPAddressAllocationReconciler) setReadyStatusFalse(ctx context.Context, ipaddressallocation *v1alpha1.IPAddressAllocation, transitionTime metav1.Time, err *error) {
conditions := []v1alpha1.Condition{
{
Type: v1alpha1.Ready,
Expand All @@ -83,13 +83,13 @@ func (r *IPAddressAllocationReconciler) setReadyStatusFalse(ctx *context.Context
},
}
ipaddressallocation.Status.Conditions = conditions
e := r.Client.Status().Update(*ctx, ipaddressallocation)
e := r.Client.Status().Update(ctx, ipaddressallocation)
if e != nil {
log.Error(e, "unable to update IPAddressAllocation status", "IPAddressAllocation", ipaddressallocation)
}
}

func (r *IPAddressAllocationReconciler) setReadyStatusTrue(ctx *context.Context, ipaddressallocation *v1alpha1.IPAddressAllocation, transitionTime metav1.Time) {
func (r *IPAddressAllocationReconciler) setReadyStatusTrue(ctx context.Context, ipaddressallocation *v1alpha1.IPAddressAllocation, transitionTime metav1.Time) {
conditions := []v1alpha1.Condition{
{
Type: v1alpha1.Ready,
Expand All @@ -100,7 +100,7 @@ func (r *IPAddressAllocationReconciler) setReadyStatusTrue(ctx *context.Context,
},
}
ipaddressallocation.Status.Conditions = conditions
e := r.Client.Status().Update(*ctx, ipaddressallocation)
e := r.Client.Status().Update(ctx, ipaddressallocation)
if e != nil {
log.Error(e, "unable to update IPAddressAllocation status", "IPAddressAllocation", ipaddressallocation)
}
Expand All @@ -127,19 +127,19 @@ func (r *IPAddressAllocationReconciler) handleUpdate(ctx context.Context, req ct
controllerutil.AddFinalizer(obj, servicecommon.IPAddressAllocationFinalizerName)
if err := r.Client.Update(ctx, obj); err != nil {
log.Error(err, "add finalizer", "IPAddressAllocation", req.NamespacedName)
updateFail(r, &ctx, obj, &err)
updateFail(r, ctx, obj, &err)
return resultRequeue, err
}
log.V(1).Info("added finalizer on IPAddressAllocation CR", "IPAddressAllocation", req.NamespacedName)
}

updated, err := r.Service.CreateOrUpdateIPAddressAllocation(obj)
if err != nil {
updateFail(r, &ctx, obj, &err)
updateFail(r, ctx, obj, &err)
return resultRequeue, err
}
if updated {
updateSuccess(r, &ctx, obj)
updateSuccess(r, ctx, obj)
}
return resultNormal, nil
}
Expand All @@ -149,17 +149,17 @@ func (r *IPAddressAllocationReconciler) handleDeletion(ctx context.Context, req
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResType)
if err := r.Service.DeleteIPAddressAllocation(obj); err != nil {
log.Error(err, "deletion failed, would retry exponentially", "IPAddressAllocation", req.NamespacedName)
deleteFail(r, &ctx, obj, &err)
deleteFail(r, ctx, obj, &err)
return resultRequeue, err
}
controllerutil.RemoveFinalizer(obj, servicecommon.IPAddressAllocationFinalizerName)
if err := r.Client.Update(ctx, obj); err != nil {
log.Error(err, "deletion failed, would retry exponentially", "IPAddressAllocation", req.NamespacedName)
deleteFail(r, &ctx, obj, &err)
deleteFail(r, ctx, obj, &err)
return resultRequeue, err
}
log.V(1).Info("removed finalizer on IPAddressAllocation CR", "IPAddressAllocation", req.NamespacedName)
deleteSuccess(r, &ctx, obj)
deleteSuccess(r, ctx, obj)
log.Info("successfully deleted IPAddressAllocation CR and all subnets", "IPAddressAllocation", obj)
} else {
// only print a message because it's not a normal case
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestIPAddressAllocationController_setReadyStatusTrue(t *testing.T) {
LastTransitionTime: transitionTime,
},
}
r.setReadyStatusTrue(&ctx, dummyIPAddressAllocation, transitionTime)
r.setReadyStatusTrue(ctx, dummyIPAddressAllocation, transitionTime)

if !reflect.DeepEqual(dummyIPAddressAllocation.Status.Conditions, newConditions) {
t.Fatalf("Failed to correctly update Status Conditions when conditions haven't changed")
Expand Down
14 changes: 7 additions & 7 deletions pkg/controllers/namespace/namespace_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ func (r *NamespaceReconciler) getDefaultNetworkConfigName() (string, error) {
return nc.Name, nil
}

func (r *NamespaceReconciler) createNetworkInfoCR(ctx *context.Context, obj client.Object, ns string, ncName string) (*v1alpha1.NetworkInfo, error) {
func (r *NamespaceReconciler) createNetworkInfoCR(ctx context.Context, obj client.Object, ns string, ncName string) (*v1alpha1.NetworkInfo, error) {
networkInfos := &v1alpha1.NetworkInfoList{}
r.Client.List(*ctx, networkInfos, client.InNamespace(ns))
r.Client.List(ctx, networkInfos, client.InNamespace(ns))
if len(networkInfos.Items) > 0 {
// if there is already one networkInfo, return this networkInfo
log.Info("networkInfo already exists", "networkInfo", networkInfos.Items[0].Name, "Namespace", ns)
Expand All @@ -67,7 +67,7 @@ func (r *NamespaceReconciler) createNetworkInfoCR(ctx *context.Context, obj clie
},
VPCs: []v1alpha1.VPCState{},
}
err := r.Client.Create(*ctx, networkInfoCR)
err := r.Client.Create(ctx, networkInfoCR)
if err != nil {
message := "failed to create NetworkInfo CR"
r.namespaceError(ctx, obj, message, err)
Expand Down Expand Up @@ -137,7 +137,7 @@ func (r *NamespaceReconciler) createDefaultSubnetSet(ns string) error {
return nil
}

func (r *NamespaceReconciler) namespaceError(ctx *context.Context, k8sObj client.Object, msg string, err error) {
func (r *NamespaceReconciler) namespaceError(ctx context.Context, k8sObj client.Object, msg string, err error) {
logErr := util.If(err == nil, errors.New(msg), err).(error)
log.Error(logErr, msg)
changes := map[string]string{AnnotationNamespaceVPCError: msg}
Expand Down Expand Up @@ -224,17 +224,17 @@ func (r *NamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
nc, ncExist := r.VPCService.GetVPCNetworkConfig(ncName)
if !ncExist {
message := fmt.Sprintf("missing network config %s for namespace %s", ncName, ns)
r.namespaceError(&ctx, obj, message, nil)
r.namespaceError(ctx, obj, message, nil)
return common.ResultRequeueAfter10sec, nil
}
if !r.VPCService.ValidateNetworkConfig(nc) {
// if network config is not valid, no need to retry, skip processing
message := fmt.Sprintf("invalid network config %s for namespace %s, missing private cidr", ncName, ns)
r.namespaceError(&ctx, obj, message, nil)
r.namespaceError(ctx, obj, message, nil)
return common.ResultRequeueAfter10sec, nil
}

if _, err := r.createNetworkInfoCR(&ctx, obj, ns, ncName); err != nil {
if _, err := r.createNetworkInfoCR(ctx, obj, ns, ncName); err != nil {
return common.ResultRequeueAfter10sec, nil
}
if err := r.createDefaultSubnetSet(ns); err != nil {
Expand Down
39 changes: 19 additions & 20 deletions pkg/controllers/networkinfo/networkinfo_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request)
controllerutil.AddFinalizer(obj, commonservice.NetworkInfoFinalizerName)
if err := r.Client.Update(ctx, obj); err != nil {
log.Error(err, "add finalizer", "NetworkInfo", req.NamespacedName)
updateFail(r, &ctx, obj, &err, r.Client, nil)
updateFail(r, ctx, obj, &err, r.Client, nil)
return common.ResultRequeue, err
}
log.V(1).Info("added finalizer on NetworkInfo CR", "NetworkInfo", req.NamespacedName)
Expand All @@ -69,25 +69,25 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request)
ncName, err := r.Service.GetNetworkconfigNameFromNS(obj.Namespace)
if err != nil {
log.Error(err, "failed to get network config name for VPC when creating NSX VPC", "VPC", obj.Name)
updateFail(r, &ctx, obj, &err, r.Client, nil)
updateFail(r, ctx, obj, &err, r.Client, nil)
return common.ResultRequeueAfter10sec, err
}
nc, _exist := r.Service.GetVPCNetworkConfig(ncName)
if !_exist {
message := fmt.Sprintf("failed to read network config %s when creating NSX VPC", ncName)
log.Info(message)
updateFail(r, &ctx, obj, &err, r.Client, nil)
updateFail(r, ctx, obj, &err, r.Client, nil)
return common.ResultRequeueAfter10sec, errors.New(message)
}
log.Info("got network config from store", "NetworkConfig", ncName)
vpcNetworkConfiguration := &v1alpha1.VPCNetworkConfiguration{}
err = r.Client.Get(ctx, types.NamespacedName{Name: commonservice.SystemVPCNetworkConfigurationName}, vpcNetworkConfiguration)
if err != nil {
log.Error(err, "failed to get system VPCNetworkConfiguration")
updateFail(r, &ctx, obj, &err, r.Client, nil)
updateFail(r, ctx, obj, &err, r.Client, nil)
return common.ResultRequeueAfter10sec, err
}
gatewayConnectionReady, _, err := getGatewayConnectionStatus(&ctx, vpcNetworkConfiguration)
gatewayConnectionReady, _, err := getGatewayConnectionStatus(ctx, vpcNetworkConfiguration)
if err != nil {
log.Error(err, "failed to get the gateway connection status", "req", req.NamespacedName)
return common.ResultRequeueAfter10sec, err
Expand All @@ -100,17 +100,17 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request)
log.Info("got the gateway connection status", "gatewayConnectionReady", gatewayConnectionReady, "reason", reason)
if err != nil {
log.Error(err, "failed to validate the edge and gateway connection", "org", nc.Org, "project", nc.NSXProject)
updateFail(r, &ctx, obj, &err, r.Client, nil)
updateFail(r, ctx, obj, &err, r.Client, nil)
return common.ResultRequeueAfter10sec, err
}
vpcNetworkConfiguration := &v1alpha1.VPCNetworkConfiguration{}
err := r.Client.Get(ctx, types.NamespacedName{Name: ncName}, vpcNetworkConfiguration)
if err != nil {
log.Error(err, "failed to get VPCNetworkConfiguration", "Name", ncName)
updateFail(r, &ctx, obj, &err, r.Client, nil)
updateFail(r, ctx, obj, &err, r.Client, nil)
return common.ResultRequeueAfter10sec, err
}
setVPCNetworkConfigurationStatusWithGatewayConnection(&ctx, r.Client, vpcNetworkConfiguration, gatewayConnectionReady, reason)
setVPCNetworkConfigurationStatusWithGatewayConnection(ctx, r.Client, vpcNetworkConfiguration, gatewayConnectionReady, reason)
} else {
log.Info("skipping reconciling the network info because the system gateway connection is not ready", "NetworkInfo", req.NamespacedName)
return common.ResultRequeueAfter60sec, nil
Expand All @@ -120,7 +120,7 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request)
createdVpc, err := r.Service.CreateOrUpdateVPC(obj, &nc)
if err != nil {
log.Error(err, "create vpc failed, would retry exponentially", "VPC", req.NamespacedName)
updateFail(r, &ctx, obj, &err, r.Client, nil)
updateFail(r, ctx, obj, &err, r.Client, nil)
return common.ResultRequeueAfter10sec, err
}

Expand Down Expand Up @@ -172,7 +172,7 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request)
LoadBalancerIPAddresses: "",
PrivateIPs: privateIPs,
}
updateFail(r, &ctx, obj, &err, r.Client, state)
updateFail(r, ctx, obj, &err, r.Client, state)
return common.ResultRequeueAfter10sec, err
}
}
Expand All @@ -181,16 +181,15 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request)
err := r.Client.Get(ctx, types.NamespacedName{Name: ncName}, vpcNetworkConfiguration)
if err != nil {
log.Error(err, "failed to get VPCNetworkConfiguration", "Name", ncName)
updateFail(r, &ctx, obj, &err, r.Client, nil)
updateFail(r, ctx, obj, &err, r.Client, nil)
return common.ResultRequeueAfter10sec, err
}
if autoSnatEnabled {
log.Info("detected that the AutoSnat is enabled", "req", req.NamespacedName)
setVPCNetworkConfigurationStatusWithSnatEnabled(&ctx, r.Client,
vpcNetworkConfiguration, true)
setVPCNetworkConfigurationStatusWithSnatEnabled(ctx, r.Client, vpcNetworkConfiguration, true)
} else {
log.Info("detected that the AutoSnat is disabled", "req", req.NamespacedName)
setVPCNetworkConfigurationStatusWithSnatEnabled(&ctx, r.Client, vpcNetworkConfiguration, false)
setVPCNetworkConfigurationStatusWithSnatEnabled(ctx, r.Client, vpcNetworkConfiguration, false)
}
}

Expand All @@ -208,7 +207,7 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request)
LoadBalancerIPAddresses: "",
PrivateIPs: privateIPs,
}
updateFail(r, &ctx, obj, &err, r.Client, state)
updateFail(r, ctx, obj, &err, r.Client, state)
return common.ResultRequeueAfter10sec, err
}
}
Expand All @@ -221,8 +220,8 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request)
PrivateIPs: privateIPs,
}
// AKO needs to know the AVI subnet path created by NSX
setVPCNetworkConfigurationStatusWithLBS(&ctx, r.Client, ncName, state.Name, path, r.Service.GetNSXLBSPath(*createdVpc.Id))
updateSuccess(r, &ctx, obj, r.Client, state, nc.Name, path)
setVPCNetworkConfigurationStatusWithLBS(ctx, r.Client, ncName, state.Name, path, r.Service.GetNSXLBSPath(*createdVpc.Id))
updateSuccess(r, ctx, obj, r.Client, state, nc.Name, path)
} else {
if controllerutil.ContainsFinalizer(obj, commonservice.NetworkInfoFinalizerName) {
metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, common.MetricResTypeNetworkInfo)
Expand All @@ -241,7 +240,7 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request)
// first delete vpc and then ipblock or else it will fail arguing it is being referenced by other objects
if err := r.Service.DeleteVPC(*vpc.Path); err != nil {
log.Error(err, "failed to delete nsx VPC, would retry exponentially", "NetworkInfo", req.NamespacedName)
deleteFail(r, &ctx, obj, &err, r.Client)
deleteFail(r, ctx, obj, &err, r.Client)
return common.ResultRequeueAfter10sec, err
}
if err := r.Service.DeleteIPBlockInVPC(*vpc); err != nil {
Expand All @@ -252,11 +251,11 @@ func (r *NetworkInfoReconciler) Reconcile(ctx context.Context, req ctrl.Request)

controllerutil.RemoveFinalizer(obj, commonservice.NetworkInfoFinalizerName)
if err := r.Client.Update(ctx, obj); err != nil {
deleteFail(r, &ctx, obj, &err, r.Client)
deleteFail(r, ctx, obj, &err, r.Client)
return common.ResultRequeue, err
}
log.V(1).Info("removed finalizer", "NetworkInfo", req.NamespacedName)
deleteSuccess(r, &ctx, obj)
deleteSuccess(r, ctx, obj)
} else {
// only print a message because it's not a normal case
log.Info("finalizers cannot be recognized", "NetworkInfo", req.NamespacedName)
Expand Down
Loading

0 comments on commit 7331b75

Please sign in to comment.