fix: resolved offerings: calculate if a container is missing (#2480)

Signed-off-by: Grant Linville <[email protected]>

Author: g-linville

pkg/controller/appdefinition/deploy.go

@@ -628,8 +628,8 @@ func containerAnnotation(container v1.Container) string {
 	return string(json)
 }
 
-func resolvedOfferingsAnnotation(appInstance *v1.AppInstance, container v1.Container) (string, error) {
-	if resolved, exists := appInstance.Status.ResolvedOfferings.Containers[container.Name]; exists {
+func resolvedOfferingsAnnotation(appInstance *v1.AppInstance, containerName string) (string, error) {
+	if resolved, exists := appInstance.Status.ResolvedOfferings.Containers[containerName]; exists {
 		data, err := convert.EncodeToMap(resolved)
 		if err != nil {
 			return "", err
@@ -645,13 +645,13 @@ func resolvedOfferingsAnnotation(appInstance *v1.AppInstance, container v1.Conta
 	return "", nil
 }
 
-func podAnnotations(appInstance *v1.AppInstance, container v1.Container) map[string]string {
+func podAnnotations(appInstance *v1.AppInstance, containerName string, container v1.Container) map[string]string {
 	annotations := map[string]string{
 		labels.AcornContainerSpec: containerAnnotation(container),
 	}
 	addPrometheusAnnotations(annotations, container)
 
-	offerings, err := resolvedOfferingsAnnotation(appInstance, container)
+	offerings, err := resolvedOfferingsAnnotation(appInstance, containerName)
 	if err == nil && offerings != "" {
 		annotations[labels.AcornContainerResolvedOfferings] = offerings
 	}
@@ -814,7 +814,7 @@ func toFunctionDeployment(req router.Request, appInstance *v1.AppInstance, tag n
 			Template: corev1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels:      podLabels,
-					Annotations: typed.Concat(deploymentAnnotations, podAnnotations(appInstance, container), secretAnnotations),
+					Annotations: typed.Concat(deploymentAnnotations, podAnnotations(appInstance, name, container), secretAnnotations),
 				},
 				Spec: corev1.PodSpec{
 					Affinity:           appInstance.Status.Scheduling[name].Affinity,
@@ -906,7 +906,7 @@ func toDeployment(req router.Request, appInstance *v1.AppInstance, tag name.Refe
 			Template: corev1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels:      podLabels,
-					Annotations: typed.Concat(deploymentAnnotations, podAnnotations(appInstance, container), secretAnnotations),
+					Annotations: typed.Concat(deploymentAnnotations, podAnnotations(appInstance, name, container), secretAnnotations),
 				},
 				Spec: corev1.PodSpec{
 					Affinity:                      appInstance.Status.Scheduling[name].Affinity,

pkg/controller/appdefinition/jobs.go

@@ -147,7 +147,7 @@ func toJob(req router.Request, appInstance *v1.AppInstance, pullSecrets *PullSec
 		Template: corev1.PodTemplateSpec{
 			ObjectMeta: metav1.ObjectMeta{
 				Labels:      podLabels,
-				Annotations: labels.Merge(podAnnotations(appInstance, container), baseAnnotations),
+				Annotations: labels.Merge(podAnnotations(appInstance, name, container), baseAnnotations),
 			},
 			Spec: corev1.PodSpec{
 				Affinity:                      appInstance.Status.Scheduling[name].Affinity,

pkg/controller/resolvedofferings/resolvedofferings.go

@@ -8,11 +8,6 @@ import (
 )
 
 // Calculate is a handler that sets the resolved offerings for an AppInstance to its status.
-//
-// This is necessary because querying for resolved offerings will result in all running
-// AppInstances using that default to redeploy when a default changes. By
-// calculating the resolved offerings only when the generation changes, we can ensure that
-// updated resolved offerings are only applied when an AppInstance is updated directly.
 func Calculate(req router.Request, resp router.Response) (err error) {
 	appInstance := req.Object.(*internalv1.AppInstance)
 	status := condition.Setter(appInstance, resp, internalv1.AppInstanceConditionResolvedOfferings)
@@ -34,7 +29,10 @@ func Calculate(req router.Request, resp router.Response) (err error) {
 		return err
 	}
 
-	if appInstance.Generation != appInstance.Status.ObservedGeneration {
+	// Calculate the resolved offerings for the AppInstance if its generation changed, or if at least one of
+	// the containers has not been resolved yet. The generation check prevents the app from redeploying
+	// if its compute class gets changed.
+	if appInstance.Generation != appInstance.Status.ObservedGeneration || needsCalculation(appInstance) {
 		if err = calculate(req, appInstance); err != nil {
 			return err
 		}
@@ -61,3 +59,12 @@ func calculate(req router.Request, appInstance *internalv1.AppInstance) error {
 
 	return nil
 }
+
+func needsCalculation(appInstance *internalv1.AppInstance) bool {
+	for _, name := range appInstance.GetAllContainerNames() {
+		if _, ok := appInstance.Status.ResolvedOfferings.Containers[name]; !ok {
+			return true
+		}
+	}
+	return false
+}