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

Jump to bottom

Fix DVM/DVMP hotlooping, remove remote watch TouchAnnotation, fix requeues so they used values from r.Migrate() #1013

Merged
merged 3 commits into from
Mar 26, 2021
Merged

Fix DVM/DVMP hotlooping, remove remote watch TouchAnnotation, fix requeues so they used values from r.Migrate() #1013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
719eb24
Fix DVM/DVMP hotlooping, remove remote watch TouchAnnotation, fix req…
djwhatle Mar 18, 2021
e5f12b5
Revert reconcile.Result{} style change
djwhatle Mar 25, 2021
9e0f4ac
Use explicit requeue bools
djwhatle Mar 26, 2021
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
3 changes: 0 additions & 3 deletions pkg/apis/migration/v1alpha1/directvolumemigrationprogress_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package v1alpha1
import (
"context"

"github.com/google/uuid"
liberr "github.com/konveyor/controller/pkg/error"
kapi "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -108,11 +107,9 @@ type DirectVolumeMigrationProgress struct {
}

func (d *DirectVolumeMigrationProgress) MarkReconciled() {
u, _ := uuid.NewUUID()
if d.Annotations == nil {
d.Annotations = map[string]string{}
}
d.Annotations[TouchAnnotation] = u.String()
djwhatle marked this conversation as resolved.
Show resolved Hide resolved
d.Status.ObservedDigest = digest(d.Spec)
}

Expand Down
6 changes: 0 additions & 6 deletions pkg/apis/migration/v1alpha1/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,9 @@ func (r *DirectVolumeMigration) GetName() string {
}

func (r *DirectVolumeMigration) MarkReconciled() {
uuid, _ := uuid.NewUUID()
if r.Annotations == nil {
r.Annotations = map[string]string{}
}
r.Annotations[TouchAnnotation] = uuid.String()
r.Status.ObservedDigest = digest(r.Spec)
}

Expand Down Expand Up @@ -269,11 +267,9 @@ func (r *DirectImageMigration) GetName() string {
}

func (r *DirectImageMigration) MarkReconciled() {
uuid, _ := uuid.NewUUID()
if r.Annotations == nil {
r.Annotations = map[string]string{}
}
r.Annotations[TouchAnnotation] = uuid.String()
r.Status.ObservedDigest = digest(r.Spec)
}

Expand Down Expand Up @@ -303,11 +299,9 @@ func (r *DirectImageStreamMigration) GetName() string {
}

func (r *DirectImageStreamMigration) MarkReconciled() {
uuid, _ := uuid.NewUUID()
if r.Annotations == nil {
r.Annotations = map[string]string{}
}
r.Annotations[TouchAnnotation] = uuid.String()
r.Status.ObservedDigest = digest(r.Spec)
}

Expand Down
20 changes: 14 additions & 6 deletions pkg/controller/directimagemigration/directimagemigration_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package directimagemigration

import (
"context"
"time"

"github.com/konveyor/controller/pkg/logging"
migapi "github.com/konveyor/mig-controller/pkg/apis/migration/v1alpha1"
Expand Down Expand Up @@ -119,10 +120,10 @@ func (r *ReconcileDirectImageMigration) Reconcile(request reconcile.Request) (re
if errors.IsNotFound(err) {
// Object not found, return. Created objects are automatically garbage collected.
// For additional cleanup logic use finalizers.
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's more obvious what's happening if we explicitly use the requeue flag

}
// Error reading the object - requeue the request.
return reconcile.Result{}, err
return reconcile.Result{Requeue: true}, err
}

// Set up jaeger tracing
Expand All @@ -133,7 +134,7 @@ func (r *ReconcileDirectImageMigration) Reconcile(request reconcile.Request) (re

// Completed.
if imageMigration.Status.Phase == Completed {
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}

// Begin staging conditions
Expand All @@ -146,8 +147,11 @@ func (r *ReconcileDirectImageMigration) Reconcile(request reconcile.Request) (re
return reconcile.Result{Requeue: true}, nil
}

// Default to PollReQ, can be overridden by r.migrate phase-specific ReQ interval
requeueAfter := time.Duration(PollReQ)

if !imageMigration.Status.HasBlockerCondition() {
_, err = r.migrate(imageMigration, reconcileSpan)
requeueAfter, err = r.migrate(imageMigration, reconcileSpan)
if err != nil {
log.Trace(err)
return reconcile.Result{Requeue: true}, nil
Expand All @@ -171,6 +175,10 @@ func (r *ReconcileDirectImageMigration) Reconcile(request reconcile.Request) (re
return reconcile.Result{Requeue: true}, nil
}

// Done
return reconcile.Result{}, nil
// Requeue
if requeueAfter > 0 {
return reconcile.Result{RequeueAfter: requeueAfter}, nil
}

return reconcile.Result{Requeue: false}, nil
}
20 changes: 14 additions & 6 deletions pkg/controller/directimagestreammigration/directimagestreammigration_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package directimagestreammigration

import (
"context"
"time"

"github.com/konveyor/controller/pkg/logging"
migapi "github.com/konveyor/mig-controller/pkg/apis/migration/v1alpha1"
Expand Down Expand Up @@ -110,10 +111,10 @@ func (r *ReconcileDirectImageStreamMigration) Reconcile(request reconcile.Reques
if errors.IsNotFound(err) {
// Object not found, return. Created objects are automatically garbage collected.
// For additional cleanup logic use finalizers.
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}
// Error reading the object - requeue the request.
return reconcile.Result{}, err
return reconcile.Result{Requeue: true}, err
}

// Set up jaeger tracing
Expand All @@ -124,7 +125,7 @@ func (r *ReconcileDirectImageStreamMigration) Reconcile(request reconcile.Reques

// Completed.
if imageStreamMigration.Status.Phase == Completed {
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}

// Begin staging conditions
Expand All @@ -137,8 +138,11 @@ func (r *ReconcileDirectImageStreamMigration) Reconcile(request reconcile.Reques
return reconcile.Result{Requeue: true}, nil
}

// Default to PollReQ, can be overridden by r.migrate phase-specific ReQ interval
requeueAfter := time.Duration(PollReQ)

if !imageStreamMigration.Status.HasBlockerCondition() {
_, err = r.migrate(imageStreamMigration, reconcileSpan)
requeueAfter, err = r.migrate(imageStreamMigration, reconcileSpan)
if err != nil {
log.Trace(err)
return reconcile.Result{Requeue: true}, nil
Expand All @@ -162,6 +166,10 @@ func (r *ReconcileDirectImageStreamMigration) Reconcile(request reconcile.Reques
return reconcile.Result{Requeue: true}, nil
}

// Done
return reconcile.Result{}, nil
// Requeue
if requeueAfter > 0 {
return reconcile.Result{RequeueAfter: requeueAfter}, nil
}

return reconcile.Result{Requeue: false}, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In all the other controllers we have return reconcile.Result{}, nil. Wonder if we should drop this OR update the others. I am fine with either as long as we are consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, will update.

}
19 changes: 14 additions & 5 deletions pkg/controller/directvolumemigration/directvolumemigration_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package directvolumemigration

import (
"context"
"time"

"github.com/konveyor/controller/pkg/logging"
migapi "github.com/konveyor/mig-controller/pkg/apis/migration/v1alpha1"
Expand Down Expand Up @@ -100,10 +101,10 @@ func (r *ReconcileDirectVolumeMigration) Reconcile(request reconcile.Request) (r
if errors.IsNotFound(err) {
// Object not found, return. Created objects are automatically garbage collected.
// For additional cleanup logic use finalizers.
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}
// Error reading the object - requeue the request.
return reconcile.Result{}, err
return reconcile.Result{Requeue: true}, err
}

// Set up jaeger tracing
Expand All @@ -117,7 +118,7 @@ func (r *ReconcileDirectVolumeMigration) Reconcile(request reconcile.Request) (r

// Check if completed
if direct.Status.Phase == Completed {
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}

// Begin staging conditions
Expand All @@ -130,8 +131,11 @@ func (r *ReconcileDirectVolumeMigration) Reconcile(request reconcile.Request) (r
return reconcile.Result{Requeue: true}, nil
}

// Default to PollReQ, can be overridden by r.migrate phase-specific ReQ interval
requeueAfter := time.Duration(PollReQ)

if !direct.Status.HasBlockerCondition() {
_, err = r.migrate(direct, reconcileSpan)
requeueAfter, err = r.migrate(direct, reconcileSpan)
if err != nil {
log.Trace(err)
return reconcile.Result{Requeue: true}, nil
Expand All @@ -155,6 +159,11 @@ func (r *ReconcileDirectVolumeMigration) Reconcile(request reconcile.Request) (r
return reconcile.Result{Requeue: true}, nil
}

// Requeue
if requeueAfter > 0 {
return reconcile.Result{RequeueAfter: requeueAfter}, nil
}

// Done
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}
8 changes: 2 additions & 6 deletions pkg/controller/directvolumemigrationprogress/directvolumemigrationprogress_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ func (r *ReconcileDirectVolumeMigrationProgress) Reconcile(request reconcile.Req
err = r.Get(context.TODO(), request.NamespacedName, pvProgress)
if err != nil {
if errors.IsNotFound(err) {
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}
return reconcile.Result{}, err
return reconcile.Result{Requeue: true}, err
}

// Set up jaeger tracing
Expand Down Expand Up @@ -178,10 +178,6 @@ func (r *ReconcileDirectVolumeMigrationProgress) Reconcile(request reconcile.Req
return reconcile.Result{Requeue: true}, nil
}

if !pvProgress.Status.IsReady() {
return reconcile.Result{Requeue: true}, nil
}

// we will requeue this every 5 seconds
return reconcile.Result{Requeue: true, RequeueAfter: time.Second * 5}, nil
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/controller/discovery/container/ds.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,17 @@ package container
import (
"database/sql"
"errors"
"time"

migapi "github.com/konveyor/mig-controller/pkg/apis/migration/v1alpha1"
"github.com/konveyor/mig-controller/pkg/controller/discovery/model"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime"
controllerruntime "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/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"time"
)

type Collections []Collection
Expand Down Expand Up @@ -68,7 +69,7 @@ func (r *DataSource) IsReady() bool {
// is for watches added by each collection reference a predicate that handles the change
// rather than queuing a reconcile event.
func (r *DataSource) Reconcile(request reconcile.Request) (reconcile.Result, error) {
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}

//
Expand Down
6 changes: 3 additions & 3 deletions pkg/controller/discovery/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,13 @@ func (r *ReconcileDiscovery) Reconcile(request reconcile.Request) (reconcile.Res
if err != nil {
if errors.IsNotFound(err) {
r.container.Delete(request.NamespacedName)
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}
log.Trace(err)
return reQueue, nil
}
if !r.IsValid(cluster) {
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}
err = r.container.Add(
cluster,
Expand All @@ -174,7 +174,7 @@ func (r *ReconcileDiscovery) Reconcile(request reconcile.Request) (reconcile.Res
return reQueue, nil
}

return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}

func (r *ReconcileDiscovery) IsValid(cluster *migapi.MigCluster) bool {
Expand Down
6 changes: 3 additions & 3 deletions pkg/controller/miganalytic/miganalytics_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func (r *ReconcileMigAnalytic) Reconcile(request reconcile.Request) (reconcile.R
err = r.Get(context.TODO(), request.NamespacedName, analytic)
if err != nil {
if errors.IsNotFound(err) {
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}
log.Trace(err)
return reconcile.Result{Requeue: true}, nil
Expand All @@ -141,7 +141,7 @@ func (r *ReconcileMigAnalytic) Reconcile(request reconcile.Request) (reconcile.R
// Exit early if the MigAnalytic already has a ready condition
// and Refresh boolean is unset
if analytic.Status.IsReady() && !analytic.Spec.Refresh {
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}

// Report reconcile error.
Expand Down Expand Up @@ -211,7 +211,7 @@ func (r *ReconcileMigAnalytic) Reconcile(request reconcile.Request) (reconcile.R
}

// Done
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}

func (r *ReconcileMigAnalytic) analyze(analytic *migapi.MigAnalytic) error {
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/migcluster/migcluster_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (r *ReconcileMigCluster) Reconcile(request reconcile.Request) (reconcile.Re
err = r.Get(context.TODO(), request.NamespacedName, cluster)
if err != nil {
if errors.IsNotFound(err) {
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}
log.Trace(err)
return reconcile.Result{Requeue: true}, nil
Expand Down Expand Up @@ -180,5 +180,5 @@ func (r *ReconcileMigCluster) Reconcile(request reconcile.Request) (reconcile.Re
}

// Done
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}
5 changes: 3 additions & 2 deletions pkg/controller/mighook/mighook_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package mighook

import (
"context"

"github.com/konveyor/mig-controller/pkg/errorutil"

"github.com/konveyor/controller/pkg/logging"
Expand Down Expand Up @@ -86,7 +87,7 @@ func (r *ReconcileMigHook) Reconcile(request reconcile.Request) (reconcile.Resul
err = r.Get(context.TODO(), request.NamespacedName, hook)
if err != nil {
if errors.IsNotFound(err) {
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}
log.Trace(err)
return reconcile.Result{Requeue: true}, nil
Expand Down Expand Up @@ -134,5 +135,5 @@ func (r *ReconcileMigHook) Reconcile(request reconcile.Request) (reconcile.Resul
}

// Done
return reconcile.Result{}, nil
return reconcile.Result{Requeue: false}, nil
}
Loading