diff --git a/bundle/manifests/amalthea.clusterserviceversion.yaml b/bundle/manifests/amalthea.clusterserviceversion.yaml index 573864c8..6476e843 100644 --- a/bundle/manifests/amalthea.clusterserviceversion.yaml +++ b/bundle/manifests/amalthea.clusterserviceversion.yaml @@ -132,6 +132,7 @@ spec: - ingresses verbs: - create + - delete - get - list - patch diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 086efe12..b6612d36 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -96,6 +96,7 @@ rules: - ingresses verbs: - create + - delete - get - list - patch diff --git a/helm-chart/amalthea-sessions/templates/manager-rbac.yaml b/helm-chart/amalthea-sessions/templates/manager-rbac.yaml index 68bcba1e..8178a70e 100644 --- a/helm-chart/amalthea-sessions/templates/manager-rbac.yaml +++ b/helm-chart/amalthea-sessions/templates/manager-rbac.yaml @@ -66,7 +66,7 @@ rules: - networking.k8s.io resources: - ingresses - verbs: [create, get, list, watch, patch, update] + verbs: [create, delete, get, list, watch, patch, update] # Required for hibernating sessions - apiGroups: ["apps"] resources: ["statefulsets"] diff --git a/internal/controller/amaltheasession_controller.go b/internal/controller/amaltheasession_controller.go index 3ec1c887..4b5757e6 100644 --- a/internal/controller/amaltheasession_controller.go +++ b/internal/controller/amaltheasession_controller.go @@ -65,7 +65,7 @@ const secretCleanupFinalizerName = "amalthea.dev/secrets-finalizer" // +kubebuilder:rbac:groups=core,resources=services,verbs=get;list;watch;create;update;patch // +kubebuilder:rbac:groups=core,resources=events,verbs=get;list;watch // +kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=get;list;watch;create;update;patch -// +kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=get;list;watch;create;update;patch +// +kubebuilder:rbac:groups=networking.k8s.io,resources=ingresses,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=metrics.k8s.io,resources=pods,verbs=get;list;watch // Reconcile is part of the main kubernetes reconciliation loop which aims to @@ -214,6 +214,7 @@ func (r *AmaltheaSessionReconciler) reconcileInner(ctx context.Context, req ctrl amaltheasession.Status.RunID = runID children, err := NewChildResources(amaltheasession, r.Configuration) + if err != nil { logger.Error( err, @@ -276,7 +277,11 @@ func (r *AmaltheaSessionReconciler) reconcileInner(ctx context.Context, req ctrl // If the status is evolving we should requeue faster requeueAfter = 0 } - return ctrl.Result{Requeue: true, RequeueAfter: requeueAfter}, nil + + if requeueAfter > 0 { + return ctrl.Result{RequeueAfter: requeueAfter}, nil + } + return ctrl.Result{Requeue: true}, nil } func (r *AmaltheaSessionReconciler) deleteSecrets(ctx context.Context, cr *amaltheadevv1alpha1.AmaltheaSession) error { diff --git a/internal/controller/children.go b/internal/controller/children.go index f97881e1..0e0f0d3f 100644 --- a/internal/controller/children.go +++ b/internal/controller/children.go @@ -101,6 +101,13 @@ func (c ChildResource[T]) Reconcile(ctx context.Context, clnt client.Client, cr } switch current := any(c.Current).(type) { case *networkingv1.Ingress: + if cr.Spec.Hibernated { + err := clnt.Delete(ctx, current) + if apierrors.IsNotFound(err) { + return ChildResourceUpdate[T]{c.Current, controllerutil.OperationResultNone, nil, nil} + } + return ChildResourceUpdate[T]{c.Current, "deleted", err, nil} + } res, err := controllerutil.CreateOrPatch(ctx, clnt, current, func() error { // NOTE: the callback function in CreateOrPatch will load the // state of the object referenced from k8s, then run the callback to update diff --git a/test/e2e/general_test.go b/test/e2e/general_test.go index 9d7923db..5aaba7fe 100644 --- a/test/e2e/general_test.go +++ b/test/e2e/general_test.go @@ -9,7 +9,9 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" schedv1 "k8s.io/api/scheduling/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" resource "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -216,10 +218,11 @@ var _ = Describe("reconcile strategies", Ordered, func() { g.Expect(sessionPod).To(BeNil()) }, "60s").WithContext(ctx).Should(Succeed()) By("Resuming the session we should see the new changes") - patched = &amaltheadevv1alpha1.AmaltheaSession{} - Expect(k8sClient.Get(ctx, typeNamespacedName, patched)).To(Succeed()) - patched.Spec.Hibernated = false - Expect(k8sClient.Update(ctx, patched)).To(Succeed()) + Eventually(func(g Gomega) { + Expect(k8sClient.Get(ctx, typeNamespacedName, patched)).To(Succeed()) + patched.Spec.Hibernated = false + Expect(k8sClient.Update(ctx, patched)).To(Succeed()) + }, "60s").WithContext(ctx).Should(Succeed()) Eventually(func(g Gomega) { sessionPod, err = amaltheasession.GetPod(ctx, k8sClient) g.Expect(err).NotTo(HaveOccurred()) @@ -228,6 +231,64 @@ var _ = Describe("reconcile strategies", Ordered, func() { g.Expect(sessionPod.Spec.Containers[0].Resources.Requests.Memory()).To(Equal(&newMemory)) }).WithContext(ctx).WithTimeout(time.Minute).Should(Succeed()) }) + + It( + "should delete the Ingress when hibernating", + func(ctx SpecContext, + ) { + var err error + var sessionPod *corev1.Pod + + By("Adding an ingress to the session") + patched := amaltheasession.DeepCopy() + Expect(k8sClient.Get(ctx, typeNamespacedName, patched)).To(Succeed()) + patched.Spec.Ingress = &amaltheadevv1alpha1.Ingress{ + Host: "amaltheasession.localhost", + } + Expect(k8sClient.Update(ctx, patched)).To(Succeed()) + + By("Checking the ingress was created") + Eventually(func(g Gomega) { + ingress := &networkingv1.Ingress{} + g.Expect(k8sClient.Get(ctx, typeNamespacedName, ingress)).To(Succeed()) + g.Expect(ingress).NotTo(BeNil()) + }, "60s").WithContext(ctx).Should(Succeed()) + + By("Hibernating the session") + Eventually(func(g Gomega) { + patched = &amaltheadevv1alpha1.AmaltheaSession{} + Expect(k8sClient.Get(ctx, typeNamespacedName, patched)).To(Succeed()) + patched.Spec.Hibernated = true + Expect(k8sClient.Update(ctx, patched)).To(Succeed()) + }, "60s").WithContext(ctx).Should(Succeed()) + // Make sure the session has stopped, and the pod has been cleaned up + Eventually(func(g Gomega) { + sessionPod, err = amaltheasession.GetPod(ctx, k8sClient) + g.Expect(err).To(HaveOccurred()) + g.Expect(sessionPod).To(BeNil()) + }, "60s").WithContext(ctx).Should(Succeed()) + + By("Checking the ingress has gone") + Eventually(func(g Gomega) { + ingress := &networkingv1.Ingress{} + err := k8sClient.Get(ctx, typeNamespacedName, ingress) + g.Expect(apierrors.IsNotFound(err)).To(BeTrue()) + }, "60s").WithContext(ctx).Should(Succeed()) + + By("Resuming the session we should see the ingress again") + Eventually(func(g Gomega) { + patched = &amaltheadevv1alpha1.AmaltheaSession{} + Expect(k8sClient.Get(ctx, typeNamespacedName, patched)).To(Succeed()) + patched.Spec.Hibernated = false + Expect(k8sClient.Update(ctx, patched)).To(Succeed()) + }, "60s").WithContext(ctx).Should(Succeed()) + Eventually(func(g Gomega) { + ingress := &networkingv1.Ingress{} + g.Expect(k8sClient.Get(ctx, typeNamespacedName, ingress)).To(Succeed()) + g.Expect(ingress).NotTo(BeNil()) + }).WithContext(ctx).WithTimeout(time.Minute).Should(Succeed()) + + }) }) Context("When the session is failing", func() { diff --git a/test/utils/utils.go b/test/utils/utils.go index 65fa982d..165fa73b 100644 --- a/test/utils/utils.go +++ b/test/utils/utils.go @@ -33,6 +33,7 @@ import ( amaltheadevv1alpha1 "github.com/SwissDataScienceCenter/amalthea/api/v1alpha1" "github.com/SwissDataScienceCenter/amalthea/internal/controller" corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" metricsv "k8s.io/metrics/pkg/client/clientset/versioned" @@ -332,6 +333,7 @@ func GetK8sClient(ctx context.Context, namespace string) (client.Client, error) &amaltheadevv1alpha1.AmaltheaSession{}, &corev1.Pod{}, &corev1.Event{}, + &networkingv1.Ingress{}, }, }, },