From e0d9ffb9da1f9b177a83eb1d36e62791bd2c76f6 Mon Sep 17 00:00:00 2001 From: Theo Barber-Bany Date: Thu, 17 Feb 2022 15:40:16 +0000 Subject: [PATCH 1/4] Update console types with conditions Update console types with failure/termination conditions. We want to update these using a new controller, in which we can keep logic to determine failure / termination states and relay that to the user via a condition and to Kaiju by pubsub --- apis/workloads/v1alpha1/console_types.go | 54 +++++++++++++++++++++++- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/apis/workloads/v1alpha1/console_types.go b/apis/workloads/v1alpha1/console_types.go index 2bd387a3..19d38259 100644 --- a/apis/workloads/v1alpha1/console_types.go +++ b/apis/workloads/v1alpha1/console_types.go @@ -57,10 +57,60 @@ type ConsoleStatus struct { PodName string `json:"podName"` ExpiryTime *metav1.Time `json:"expiryTime,omitempty"` // Time at which the job completed successfully - CompletionTime *metav1.Time `json:"completionTime,omitempty"` - Phase ConsolePhase `json:"phase"` + CompletionTime *metav1.Time `json:"completionTime,omitempty"` + Phase ConsolePhase `json:"phase"` + Conditions []ConsoleCondition `json:"conditions,omitempty"` } +// Console conditions describe a valid condition for a console +type ConsoleConditionType string + +const ( + // ConsoleTerminatedType describes the type of condition where the console + // is terminated + ConsoleTerminatedType ConsoleConditionType = "ConsoleTerminated" + + // ConsoleFailedType describes the type of condition where the controller + // tried to start the console, but there was some error causing it to fail + ConsoleFailedType ConsoleConditionType = "ConsoleFailed" +) + +// ConsoleCondition is a status condition for a console +type ConsoleCondition struct { + // Type of this condition + Type ConsoleConditionType `json:"type"` + + // Status of this condition + Status corev1.ConditionStatus `json:"status"` + + // LastUpdateTime of this condition + LastUpdateTime metav1.Time `json:"lastUpdateTime,omitempty"` + + // LastTransitionTime of this condition + LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"` + + // Reason for the current status of this condition + Reason ConsoleConditionReason `json:"reason,omitempty"` + + // Message associated with this condition + Message string `json:"message,omitempty"` +} + +// ConsoleConditionReason represents a valid condition reason for a console +type ConsoleConditionReason string + +const ( + // ReasonTlogNotFound is a console condition for a failed console because + // tlog-rec was not found in the image + ReasonTlogNotFound ConsoleConditionReason = "tlog-rec binary not found" + + ReasonCrashLoopBackoff ConsoleConditionReason = "pod for console is in CrashLoopBackoff" + + ReasonImagePull ConsoleConditionReason = "specified image failed to pull" + + ReasonTimedOutAuthorize ConsoleConditionReason = "specified image failed to pull" +) + // +kubebuilder:object:root=true // +kubebuilder:storageversion From c9d7cc693bd6dcd47cf28d10574af14b586cafb0 Mon Sep 17 00:00:00 2001 From: Theo Barber-Bany Date: Thu, 17 Feb 2022 15:41:21 +0000 Subject: [PATCH 2/4] Add barebones termination controller A very bare bones termination controller to watch Consoles, determine why their pods fail and update conditions on their status. --- cmd/workloads-manager/main.go | 14 +++ .../console/termination/handler/handler.go | 29 ++++++ .../console/termination/status/status.go | 10 ++ .../console/termination/status/types.go | 13 +++ .../termination/termination_controller.go | 95 +++++++++++++++++++ 5 files changed, 161 insertions(+) create mode 100644 controllers/workloads/console/termination/handler/handler.go create mode 100644 controllers/workloads/console/termination/status/status.go create mode 100644 controllers/workloads/console/termination/status/types.go create mode 100644 controllers/workloads/console/termination/termination_controller.go diff --git a/cmd/workloads-manager/main.go b/cmd/workloads-manager/main.go index 280247f8..bd8eebcb 100644 --- a/cmd/workloads-manager/main.go +++ b/cmd/workloads-manager/main.go @@ -16,6 +16,7 @@ import ( workloadsv1alpha1 "github.com/gocardless/theatre/v3/apis/workloads/v1alpha1" "github.com/gocardless/theatre/v3/cmd" consolecontroller "github.com/gocardless/theatre/v3/controllers/workloads/console" + terminationcontroller "github.com/gocardless/theatre/v3/controllers/workloads/console/termination" "github.com/gocardless/theatre/v3/pkg/signals" "github.com/gocardless/theatre/v3/pkg/workloads/console/events" ) @@ -102,6 +103,19 @@ func main() { app.Fatalf("failed to create controller: %v", err) } + // controller to observe termination states + if err = (&terminationcontroller.TerminationReconciler{ + // Client: mgr.GetClient(), + // LifecycleRecorder: lifecycleRecorder, + // ConsoleIdBuilder: idBuilder, + // Log: ctrl.Log.WithName("controllers").WithName("termination-watcher"), + // Scheme: mgr.GetScheme(), + // SessionPubsubProjectId: *sessionPubsubProjectId, + // SessionPubsubTopicId: *sessionPubsubTopicId, + }).SetupWithManager(ctx, mgr); err != nil { + app.Fatalf("failed to create termination watching controller: %v", err) + } + // console authenticator webhook mgr.GetWebhookServer().Register("/mutate-consoles", &admission.Webhook{ Handler: workloadsv1alpha1.NewConsoleAuthenticatorWebhook( diff --git a/controllers/workloads/console/termination/handler/handler.go b/controllers/workloads/console/termination/handler/handler.go new file mode 100644 index 00000000..f53ce817 --- /dev/null +++ b/controllers/workloads/console/termination/handler/handler.go @@ -0,0 +1,29 @@ +package handler + +import ( + workloadsv1alpha1 "github.com/gocardless/theatre/v3/apis/workloads/v1alpha1" + "github.com/gocardless/theatre/v3/controllers/workloads/console/termination/status" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func main() { +} + +type TerminationHandler struct { + client client.Client +} + +type Options struct{} + +func NewTerminationHandler(c client.Client, opts *Options) *TerminationHandler { + return &TerminationHandler{ + client: c, + } +} + +func (h *TerminationHandler) Handle(instance *workloadsv1alpha1.Console) (*status.Result, error) { + switch instance.Status.Phase { + default: + return nil, nil + } +} diff --git a/controllers/workloads/console/termination/status/status.go b/controllers/workloads/console/termination/status/status.go new file mode 100644 index 00000000..f69d77c4 --- /dev/null +++ b/controllers/workloads/console/termination/status/status.go @@ -0,0 +1,10 @@ +package status + +import ( + workloadsv1alpha1 "github.com/gocardless/theatre/v3/apis/workloads/v1alpha1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func UpdateStatus(c client.Client, instance *workloadsv1alpha1.Console, result *Result) error { + return nil +} diff --git a/controllers/workloads/console/termination/status/types.go b/controllers/workloads/console/termination/status/types.go new file mode 100644 index 00000000..53c981f0 --- /dev/null +++ b/controllers/workloads/console/termination/status/types.go @@ -0,0 +1,13 @@ +package status + +import ( + workloadsv1alpha1 "github.com/gocardless/theatre/v3/apis/workloads/v1alpha1" +) + +// Result is the basis for updating the status of a Console with termination +// events +type Result struct { + Phase *workloadsv1alpha1.ConsolePhase + Condition workloadsv1alpha1.ConsoleConditionType + Reason workloadsv1alpha1.ConsoleConditionReason +} diff --git a/controllers/workloads/console/termination/termination_controller.go b/controllers/workloads/console/termination/termination_controller.go new file mode 100644 index 00000000..3c9efd2d --- /dev/null +++ b/controllers/workloads/console/termination/termination_controller.go @@ -0,0 +1,95 @@ +package termination + +import ( + "context" + "fmt" + + workloadsv1alpha1 "github.com/gocardless/theatre/v3/apis/workloads/v1alpha1" + + "github.com/gocardless/theatre/v3/controllers/workloads/console/termination/handler" + "github.com/gocardless/theatre/v3/controllers/workloads/console/termination/status" + batchv1 "k8s.io/api/batch/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + watchhandler "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/controller-runtime/pkg/source" +) + +// newReconciler returns a new reconcile.Reconciler +func newReconciler(mgr manager.Manager) reconcile.Reconciler { + h := handler.NewTerminationHandler(mgr.GetClient(), &handler.Options{}) + return &TerminationReconciler{Client: mgr.GetClient(), handler: h, scheme: mgr.GetScheme()} +} + +func add(mgr manager.Manager, r reconcile.Reconciler) error { + // create the controller + c, err := controller.New("console-termination-controller", mgr, controller.Options{Reconciler: r}) + if err != nil { + return err + } + + // Watch for Console changes + err = c.Watch(&source.Kind{Type: &workloadsv1alpha1.Console{}}, &watchhandler.EnqueueRequestForOwner{ + IsController: true, + }) + if err != nil { + return nil + } + + // Watch for job changes + err = c.Watch(&source.Kind{Type: &batchv1.Job{}}, &watchhandler.EnqueueRequestForOwner{ + OwnerType: &workloadsv1alpha1.Console{}, + IsController: true, + }) + if err != nil { + return nil + } + + // err = mgr.GetCache().IndexField(&batchv1.Job{}, "metadata.labels" ,func(obj runtime.Object) []string + + return nil +} + +var _ reconcile.Reconciler = &TerminationReconciler{} + +type TerminationReconciler struct { + client.Client + handler *handler.TerminationHandler + scheme *runtime.Scheme +} + +func (r *TerminationReconciler) SetupWithManager(ctx context.Context, mgr manager.Manager) error { + return add(mgr, newReconciler(mgr)) +} + +func (r *TerminationReconciler) Reconcile(request reconcile.Request) (reconcile.Result, error) { + instance := &workloadsv1alpha1.Console{} + err := r.Get(context.Background(), request.NamespacedName, instance) + if err != nil { + if errors.IsNotFound(err) { + return reconcile.Result{}, nil + } + return reconcile.Result{}, err + } + + result, err := r.handler.Handle(instance) + if err != nil { + statusErr := status.UpdateStatus(r.Client, instance, result) + if statusErr != nil { + // todo: logging good + panic("failed to update status") + } + + return reconcile.Result{}, fmt.Errorf("error handling console %s: %v", instance.GetName(), err) + } + + err = status.UpdateStatus(r.Client, instance, result) + if err != nil { + return reconcile.Result{}, fmt.Errorf("error updating status: %v", err) + } + return reconcile.Result{}, nil +} From eeee97fbb0ac43e2f27002cfd35962a3cf9a3628 Mon Sep 17 00:00:00 2001 From: Talal Al-Tamimi Date: Thu, 24 Feb 2022 18:10:09 +0000 Subject: [PATCH 3/4] Generate code Run `make generate` --- apis/rbac/v1alpha1/zz_generated.deepcopy.go | 1 + .../v1alpha1/zz_generated.deepcopy.go | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/apis/rbac/v1alpha1/zz_generated.deepcopy.go b/apis/rbac/v1alpha1/zz_generated.deepcopy.go index 885030f6..14402a41 100644 --- a/apis/rbac/v1alpha1/zz_generated.deepcopy.go +++ b/apis/rbac/v1alpha1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated // Code generated by controller-gen. DO NOT EDIT. diff --git a/apis/workloads/v1alpha1/zz_generated.deepcopy.go b/apis/workloads/v1alpha1/zz_generated.deepcopy.go index 129cc38a..325dff56 100644 --- a/apis/workloads/v1alpha1/zz_generated.deepcopy.go +++ b/apis/workloads/v1alpha1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated // Code generated by controller-gen. DO NOT EDIT. @@ -197,6 +198,23 @@ func (in *ConsoleAuthorisers) DeepCopy() *ConsoleAuthorisers { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ConsoleCondition) DeepCopyInto(out *ConsoleCondition) { + *out = *in + in.LastUpdateTime.DeepCopyInto(&out.LastUpdateTime) + in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConsoleCondition. +func (in *ConsoleCondition) DeepCopy() *ConsoleCondition { + if in == nil { + return nil + } + out := new(ConsoleCondition) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ConsoleList) DeepCopyInto(out *ConsoleList) { *out = *in @@ -271,6 +289,13 @@ func (in *ConsoleStatus) DeepCopyInto(out *ConsoleStatus) { in, out := &in.CompletionTime, &out.CompletionTime *out = (*in).DeepCopy() } + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]ConsoleCondition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConsoleStatus. From 3346b601bb9257287dd98796c924fa6749154459 Mon Sep 17 00:00:00 2001 From: Talal Al-Tamimi Date: Thu, 24 Feb 2022 18:12:24 +0000 Subject: [PATCH 4/4] Generate manifests Run `make manifests` --- ...workloads.crd.gocardless.com_consoles.yaml | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/config/base/crds/workloads.crd.gocardless.com_consoles.yaml b/config/base/crds/workloads.crd.gocardless.com_consoles.yaml index 13f249e7..653b9e9a 100644 --- a/config/base/crds/workloads.crd.gocardless.com_consoles.yaml +++ b/config/base/crds/workloads.crd.gocardless.com_consoles.yaml @@ -93,6 +93,35 @@ spec: description: Time at which the job completed successfully format: date-time type: string + conditions: + items: + description: ConsoleCondition is a status condition for a console + properties: + lastTransitionTime: + description: LastTransitionTime of this condition + format: date-time + type: string + lastUpdateTime: + description: LastUpdateTime of this condition + format: date-time + type: string + message: + description: Message associated with this condition + type: string + reason: + description: Reason for the current status of this condition + type: string + status: + description: Status of this condition + type: string + type: + description: Type of this condition + type: string + required: + - status + - type + type: object + type: array expiryTime: format: date-time type: string