fix: requeue without delay if the status is changing (#704)

* fix: requeue without delay if the status is changing

The status for amalthea sessions takes a long time to show up. This
should address this problem.

* fix: show default status properties

This avoids having a completely empty status when the session is first
created.

* fix: use same fsgroup and user group

* fix: path for oauth2proxy should end with /

api/v1alpha1/amaltheasession_children.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/url"
+	"strings"
 	"time"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -146,6 +147,12 @@ func (cr *AmaltheaSession) StatefulSet() appsv1.StatefulSet {
 		})
 
 		if auth.Type == Oidc {
+			sessionURL := cr.sessionLocalhostURL().String()
+			if !strings.HasSuffix(sessionURL, "/") {
+				// NOTE: If the url does not end with "/" then the oauth2proxy proxies only the exact path
+				// and does not proxy subpaths
+				sessionURL += "/"
+			}
 			authContainer := v1.Container{
 				Image: oauth2ProxyImage,
 				Name:  "oauth2-proxy",
@@ -154,7 +161,7 @@ func (cr *AmaltheaSession) StatefulSet() appsv1.StatefulSet {
 					RunAsNonRoot:             ptr.To(true),
 				},
 				Args: []string{
-					fmt.Sprintf("--upstream=%s", cr.sessionLocalhostURL().String()),
+					fmt.Sprintf("--upstream=%s", sessionURL),
 					fmt.Sprintf("--http-address=:%d", authProxyPort),
 					"--silence-ping-logging",
 					"--config=/etc/oauth2-proxy/" + auth.SecretRef.Key,
@@ -221,6 +228,7 @@ func (cr *AmaltheaSession) StatefulSet() appsv1.StatefulSet {
 					Labels: labels,
 				},
 				Spec: v1.PodSpec{
+					SecurityContext: &v1.PodSecurityContext{FSGroup: &cr.Spec.Session.RunAsGroup},
 					Containers:     containers,
 					InitContainers: initContainers,
 					Volumes:        volumes,
@@ -435,14 +443,16 @@ func (cr *AmaltheaSession) initClones() ([]v1.Container, []v1.Volume) {
 func (cr *AmaltheaSession) AllSecrets() v1.SecretList {
 	secrets := v1.SecretList{}
 
-	if cr.Spec.Ingress != nil && cr.Spec.Ingress.TLSSecretName != nil {
-		secrets.Items = append(secrets.Items, v1.Secret{
-			ObjectMeta: metav1.ObjectMeta{
-				Namespace: cr.Namespace,
-				Name:      *cr.Spec.Ingress.TLSSecretName,
-			},
-		})
-	}
+	// Removing the TLS secret will cause certificates to be re-issued
+	// we should treat this differently.
+	// if cr.Spec.Ingress != nil && cr.Spec.Ingress.TLSSecretName != nil {
+	// 	secrets.Items = append(secrets.Items, v1.Secret{
+	// 		ObjectMeta: metav1.ObjectMeta{
+	// 			Namespace: cr.Namespace,
+	// 			Name:      *cr.Spec.Ingress.TLSSecretName,
+	// 		},
+	// 	})
+	// }
 
 	if auth := cr.Spec.Authentication; auth != nil && auth.Enabled {
 		secrets.Items = append(secrets.Items, v1.Secret{

api/v1alpha1/amaltheasession_types.go

@@ -17,6 +17,9 @@ limitations under the License.
 package v1alpha1
 
 import (
+	"net/url"
+	"strings"
+
 	v1 "k8s.io/api/core/v1"
 	resource "k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -108,6 +111,8 @@ type Session struct {
 	// +optional
 	// +kubebuilder:default:=1000
 	// +kubebuilder:validation:Minimum:=0
+	// The group is set on the session and this value is also set as the fsgroup for the whole pod and all session
+	// contianers.
 	RunAsGroup int64 `json:"runAsGroup,omitempty"`
 	// +optional
 	// +kubebuilder:default:="/"
@@ -293,6 +298,7 @@ type AmaltheaSessionStatus struct {
 	URL                 string          `json:"url,omitempty"`
 	ContainerCounts     ContainerCounts `json:"containerCounts,omitempty"`
 	InitContainerCounts ContainerCounts `json:"initContainerCounts,omitempty"`
+	// +kubebuilder:default:=false
 	Idle                bool            `json:"idle,omitempty"`
 	// +kubebuilder:validation:Format:=date-time
 	IdleSince metav1.Time `json:"idleSince,omitempty"`
@@ -316,6 +322,7 @@ type AmaltheaSession struct {
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
 	Spec   AmaltheaSessionSpec   `json:"spec,omitempty"`
+	// +kubebuilder:default:={}
 	Status AmaltheaSessionStatus `json:"status,omitempty"`
 }
 
@@ -349,3 +356,27 @@ type AmaltheaSessionCondition struct {
 func init() {
 	SchemeBuilder.Register(&AmaltheaSession{}, &AmaltheaSessionList{})
 }
+
+func (a *AmaltheaSession) GetURLString() string {
+	sessionURL := a.GetURL()
+	if sessionURL == nil {
+		return "None"
+	}
+	return strings.TrimSuffix(sessionURL.String(), "/")
+}
+
+func (a *AmaltheaSession) GetURL() *url.URL {
+	if a.Spec.Ingress == nil || a.Spec.Ingress.Host == "" {
+		return nil
+	}
+	urlScheme := "http"
+	if a.Spec.Ingress.TLSSecretName != nil {
+		urlScheme = "https"
+	}
+	sessionURL := url.URL{
+		Scheme: urlScheme,
+		Path:   a.Spec.Session.URLPath,
+		Host:   a.Spec.Ingress.Host,
+	}
+	return &sessionURL
+}

config/crd/bases/amalthea.dev_amaltheasessions.yaml

@@ -4792,6 +4792,9 @@ spec:
                     type: object
                   runAsGroup:
                     default: 1000
+                    description: |-
+                      The group is set on the session and this value is also set as the fsgroup for the whole pod and all session
+                      contianers.
                     format: int64
                     minimum: 0
                     type: integer
@@ -4845,6 +4848,7 @@ spec:
             - session
             type: object
           status:
+            default: {}
             description: AmaltheaSessionStatus defines the observed state of AmaltheaSession
             properties:
               conditions:
@@ -4885,6 +4889,7 @@ spec:
                 format: date-time
                 type: string
               idle:
+                default: false
                 type: boolean
               idleSince:
                 format: date-time

internal/controller/amaltheasession_controller.go

@@ -19,6 +19,7 @@ package controller
 import (
 	"context"
 	"errors"
+	"reflect"
 	"time"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -84,6 +85,21 @@ func (r *AmaltheaSessionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	}
 
 	if amaltheasession.GetDeletionTimestamp() == nil {
+		if reflect.DeepEqual(amaltheasession.Status, amaltheadevv1alpha1.AmaltheaSessionStatus{State: amaltheadevv1alpha1.NotReady, Idle: false}) {
+			// First status update/render
+			amaltheasession.Status.URL = amaltheasession.GetURLString()
+			err := r.Status().Update(ctx, amaltheasession)
+			if err != nil {
+				err = r.Get(ctx, req.NamespacedName, amaltheasession)
+				if err != nil {
+					return ctrl.Result{}, err
+				}
+				err = r.Status().Update(ctx, amaltheasession)
+				if err != nil {
+					return ctrl.Result{}, err
+				}
+			}
+		}
 		// The object is not being deleted, so if it does not have our finalizer,
 		// then lets add the finalizer and update the object. This is equivalent
 		// to registering our finalizer.
@@ -135,6 +151,7 @@ func (r *AmaltheaSessionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	}
 
 	newStatus := updates.Status(ctx, r.Client, amaltheasession)
+	statusChanged := reflect.DeepEqual(amaltheasession.Status, newStatus)
 	amaltheasession.Status = newStatus
 	err = r.Status().Update(ctx, amaltheasession)
 	if err != nil {
@@ -162,7 +179,12 @@ func (r *AmaltheaSessionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	}
 
 	// Now requeue to make sure we can watch for idleness and other status changes
-	return ctrl.Result{Requeue: true, RequeueAfter: time.Second * 10}, nil
+	requeueAfter := time.Second * 10
+	if statusChanged {
+		// If the status is evolving we should requeue faster
+		requeueAfter = 0
+	}
+	return ctrl.Result{Requeue: true, RequeueAfter: requeueAfter}, nil
 }
 
 func (r *AmaltheaSessionReconciler) deleteSecrets(ctx context.Context, cr *amaltheadevv1alpha1.AmaltheaSession) error {

internal/controller/children.go

@@ -3,7 +3,6 @@ package controller
 import (
 	"context"
 	"fmt"
-	"net/url"
 	"strings"
 	"time"
 
@@ -228,20 +227,6 @@ func (c ChildResourceUpdates) Status(ctx context.Context, clnt client.Client, cr
 		failingSince = metav1.Time{}
 	}
 
-	sessionURLStr := "None"
-	if cr.Spec.Ingress != nil {
-		urlScheme := "http"
-		if cr.Spec.Ingress.TLSSecretName != nil {
-			urlScheme = "https"
-		}
-		sessionURL := url.URL{
-			Scheme: urlScheme,
-			Path:   cr.Spec.Session.URLPath,
-			Host:   cr.Spec.Ingress.Host,
-		}
-		sessionURLStr = strings.TrimSuffix(sessionURL.String(), "/")
-	}
-
 	state := c.State(cr, pod)
 	conditions := cr.Status.Conditions
 	if len(conditions) == 0 {
@@ -289,7 +274,7 @@ func (c ChildResourceUpdates) Status(ctx context.Context, clnt client.Client, cr
 	status := amaltheadevv1alpha1.AmaltheaSessionStatus{
 		Conditions:      conditions,
 		State:           state,
-		URL:             sessionURLStr,
+		URL:             cr.GetURLString(),
 		Idle:            idle,
 		IdleSince:       idleSince,
 		FailingSince:    failingSince,