Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(recordings): remove 'transient' active recordings handling #858

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Mar 21, 2025

Welcome to Cryostat! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits using a GPG signature

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main


Related to #269

Description of the change:

Removes old, disabled-by-default implementation of 'transient' active recordings.

Motivation for the change:

The original idea here was that active recording streams could always be transformed into archived recordings by simply archiving the active source recording, then using the archived recording in its place.

This was originally intended to allow us to restructure internal implementations to only need to deal with archived recordings URLs from object storage, and not active recording input streams. However, this runs into problems where the transient files may never be cleaned from object storage and pollute storage, or may only be cleaned on a long periodic schedule. This would also mean that any operations on active recordings could never be performed without object storage being both configured and currently available - storage outages would block almost any Cryostat activity.

The plan here is to instead use #269 to handle archived recordings as object storage URLs, and active recordings always as direct "piped" streams between components.

How to manually test:

  1. Run CRYOSTAT_IMAGE=quay.io... bash smoketest.bash...
  2. ...

@andrewazores andrewazores added chore Refactor, rename, cleanup, etc. safe-to-test labels Mar 21, 2025
@andrewazores
Copy link
Member Author

/build_test

Copy link

Workflow started at 3/21/2025, 3:21:41 PM. View Actions Run.

Copy link

OpenAPI schema change detected:

diff --git a/schema/openapi.yaml b/schema/openapi.yaml
index 635d739..a8f837d 100644
--- a/schema/openapi.yaml
+++ b/schema/openapi.yaml
@@ -287,24 +287,20 @@ components:
     HttpServerResponse:
       properties:
         chunked:
           type: boolean
         statusCode:
           format: int32
           type: integer
         statusMessage:
           type: string
       type: object
-    Instant:
-      example: 2022-03-10T16:15:50Z
-      format: date-time
-      type: string
     JsonObject:
       items:
         properties:
           key:
             type: string
           value: {}
         type: object
       type: array
     LinkedRecordingDescriptor:
       properties:
@@ -365,22 +361,20 @@ components:
         id:
           format: int64
           type: integer
         targets:
           items:
             $ref: '#/components/schemas/Target'
           type: array
       type: object
     Metadata:
       properties:
-        expiry:
-          $ref: '#/components/schemas/Instant'
         labels:
           additionalProperties:
             type: string
           type: object
       type: object
     MethodParameter:
       properties:
         contentType:
           $ref: '#/components/schemas/ContentType'
         converter:

Copy link

GraphQL schema change detected:

diff --git a/schema/schema.graphql b/schema/schema.graphql
index 0ee8fa1..ff79cc7 100644
--- a/schema/schema.graphql
+++ b/schema/schema.graphql
@@ -103,22 +103,20 @@ type MemoryMetrics {
 }
 
 type MemoryUtilization {
   committed: BigInteger!
   init: BigInteger!
   max: BigInteger!
   used: BigInteger!
 }
 
 type Metadata {
-  "ISO-8601"
-  expiry: DateTime
   labels(
     "Get entry/entries for a certain key/s"
     key: [String]
   ): [Entry_String_String]
 }
 
 "Mutation root"
 type Mutation {
   "Archive an existing Flight Recording matching the given filter, on all Targets under the subtrees of the discovery nodes matching the given filter"
   archiveRecording(nodes: DiscoveryNodeFilterInput!, recordings: ActiveRecordingsFilterInput): [ArchivedRecording]

Copy link

Schema changes committed by the CI.

Copy link

CI build and push: All tests pass ✅
https://github.com/cryostatio/cryostat/actions/runs/13999347978

@andrewazores andrewazores requested a review from a team March 21, 2025 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Refactor, rename, cleanup, etc. safe-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant