Skip to content

Commit

Permalink
Rename ProgressReporter to StatusReporter
Browse files Browse the repository at this point in the history 
Should have been done back when annotations were addded to "progress"

Also, if pvc is bound do not call phase Reconcile functions only Status

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
  • Loading branch information
@mhenriks
mhenriks committed Jun 27, 2023
1 parent f605718 commit 7edeaee
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 43 deletions.
8 changes: 4 additions & 4 deletions pkg/controller/clone/host-clone.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type HostClonePhase struct {

var _ Phase = &HostClonePhase{}

var _ ProgressReporter = &HostClonePhase{}
var _ StatusReporter = &HostClonePhase{}

var httpClient *http.Client

Expand All @@ -49,9 +49,9 @@ func (p *HostClonePhase) Name() string {
return HostClonePhaseName
}

// Progress returns the phase progress
func (p *HostClonePhase) Progress(ctx context.Context) (*PhaseProgress, error) {
result := &PhaseProgress{}
// Status returns the phase status
func (p *HostClonePhase) Status(ctx context.Context) (*PhaseStatus, error) {
result := &PhaseStatus{}
pvc := &corev1.PersistentVolumeClaim{}
exists, err := getResource(ctx, p.Client, p.Namespace, p.DesiredClaim.Name, pvc)
if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions pkg/controller/clone/planner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,15 @@ type Phase interface {
Reconcile(context.Context) (*reconcile.Result, error)
}

// PhaseProgress contains phase progress data
type PhaseProgress struct {
// PhaseStatus contains phase status data
type PhaseStatus struct {
Progress string
Annotations map[string]string
}

// ProgressReporter allows a phase to report progress
type ProgressReporter interface {
Progress(context.Context) (*PhaseProgress, error)
// StatusReporter allows a phase to report status
type StatusReporter interface {
Status(context.Context) (*PhaseStatus, error)
}

// list of all possible (core) types created
Expand Down
55 changes: 30 additions & 25 deletions pkg/controller/populators/clone-populator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,15 @@ func (r *ClonePopulatorReconciler) Reconcile(ctx context.Context, req reconcile.
}

hasFinalizer := cc.HasFinalizer(pvc, cloneFinalizer)
isBound := cc.IsBound(pvc)
isDeleted := !pvc.DeletionTimestamp.IsZero()
isSucceeded := isClonePhaseSucceeded(pvc)

log.V(3).Info("pvc state", "hasFinalizer", hasFinalizer,
"isBound", cc.IsBound(pvc), "isDeleted", isDeleted, "isSucceeded", isSucceeded)
"isBound", isBound, "isDeleted", isDeleted, "isSucceeded", isSucceeded)

if !isDeleted && !isSucceeded {
return r.reconcilePending(ctx, log, pvc)
return r.reconcilePending(ctx, log, pvc, isBound)
}

if hasFinalizer {
Expand All @@ -168,7 +169,7 @@ func (r *ClonePopulatorReconciler) Reconcile(ctx context.Context, req reconcile.
return reconcile.Result{}, nil
}

func (r *ClonePopulatorReconciler) reconcilePending(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim) (reconcile.Result, error) {
func (r *ClonePopulatorReconciler) reconcilePending(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, statusOnly bool) (reconcile.Result, error) {
ready, _, err := claimReadyForPopulation(ctx, r.client, pvc)
if err != nil {
return reconcile.Result{}, r.updateClonePhaseError(ctx, log, pvc, err)
Expand Down Expand Up @@ -222,7 +223,7 @@ func (r *ClonePopulatorReconciler) reconcilePending(ctx context.Context, log log
Strategy: *cs,
}

return r.planAndExecute(ctx, log, pvc, args)
return r.planAndExecute(ctx, log, pvc, statusOnly, args)
}

func (r *ClonePopulatorReconciler) getCloneStrategy(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, vcs *cdiv1.VolumeCloneSource) (*cdiv1.CDICloneStrategy, error) {
Expand All @@ -245,40 +246,44 @@ func (r *ClonePopulatorReconciler) getCloneStrategy(ctx context.Context, log log
return cs, nil
}

func (r *ClonePopulatorReconciler) planAndExecute(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, args *clone.PlanArgs) (reconcile.Result, error) {
func (r *ClonePopulatorReconciler) planAndExecute(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, statusOnly bool, args *clone.PlanArgs) (reconcile.Result, error) {
phases, err := r.planner.Plan(ctx, args)
if err != nil {
return reconcile.Result{}, r.updateClonePhaseError(ctx, log, pvc, err)
}

log.V(3).Info("created phases", "num", len(phases))

var progressResults []*clone.PhaseProgress
var statusResults []*clone.PhaseStatus
for _, p := range phases {
var result *reconcile.Result
var err error
var progress string
result, err := p.Reconcile(ctx)
if err != nil {
return reconcile.Result{}, r.updateClonePhaseError(ctx, log, pvc, err)
if !statusOnly {
result, err = p.Reconcile(ctx)
if err != nil {
return reconcile.Result{}, r.updateClonePhaseError(ctx, log, pvc, err)
}
}

if pr, ok := p.(clone.ProgressReporter); ok {
pp, err := pr.Progress(ctx)
if sr, ok := p.(clone.StatusReporter); ok {
ps, err := sr.Status(ctx)
if err != nil {
return reconcile.Result{}, r.updateClonePhaseError(ctx, log, pvc, err)
}
progress = pp.Progress
progressResults = append(progressResults, pp)
progress = ps.Progress
statusResults = append(statusResults, ps)
}

if result != nil {
log.V(1).Info("currently in phase, returning", "name", p.Name(), "progress", progress)
return *result, r.updateClonePhase(ctx, log, pvc, p.Name(), progressResults)
return *result, r.updateClonePhase(ctx, log, pvc, p.Name(), statusResults)
}
}

log.V(3).Info("executed all phases, setting phase to Succeeded")

return reconcile.Result{}, r.updateClonePhaseSucceeded(ctx, log, pvc, progressResults)
return reconcile.Result{}, r.updateClonePhaseSucceeded(ctx, log, pvc, statusResults)
}

func (r *ClonePopulatorReconciler) validateCrossNamespace(pvc *corev1.PersistentVolumeClaim, vcs *cdiv1.VolumeCloneSource) error {
Expand Down Expand Up @@ -334,25 +339,25 @@ func (r *ClonePopulatorReconciler) updateClonePhasePending(ctx context.Context,
return r.updateClonePhase(ctx, log, pvc, clone.PendingPhaseName, nil)
}

func (r *ClonePopulatorReconciler) updateClonePhaseSucceeded(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, progress []*clone.PhaseProgress) error {
if progress == nil {
progress = []*clone.PhaseProgress{{}}
func (r *ClonePopulatorReconciler) updateClonePhaseSucceeded(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, status []*clone.PhaseStatus) error {
if status == nil {
status = []*clone.PhaseStatus{{}}
}
progress[len(progress)-1].Progress = cc.ProgressDone
return r.updateClonePhase(ctx, log, pvc, clone.SucceededPhaseName, progress)
status[len(status)-1].Progress = cc.ProgressDone
return r.updateClonePhase(ctx, log, pvc, clone.SucceededPhaseName, status)
}

func (r *ClonePopulatorReconciler) updateClonePhase(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, phase string, progress []*clone.PhaseProgress) error {
func (r *ClonePopulatorReconciler) updateClonePhase(ctx context.Context, log logr.Logger, pvc *corev1.PersistentVolumeClaim, phase string, status []*clone.PhaseStatus) error {
claimCpy := pvc.DeepCopy()
delete(claimCpy.Annotations, AnnCloneError)
cc.AddAnnotation(claimCpy, AnnClonePhase, phase)

var mergedAnnotations = make(map[string]string)
for _, pp := range progress {
if pp.Progress != "" {
cc.AddAnnotation(claimCpy, cc.AnnPopulatorProgress, pp.Progress)
for _, ps := range status {
if ps.Progress != "" {
cc.AddAnnotation(claimCpy, cc.AnnPopulatorProgress, ps.Progress)
}
for k, v := range pp.Annotations {
for k, v := range ps.Annotations {
mergedAnnotations[k] = v
if _, ok := desiredCloneAnnotations[k]; ok {
cc.AddAnnotation(claimCpy, k, v)
Expand Down
18 changes: 9 additions & 9 deletions pkg/controller/populators/clone-populator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,12 +304,12 @@ var _ = Describe("Clone populator tests", func() {
&fakePhase{
name: "phase1",
},
&fakePhaseWithProgress{
&fakePhaseWithStatus{
fakePhase: fakePhase{
name: "phase2",
result: &reconcile.Result{},
},
progress: &clone.PhaseProgress{
status: &clone.PhaseStatus{
Progress: "50.0%",
Annotations: map[string]string{
"foo": "bar",
Expand Down Expand Up @@ -346,12 +346,12 @@ var _ = Describe("Clone populator tests", func() {
reconciler := createClonePopulatorReconciler(target, storageClass(), source)
reconciler.planner = &fakePlanner{
planResult: []clone.Phase{
&fakePhaseWithProgress{
&fakePhaseWithStatus{
fakePhase: fakePhase{
name: "phase1",
result: &reconcile.Result{},
},
proogressErr: fmt.Errorf("progress error"),
statusErr: fmt.Errorf("progress error"),
},
},
}
Expand Down Expand Up @@ -430,14 +430,14 @@ func (p *fakePhase) Reconcile(ctx context.Context) (*reconcile.Result, error) {
return p.result, p.err
}

type fakePhaseWithProgress struct {
type fakePhaseWithStatus struct {
fakePhase
progress *clone.PhaseProgress
proogressErr error
status *clone.PhaseStatus
statusErr error
}

func (p *fakePhaseWithProgress) Progress(ctx context.Context) (*clone.PhaseProgress, error) {
return p.progress, p.proogressErr
func (p *fakePhaseWithStatus) Status(ctx context.Context) (*clone.PhaseStatus, error) {
return p.status, p.statusErr
}

func createClonePopulatorReconciler(objects ...runtime.Object) *ClonePopulatorReconciler {
Expand Down

0 comments on commit 7edeaee

Please sign in to comment.