Skip to content

Commit

Permalink
[WIP] Bump observedGeneration first
Browse files Browse the repository at this point in the history
Move observedGeneration bump to the top of reconcile()

Fixes knative#4937
  • Loading branch information
Dan Gerdesmeier committed Dec 5, 2019
1 parent fc4ea32 commit b944ef5
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 11 deletions.
7 changes: 3 additions & 4 deletions pkg/reconciler/configuration/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ func (c *Reconciler) reconcile(ctx context.Context, config *v1alpha1.Configurati
// assumptions about defaulting.
config.SetDefaults(v1.WithUpgradeViaDefaulting(ctx))
config.Status.InitializeConditions()
// Bump observed generation to denote that we have processed this
// generation regardless of success or failure.
config.Status.ObservedGeneration = config.Generation

if err := config.ConvertUp(ctx, &v1beta1.Configuration{}); err != nil {
if ce, ok := err.(*v1alpha1.CannotConvertError); ok {
Expand All @@ -129,10 +132,6 @@ func (c *Reconciler) reconcile(ctx context.Context, config *v1alpha1.Configurati
return err
}

// Bump observed generation to denote that we have processed this
// generation regardless of success or failure.
config.Status.ObservedGeneration = config.Generation

// First, fetch the revision that should exist for the current generation.
lcr, err := c.latestCreatedRevision(config)
if errors.IsNotFound(err) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/revision/revision.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ func (c *Reconciler) reconcile(ctx context.Context, rev *v1alpha1.Revision) erro
rev.SetDefaults(v1.WithUpgradeViaDefaulting(ctx))

rev.Status.InitializeConditions()
rev.Status.ObservedGeneration = rev.Generation
c.updateRevisionLoggingURL(ctx, rev)

if err := rev.ConvertUp(ctx, &v1beta1.Revision{}); err != nil {
Expand Down Expand Up @@ -210,7 +211,6 @@ func (c *Reconciler) reconcile(ctx context.Context, rev *v1alpha1.Revision) erro
"Revision becomes ready upon all resources being ready")
}

rev.Status.ObservedGeneration = rev.Generation
return nil
}

Expand Down
6 changes: 1 addition & 5 deletions pkg/reconciler/route/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ func (c *Reconciler) reconcile(ctx context.Context, r *v1alpha1.Route) error {
// assumptions about defaulting.
r.SetDefaults(v1.WithUpgradeViaDefaulting(ctx))
r.Status.InitializeConditions()
r.Status.ObservedGeneration = r.Generation

if err := r.ConvertUp(ctx, &v1beta1.Route{}); err != nil {
return err
Expand All @@ -204,10 +205,6 @@ func (c *Reconciler) reconcile(ctx context.Context, r *v1alpha1.Route) error {
traffic, err := c.configureTraffic(ctx, r, serviceNames.desiredClusterLocalServiceNames)
if traffic == nil || err != nil {
// Traffic targets aren't ready, no need to configure child resources.
// Need to update ObservedGeneration, otherwise Route's Ready state won't
// be propagated to Service and the Service's RoutesReady will stay in
// 'Unknown'.
r.Status.ObservedGeneration = r.Generation
return err
}

Expand Down Expand Up @@ -256,7 +253,6 @@ func (c *Reconciler) reconcile(ctx context.Context, r *v1alpha1.Route) error {
return err
}

r.Status.ObservedGeneration = r.Generation
logger.Info("Route successfully synced")
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ func (c *Reconciler) reconcile(ctx context.Context, service *v1alpha1.Service) e
// assumptions about defaulting.
service.SetDefaults(v1.WithUpgradeViaDefaulting(ctx))
service.Status.InitializeConditions()
service.Status.ObservedGeneration = service.Generation

if err := service.ConvertUp(ctx, &v1beta1.Service{}); err != nil {
if ce, ok := err.(*v1alpha1.CannotConvertError); ok {
Expand Down Expand Up @@ -192,7 +193,6 @@ func (c *Reconciler) reconcile(ctx context.Context, service *v1alpha1.Service) e
}

c.checkRoutesNotReady(config, logger, route, service)
service.Status.ObservedGeneration = service.Generation

return nil
}
Expand Down

0 comments on commit b944ef5

Please sign in to comment.