Merge pull request #2310 from thedadams/status-adjustments

Adjust status calculations

.gitignore

@@ -15,4 +15,5 @@
 /vendor
 apiserver.local.config
 /.envrc
+/depot.json
 **/.DS_Store

pkg/controller/appdefinition/depends.go

@@ -1,16 +1,11 @@
 package appdefinition
 
 import (
-	"fmt"
-	"slices"
 	"strconv"
-	"strings"
 
 	"github.com/acorn-io/baaah/pkg/apply"
-	"github.com/acorn-io/baaah/pkg/typed"
 	v1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1"
 	"github.com/acorn-io/runtime/pkg/labels"
-	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 func getDependencyAnnotations(app *v1.AppInstance, containerOrJobName string, deps []v1.Dependency) map[string]string {
@@ -72,14 +67,6 @@ func getDependencyAnnotations(app *v1.AppInstance, containerOrJobName string, de
 		s := app.Status.AppStatus.Containers[containerOrJobName]
 		s.Dependencies = depStatus
 
-		if !s.Ready {
-			msg, blocked := isBlocked(s.Dependencies, s.ExpressionErrors)
-			if blocked {
-				s.State = "waiting"
-			}
-			s.TransitioningMessages = append(s.TransitioningMessages, msg...)
-		}
-
 		if app.Status.AppStatus.Containers == nil {
 			app.Status.AppStatus.Containers = map[string]v1.ContainerStatus{}
 		}
@@ -88,14 +75,6 @@ func getDependencyAnnotations(app *v1.AppInstance, containerOrJobName string, de
 		s := app.Status.AppStatus.Jobs[containerOrJobName]
 		s.Dependencies = depStatus
 
-		if !s.Ready {
-			msg, blocked := isBlocked(s.Dependencies, s.ExpressionErrors)
-			if blocked {
-				s.State = "waiting"
-			}
-			s.TransitioningMessages = append(s.TransitioningMessages, msg...)
-		}
-
 		if app.Status.AppStatus.Jobs == nil {
 			app.Status.AppStatus.Jobs = map[string]v1.JobStatus{}
 		}
@@ -104,35 +83,3 @@ func getDependencyAnnotations(app *v1.AppInstance, containerOrJobName string, de
 
 	return result
 }
-
-func isBlocked(dependencies map[string]v1.DependencyStatus, expressionErrors []v1.ExpressionError) (result []string, _ bool) {
-	groupedByTypeName := map[string][]string{}
-
-	for depName, dep := range dependencies {
-		var key string
-		if dep.Missing {
-			key = string(dep.DependencyType) + " to be created"
-		} else if !dep.Ready {
-			key = string(dep.DependencyType) + " to be ready"
-		} else {
-			continue
-		}
-		groupedByTypeName[key] = append(groupedByTypeName[key], depName)
-	}
-
-	for _, exprError := range expressionErrors {
-		if exprError.DependencyNotFound != nil && exprError.DependencyNotFound.SubKey == "" {
-			key := string(exprError.DependencyNotFound.DependencyType) + " to be created"
-			groupedByTypeName[key] = append(groupedByTypeName[key], exprError.DependencyNotFound.Name)
-		}
-	}
-
-	for _, key := range typed.SortedKeys(groupedByTypeName) {
-		values := sets.New(groupedByTypeName[key]...).UnsortedList()
-		slices.Sort(values)
-		msg := fmt.Sprintf("waiting for %s [%s]", key, strings.Join(values, ", "))
-		result = append(result, msg)
-	}
-
-	return result, len(result) > 0
-}

pkg/controller/appdefinition/testdata/depends/expected.golden

@@ -303,9 +303,6 @@ status:
         dependencies:
           container-name:
             serviceType: container
-        state: waiting
-        transitioningMessages:
-        - waiting for container to be ready [container-name]
   columns: {}
   conditions:
     reason: Success

pkg/controller/appstatus/appstatus.go

@@ -29,11 +29,11 @@ type appStatusRenderer struct {
 // fields are specifically fields with aggregated values.
 // Additionally, some corner cases are handled in this code where fields need to be initialized in a special way
 func resetHandlerControlledFields(app *v1.AppInstance) {
-	appUpdated := app.Generation == app.Status.ObservedGeneration && app.Status.AppImage.Digest == app.Status.Staged.AppImage.Digest
+	appUpToDate := app.Generation == app.Status.ObservedGeneration && app.Status.AppImage.Digest == app.Status.ObservedImageDigest
 	for name, status := range app.Status.AppStatus.Containers {
 		// If the app is being updated, then set the containers to not ready so that the controller will run them again and the
 		// dependency status will be set correctly.
-		status.Ready = status.Ready && appUpdated
+		status.Ready = status.Ready && appUpToDate
 		status.ExpressionErrors = nil
 		status.Dependencies = nil
 		app.Status.AppStatus.Containers[name] = status
@@ -42,7 +42,7 @@ func resetHandlerControlledFields(app *v1.AppInstance) {
 	for name, status := range app.Status.AppStatus.Jobs {
 		status.ExpressionErrors = nil
 		status.Dependencies = nil
-		if !appUpdated && jobs.ShouldRun(name, app) {
+		if !appUpToDate && jobs.ShouldRun(name, app) {
 			// If a job is going to run again, then set its status to not ready so that the controller will run it again and the
 			// dependency status will be set correctly.
 			status.Ready = false
@@ -51,18 +51,21 @@ func resetHandlerControlledFields(app *v1.AppInstance) {
 	}
 
 	for name, status := range app.Status.AppStatus.Services {
+		status.Ready = status.Ready && appUpToDate
 		status.ExpressionErrors = nil
 		status.MissingConsumerPermissions = nil
 		app.Status.AppStatus.Services[name] = status
 	}
 
 	for name, status := range app.Status.AppStatus.Secrets {
+		status.Ready = status.Ready && appUpToDate
 		status.LookupErrors = nil
 		status.LookupTransitioning = nil
 		app.Status.AppStatus.Secrets[name] = status
 	}
 
 	for name, status := range app.Status.AppStatus.Routers {
+		status.Ready = status.Ready && appUpToDate
 		status.MissingTargets = nil
 		app.Status.AppStatus.Routers[name] = status
 	}

pkg/controller/appstatus/blocked.go

@@ -0,0 +1,41 @@
+package appstatus
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/acorn-io/baaah/pkg/typed"
+	v1 "github.com/acorn-io/runtime/pkg/apis/internal.acorn.io/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
+)
+
+func isBlocked(dependencies map[string]v1.DependencyStatus, expressionErrors []v1.ExpressionError) (result []string, _ bool) {
+	groupedByTypeName := map[string][]string{}
+
+	for depName, dep := range dependencies {
+		var key string
+		if dep.Missing {
+			key = string(dep.DependencyType) + " to be created"
+		} else if !dep.Ready {
+			key = string(dep.DependencyType) + " to be ready"
+		} else {
+			continue
+		}
+		groupedByTypeName[key] = append(groupedByTypeName[key], depName)
+	}
+
+	for _, exprError := range expressionErrors {
+		if exprError.DependencyNotFound != nil && exprError.DependencyNotFound.SubKey == "" {
+			key := string(exprError.DependencyNotFound.DependencyType) + " to be created"
+			groupedByTypeName[key] = append(groupedByTypeName[key], exprError.DependencyNotFound.Name)
+		}
+	}
+
+	for _, key := range typed.SortedKeys(groupedByTypeName) {
+		values := sets.NewString(groupedByTypeName[key]...).List()
+		msg := fmt.Sprintf("waiting for %s [%s]", key, strings.Join(values, ", "))
+		result = append(result, msg)
+	}
+
+	return result, len(result) > 0
+}

pkg/controller/appstatus/containers.go

@@ -137,6 +137,14 @@ func setContainerMessages(app *v1.AppInstance) {
 			}
 		}
 
+		if !cs.Ready {
+			msg, blocked := isBlocked(cs.Dependencies, cs.ExpressionErrors)
+			if blocked {
+				cs.State = "waiting"
+			}
+			cs.TransitioningMessages = append(cs.TransitioningMessages, msg...)
+		}
+
 		// Add informative messages if all else is healthy
 		if len(cs.TransitioningMessages) == 0 && len(cs.ErrorMessages) == 0 {
 			if cs.RunningReplicaCount > 1 {

pkg/controller/appstatus/jobs.go

@@ -184,6 +184,14 @@ func setJobMessages(app *v1.AppInstance) {
 			}
 		}
 
+		if !c.Ready {
+			msg, blocked := isBlocked(c.Dependencies, c.ExpressionErrors)
+			if blocked {
+				c.State = "waiting"
+			}
+			c.TransitioningMessages = append(c.TransitioningMessages, msg...)
+		}
+
 		app.Status.AppStatus.Jobs[jobName] = c
 	}
 }

pkg/controller/appstatus/secrets.go

@@ -91,9 +91,6 @@ func (a *appStatusRenderer) readSecrets() error {
 
 func setSecretMessages(app *v1.AppInstance) {
 	for secretName, s := range app.Status.AppStatus.Secrets {
-		s.ErrorMessages = s.LookupErrors
-		s.TransitioningMessages = s.LookupTransitioning
-
 		// Not ready if we have any error messages
 		if len(s.ErrorMessages) > 0 {
 			s.Ready = false

pkg/controller/routes.go

@@ -60,13 +60,13 @@ func routes(router *router.Router, cfg *rest.Config, registryTransport http.Roun
 	router.OnErrorHandler = appdefinition.OnError
 
 	appRouter := router.Type(&v1.AppInstance{}).Middleware(devsession.OverlayDevSession).IncludeFinalizing()
-	appRouter.HandlerFunc(appstatus.PrepareStatus)
 	appRouter.HandlerFunc(appdefinition.AssignNamespace)
 
 	// AppImage preparation, checks and promotion
 	appRouter.HandlerFunc(appdefinition.PullAppImage(registryTransport, recorder)) // pulls image to .status.Staged.AppImage
 	appRouter.HandlerFunc(permissions.CheckPermissions)                            // checks if newly staged image is allowed and that permissions are requested and granted properly
 	appRouter.HandlerFunc(permissions.CopyPromoteStagedAppImage)                   // if the above checks pass, the staged image is promoted to app.Status.AppImage and everything below will use that image
+	appRouter.HandlerFunc(appstatus.PrepareStatus)
 
 	// ---
 	appRouter.HandlerFunc(images.CreateImages)
@@ -82,14 +82,14 @@ func routes(router *router.Router, cfg *rest.Config, registryTransport http.Roun
 	appHasNamespace.HandlerFunc(quota.WaitForAllocation)
 
 	appMeetsPreconditions := appHasNamespace.Middleware(appstatus.CheckStatus)
-	appMeetsPreconditions.HandlerFunc(appstatus.GetStatus)
 	appMeetsPreconditions.Middleware(appdefinition.ImagePulled).HandlerFunc(permissions.ConsumerPermissions)
 	appMeetsPreconditions.Middleware(appdefinition.ImagePulled).HandlerFunc(appdefinition.DeploySpec)
 	appMeetsPreconditions.Middleware(appdefinition.ImagePulled).HandlerFunc(secrets.CreateSecrets)
 	appMeetsPreconditions.HandlerFunc(networkpolicy.ForApp)
 	appMeetsPreconditions.HandlerFunc(appdefinition.AddAcornProjectLabel)
 	appMeetsPreconditions.HandlerFunc(appdefinition.UpdateObservedFields)
 
+	appRouter.HandlerFunc(appstatus.GetStatus)
 	appRouter.HandlerFunc(appstatus.SetStatus)
 	appRouter.HandlerFunc(appstatus.ReadyStatus)
 	appRouter.HandlerFunc(appstatus.CLIStatus)