Skip to content

Commit

Permalink
controllers: reuse the context from reconcile
Browse files Browse the repository at this point in the history 
We dont need to create a new context for each
time for a reconcile we should use the ctx
from the Reconcile for all the dependent operations.

Signed-off-by: Madhu Rajanna <[email protected]>
  • Loading branch information
@Madhu-1
Madhu-1 committed Mar 26, 2024
1 parent cba970f commit 74f781f
Show file tree
Hide file tree
Showing 13 changed files with 99 additions and 97 deletions.
21 changes: 11 additions & 10 deletions controllers/clusterversion_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ type ClusterVersionReconciler struct {
func (r *ClusterVersionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
logger := log.FromContext(ctx)
instance := configv1.ClusterVersion{}
if err := r.Client.Get(context.TODO(), req.NamespacedName, &instance); err != nil {
if err := r.Client.Get(ctx, req.NamespacedName, &instance); err != nil {
return ctrl.Result{}, err
}
if err := r.ensureConsolePlugin(instance.Status.Desired.Version); err != nil {
if err := r.ensureConsolePlugin(ctx, instance.Status.Desired.Version); err != nil {
logger.Error(err, "Could not ensure compatibility for ODF consolePlugin")
return ctrl.Result{}, err
}
Expand All @@ -68,13 +68,14 @@ func (r *ClusterVersionReconciler) Reconcile(ctx context.Context, req ctrl.Reque

// SetupWithManager sets up the controller with the Manager.
func (r *ClusterVersionReconciler) SetupWithManager(mgr ctrl.Manager) error {
ctx := context.TODO()
err := mgr.Add(manager.RunnableFunc(func(context.Context) error {
clusterVersion, err := util.DetermineOpenShiftVersion(r.Client)
clusterVersion, err := util.DetermineOpenShiftVersion(ctx, r.Client)
if err != nil {
return err
}

return r.ensureConsolePlugin(clusterVersion)
return r.ensureConsolePlugin(ctx, clusterVersion)
}))
if err != nil {
return err
Expand All @@ -85,15 +86,15 @@ func (r *ClusterVersionReconciler) SetupWithManager(mgr ctrl.Manager) error {
Complete(r)
}

func (r *ClusterVersionReconciler) ensureConsolePlugin(clusterVersion string) error {
logger := log.FromContext(context.TODO())
func (r *ClusterVersionReconciler) ensureConsolePlugin(ctx context.Context, clusterVersion string) error {
logger := log.FromContext(ctx)
// The base path to where the request are sent
basePath := console.GetBasePath(clusterVersion)
nginxConf := console.GetNginxConfiguration()

// Get ODF console Deployment
odfConsoleDeployment := console.GetDeployment(OperatorNamespace)
err := r.Client.Get(context.TODO(), types.NamespacedName{
err := r.Client.Get(ctx, types.NamespacedName{
Name: odfConsoleDeployment.Name,
Namespace: odfConsoleDeployment.Namespace,
}, odfConsoleDeployment)
Expand All @@ -103,7 +104,7 @@ func (r *ClusterVersionReconciler) ensureConsolePlugin(clusterVersion string) er

// Create/Update ODF console ConfigMap (nginx configuration)
odfConsoleConfigMap := console.GetNginxConfConfigMap(OperatorNamespace)
_, err = controllerutil.CreateOrUpdate(context.TODO(), r.Client, odfConsoleConfigMap, func() error {
_, err = controllerutil.CreateOrUpdate(ctx, r.Client, odfConsoleConfigMap, func() error {
if odfConsoleConfigMapData := odfConsoleConfigMap.Data["nginx.conf"]; odfConsoleConfigMapData != nginxConf {
logger.Info(fmt.Sprintf("Set the ConfigMap odf-console-nginx-conf data as '%s'", nginxConf))
odfConsoleConfigMap.Data["nginx.conf"] = nginxConf
Expand All @@ -116,7 +117,7 @@ func (r *ClusterVersionReconciler) ensureConsolePlugin(clusterVersion string) er

// Create/Update ODF console Service
odfConsoleService := console.GetService(r.ConsolePort, OperatorNamespace)
_, err = controllerutil.CreateOrUpdate(context.TODO(), r.Client, odfConsoleService, func() error {
_, err = controllerutil.CreateOrUpdate(ctx, r.Client, odfConsoleService, func() error {
return controllerutil.SetControllerReference(odfConsoleDeployment, odfConsoleService, r.Scheme)
})
if err != nil && !errors.IsAlreadyExists(err) {
Expand All @@ -125,7 +126,7 @@ func (r *ClusterVersionReconciler) ensureConsolePlugin(clusterVersion string) er

// Create/Update ODF console ConsolePlugin
odfConsolePlugin := console.GetConsolePluginCR(r.ConsolePort, OperatorNamespace)
_, err = controllerutil.CreateOrUpdate(context.TODO(), r.Client, odfConsolePlugin, func() error {
_, err = controllerutil.CreateOrUpdate(ctx, r.Client, odfConsolePlugin, func() error {
if currentBasePath := odfConsolePlugin.Spec.Service.BasePath; currentBasePath != basePath {
logger.Info(fmt.Sprintf("Set the BasePath for odf-console plugin as '%s'", basePath))
odfConsolePlugin.Spec.Service.BasePath = basePath
Expand Down
12 changes: 6 additions & 6 deletions controllers/delete.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,18 @@ import (
odfv1alpha1 "github.com/red-hat-storage/odf-operator/api/v1alpha1"
)

func (r *StorageSystemReconciler) deleteResources(instance *odfv1alpha1.StorageSystem, logger logr.Logger) error {
func (r *StorageSystemReconciler) deleteResources(ctx context.Context, instance *odfv1alpha1.StorageSystem, logger logr.Logger) error {

var backendStorage client.Object

r.deleteQuickStarts(logger, instance)
r.deleteQuickStarts(ctx, logger, instance)
if instance.Spec.Kind == VendorStorageCluster() {
backendStorage = &ocsv1.StorageCluster{}
} else if instance.Spec.Kind == VendorFlashSystemCluster() {
backendStorage = &ibmv1alpha1.FlashSystemCluster{}
}

err := r.Client.Get(context.TODO(), types.NamespacedName{
err := r.Client.Get(ctx, types.NamespacedName{
Name: instance.Spec.Name, Namespace: instance.Spec.Namespace},
backendStorage)

Expand All @@ -53,7 +53,7 @@ func (r *StorageSystemReconciler) deleteResources(instance *odfv1alpha1.StorageS
return err
}

err = r.Client.Delete(context.TODO(), backendStorage)
err = r.Client.Delete(ctx, backendStorage)
if err != nil {
logger.Error(err, "Failed to delete", "Kind", instance.Spec.Kind, "Name", instance.Spec.Name)
return err
Expand All @@ -64,10 +64,10 @@ func (r *StorageSystemReconciler) deleteResources(instance *odfv1alpha1.StorageS
return fmt.Errorf("Waiting for %s %s to be deleted", instance.Spec.Kind, instance.Spec.Name)
}

func (r *StorageSystemReconciler) areAllStorageSystemsMarkedForDeletion(namespace string) (bool, error) {
func (r *StorageSystemReconciler) areAllStorageSystemsMarkedForDeletion(ctx context.Context, namespace string) (bool, error) {

var storageSystems odfv1alpha1.StorageSystemList
err := r.Client.List(context.TODO(), &storageSystems, &client.ListOptions{Namespace: namespace})
err := r.Client.List(ctx, &storageSystems, &client.ListOptions{Namespace: namespace})
if err != nil {
return false, err
}
Expand Down
9 changes: 5 additions & 4 deletions controllers/delete_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ func TestDeleteResources(t *testing.T) {
}

for i, tc := range testCases {
ctx := context.TODO()
t.Logf("Case %d: %s\n", i+1, tc.label)

fakeStorageSystem := GetFakeStorageSystem(tc.kind)
Expand All @@ -83,24 +84,24 @@ func TestDeleteResources(t *testing.T) {

fakeStorageSystem.Spec.Name = vendorSystem.GetName()
fakeStorageSystem.Spec.Namespace = vendorSystem.GetNamespace()
err = fakeReconciler.Client.Create(context.TODO(), vendorSystem)
err = fakeReconciler.Client.Create(ctx, vendorSystem)
assert.NoError(t, err)
}

err = fakeReconciler.deleteResources(fakeStorageSystem, fakeReconciler.Log)
err = fakeReconciler.deleteResources(ctx, fakeStorageSystem, fakeReconciler.Log)
assert.True(t, (tc.expectedError == (err != nil)))

// verify resource does not exist
if tc.kind == VendorStorageCluster() {
storageCluster := &ocsv1.StorageCluster{}
err = fakeReconciler.Client.Get(context.TODO(), types.NamespacedName{
err = fakeReconciler.Client.Get(ctx, types.NamespacedName{
Name: fakeStorageSystem.Spec.Name, Namespace: fakeStorageSystem.Namespace},
storageCluster)
assert.True(t, errors.IsNotFound(err))

} else if tc.kind == VendorFlashSystemCluster() {
flashSystemCluster := &ibmv1alpha1.FlashSystemCluster{}
err = fakeReconciler.Client.Get(context.TODO(), types.NamespacedName{
err = fakeReconciler.Client.Get(ctx, types.NamespacedName{
Name: fakeStorageSystem.Spec.Name, Namespace: fakeStorageSystem.Namespace},
flashSystemCluster)
assert.True(t, errors.IsNotFound(err))
Expand Down
14 changes: 7 additions & 7 deletions controllers/quickstarts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
odfv1alpha1 "github.com/red-hat-storage/odf-operator/api/v1alpha1"
)

func (r *StorageSystemReconciler) ensureQuickStarts(logger logr.Logger) error {
func (r *StorageSystemReconciler) ensureQuickStarts(ctx context.Context, logger logr.Logger) error {
if len(AllQuickStarts) == 0 {
logger.Info("No quickstarts found")
return nil
Expand All @@ -41,10 +41,10 @@ func (r *StorageSystemReconciler) ensureQuickStarts(logger logr.Logger) error {
continue
}
found := consolev1.ConsoleQuickStart{}
err = r.Client.Get(context.TODO(), types.NamespacedName{Name: cqs.Name, Namespace: cqs.Namespace}, &found)
err = r.Client.Get(ctx, types.NamespacedName{Name: cqs.Name, Namespace: cqs.Namespace}, &found)
if err != nil {
if errors.IsNotFound(err) {
err = r.Client.Create(context.TODO(), &cqs)
err = r.Client.Create(ctx, &cqs)
if err != nil {
logger.Error(err, "Failed to create quickstart", "Name", cqs.Name, "Namespace", cqs.Namespace)
return nil
Expand All @@ -56,7 +56,7 @@ func (r *StorageSystemReconciler) ensureQuickStarts(logger logr.Logger) error {
return nil
}
found.Spec = cqs.Spec
err = r.Client.Update(context.TODO(), &found)
err = r.Client.Update(ctx, &found)
if err != nil {
logger.Error(err, "Failed to update quickstart", "Name", cqs.Name, "Namespace", cqs.Namespace)
return nil
Expand All @@ -66,12 +66,12 @@ func (r *StorageSystemReconciler) ensureQuickStarts(logger logr.Logger) error {
return nil
}

func (r *StorageSystemReconciler) deleteQuickStarts(logger logr.Logger, instance *odfv1alpha1.StorageSystem) {
func (r *StorageSystemReconciler) deleteQuickStarts(ctx context.Context, logger logr.Logger, instance *odfv1alpha1.StorageSystem) {
if len(AllQuickStarts) == 0 {
logger.Info("No quickstarts found.")
}

allSSDeleted, err := r.areAllStorageSystemsMarkedForDeletion(instance.Namespace)
allSSDeleted, err := r.areAllStorageSystemsMarkedForDeletion(ctx, instance.Namespace)
if err != nil {
// Log the error but not fail the operator
logger.Error(err, "Failed to List", "Kind", "StorageSystem")
Expand All @@ -90,7 +90,7 @@ func (r *StorageSystemReconciler) deleteQuickStarts(logger logr.Logger, instance
continue
}

err = r.Client.Delete(context.TODO(), &cqs)
err = r.Client.Delete(ctx, &cqs)
if err != nil {
if errors.IsNotFound(err) {
continue
Expand Down
12 changes: 6 additions & 6 deletions controllers/quickstarts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestEnsureQuickStarts(t *testing.T) {
}

fakeReconciler := GetFakeStorageSystemReconciler(t)
err := fakeReconciler.ensureQuickStarts(fakeReconciler.Log)
err := fakeReconciler.ensureQuickStarts(context.TODO(), fakeReconciler.Log)
assert.NoError(t, err)
for _, c := range cases {
qs := consolev1.ConsoleQuickStart{}
Expand Down Expand Up @@ -140,23 +140,23 @@ func TestDeleteQuickStarts(t *testing.T) {

for i, tc := range testCases {
t.Logf("Case %d: %s\n", i+1, tc.label)

ctx := context.TODO()
fakeReconciler := GetFakeStorageSystemReconciler(t)

err := fakeReconciler.ensureQuickStarts(fakeReconciler.Log)
err := fakeReconciler.ensureQuickStarts(ctx, fakeReconciler.Log)
assert.NoError(t, err)

var quickstarts []consolev1.ConsoleQuickStart = getActualQuickStarts(t, cases, fakeReconciler)
assert.Equal(t, 2, len(quickstarts))

for i := range tc.createStorageSystems {
err = fakeReconciler.Client.Create(context.TODO(), &tc.createStorageSystems[i])
err = fakeReconciler.Client.Create(ctx, &tc.createStorageSystems[i])
assert.NoError(t, err)
}
for i := range tc.deleteStorageSystems {
err := fakeReconciler.Client.Delete(context.TODO(), &tc.deleteStorageSystems[i])
err := fakeReconciler.Client.Delete(ctx, &tc.deleteStorageSystems[i])
assert.NoError(t, err)
err = fakeReconciler.deleteResources(&tc.deleteStorageSystems[i], fakeReconciler.Log)
err = fakeReconciler.deleteResources(ctx, &tc.deleteStorageSystems[i], fakeReconciler.Log)
assert.NoError(t, err)
}
quickstarts = getActualQuickStarts(t, cases, fakeReconciler)
Expand Down
6 changes: 3 additions & 3 deletions controllers/storagecluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (r *StorageClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque
logger := log.FromContext(ctx)

instance := &ocsv1.StorageCluster{}
err := r.Client.Get(context.TODO(), req.NamespacedName, instance)
err := r.Client.Get(ctx, req.NamespacedName, instance)
if err != nil {
if errors.IsNotFound(err) {
logger.Info("StorageCluster instance not found.")
Expand All @@ -71,7 +71,7 @@ func (r *StorageClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque

// get list of StorageSystems
storageSystemList := &odfv1alpha1.StorageSystemList{}
err = r.Client.List(context.TODO(), storageSystemList, &client.ListOptions{Namespace: instance.Namespace})
err = r.Client.List(ctx, storageSystemList, &client.ListOptions{Namespace: instance.Namespace})
if err != nil {
logger.Error(err, "Failed to list the StorageSystem.")
return ctrl.Result{}, err
Expand All @@ -94,7 +94,7 @@ func (r *StorageClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reque
}

// create StorageSystem for storageCluster
err = r.Client.Create(context.TODO(), storageSystem)
err = r.Client.Create(ctx, storageSystem)
if err != nil {
logger.Error(err, "Failed to create StorageSystem.", "StorageSystem", storageSystem.Name)
return ctrl.Result{}, err
Expand Down
Loading

0 comments on commit 74f781f

Please sign in to comment.