Skip to content

Commit

Permalink
Fix DVM/DVMP hotlooping, remove remote watch TouchAnnotation, fix req…
Browse files Browse the repository at this point in the history
…ueues so they used values from r.Migrate() (#1013)

* Fix DVM/DVMP hotlooping, remove remote watch TouchAnnotation, fix requeue times

Move default PollReQ setting for better locality

* Use explicit requeue bools
  • Loading branch information
Derek Whatley authored Mar 26, 2021
1 parent 6d270c1 commit 3855b9e
Show file tree
Hide file tree
Showing 14 changed files with 72 additions and 57 deletions.
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()
d.Status.ObservedDigest = digest(d.Spec)
}

Expand Down
6 changes: 0 additions & 6 deletions pkg/apis/migration/v1alpha1/resource.go
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
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
}
// 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
}
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
}
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
}
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
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
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
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
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
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

0 comments on commit 3855b9e

Please sign in to comment.