Skip to content

Commit

Permalink
fix: Add retries around the test enclave destroy logic (#2255)
Browse files Browse the repository at this point in the history
## Description:
This change is part of debugging this
[issue](#2247).

## Is this change user facing?
NO

## References (if applicable):
#2247
  • Loading branch information
laurentluce committed Mar 5, 2024
1 parent 2a60e4f commit 6245ddf
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 23 deletions.
34 changes: 17 additions & 17 deletions enclave-manager/web/README.md
Expand Up @@ -85,23 +85,23 @@ You don’t have to ever use `eject`. The curated feature set is suitable for sm

### Key libraries used

* Typescript
* React
* react-scripts - these packages were created with Create React App.
* React router (v6) - we avoid using route actions and loaders as they didn't seem to have good Typescript support, created codepaths that were tricky to follow (by excessively fragmenting code) and generally seemed to increase complexity rather than improve development velocity.
* Chakra UI
* TanStack table - used for managing the state of tables throughout the application
* React hook form - used for forms where any validation is required. Generally the [Smart Form Component pattern](https://react-hook-form.com/advanced-usage#SmartFormComponent) is followed to create form components that can be composed together arbitrarily.
* Reactflow - used for the graph in the enclave builder.
* Virtuoso - used for rendering streams of logs without dumping all of the text into the DOM.
* Monaco - used anywhere we want to display blocks of code, or for editing code in the browser.
* connect-es - this is used to create the client used for accessing kurtosis enclave manager and package indexer api's.
- Typescript
- React
- react-scripts - these packages were created with Create React App.
- React router (v6) - we avoid using route actions and loaders as they didn't seem to have good Typescript support, created codepaths that were tricky to follow (by excessively fragmenting code) and generally seemed to increase complexity rather than improve development velocity.
- Chakra UI
- TanStack table - used for managing the state of tables throughout the application
- React hook form - used for forms where any validation is required. Generally the [Smart Form Component pattern](https://react-hook-form.com/advanced-usage#SmartFormComponent) is followed to create form components that can be composed together arbitrarily.
- Reactflow - used for the graph in the enclave builder.
- Virtuoso - used for rendering streams of logs without dumping all of the text into the DOM.
- Monaco - used anywhere we want to display blocks of code, or for editing code in the browser.
- connect-es - this is used to create the client used for accessing kurtosis enclave manager and package indexer api's.

### Gotchas

* When you first start building this codebase you need to have a local build of the `enclave-manager-sdk`. This is because the `enclave-manager-sdk` is not a published library. The easiest way to make sure this is in place is to run `scripts/build.sh` from the root of this repository, or `enclave-manager/api/typescript/scripts/build.sh`.
* When you run cypress tests locally the tests are running against the app served by your local engine, not the one running from your development server. An easy way to temporarily work around this is to modify the port used from `9711` to `4000` in `cypress/supports/commands.ts`
* The wrapping library we use (`@monaco/react`) is a handy abstraction around Monaco, but using plugins with it seems very challenging without ejecting unfortunately.
* We use `react-mentions` for mention inputs in the enclave builder. This works fine, but better autocomplete could be built using Monaco.
* The enclave builder shows a graph of nested forms - because rendering it can be quite expensive, sensible use of `memo` around functional components in this part of the codebase can be particularly important. The enclave builder has two data models - one is the VariableContext which contains all of the form state across the graph, and provides the universe of available variables to mention in inputs; the other is the reactflow internal state which tracks node positions, types and sizes on the graph. The data models are linked by using the same `id` to refer to nodes.
* There is an issue with stream connections to the backend where they can be unexpectedly disconnected in some browsers, it's unclear what is causing this, but from [this bug](https://github.com/connectrpc/connect-es/issues/907) it seems like it could be in the kurtosis enclave manager server.
- When you first start building this codebase you need to have a local build of the `enclave-manager-sdk`. This is because the `enclave-manager-sdk` is not a published library. The easiest way to make sure this is in place is to run `scripts/build.sh` from the root of this repository, or `enclave-manager/api/typescript/scripts/build.sh`.
- When you run cypress tests locally the tests are running against the app served by your local engine, not the one running from your development server. An easy way to temporarily work around this is to modify the port used from `9711` to `4000` in `cypress/supports/commands.ts`
- The wrapping library we use (`@monaco/react`) is a handy abstraction around Monaco, but using plugins with it seems very challenging without ejecting unfortunately.
- We use `react-mentions` for mention inputs in the enclave builder. This works fine, but better autocomplete could be built using Monaco.
- The enclave builder shows a graph of nested forms - because rendering it can be quite expensive, sensible use of `memo` around functional components in this part of the codebase can be particularly important. The enclave builder has two data models - one is the VariableContext which contains all of the form state across the graph, and provides the universe of available variables to mention in inputs; the other is the reactflow internal state which tracks node positions, types and sizes on the graph. The data models are linked by using the same `id` to refer to nodes.
- There is an issue with stream connections to the backend where they can be unexpectedly disconnected in some browsers, it's unclear what is causing this, but from [this bug](https://github.com/connectrpc/connect-es/issues/907) it seems like it could be in the kurtosis enclave manager server.
26 changes: 20 additions & 6 deletions internal_testsuites/golang/test_helpers/enclave_setup.go
Expand Up @@ -3,16 +3,21 @@ package test_helpers
import (
"context"
"fmt"
"testing"
"time"

"github.com/kurtosis-tech/kurtosis/api/golang/core/lib/enclaves"
"github.com/kurtosis-tech/kurtosis/api/golang/util"
"github.com/kurtosis-tech/stacktrace"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
"testing"
"time"
)

const (
testsuiteNameEnclaveIDFragment = "go-test"

destroyEnclaveRetries = 3
destroyEnclaveRetriesDelayMilliseconds = 1000
)

func CreateEnclave(t *testing.T, ctx context.Context, testName string) (resultEnclaveCtx *enclaves.EnclaveContext, resultStopEnclaveFunc func(), resultDestroyEnclaveFunc func() error, resultErr error) {
Expand All @@ -32,10 +37,19 @@ func CreateEnclave(t *testing.T, ctx context.Context, testName string) (resultEn

}
destroyEnclaveFuncWrapped := func() error {
if err := destroyEnclaveFunc(); err != nil {
logrus.Errorf("An error occurred destroying enclave '%v' that we created for this test:\n%v", enclaveName, err)
logrus.Errorf("ACTION REQUIRED: You'll need to destroy enclave '%v' manually!!!!", enclaveName)
return err
for i := 0; i < destroyEnclaveRetries; i++ {
if err := destroyEnclaveFunc(); err != nil {
logrus.Warnf("An error occurred destroying enclave '%v' that we created for this test:\n%v", enclaveName, err)
if i == destroyEnclaveRetries-1 {
logrus.Errorf("An error occurred destroying enclave '%v' that we created for this test:\n%v", enclaveName, err)
logrus.Errorf("ACTION REQUIRED: You'll need to destroy enclave '%v' manually!!!!", enclaveName)
return stacktrace.NewError("An error occurred after trying to destroy the enclave '%v' %d times", enclaveName, destroyEnclaveRetries)
}
logrus.Warnf("Retrying %d more time(s)", destroyEnclaveRetries-i-1)
time.Sleep(time.Duration(destroyEnclaveRetriesDelayMilliseconds) * time.Millisecond)
} else {
break
}
}
return nil
}
Expand Down

0 comments on commit 6245ddf

Please sign in to comment.