Add archive property, update service controller, disable flightrecord…

…er controller

deploy/crds/rhjmc.redhat.com_recordings_crd.yaml

@@ -31,9 +31,15 @@ spec:
         spec:
           description: RecordingSpec defines the desired state of Recording
           properties:
+            archive:
+              description: Whether this recording should be saved to persistent storage.
+                If true, the JFR file will be retained until this object is deleted.
+                If false, the JFR file will be deleted when its corresponding JVM
+                exits.
+              type: boolean
             duration:
               description: The requested total duration of the recording, a zero value
-                will record indefinitely
+                will record indefinitely.
               type: string
             eventOptions:
               description: 'A list of event options to use when creating the recording.
@@ -43,19 +49,17 @@ spec:
                 type: string
               type: array
             name:
-              description: Name of the recording to be created
+              description: Name of the recording to be created.
               type: string
             state:
               description: Desired state of the recording. If omitted, RUNNING will
-                be assumed TODO Should we not allow CREATED and STOPPING for this?
-                Do they make sense?
+                be assumed.
               enum:
-              - CREATED
               - RUNNING
-              - STOPPING
               - STOPPED
               type: string
           required:
+          - archive
           - duration
           - eventOptions
           - name
@@ -64,17 +68,17 @@ spec:
           description: RecordingStatus defines the observed state of Recording
           properties:
             downloadURL:
-              description: A URL to download the JFR file for the recording
+              description: A URL to download the JFR file for the recording.
               type: string
             duration:
-              description: The duration of the recording specified during creation
+              description: The duration of the recording specified during creation.
               type: string
             startTime:
-              description: The date/time when the recording started
+              description: The date/time when the recording started.
               format: date-time
               type: string
             state:
-              description: Current state of the recording
+              description: Current state of the recording.
               enum:
               - CREATED
               - RUNNING

pkg/apis/rhjmc/v1alpha1/flightrecorder_types.go

@@ -71,6 +71,7 @@ type RecordingInfo struct {
 // +k8s:openapi-gen=true
 // +kubebuilder:subresource:status
 // +kubebuilder:resource:path=flightrecorders,scope=Namespaced
+// +kubebuilder:skipversion
 type FlightRecorder struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`

pkg/apis/rhjmc/v1alpha2/flightrecorder_types.go

@@ -31,6 +31,7 @@ type FlightRecorderStatus struct {
 	// Reference to the pod/service that this object controls JFR for
 	Target *corev1.ObjectReference `json:"target"`
 	// JMX port for target JVM
+	// +kubebuilder:validation:Minimum=0
 	Port int32 `json:"port"`
 	// Recordings that match this selector belong to this FlightRecorder
 	RecordingSelector *metav1.LabelSelector `json:"recordingSelector"`
@@ -59,7 +60,8 @@ type OptionDescriptor struct {
 // +k8s:openapi-gen=true
 // +kubebuilder:subresource:status
 // +kubebuilder:resource:path=flightrecorders,scope=Namespaced
-type FlightRecorder struct {
+// +kubebuilder:storageversion
+type FlightRecorder struct { // TODO Conversion with v1alpha1? Might not be possible due to moving data to Recording
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 

pkg/apis/rhjmc/v1alpha2/recording_types.go

@@ -13,20 +13,23 @@ type RecordingSpec struct {
 	// INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
 	// Important: Run "operator-sdk generate k8s" to regenerate code after modifying this file
 	// Add custom validation using kubebuilder tags: https://book-v1.book.kubebuilder.io/beyond_basics/generating_crd.html
+	// TODO Full validation marker list: https://book.kubebuilder.io/reference/markers/crd-validation.html
 
-	// Name of the recording to be created
+	// Name of the recording to be created.
 	Name string `json:"name"`
 	// A list of event options to use when creating the recording.
 	// These are used to enable and fine-tune individual events.
 	// Examples: "jdk.ExecutionSample:enabled=true", "jdk.ExecutionSample:period=200ms"
 	// +listType=set
 	EventOptions []string `json:"eventOptions"` // TODO Maybe replace with more specific type (e.g. "typeID, option, value" tuples)
-	// The requested total duration of the recording, a zero value will record indefinitely
+	// The requested total duration of the recording, a zero value will record indefinitely.
 	Duration metav1.Duration `json:"duration"`
-	// Desired state of the recording. If omitted, RUNNING will be assumed
-	// TODO Should we not allow CREATED and STOPPING for this? Do they make sense?
-	// +kubebuilder:validation:Enum=CREATED;RUNNING;STOPPING;STOPPED
+	// Desired state of the recording. If omitted, RUNNING will be assumed.
+	// +kubebuilder:validation:Enum=RUNNING;STOPPED
 	State RecordingState `json:"state,omitempty"`
+	// Whether this recording should be saved to persistent storage. If true, the JFR file will be retained until
+	// this object is deleted. If false, the JFR file will be deleted when its corresponding JVM exits.
+	Archive bool `json:"archive"`
 }
 
 // RecordingState describes the current state of the recording according
@@ -35,16 +38,16 @@ type RecordingState string // FIXME From client/command_types.go
 
 const (
 	// RecordingStateCreated means the recording has been accepted, but
-	// has not started yet
+	// has not started yet.
 	RecordingStateCreated RecordingState = "CREATED"
 	// RecordingStateRunning means the recording has started and is
-	// currently running
+	// currently running.
 	RecordingStateRunning RecordingState = "RUNNING"
 	// RecordingStateStopping means that the recording is in the process
-	// of finishing
+	// of finishing.
 	RecordingStateStopping RecordingState = "STOPPING"
 	// RecordingStateStopped means the recording has completed and the
-	// JFR file is fully written
+	// JFR file is fully written.
 	RecordingStateStopped RecordingState = "STOPPED"
 )
 
@@ -55,14 +58,14 @@ type RecordingStatus struct {
 	// Important: Run "operator-sdk generate k8s" to regenerate code after modifying this file
 	// Add custom validation using kubebuilder tags: https://book-v1.book.kubebuilder.io/beyond_basics/generating_crd.html
 
-	// Current state of the recording
+	// Current state of the recording.
 	// +kubebuilder:validation:Enum=CREATED;RUNNING;STOPPING;STOPPED
 	State RecordingState `json:"state"`
-	// The date/time when the recording started
+	// The date/time when the recording started.
 	StartTime metav1.Time `json:"startTime"`
-	// The duration of the recording specified during creation
+	// The duration of the recording specified during creation.
 	Duration metav1.Duration `json:"duration"` // FIXME Needed?
-	// A URL to download the JFR file for the recording
+	// A URL to download the JFR file for the recording.
 	DownloadURL string `json:"downloadURL,omitempty"`
 	// TODO Consider adding Conditions:
 	// https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties

pkg/apis/rhjmc/v1alpha2/zz_generated.openapi.go

@@ -181,7 +181,7 @@ func schema_pkg_apis_rhjmc_v1alpha2_RecordingSpec(ref common.ReferenceCallback)
 				Properties: map[string]spec.Schema{
 					"name": {
 						SchemaProps: spec.SchemaProps{
-							Description: "Name of the recording to be created",
+							Description: "Name of the recording to be created.",
 							Type:        []string{"string"},
 							Format:      "",
 						},
@@ -207,19 +207,26 @@ func schema_pkg_apis_rhjmc_v1alpha2_RecordingSpec(ref common.ReferenceCallback)
 					},
 					"duration": {
 						SchemaProps: spec.SchemaProps{
-							Description: "The requested total duration of the recording, a zero value will record indefinitely",
+							Description: "The requested total duration of the recording, a zero value will record indefinitely.",
 							Ref:         ref("k8s.io/apimachinery/pkg/apis/meta/v1.Duration"),
 						},
 					},
 					"state": {
 						SchemaProps: spec.SchemaProps{
-							Description: "Desired state of the recording. If omitted, RUNNING will be assumed",
+							Description: "Desired state of the recording. If omitted, RUNNING will be assumed.",
 							Type:        []string{"string"},
 							Format:      "",
 						},
 					},
+					"archive": {
+						SchemaProps: spec.SchemaProps{
+							Description: "Whether this recording should be saved to persistent storage. If true, the JFR file will be retained until this object is deleted. If false, the JFR file will be deleted when its corresponding JVM exits.",
+							Type:        []string{"boolean"},
+							Format:      "",
+						},
+					},
 				},
-				Required: []string{"name", "eventOptions", "duration"},
+				Required: []string{"name", "eventOptions", "duration", "archive"},
 			},
 		},
 		Dependencies: []string{
@@ -236,26 +243,26 @@ func schema_pkg_apis_rhjmc_v1alpha2_RecordingStatus(ref common.ReferenceCallback
 				Properties: map[string]spec.Schema{
 					"state": {
 						SchemaProps: spec.SchemaProps{
-							Description: "Current state of the recording",
+							Description: "Current state of the recording.",
 							Type:        []string{"string"},
 							Format:      "",
 						},
 					},
 					"startTime": {
 						SchemaProps: spec.SchemaProps{
-							Description: "The date/time when the recording started",
+							Description: "The date/time when the recording started.",
 							Ref:         ref("k8s.io/apimachinery/pkg/apis/meta/v1.Time"),
 						},
 					},
 					"duration": {
 						SchemaProps: spec.SchemaProps{
-							Description: "The duration of the recording specified during creation",
+							Description: "The duration of the recording specified during creation.",
 							Ref:         ref("k8s.io/apimachinery/pkg/apis/meta/v1.Duration"),
 						},
 					},
 					"downloadURL": {
 						SchemaProps: spec.SchemaProps{
-							Description: "A URL to download the JFR file for the recording",
+							Description: "A URL to download the JFR file for the recording.",
 							Type:        []string{"string"},
 							Format:      "",
 						},

pkg/controller/flightrecorder/flightrecorder_controller.go

@@ -33,7 +33,8 @@ var log = logf.Log.WithName("controller_flightrecorder")
 // Add creates a new FlightRecorder Controller and adds it to the Manager. The Manager will set fields on the Controller
 // and Start it when the Manager is Started.
 func Add(mgr manager.Manager) error {
-	return add(mgr, newReconciler(mgr))
+	//return add(mgr, newReconciler(mgr))
+	return nil // FIXME
 }
 
 // newReconciler returns a new reconcile.Reconciler

pkg/controller/service/service_controller.go

@@ -4,7 +4,7 @@ import (
 	"context"
 	"errors"
 
-	rhjmcv1alpha1 "github.com/rh-jmc-team/container-jfr-operator/pkg/apis/rhjmc/v1alpha1"
+	rhjmcv1alpha2 "github.com/rh-jmc-team/container-jfr-operator/pkg/apis/rhjmc/v1alpha2"
 	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -49,7 +49,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
 	}
 
 	// Watch for changes to secondary resource FlightRecorder and requeue the owner Service
-	err = c.Watch(&source.Kind{Type: &rhjmcv1alpha1.FlightRecorder{}}, &handler.EnqueueRequestForOwner{
+	err = c.Watch(&source.Kind{Type: &rhjmcv1alpha2.FlightRecorder{}}, &handler.EnqueueRequestForOwner{
 		IsController: true,
 		OwnerType:    &corev1.Service{},
 	})
@@ -99,25 +99,26 @@ func (r *ReconcileService) Reconcile(request reconcile.Request) (reconcile.Resul
 	jmxPort, err := getServiceJMXPort(svc)
 	jmxCompatible := err == nil
 
-	// Define a new FlightRecorder object for this service
-	jfr, err := r.newFlightRecorderForService(svc)
-	if err != nil {
-		return reconcile.Result{}, err
-	}
-
-	// Set Service instance as the owner and controller
-	if err := controllerutil.SetControllerReference(svc, jfr, r.scheme); err != nil {
-		return reconcile.Result{}, err
-	}
-
 	// Check if this FlightRecorder already exists
-	found := &rhjmcv1alpha1.FlightRecorder{}
-	err = r.client.Get(ctx, types.NamespacedName{Name: jfr.Name, Namespace: jfr.Namespace}, found)
+	found := &rhjmcv1alpha2.FlightRecorder{}
+	jfrName := svc.Name
+	err = r.client.Get(ctx, types.NamespacedName{Name: jfrName, Namespace: request.Namespace}, found)
 	if err != nil && kerrors.IsNotFound(err) {
 
 		if jmxCompatible {
-			reqLogger.Info("Creating a new FlightRecorder", "Namespace", jfr.Namespace, "Name", jfr.Name)
-			jfr.Spec.Port = jmxPort
+			reqLogger.Info("Creating a new FlightRecorder", "Namespace", request.Namespace, "Name", jfrName)
+
+			// Define a new FlightRecorder object for this service
+			jfr, err := r.newFlightRecorderForService(jfrName, svc, jmxPort)
+			if err != nil {
+				return reconcile.Result{}, err
+			}
+
+			// Set Service instance as the owner and controller
+			if err := controllerutil.SetControllerReference(svc, jfr, r.scheme); err != nil {
+				return reconcile.Result{}, err
+			}
+
 			err = r.client.Create(ctx, jfr)
 			if err != nil {
 				return reconcile.Result{}, err
@@ -141,18 +142,18 @@ func (r *ReconcileService) Reconcile(request reconcile.Request) (reconcile.Resul
 		if !jmxCompatible {
 			// Service was previously JMX-compatible but is not anymore,
 			// so delete its FlightRecorder and do not requeue a new one
-			reqLogger.Info("Deleting dangling FlightRecorder", "Namespace", jfr.Namespace, "Name", jfr.Name)
+			reqLogger.Info("Deleting dangling FlightRecorder", "Namespace", request.Namespace, "Name", jfrName)
 			err = r.client.Delete(ctx, found)
 			if err != nil {
 				return reconcile.Result{}, err
 			}
 			return reconcile.Result{}, nil
 		}
 
-		if found.Spec.Port != jmxPort {
+		if found.Status.Port != jmxPort {
 			// FlightRecorder is incorrect - service was likely modified. Delete outdated FlightRecorder
 			// and requeue creation of corrected one
-			reqLogger.Info("Deleting outdated FlightRecorder", "Namespace", jfr.Namespace, "Name", jfr.Name)
+			reqLogger.Info("Deleting outdated FlightRecorder", "Namespace", request.Namespace, "Name", jfrName)
 			err = r.client.Delete(ctx, found)
 			if err != nil {
 				return reconcile.Result{}, err
@@ -184,30 +185,38 @@ func getServiceJMXPort(svc *corev1.Service) (int32, error) {
 }
 
 // newFlightRecorderForService returns a FlightRecorder with the same name/namespace as the service
-func (r *ReconcileService) newFlightRecorderForService(svc *corev1.Service) (*rhjmcv1alpha1.FlightRecorder, error) {
+func (r *ReconcileService) newFlightRecorderForService(name string, svc *corev1.Service, jmxPort int32) (*rhjmcv1alpha2.FlightRecorder, error) {
+	// Inherit "app" label from service
 	appLabel := svc.Name // Use service name as fallback
 	if label, pres := svc.Labels["app"]; pres {
 		appLabel = label
 	}
 	labels := map[string]string{
 		"app": appLabel,
 	}
+
+	// Add reference to service to this FlightRecorder
 	ref, err := reference.GetReference(r.scheme, svc)
 	if err != nil {
 		return nil, err
 	}
-	return &rhjmcv1alpha1.FlightRecorder{ // TODO should we use OwnerReference for this?
+
+	// Use label selector matching the name of this FlightRecorder
+	selector := &metav1.LabelSelector{}
+	selector = metav1.AddLabelToSelector(selector, rhjmcv1alpha2.RecordingLabel, name)
+
+	return &rhjmcv1alpha2.FlightRecorder{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      svc.Name,
+			Name:      name,
 			Namespace: svc.Namespace,
 			Labels:    labels,
 		},
-		Spec: rhjmcv1alpha1.FlightRecorderSpec{
-			RecordingRequests: []rhjmcv1alpha1.RecordingRequest{},
-		},
-		Status: rhjmcv1alpha1.FlightRecorderStatus{
-			Target:     ref,
-			Recordings: []rhjmcv1alpha1.RecordingInfo{},
+		Spec: rhjmcv1alpha2.FlightRecorderSpec{},
+		Status: rhjmcv1alpha2.FlightRecorderStatus{
+			Events:            []rhjmcv1alpha2.EventInfo{},
+			Target:            ref,
+			Port:              jmxPort,
+			RecordingSelector: selector,
 		},
 	}, nil
 }