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: disable data store deletion on both CLI and Backend #3464

Merged
merged 4 commits into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/pull-request.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ jobs:
- tracetest-jaeger
- tracetest-opensearch
- tracetest-tempo
- tracetest-no-tracing
- tracetest-provisioning-env
- tracetest-signoz
steps:
Expand Down
1 change: 0 additions & 1 deletion cli/cmd/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ var (
return nil
},
}),
resourcemanager.WithDeleteSuccessMessage("DataStore removed. Defaulting back to no-tracing mode"),
resourcemanager.WithResourceType("DataStore"),
),
).
Expand Down
36 changes: 0 additions & 36 deletions examples/tracetest-no-tracing/docker-compose.yml

This file was deleted.

17 changes: 0 additions & 17 deletions examples/tracetest-no-tracing/tests/list-tests.yaml

This file was deleted.

7 changes: 0 additions & 7 deletions examples/tracetest-no-tracing/tracetest-config.yaml

This file was deleted.

1 change: 1 addition & 0 deletions server/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@ func registerDataStoreResource(repository *datastore.Repository, router *mux.Rou
datastore.ResourceNamePlural,
repository,
resourcemanager.WithTracer(tracer),
resourcemanager.DisableDelete(),
)
manager.RegisterRoutes(router)
provisioner.AddResourceProvisioner(manager)
Expand Down
35 changes: 11 additions & 24 deletions server/datastore/datastore_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ func (r *Repository) SetID(dataStore DataStore, id id.ID) DataStore {

const DataStoreSingleID id.ID = "current"

var defaultDataStore = DataStore{
ID: DataStoreSingleID,
Name: "OTLP",
Type: DataStoreTypeOTLP,
Default: true,
Values: DataStoreValues{},
}

const insertQuery = `
INSERT INTO data_stores (
"id",
Expand Down Expand Up @@ -123,27 +131,6 @@ func (r *Repository) Update(ctx context.Context, dataStore DataStore) (DataStore
return dataStore, nil
}

func (r *Repository) Delete(ctx context.Context, id id.ID) error {
tx, err := r.db.BeginTx(ctx, nil)
if err != nil {
return err
}
defer tx.Rollback()

query, params := sqlutil.Tenant(ctx, deleteQuery, id)
_, err = tx.ExecContext(ctx, query, params...)
if err != nil {
return fmt.Errorf("datastore repository sql exec delete: %w", err)
}

err = tx.Commit()
if err != nil {
return fmt.Errorf("commit: %w", err)
}

return nil
}

const getQuery = `
SELECT
"id",
Expand All @@ -170,9 +157,9 @@ func (r *Repository) Get(ctx context.Context, id id.ID) (DataStore, error) {

dataStore, err := r.readRow(row)
if err != nil && errors.Is(err, sql.ErrNoRows) {
return DataStore{
CreatedAt: newCreateAtDateString(),
}, nil // Assumes an empty datastore
dataStore := defaultDataStore
dataStore.CreatedAt = newCreateAtDateString()
return dataStore, nil // Assumes default datastore
}
if err != nil {
return DataStore{}, fmt.Errorf("datastore repository get sql query: %w", err)
Expand Down
2 changes: 1 addition & 1 deletion server/datastore/datastore_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ var (
excludedOperations = rmtests.ExcludeOperations(
rmtests.OperationUpdateNotFound,
rmtests.OperationGetNotFound,
rmtests.OperationDeleteNotFound,
rmtests.OperationListSortSuccess,
rmtests.OperationListNoResults,
)
Expand Down Expand Up @@ -45,6 +44,7 @@ func registerManagerFn(router *mux.Router, db *sql.DB) resourcemanager.Manager {
datastore.ResourceNamePlural,
dataStoreRepository,
resourcemanager.WithIDGen(id.GenerateID),
resourcemanager.DisableDelete(),
)
manager.RegisterRoutes(router)

Expand Down
1 change: 1 addition & 0 deletions server/provisioning/provisioning_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ func setup(db *sql.DB) provisioningFixture {
datastore.ResourceName,
datastore.ResourceNamePlural,
f.dataStores,
resourcemanager.DisableDelete(),
)

f.provisioner = provisioning.New(provisioning.WithResourceProvisioners(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,18 +42,8 @@ func TestDeleteDatastore(t *testing.T) {
require.True(dataStore.Spec.Default)

// When I try to delete the datastore
// Then it should delete with success
// Then it should return a error message, showing that we cannot delete a datastore
result = tracetestcli.Exec(t, "delete datastore --id current", tracetestcli.WithCLIConfig(cliConfig))
helpers.RequireExitCodeEqual(t, result, 0)
require.Contains(result.StdOut, "DataStore removed. Defaulting back to no-tracing mode")

// When I try to get a datastore again
// Then it should return an empty datastore
result = tracetestcli.Exec(t, "get datastore --id current", tracetestcli.WithCLIConfig(cliConfig))
// TODO: we haven't defined a valid output to tell to the user that we are on `no-tracing mode`
helpers.RequireExitCodeEqual(t, result, 0)

dataStore = helpers.UnmarshalYAML[types.DataStoreResource](t, result.StdOut)
require.Equal("DataStore", dataStore.Type)
require.False(dataStore.Spec.Default)
helpers.RequireExitCodeEqual(t, result, 1)
require.Contains(result.StdErr, "resource DataStore does not support the action")
}
13 changes: 10 additions & 3 deletions testing/cli-e2etest/testscenarios/datastore/list_datastore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,17 @@ func TestListDatastore(t *testing.T) {
// And I have my server recently created

// When I try to list datastore on pretty mode and there is no datastore
// Then it should print an empty table
// Then it should list the default datastore
result := tracetestcli.Exec(t, "list datastore --output pretty", tracetestcli.WithCLIConfig(cliConfig))
helpers.RequireExitCodeEqual(t, result, 0)
require.NotContains(result.StdOut, "current")

parsedTable := helpers.UnmarshalTable(t, result.StdOut)
require.Len(parsedTable, 1)

singleLine := parsedTable[0]

require.Equal("current", singleLine["ID"])
require.Equal("OTLP", singleLine["NAME"])
require.Equal("*", singleLine["DEFAULT"])
})

addListDatastorePreReqs(t, env)
Expand Down