Skip to content

Commit

Permalink
feat: [log retention improvements pt. 2] add flag for configuring log…
Browse files Browse the repository at this point in the history
… retention period (#2536)

## Description
Log Retention Improvements:

This PR allows users to configure log retention period with
`--log-retention-period` flag on engine starts/restarts. Default is set
to 1 week.

- [x] PR 1: Introduce `LogFileLayout` and `PerWeekLogFileLayout` and
adjust `LogFileManager` and `LogsDatabaseClient` to use this for
retrieving log file paths
- [x] PR 2: Make retention configurable via a CLI flag
- [ ] PR 3: Implement `PerHourFileLayout` and swap storage format from
`PerWeekFileLayout` to `PerHourFileLayout`
- [ ] PR 4: Make `LogsAggregator` use `LogFileLayout` for determining
storage format and set it to use `PerHourFileLayout`
------------------------------

## Is this change user facing?
NO

## References 
#2443
#2190
#2534
  • Loading branch information
tedim52 authored Aug 29, 2024
1 parent b6aa522 commit 185dc6f
Show file tree
Hide file tree
Showing 17 changed files with 93 additions and 22 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

### Features

* [log retention improvements pt. 1] introduce file layout interface ([#2534](https://github.com/kurtosis-tech/kurtosis/issues/2534)) ([a494278](https://github.com/kurtosis-tech/kurtosis/commit/a4942781038330bd06d6dc3e9bc4c4550d2048dc))
* log retention improvements pt. 1: introduce file layout interface ([#2534](https://github.com/kurtosis-tech/kurtosis/issues/2534)) ([a494278](https://github.com/kurtosis-tech/kurtosis/commit/a4942781038330bd06d6dc3e9bc4c4550d2048dc))
* make kurtosis service logs faster ([#2525](https://github.com/kurtosis-tech/kurtosis/issues/2525)) ([d6b246a](https://github.com/kurtosis-tech/kurtosis/commit/d6b246aebe36d81dfca38e6da7fb3bff8d22e36d))

## [1.0.0](https://github.com/kurtosis-tech/kurtosis/compare/0.90.1...1.0.0) (2024-07-03)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func (cmd *EngineConsumingKurtosisCommand) getSetupFunc() func(context.Context)
kurtosisBackend := engineManager.GetKurtosisBackend()

dontRestartAPIContainers := false
engineClient, closeClientFunc, err := engineManager.StartEngineIdempotentlyWithDefaultVersion(ctx, defaults.DefaultEngineLogLevel, defaults.DefaultEngineEnclavePoolSize, defaults.DefaultGitHubAuthTokenOverride, dontRestartAPIContainers, defaults.DefaultDomain)
engineClient, closeClientFunc, err := engineManager.StartEngineIdempotentlyWithDefaultVersion(ctx, defaults.DefaultEngineLogLevel, defaults.DefaultEngineEnclavePoolSize, defaults.DefaultGitHubAuthTokenOverride, dontRestartAPIContainers, defaults.DefaultDomain, defaults.DefaultLogRetentionPeriod)
if err != nil {
return nil, stacktrace.Propagate(err, "An error occurred creating a new Kurtosis engine client")
}
Expand Down
2 changes: 1 addition & 1 deletion cli/cli/commands/cluster/set/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func run(ctx context.Context, flags *flags.ParsedFlags, args *args.ParsedArgs) e
// TODO - fix the idempotent starter longer term
if engineStatus == engine_manager.EngineStatus_Stopped {
dontRestartAPIContainers := false
_, engineClientCloseFunc, err := engineManagerNewCluster.StartEngineIdempotentlyWithDefaultVersion(ctx, defaults.DefaultEngineLogLevel, defaults.DefaultEngineEnclavePoolSize, defaults.DefaultGitHubAuthTokenOverride, dontRestartAPIContainers, defaults.DefaultDomain)
_, engineClientCloseFunc, err := engineManagerNewCluster.StartEngineIdempotentlyWithDefaultVersion(ctx, defaults.DefaultEngineLogLevel, defaults.DefaultEngineEnclavePoolSize, defaults.DefaultGitHubAuthTokenOverride, dontRestartAPIContainers, defaults.DefaultDomain, defaults.DefaultLogRetentionPeriod)
if err != nil {
return stacktrace.Propagate(err, "Engine could not be started after cluster was updated. Its status can be retrieved "+
"running 'kurtosis %s %s' and it can potentially be started running 'kurtosis %s %s'",
Expand Down
2 changes: 1 addition & 1 deletion cli/cli/commands/enclave/add/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func run(
return stacktrace.Propagate(err, "An error occurred creating an engine manager.")
}

engineClient, closeClientFunc, err := engineManager.StartEngineIdempotentlyWithDefaultVersion(ctx, defaults.DefaultEngineLogLevel, defaults.DefaultEngineEnclavePoolSize, defaults.DefaultGitHubAuthTokenOverride, dontRestartAPIContainers, defaults.DefaultDomain)
engineClient, closeClientFunc, err := engineManager.StartEngineIdempotentlyWithDefaultVersion(ctx, defaults.DefaultEngineLogLevel, defaults.DefaultEngineEnclavePoolSize, defaults.DefaultGitHubAuthTokenOverride, dontRestartAPIContainers, defaults.DefaultDomain, defaults.DefaultLogRetentionPeriod)
if err != nil {
return stacktrace.Propagate(err, "An error occurred creating a new Kurtosis engine client")
}
Expand Down
20 changes: 19 additions & 1 deletion cli/cli/commands/engine/restart/restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"strconv"
"strings"
"time"

"github.com/kurtosis-tech/kurtosis/cli/cli/command_framework/lowlevel"
"github.com/kurtosis-tech/kurtosis/cli/cli/command_framework/lowlevel/args"
Expand All @@ -24,6 +25,7 @@ const (
logLevelFlagKey = "log-level"
enclavePoolSizeFlagKey = "enclave-pool-size"
githubAuthTokenOverrideFlagKey = "github-auth-token"
logRetentionPeriodFlagKey = "log-retention-period"

defaultEngineVersion = ""
restartEngineOnSameVersionIfAnyRunning = false
Expand Down Expand Up @@ -92,6 +94,13 @@ var RestartCmd = &lowlevel.LowlevelKurtosisCommand{
Type: flags.FlagType_String,
Default: defaultDomain,
},
{
Key: logRetentionPeriodFlagKey,
Usage: "The length of time that Kurtosis should keep logs for. Eg. if set to 168h, Kurtosis will remove all logs beyond 1 week. You can specify hours using 'h' however Kurtosis currently only supports setting retention on a weekly basis.",
Shorthand: "",
Type: flags.FlagType_String,
Default: defaults.DefaultLogRetentionPeriod,
},
},
PreValidationAndRunFunc: nil,
RunFunc: run,
Expand Down Expand Up @@ -158,9 +167,18 @@ func run(_ context.Context, flags *flags.ParsedFlags, _ *args.ParsedArgs) error
return stacktrace.Propagate(err, "An error occurred while getting the Kurtosis engine enclave manager UI domain name using the flag with key '%v'; this is a bug in Kurtosis", domainFlagKey)
}

logRetentionPeriodStr, err := flags.GetString(logRetentionPeriodFlagKey)
if err != nil {
return stacktrace.Propagate(err, "An error occurred while getting the log retention period string from flag: '%v'", logRetentionPeriodStr)
}
_, err = time.ParseDuration(logRetentionPeriodStr)
if err != nil {
return stacktrace.Propagate(err, "An error occurred parsing provided log retention period '%v' into a duration. Ensure the provided value has the proper format of hours using 'h'.", logRetentionPeriodStr)
}

var engineClientCloseFunc func() error
var restartEngineErr error
_, engineClientCloseFunc, restartEngineErr = engineManager.RestartEngineIdempotently(ctx, logLevel, engineVersion, restartEngineOnSameVersionIfAnyRunning, enclavePoolSize, shouldStartInDebugMode, githubAuthTokenOverride, shouldRestartAPIContainers, domain)
_, engineClientCloseFunc, restartEngineErr = engineManager.RestartEngineIdempotently(ctx, logLevel, engineVersion, restartEngineOnSameVersionIfAnyRunning, enclavePoolSize, shouldStartInDebugMode, githubAuthTokenOverride, shouldRestartAPIContainers, domain, logRetentionPeriodStr)
if restartEngineErr != nil {
return stacktrace.Propagate(restartEngineErr, "An error occurred restarting the Kurtosis engine")
}
Expand Down
24 changes: 21 additions & 3 deletions cli/cli/commands/engine/start/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"strconv"
"strings"
"time"

"github.com/kurtosis-tech/kurtosis/cli/cli/command_framework/lowlevel"
"github.com/kurtosis-tech/kurtosis/cli/cli/command_framework/lowlevel/args"
Expand All @@ -24,6 +25,7 @@ const (
logLevelFlagKey = "log-level"
enclavePoolSizeFlagKey = "enclave-pool-size"
githubAuthTokenOverrideFlagKey = "github-auth-token"
logRetentionPeriodFlagKey = "log-retention-period"

defaultEngineVersion = ""
kurtosisTechEngineImagePrefix = "kurtosistech/engine"
Expand Down Expand Up @@ -93,6 +95,13 @@ var StartCmd = &lowlevel.LowlevelKurtosisCommand{
Type: flags.FlagType_String,
Default: defaultDomain,
},
{
Key: logRetentionPeriodFlagKey,
Usage: "The length of time that Kurtosis should keep logs for. Eg. if set to 168h, Kurtosis will remove all logs beyond 1 week. You can specify hours using 'h' however Kurtosis currently only supports setting retention on a weekly basis.",
Shorthand: "",
Type: flags.FlagType_String,
Default: defaults.DefaultLogRetentionPeriod,
},
},
PreValidationAndRunFunc: nil,
RunFunc: run,
Expand Down Expand Up @@ -154,16 +163,25 @@ func run(_ context.Context, flags *flags.ParsedFlags, _ *args.ParsedArgs) error
return stacktrace.Propagate(err, "An error occurred while getting the Kurtosis engine enclave manager UI domain name using the flag with key '%v'; this is a bug in Kurtosis", domainFlagKey)
}

logRetentionPeriodStr, err := flags.GetString(logRetentionPeriodFlagKey)
if err != nil {
return stacktrace.Propagate(err, "An error occurred while getting the log retention period string from flag: '%v'", logRetentionPeriodStr)
}
_, err = time.ParseDuration(logRetentionPeriodStr)
if err != nil {
return stacktrace.Propagate(err, "An error occurred parsing provided log retention period '%v' into a duration. Ensure the provided value has the proper format of hours using 'h'.", logRetentionPeriodStr)
}

if engineVersion == defaultEngineVersion && isDebugMode {
engineDebugVersion := fmt.Sprintf("%s-%s", kurtosis_version.KurtosisVersion, defaults.DefaultKurtosisContainerDebugImageNameSuffix)
logrus.Infof("Starting Kurtosis engine in debug mode from image '%v%v%v'...", kurtosisTechEngineImagePrefix, imageVersionDelimiter, engineDebugVersion)
_, engineClientCloseFunc, startEngineErr = engineManager.StartEngineIdempotentlyWithCustomVersion(ctx, engineDebugVersion, logLevel, enclavePoolSize, true, githubAuthTokenOverride, shouldRestartAPIContainers, domain)
_, engineClientCloseFunc, startEngineErr = engineManager.StartEngineIdempotentlyWithCustomVersion(ctx, engineDebugVersion, logLevel, enclavePoolSize, true, githubAuthTokenOverride, shouldRestartAPIContainers, domain, logRetentionPeriodStr)
} else if engineVersion == defaultEngineVersion {
logrus.Infof("Starting Kurtosis engine from image '%v%v%v'...", kurtosisTechEngineImagePrefix, imageVersionDelimiter, kurtosis_version.KurtosisVersion)
_, engineClientCloseFunc, startEngineErr = engineManager.StartEngineIdempotentlyWithDefaultVersion(ctx, logLevel, enclavePoolSize, githubAuthTokenOverride, shouldRestartAPIContainers, domain)
_, engineClientCloseFunc, startEngineErr = engineManager.StartEngineIdempotentlyWithDefaultVersion(ctx, logLevel, enclavePoolSize, githubAuthTokenOverride, shouldRestartAPIContainers, domain, logRetentionPeriodStr)
} else {
logrus.Infof("Starting Kurtosis engine from image '%v%v%v'...", kurtosisTechEngineImagePrefix, imageVersionDelimiter, engineVersion)
_, engineClientCloseFunc, startEngineErr = engineManager.StartEngineIdempotentlyWithCustomVersion(ctx, engineVersion, logLevel, enclavePoolSize, defaults.DefaultEnableDebugMode, githubAuthTokenOverride, shouldRestartAPIContainers, domain)
_, engineClientCloseFunc, startEngineErr = engineManager.StartEngineIdempotentlyWithCustomVersion(ctx, engineVersion, logLevel, enclavePoolSize, defaults.DefaultEnableDebugMode, githubAuthTokenOverride, shouldRestartAPIContainers, domain, logRetentionPeriodStr)
}
if startEngineErr != nil {
return stacktrace.Propagate(startEngineErr, "An error occurred starting the Kurtosis engine")
Expand Down
2 changes: 1 addition & 1 deletion cli/cli/commands/github/login/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func RestartEngineAfterGitHubAuth(ctx context.Context) error {
var engineClientCloseFunc func() error
var restartEngineErr error
dontRestartAPIContainers := false
_, engineClientCloseFunc, restartEngineErr = engineManager.RestartEngineIdempotently(ctx, defaults.DefaultEngineLogLevel, defaultEngineVersion, restartEngineOnSameVersionIfAnyRunning, defaults.DefaultEngineEnclavePoolSize, defaults.DefaultEnableDebugMode, defaults.DefaultGitHubAuthTokenOverride, dontRestartAPIContainers, defaults.DefaultDomain)
_, engineClientCloseFunc, restartEngineErr = engineManager.RestartEngineIdempotently(ctx, defaults.DefaultEngineLogLevel, defaultEngineVersion, restartEngineOnSameVersionIfAnyRunning, defaults.DefaultEngineEnclavePoolSize, defaults.DefaultEnableDebugMode, defaults.DefaultGitHubAuthTokenOverride, dontRestartAPIContainers, defaults.DefaultDomain, defaults.DefaultLogRetentionPeriod)
if restartEngineErr != nil {
return stacktrace.Propagate(restartEngineErr, "An error occurred restarting the Kurtosis engine")
}
Expand Down
2 changes: 1 addition & 1 deletion cli/cli/commands/kurtosis_context/set/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func SetContext(
}

dontRestartAPIContainers := false
_, engineClientCloseFunc, startEngineErr := engineManager.StartEngineIdempotentlyWithDefaultVersion(ctx, logrus.InfoLevel, defaults.DefaultEngineEnclavePoolSize, defaults.DefaultGitHubAuthTokenOverride, dontRestartAPIContainers, defaults.DefaultDomain)
_, engineClientCloseFunc, startEngineErr := engineManager.StartEngineIdempotentlyWithDefaultVersion(ctx, logrus.InfoLevel, defaults.DefaultEngineEnclavePoolSize, defaults.DefaultGitHubAuthTokenOverride, dontRestartAPIContainers, defaults.DefaultDomain, defaults.DefaultLogRetentionPeriod)
if startEngineErr != nil {
logrus.Warnf("The context was successfully set to '%s' but Kurtosis failed to start an engine in "+
"this new context. A new engine should be started manually with '%s %s %s'. The error was:\n%v",
Expand Down
2 changes: 2 additions & 0 deletions cli/cli/defaults/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ const (
DefaultGitHubAuthTokenOverride = ""

DefaultDomain = ""

DefaultLogRetentionPeriod = "168h"
)

var DefaultApiContainerLogLevel = logrus.DebugLevel
Expand Down
9 changes: 9 additions & 0 deletions cli/cli/helpers/engine_manager/engine_existence_guarantor.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ type engineExistenceGuarantor struct {

// Enclave manager UI domain name
domain string

// Length of time Kurtosis will keep logs for
logRetentionPeriod string
}

func newEngineExistenceGuarantorWithDefaultVersion(
Expand All @@ -107,6 +110,7 @@ func newEngineExistenceGuarantorWithDefaultVersion(
githubAuthTokenOverride string,
restartAPIContainers bool,
domain string,
logRetentionPeriod string,
) *engineExistenceGuarantor {
return newEngineExistenceGuarantorWithCustomVersion(
ctx,
Expand All @@ -126,6 +130,7 @@ func newEngineExistenceGuarantorWithDefaultVersion(
githubAuthTokenOverride,
restartAPIContainers,
domain,
logRetentionPeriod,
)
}

Expand All @@ -147,6 +152,7 @@ func newEngineExistenceGuarantorWithCustomVersion(
githubAuthTokenOverride string,
restartAPIContainers bool,
domain string,
logRetentionPeriod string,
) *engineExistenceGuarantor {
return &engineExistenceGuarantor{
ctx: ctx,
Expand All @@ -168,6 +174,7 @@ func newEngineExistenceGuarantorWithCustomVersion(
githubAuthTokenOverride: githubAuthTokenOverride,
restartAPIContainers: restartAPIContainers,
domain: domain,
logRetentionPeriod: logRetentionPeriod,
}
}

Expand Down Expand Up @@ -228,6 +235,7 @@ func (guarantor *engineExistenceGuarantor) VisitStopped() error {
githubAuthToken,
guarantor.restartAPIContainers,
guarantor.domain,
guarantor.logRetentionPeriod,
)
} else {
_, _, engineLaunchErr = guarantor.engineServerLauncher.LaunchWithCustomVersion(
Expand All @@ -249,6 +257,7 @@ func (guarantor *engineExistenceGuarantor) VisitStopped() error {
githubAuthToken,
guarantor.restartAPIContainers,
guarantor.domain,
guarantor.logRetentionPeriod,
)
}
if engineLaunchErr != nil {
Expand Down
11 changes: 7 additions & 4 deletions cli/cli/helpers/engine_manager/engine_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (manager *EngineManager) StartEngineIdempotentlyWithDefaultVersion(
githubAuthTokenOverride string,
restartAPIContainers bool,
domain string,
) (kurtosis_engine_rpc_api_bindings.EngineServiceClient, func() error, error) {
logRetentionPeriodStr string) (kurtosis_engine_rpc_api_bindings.EngineServiceClient, func() error, error) {
status, maybeHostMachinePortBinding, engineVersion, err := manager.GetEngineStatus(ctx)
if err != nil {
return nil, nil, stacktrace.Propagate(err, "An error occurred retrieving the Kurtosis engine status, which is necessary for creating a connection to the engine")
Expand All @@ -207,6 +207,7 @@ func (manager *EngineManager) StartEngineIdempotentlyWithDefaultVersion(
githubAuthTokenOverride,
restartAPIContainers,
domain,
logRetentionPeriodStr,
)
// TODO Need to handle the Kubernetes case, where a gateway needs to be started after the engine is started but
// before we can return an EngineClient
Expand All @@ -227,7 +228,7 @@ func (manager *EngineManager) StartEngineIdempotentlyWithCustomVersion(
githubAuthTokenOverride string,
restartAPIContainers bool,
domain string,
) (kurtosis_engine_rpc_api_bindings.EngineServiceClient, func() error, error) {
logRetentionPeriodStr string) (kurtosis_engine_rpc_api_bindings.EngineServiceClient, func() error, error) {
status, maybeHostMachinePortBinding, engineVersion, err := manager.GetEngineStatus(ctx)
if err != nil {
return nil, nil, stacktrace.Propagate(err, "An error occurred retrieving the Kurtosis engine status, which is necessary for creating a connection to the engine")
Expand All @@ -252,6 +253,7 @@ func (manager *EngineManager) StartEngineIdempotentlyWithCustomVersion(
githubAuthTokenOverride,
restartAPIContainers,
domain,
logRetentionPeriodStr,
)
engineClient, engineClientCloseFunc, err := manager.startEngineWithGuarantor(ctx, status, engineGuarantor)
if err != nil {
Expand Down Expand Up @@ -353,6 +355,7 @@ func (manager *EngineManager) RestartEngineIdempotently(
githubAuthTokenOverride string,
shouldRestartAPIContainers bool,
domain string,
logRetentionPeriodStr string,
) (kurtosis_engine_rpc_api_bindings.EngineServiceClient, func() error, error) {
var versionOfNewEngine string
// We try to do our best to restart an engine on the same version the current on is on
Expand All @@ -378,9 +381,9 @@ func (manager *EngineManager) RestartEngineIdempotently(
var engineClientCloseFunc func() error
var restartEngineErr error
if versionOfNewEngine != defaultEngineVersion {
_, engineClientCloseFunc, restartEngineErr = manager.StartEngineIdempotentlyWithCustomVersion(ctx, versionOfNewEngine, logLevel, poolSize, shouldStartInDebugMode, githubAuthTokenOverride, shouldRestartAPIContainers, domain)
_, engineClientCloseFunc, restartEngineErr = manager.StartEngineIdempotentlyWithCustomVersion(ctx, versionOfNewEngine, logLevel, poolSize, shouldStartInDebugMode, githubAuthTokenOverride, shouldRestartAPIContainers, domain, logRetentionPeriodStr)
} else {
_, engineClientCloseFunc, restartEngineErr = manager.StartEngineIdempotentlyWithDefaultVersion(ctx, logLevel, poolSize, githubAuthTokenOverride, shouldRestartAPIContainers, domain)
_, engineClientCloseFunc, restartEngineErr = manager.StartEngineIdempotentlyWithDefaultVersion(ctx, logLevel, poolSize, githubAuthTokenOverride, shouldRestartAPIContainers, domain, logRetentionPeriodStr)
}
if restartEngineErr != nil {
return nil, nil, stacktrace.Propagate(restartEngineErr, "An error occurred starting a new engine")
Expand Down
2 changes: 2 additions & 0 deletions docs/docs/cli-reference/engine-restart.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,7 @@ You may optionally pass in the following flags with this command:
* `--log-level`: The level that the started engine should log at. Options include: `panic`, `fatal`, `error`, `warning`, `info`, `debug`, or `trace`. The engine logs at the `info` level by default.
* `--version`: The version (Docker tag) of the Kurtosis engine that should be started. If not set, the engine will start up with the default version.
* `--enclave-pool-size`: The size of the Kurtosis engine enclave pool. The enclave pool is a component of the Kurtosis engine that allows us to create and maintain 'n' number of idle enclaves for future use. This functionality allows to improve the performance for each new creation enclave request.
* `--github-auth-token`: The auth token to use for authorizing GitHub operations. If set, this will override the currently logged in GitHub user from `kurtosis github login`, if one exists. Note, this token does not persist when restarting the engine.
* `--log-retention-period`: The duration in which Kurtosis engine will keep logs for. The engine will remove any logs beyond this period. You can specify hours using `h`. The default is set to 1 week (168h). NOTE: Currently, Kurtosis only supports setting retention on weekly intervals. Ongoing work is occurring to make this interval more granular - see https://github.com/kurtosis-tech/kurtosis/pull/2534

CAUTION: The `--enclave-pool-size` flag is only available for Kubernetes.
Loading

0 comments on commit 185dc6f

Please sign in to comment.