diff --git a/pkg/controller/feed/reconcile.go b/pkg/controller/feed/reconcile.go index 73b3de4a6e2..714101fb668 100644 --- a/pkg/controller/feed/reconcile.go +++ b/pkg/controller/feed/reconcile.go @@ -41,21 +41,54 @@ const ( finalizerName = controllerAgentName ) -// Reconcile Feed resources +// Reconcile compares the actual state of a Feed with the desired, and attempts +// to converge the two. It then updates the Status block of the Feed with +// its current state. +// If Reconcile returns a non-nil error, the request will be retried. func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, error) { feed := &feedsv1alpha1.Feed{} err := r.client.Get(context.TODO(), request.NamespacedName, feed) + // The feed may have been deleted since it was added to the workqueue. If so + // there's nothing to be done. if errors.IsNotFound(err) { glog.Errorf("could not find Feed %v\n", request) return reconcile.Result{}, nil } + // If the feed exists but could not be retrieved, then we should retry. if err != nil { glog.Errorf("could not fetch Feed %v for %+v\n", err, request) return reconcile.Result{}, err } + // Now that we know the feed exists, we can reconcile it. An error returned + // here means the reconcile did not complete and the Feed should be requeued + // for another attempt. + // A successful reconcile does not necessarily mean the feed is in the desired + // state, it means no more can be done for now. In this case the feed will + // not be reconciled again until the resync period or a watched resource + // changes. + if err = r.reconcile(feed); err != nil { + glog.Errorf("error reconciling Feed: %v", err) + } + + // Since the reconcile is a sequence of steps, earlier steps may complete + // successfully while later steps fail. The Feed is updated on failure to + // preserve any useful status or metadata changes the non-failing steps made. + if updateErr := r.updateFeed(feed); updateErr != nil { + glog.Errorf("failed to update Feed: %v", updateErr) + // An error here means the feed should be reconciled again, regardless of + // whether the reconcile was successful or not. + return reconcile.Result{}, updateErr + } + return reconcile.Result{}, err +} + +// reconcile tries to converge the current state of the given Feed to the +// desired state. This function should not update the Feed; the calling method +// should do that. +func (r *reconciler) reconcile(feed *feedsv1alpha1.Feed) error { feed.Status.InitializeConditions() // Fetch the EventSource and EventType that is being asked for // and if they don't exist update the status to reflect this back @@ -80,13 +113,13 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err Message: t.Message, }) default: - return reconcile.Result{}, err + // This is unreachable unless getFeedSource is refactored. + // Assume this is a non-terminal state and return the error. + return fmt.Errorf("Error getting feed source: %v", err) } - if err := r.updateFeed(feed); err != nil { - glog.Errorf("failed to update Feed status: %v", err) - return reconcile.Result{}, err - } - return reconcile.Result{}, nil + // This is a terminal state, and we've noted it in the status, so return + // nil to signal that no further reconciling is required. + return nil } // TODO: Set the FeedConditionDependenciesSatisfied to true here? Or, after @@ -101,10 +134,6 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err // to do this properly. // TODO: Add issue link here. can't look up right now, no wifi r.setEventTypeOwnerReference(feed) - if err := r.updateOwnerReferences(feed); err != nil { - glog.Errorf("failed to update Feed owner references: %v", err) - return reconcile.Result{}, err - } if feed.GetDeletionTimestamp() == nil { err = r.reconcileStartJob(feed, eventSource, eventType) @@ -117,12 +146,7 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err glog.Errorf("error reconciling stop Job: %v", err) } } - - if updateErr := r.updateFeed(feed); updateErr != nil { - glog.Errorf("failed to update Feed status: %v", updateErr) - return reconcile.Result{}, updateErr - } - return reconcile.Result{}, err + return nil } func (r *reconciler) reconcileStartJob(feed *feedsv1alpha1.Feed, es *feedsv1alpha1.EventSource, et *feedsv1alpha1.EventType) error { @@ -247,20 +271,6 @@ func (r *reconciler) reconcileStopJob(feed *feedsv1alpha1.Feed, es *feedsv1alpha return nil } -func (r *reconciler) updateOwnerReferences(u *feedsv1alpha1.Feed) error { - feed := &feedsv1alpha1.Feed{} - err := r.client.Get(context.TODO(), client.ObjectKey{Namespace: u.Namespace, Name: u.Name}, feed) - if err != nil { - return err - } - - if !equality.Semantic.DeepEqual(feed.OwnerReferences, u.OwnerReferences) { - feed.SetOwnerReferences(u.ObjectMeta.OwnerReferences) - return r.client.Update(context.TODO(), feed) - } - return nil -} - func (r *reconciler) updateFinalizers(u *feedsv1alpha1.Feed) error { feed := &feedsv1alpha1.Feed{} err := r.client.Get(context.TODO(), client.ObjectKey{Namespace: u.Namespace, Name: u.Name}, feed)