diff --git a/pkg/tests/apis/provisioning/deletejob_test.go b/pkg/tests/apis/provisioning/deletejob_test.go index 5a55c75c4cc..85f3d97da13 100644 --- a/pkg/tests/apis/provisioning/deletejob_test.go +++ b/pkg/tests/apis/provisioning/deletejob_test.go @@ -6,7 +6,6 @@ import ( "path/filepath" "strings" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -26,31 +25,21 @@ func TestIntegrationProvisioning_DeleteJob(t *testing.T) { ctx := context.Background() const repo = "delete-job-test-repo" - localTmp := helper.RenderObject(t, "testdata/local-write.json.tmpl", map[string]any{ - "Name": repo, - "SyncEnabled": true, - "SyncTarget": "instance", - }) - _, err := helper.Repositories.Resource.Create(ctx, localTmp, metav1.CreateOptions{}) - require.NoError(t, err) - // Copy multiple test files to the repository - helper.CopyToProvisioningPath(t, "testdata/all-panels.json", "dashboard1.json") - helper.CopyToProvisioningPath(t, "testdata/text-options.json", "dashboard2.json") - helper.CopyToProvisioningPath(t, "testdata/timeline-demo.json", "folder/dashboard3.json") + testRepo := TestRepo{ + Name: repo, + Copies: map[string]string{ + "testdata/all-panels.json": "dashboard1.json", + "testdata/text-options.json": "dashboard2.json", + "testdata/timeline-demo.json": "folder/dashboard3.json", + }, + ExpectedDashboards: 3, + ExpectedFolders: 1, + } - // Trigger and wait for initial sync to populate resources - helper.SyncAndWait(t, repo, nil) - - // Verify initial state - should have 3 dashboards and 1 folder - dashboards, err := helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) - require.NoError(t, err) - require.Equal(t, 3, len(dashboards.Items), "should have 3 dashboards after sync") - - folders, err := helper.Folders.Resource.List(ctx, metav1.ListOptions{}) - require.NoError(t, err) - require.Equal(t, 1, len(folders.Items), "should have 1 folder after sync") + helper.CreateRepo(t, testRepo) t.Run("delete single file", func(t *testing.T) { + // FIXME: make the tests in a way that we can simply have a spec and some expectations per scenario. spec := provisioning.JobSpec{ Action: provisioning.JobActionDelete, Delete: &provisioning.DeleteJobOptions{ @@ -58,15 +47,18 @@ func TestIntegrationProvisioning_DeleteJob(t *testing.T) { }, } // Create delete job for single file - helper.TriggerJobAndWait(t, repo, spec) + helper.TriggerJobAndWaitForSuccess(t, repo, spec) + + // FIXME: create a helper to verify repository files // Verify file is deleted from repository - _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "dashboard1.json") + _, err := helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "dashboard1.json") require.Error(t, err, "file should be deleted from repository") require.True(t, apierrors.IsNotFound(err), "should be not found error") + // FIXME: create a helper to verify grafana resources // Verify dashboard is removed from Grafana after sync - dashboards, err = helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) + dashboards, err := helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) require.NoError(t, err) require.Equal(t, 2, len(dashboards.Items), "should have 2 dashboards after delete") @@ -84,10 +76,11 @@ func TestIntegrationProvisioning_DeleteJob(t *testing.T) { Paths: []string{"dashboard2.json", "folder/dashboard3.json"}, }, } - helper.TriggerJobAndWait(t, repo, spec) + helper.TriggerJobAndWaitForSuccess(t, repo, spec) + // FIXME: use helper // Verify files are deleted from repository - _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "dashboard2.json") + _, err := helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "dashboard2.json") require.Error(t, err, "dashboard2.json should be deleted") require.True(t, apierrors.IsNotFound(err)) @@ -96,18 +89,20 @@ func TestIntegrationProvisioning_DeleteJob(t *testing.T) { require.True(t, apierrors.IsNotFound(err)) // Verify all dashboards are removed from Grafana after sync - dashboards, err = helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) + dashboards, err := helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) require.NoError(t, err) require.Equal(t, 0, len(dashboards.Items), "should have 0 dashboards after deleting all") }) t.Run("delete by resource reference", func(t *testing.T) { + // FIXME: do not create this on top of the other one. Isolate the cases // Create modified test files with unique UIDs for ResourceRef testing // Read and modify the testdata files to have unique UIDs that don't conflict with existing resources allPanelsContent := helper.LoadFile("testdata/all-panels.json") textOptionsContent := helper.LoadFile("testdata/text-options.json") timelineDemoContent := helper.LoadFile("testdata/timeline-demo.json") + // FIXME: use generic objects // Modify UIDs to be unique for ResourceRef tests allPanelsModified := strings.Replace(string(allPanelsContent), `"uid": "n1jR8vnnz"`, `"uid": "resourceref1"`, 1) textOptionsModified := strings.Replace(string(textOptionsContent), `"uid": "WZ7AhQiVz"`, `"uid": "resourceref2"`, 1) @@ -155,8 +150,9 @@ func TestIntegrationProvisioning_DeleteJob(t *testing.T) { }, } - helper.TriggerJobAndWait(t, repo, spec) + helper.TriggerJobAndWaitForSuccess(t, repo, spec) + // FIXME: use helpers // Verify corresponding file is deleted from repository _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "resource-test-1.json") require.Error(t, err, "file should be deleted from repository") @@ -192,8 +188,9 @@ func TestIntegrationProvisioning_DeleteJob(t *testing.T) { }, }, } - helper.TriggerJobAndWait(t, repo, spec) + helper.TriggerJobAndWaitForSuccess(t, repo, spec) + // FIXME: use helpers // Verify both dashboards are removed from Grafana _, err = helper.DashboardsV1.Resource.Get(ctx, "resourceref2", metav1.GetOptions{}) require.Error(t, err, "text-options dashboard should be deleted") @@ -217,6 +214,7 @@ func TestIntegrationProvisioning_DeleteJob(t *testing.T) { }) t.Run("mixed deletion - paths and resources", func(t *testing.T) { + // FIXME: do not build this case on top of the other one. Isolate the cases // Setup fresh resources for mixed test - reuse the modified content with unique UIDs tmpMixed1 := filepath.Join(tmpDir, "mixed-test-1.json") tmpMixed2 := filepath.Join(tmpDir, "mixed-test-2.json") @@ -246,8 +244,9 @@ func TestIntegrationProvisioning_DeleteJob(t *testing.T) { }, } - helper.TriggerJobAndWait(t, repo, spec) + helper.TriggerJobAndWaitForSuccess(t, repo, spec) + // FIXME: use the helpers // Verify both targeted resources are deleted from Grafana _, err = helper.DashboardsV1.Resource.Get(ctx, "resourceref1", metav1.GetOptions{}) require.Error(t, err, "dashboard deleted by path should be removed") @@ -271,6 +270,7 @@ func TestIntegrationProvisioning_DeleteJob(t *testing.T) { }) t.Run("delete folder by resource reference", func(t *testing.T) { + // FIXME: do not build this case on top of the previous one. Isolate them // Create a dashboard inside a folder to automatically create the folder structure // This follows the same pattern as other tests in this file testDashboard := strings.Replace(string(allPanelsContent), `"uid": "n1jR8vnnz"`, `"uid": "folder-dash"`, 1) @@ -316,8 +316,9 @@ func TestIntegrationProvisioning_DeleteJob(t *testing.T) { }, }, } - helper.TriggerJobAndWait(t, repo, spec) + helper.TriggerJobAndWaitForSuccess(t, repo, spec) + // FIXME: use helpers // Verify folder is deleted from Grafana _, err = helper.Folders.Resource.Get(ctx, testFolderName, metav1.GetOptions{}) require.Error(t, err, "folder should be deleted from Grafana") @@ -331,114 +332,35 @@ func TestIntegrationProvisioning_DeleteJob(t *testing.T) { t.Run("delete non-existent resource by reference", func(t *testing.T) { // Create delete job for non-existent resource - result := helper.AdminREST.Post(). - Namespace("default"). - Resource("repositories"). - Name(repo). - SubResource("jobs"). - Body(asJSON(&provisioning.JobSpec{ - Action: provisioning.JobActionDelete, - Delete: &provisioning.DeleteJobOptions{ - Resources: []provisioning.ResourceRef{ - { - Name: "non-existent-uid", - Kind: "Dashboard", - Group: "dashboard.grafana.app", - }, + spec := provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Delete: &provisioning.DeleteJobOptions{ + Resources: []provisioning.ResourceRef{ + { + Name: "non-existent-uid", + Kind: "Dashboard", + Group: "dashboard.grafana.app", }, }, - })). - SetHeader("Content-Type", "application/json"). - Do(ctx) - require.NoError(t, result.Error(), "should be able to create delete job") + }, + } - // TODO: Simply all this - // Wait for job to complete - should record error but continue - require.EventuallyWithT(t, func(collect *assert.CollectT) { - list := &unstructured.UnstructuredList{} - err := helper.AdminREST.Get(). - Namespace("default"). - Resource("repositories"). - Name(repo). - SubResource("jobs"). - Do(ctx).Into(list) - assert.NoError(collect, err, "should be able to list jobs") - assert.NotEmpty(collect, list.Items, "expect at least one job") - - // Find the most recent delete job - var deleteJob *unstructured.Unstructured - for _, elem := range list.Items { - assert.Equal(collect, repo, elem.GetLabels()["provisioning.grafana.app/repository"], "should have repo label") - - action := mustNestedString(elem.Object, "spec", "action") - if action == "delete" { - // Get the most recent one (they should be ordered by creation time) - deleteJob = &elem - } - } - if !assert.NotNil(collect, deleteJob, "should find a delete job") { - return - } - - state := mustNestedString(deleteJob.Object, "status", "state") - // The job should complete but record errors for individual resource resolution failures - if state == "error" || state == "completed" || state == "success" { - // Any of these states is acceptable - the key is that resource resolution errors are recorded - // and don't fail the entire job due to error-tolerant implementation - return - } - assert.Fail(collect, "job should complete or error, but got state: %s", state) - }, time.Second*10, time.Millisecond*100, "Expected delete job to handle non-existent resource") + job := helper.TriggerJobAndWaitForComplete(t, repo, spec) + state := mustNestedString(job.Object, "status", "state") + assert.Equal(t, "error", state, "delete job should have failed due to non-existent file") }) - - // Repository cleanup is handled by the main test function }) t.Run("delete non-existent file", func(t *testing.T) { - // Create delete job for non-existent file - result := helper.AdminREST.Post(). - Namespace("default"). - Resource("repositories"). - Name(repo). - SubResource("jobs"). - Body(asJSON(&provisioning.JobSpec{ - Action: provisioning.JobActionDelete, - Delete: &provisioning.DeleteJobOptions{ - Paths: []string{"non-existent.json"}, - }, - })). - SetHeader("Content-Type", "application/json"). - Do(ctx) - require.NoError(t, result.Error(), "should be able to create delete job") + spec := provisioning.JobSpec{ + Action: provisioning.JobActionDelete, + Delete: &provisioning.DeleteJobOptions{ + Paths: []string{"non-existent.json"}, + }, + } - // TODO: Simplify this - // Wait for job to complete - should fail due to strict error handling - require.EventuallyWithT(t, func(collect *assert.CollectT) { - list := &unstructured.UnstructuredList{} - err := helper.AdminREST.Get(). - Namespace("default"). - Resource("repositories"). - Name(repo). - SubResource("jobs"). - Do(ctx).Into(list) - assert.NoError(collect, err, "should be able to list jobs") - assert.NotEmpty(collect, list.Items, "expect at least one job") - - // Find the delete job specifically - var deleteJob *unstructured.Unstructured - for _, elem := range list.Items { - assert.Equal(collect, repo, elem.GetLabels()["provisioning.grafana.app/repository"], "should have repo label") - - action := mustNestedString(elem.Object, "spec", "action") - if action == "delete" { - deleteJob = &elem - break - } - } - assert.NotNil(collect, deleteJob, "should find a delete job") - - state := mustNestedString(deleteJob.Object, "status", "state") - assert.Equal(collect, "error", state, "delete job should have failed due to non-existent file") - }, time.Second*10, time.Millisecond*100, "Expected delete job to fail with error state") + job := helper.TriggerJobAndWaitForComplete(t, repo, spec) + state := mustNestedString(job.Object, "status", "state") + assert.Equal(t, "error", state, "delete job should have failed due to non-existent file") }) } diff --git a/pkg/tests/apis/provisioning/exportjob_test.go b/pkg/tests/apis/provisioning/exportjob_test.go index 16988740b1b..25c909f6609 100644 --- a/pkg/tests/apis/provisioning/exportjob_test.go +++ b/pkg/tests/apis/provisioning/exportjob_test.go @@ -28,6 +28,7 @@ func TestProvisioning_ExportUnifiedToRepository(t *testing.T) { _, err := helper.DashboardsV0.Resource.Create(ctx, dashboard, metav1.CreateOptions{}) require.NoError(t, err, "should be able to create v0 dashboard") + // FIXME: add helper and template for dashboards in different versions dashboard = helper.LoadYAMLOrJSONFile("exportunifiedtorepository/dashboard-test-v1.yaml") _, err = helper.DashboardsV1.Resource.Create(ctx, dashboard, metav1.CreateOptions{}) require.NoError(t, err, "should be able to create v1 dashboard") @@ -42,6 +43,7 @@ func TestProvisioning_ExportUnifiedToRepository(t *testing.T) { // Now for the repository. const repo = "local-repository" + // FIXME: use the same helper to create the repository createBody := helper.RenderObject(t, "exportunifiedtorepository/repository.json.tmpl", map[string]any{"Name": repo}) _, err = helper.Repositories.Resource.Create(ctx, createBody, metav1.CreateOptions{}) require.NoError(t, err, "should be able to create repository") @@ -53,7 +55,7 @@ func TestProvisioning_ExportUnifiedToRepository(t *testing.T) { Path: "", // no prefix necessary for testing }, } - helper.TriggerJobAndWait(t, repo, spec) + helper.TriggerJobAndWaitForSuccess(t, repo, spec) type props struct { title string @@ -103,6 +105,7 @@ func TestIntegrationProvisioning_SecondRepositoryOnlyExportsNewDashboards(t *tes helper := runGrafana(t) ctx := context.Background() + // FIXME: helper to create dashboards. // Create some unmanaged dashboards directly in Grafana first dashboard1 := helper.LoadYAMLOrJSONFile("exportunifiedtorepository/dashboard-test-v1.yaml") dashboard1Obj, err := helper.DashboardsV1.Resource.Create(ctx, dashboard1, metav1.CreateOptions{}) @@ -114,6 +117,7 @@ func TestIntegrationProvisioning_SecondRepositoryOnlyExportsNewDashboards(t *tes require.NoError(t, err, "should be able to create second dashboard") dashboard2Name := dashboard2Obj.GetName() + // FIXME: use same helper to create repository // Create the first repository with sync enabled const repo1 = "first-repository" repo1Path := filepath.Join(helper.ProvisioningPath, repo1) @@ -140,7 +144,7 @@ func TestIntegrationProvisioning_SecondRepositoryOnlyExportsNewDashboards(t *tes }, } - helper.TriggerJobAndWait(t, repo1, spec) + helper.TriggerJobAndWaitForSuccess(t, repo1, spec) helper.SyncAndWait(t, repo1, nil) printFileTree(t, helper.ProvisioningPath) @@ -153,8 +157,8 @@ func TestIntegrationProvisioning_SecondRepositoryOnlyExportsNewDashboards(t *tes require.NoError(t, err) require.Equal(t, repo1, managedDash2.GetAnnotations()[utils.AnnoKeyManagerIdentity], "dashboard2 should be managed by first repo") + // FIXME: use helper to create repository // Create second repository - enable sync and set different target - const repo2 = "second-repository" repo2Path := filepath.Join(helper.ProvisioningPath, repo2) err = os.MkdirAll(repo2Path, 0750) @@ -175,6 +179,7 @@ func TestIntegrationProvisioning_SecondRepositoryOnlyExportsNewDashboards(t *tes // Wait for second repository to sync helper.SyncAndWait(t, repo2, nil) + // FIXME: use helpers to check status // Validate that folders for both repositories exist folders, err := helper.Folders.Resource.List(ctx, metav1.ListOptions{}) require.NoError(t, err, "should be able to list folders") @@ -215,7 +220,7 @@ func TestIntegrationProvisioning_SecondRepositoryOnlyExportsNewDashboards(t *tes Path: "", // no prefix necessary for testing }, } - helper.TriggerJobAndWait(t, repo2, spec) + helper.TriggerJobAndWaitForSuccess(t, repo2, spec) // Wait for both repositories to sync helper.SyncAndWait(t, repo1, nil) diff --git a/pkg/tests/apis/provisioning/files_test.go b/pkg/tests/apis/provisioning/files_test.go index d50fde289f1..0867bce6c9a 100644 --- a/pkg/tests/apis/provisioning/files_test.go +++ b/pkg/tests/apis/provisioning/files_test.go @@ -6,16 +6,12 @@ import ( "io" "net/http" "testing" - "time" "github.com/grafana/grafana/pkg/apimachinery/utils" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - - provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" ) func TestIntegrationProvisioning_DeleteResources(t *testing.T) { @@ -393,66 +389,4 @@ func TestIntegrationProvisioning_MoveResources(t *testing.T) { require.Error(t, result.Error(), "should fail when source file doesn't exist") }) }) - - t.Run("move non-existent resource by reference", func(t *testing.T) { - // Create move job for non-existent resource - result := helper.AdminREST.Post(). - Namespace("default"). - Resource("repositories"). - Name(repo). - SubResource("jobs"). - Body(asJSON(&provisioning.JobSpec{ - Action: provisioning.JobActionMove, - Move: &provisioning.MoveJobOptions{ - TargetPath: "moved-nonexistent/", - Resources: []provisioning.ResourceRef{ - { - Name: "non-existent-move-uid", - Kind: "Dashboard", - Group: "dashboard.grafana.app", - }, - }, - }, - })). - SetHeader("Content-Type", "application/json"). - Do(ctx) - require.NoError(t, result.Error(), "should be able to create move job") - - // Wait for job to complete - should record error but continue - require.EventuallyWithT(t, func(collect *assert.CollectT) { - list := &unstructured.UnstructuredList{} - err := helper.AdminREST.Get(). - Namespace("default"). - Resource("repositories"). - Name(repo). - SubResource("jobs"). - Do(ctx).Into(list) - assert.NoError(collect, err, "should be able to list jobs") - assert.NotEmpty(collect, list.Items, "expect at least one job") - - // Find the most recent move job - var moveJob *unstructured.Unstructured - for _, elem := range list.Items { - assert.Equal(collect, repo, elem.GetLabels()["provisioning.grafana.app/repository"], "should have repo label") - - action := mustNestedString(elem.Object, "spec", "action") - if action == "move" { - // Get the most recent one (they should be ordered by creation time) - moveJob = &elem - } - } - if !assert.NotNil(collect, moveJob, "should find a move job") { - return - } - - state := mustNestedString(moveJob.Object, "status", "state") - // The job should complete but record errors for individual resource resolution failures - if state == "error" || state == "completed" || state == "success" { - // Any of these states is acceptable - the key is that resource resolution errors are recorded - // and don't fail the entire job due to error-tolerant implementation - return - } - assert.Fail(collect, "job should complete or error, but got state: %s", state) - }, time.Second*10, time.Millisecond*100, "Expected move job to handle non-existent resource") - }) } diff --git a/pkg/tests/apis/provisioning/helper_test.go b/pkg/tests/apis/provisioning/helper_test.go index b2c4636d87e..8ca2f22325b 100644 --- a/pkg/tests/apis/provisioning/helper_test.go +++ b/pkg/tests/apis/provisioning/helper_test.go @@ -96,7 +96,7 @@ func (h *provisioningTestHelper) SyncAndWait(t *testing.T, repo string, options h.AwaitJobSuccess(t, t.Context(), unstruct) } -func (h *provisioningTestHelper) TriggerJobAndWait(t *testing.T, repo string, spec provisioning.JobSpec) { +func (h *provisioningTestHelper) TriggerJobAndWaitForSuccess(t *testing.T, repo string, spec provisioning.JobSpec) { t.Helper() body := asJSON(spec) @@ -126,40 +126,77 @@ func (h *provisioningTestHelper) TriggerJobAndWait(t *testing.T, repo string, sp h.AwaitJobSuccess(t, t.Context(), unstruct) } +func (h *provisioningTestHelper) TriggerJobAndWaitForComplete(t *testing.T, repo string, spec provisioning.JobSpec) *unstructured.Unstructured { + t.Helper() + + body := asJSON(spec) + result := h.AdminREST.Post(). + Namespace("default"). + Resource("repositories"). + Name(repo). + SubResource("jobs"). + Body(body). + SetHeader("Content-Type", "application/json"). + Do(t.Context()) + + if apierrors.IsAlreadyExists(result.Error()) { + // Wait for all jobs to finish as we don't have the name. + h.AwaitJobs(t, repo) + t.Errorf("repository %s already has a job running, but we expected a new one to be created", repo) + t.FailNow() + + return nil + } + + obj, err := result.Get() + require.NoError(t, err, "expecting to be able to sync repository") + + unstruct, ok := obj.(*unstructured.Unstructured) + require.True(t, ok, "expecting unstructured object, but got %T", obj) + + name := unstruct.GetName() + require.NotEmpty(t, name, "expecting name to be set") + + return h.AwaitJob(t, t.Context(), unstruct) +} + func (h *provisioningTestHelper) AwaitJobSuccess(t *testing.T, ctx context.Context, job *unstructured.Unstructured) { t.Helper() + job = h.AwaitJob(t, ctx, job) + lastErrors := mustNestedStringSlice(job.Object, "status", "errors") + require.Empty(t, lastErrors, "historic job '%s' has errors: %v", job.GetName(), lastErrors) + lastState := mustNestedString(job.Object, "status", "state") + require.Equal(t, string(provisioning.JobStateSuccess), lastState, + "historic job '%s' was not successful", job.GetName()) +} + +func (h *provisioningTestHelper) AwaitJob(t *testing.T, ctx context.Context, job *unstructured.Unstructured) *unstructured.Unstructured { + t.Helper() repo := job.GetLabels()[jobs.LabelRepository] require.NotEmpty(t, repo) - // TODO: simply this - if !assert.EventuallyWithT(t, func(collect *assert.CollectT) { + + var lastResult *unstructured.Unstructured + require.EventuallyWithT(t, func(collect *assert.CollectT) { result, err := h.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "jobs", string(job.GetUID())) if apierrors.IsNotFound(err) { - assert.Fail(collect, "job '%s' not found yet yet", job.GetName()) + collect.Errorf("job '%s' not found, still waiting for it to complete", job.GetName()) return // continue trying } - // Can fail fast here -- the jobs are immutable - require.NoError(t, err) - require.NotNil(t, result) - - errors := mustNestedStringSlice(result.Object, "status", "errors") - require.Empty(t, errors, "historic job '%s' has errors: %v", job.GetName(), errors) - state := mustNestedString(result.Object, "status", "state") - require.Equal(t, string(provisioning.JobStateSuccess), state, - "historic job '%s' was not successful", job.GetName()) - }, time.Second*10, time.Millisecond*25) { - // We also want to add the job details to the error when it fails. - job, err := h.Jobs.Resource.Get(ctx, job.GetName(), metav1.GetOptions{}) if err != nil { - t.Logf("failed to get job details for further help: %v", err) - } else { - t.Logf("job details: %+v", job.Object) + collect.Errorf("failed to get job '%s': %v", job.GetName(), err) + collect.FailNow() + return } - t.FailNow() - } + + lastResult = result + }, time.Second*10, time.Millisecond*25) + require.NotNil(t, lastResult, "expected job result to be non-nil") + + return lastResult } func (h *provisioningTestHelper) AwaitJobs(t *testing.T, repoName string) { @@ -275,6 +312,46 @@ func (h *provisioningTestHelper) CopyToProvisioningPath(t *testing.T, from, to s require.NoError(t, err, "failed to write file to provisioning path") } +type TestRepo struct { + Name string + Target string + Values map[string]any + Copies map[string]string + ExpectedDashboards int + ExpectedFolders int +} + +func (h *provisioningTestHelper) CreateRepo(t *testing.T, repo TestRepo) { + if repo.Target == "" { + repo.Target = "instance" + } + + localTmp := h.RenderObject(t, "testdata/local-write.json.tmpl", map[string]any{ + "Name": repo.Name, + "SyncEnabled": true, + "SyncTarget": repo.Target, + }) + + _, err := h.Repositories.Resource.Create(t.Context(), localTmp, metav1.CreateOptions{}) + require.NoError(t, err) + + for from, to := range repo.Copies { + h.CopyToProvisioningPath(t, from, to) + } + + // Trigger and wait for initial sync to populate resources + h.SyncAndWait(t, repo.Name, nil) + + // Verify initial state + dashboards, err := h.DashboardsV1.Resource.List(t.Context(), metav1.ListOptions{}) + require.NoError(t, err) + require.Equal(t, repo.ExpectedDashboards, len(dashboards.Items), "should the expected dashboards after sync") + + folders, err := h.Folders.Resource.List(t.Context(), metav1.ListOptions{}) + require.NoError(t, err) + require.Equal(t, repo.ExpectedFolders, len(folders.Items), "should have the expected folders after sync") +} + type grafanaOption func(opts *testinfra.GrafanaOpts) // Useful for debugging a test in development. diff --git a/pkg/tests/apis/provisioning/movejob_test.go b/pkg/tests/apis/provisioning/movejob_test.go index a7d44f89fef..c7058e6f96e 100644 --- a/pkg/tests/apis/provisioning/movejob_test.go +++ b/pkg/tests/apis/provisioning/movejob_test.go @@ -2,18 +2,15 @@ package provisioning import ( "context" - "encoding/json" "os" "path/filepath" "strings" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" provisioning "github.com/grafana/grafana/apps/provisioning/pkg/apis/provisioning/v0alpha1" ) @@ -26,26 +23,17 @@ func TestIntegrationProvisioning_MoveJob(t *testing.T) { helper := runGrafana(t) ctx := context.Background() const repo = "move-test-repo" - localTmp := helper.RenderObject(t, "testdata/local-write.json.tmpl", map[string]any{ - "Name": repo, - "SyncEnabled": true, - "SyncTarget": "instance", - }) - _, err := helper.Repositories.Resource.Create(ctx, localTmp, metav1.CreateOptions{}) - require.NoError(t, err) - // Copy multiple test files to the repository - helper.CopyToProvisioningPath(t, "testdata/all-panels.json", "dashboard1.json") - helper.CopyToProvisioningPath(t, "testdata/text-options.json", "dashboard2.json") - helper.CopyToProvisioningPath(t, "testdata/timeline-demo.json", "folder/dashboard3.json") - // Trigger and wait for initial sync to populate resources - helper.SyncAndWait(t, repo, nil) - // Verify initial state - should have 3 dashboards and 1 folder - dashboards, err := helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) - require.NoError(t, err) - require.Equal(t, 3, len(dashboards.Items), "should have 3 dashboards after sync") - folders, err := helper.Folders.Resource.List(ctx, metav1.ListOptions{}) - require.NoError(t, err) - require.Equal(t, 1, len(folders.Items), "should have 1 folder after sync") + testRepo := TestRepo{ + Name: repo, + Copies: map[string]string{ + "testdata/all-panels.json": "dashboard1.json", + "testdata/text-options.json": "dashboard2.json", + "testdata/timeline-demo.json": "folder/dashboard3.json", + }, + ExpectedDashboards: 3, + ExpectedFolders: 1, + } + helper.CreateRepo(t, testRepo) t.Run("move single file", func(t *testing.T) { spec := provisioning.JobSpec{ @@ -55,13 +43,13 @@ func TestIntegrationProvisioning_MoveJob(t *testing.T) { TargetPath: "moved/", }, } - helper.TriggerJobAndWait(t, repo, spec) - + helper.TriggerJobAndWaitForSuccess(t, repo, spec) // TODO: This additional sync should not be necessary - the move job should handle sync properly helper.SyncAndWait(t, repo, nil) + // FIXME: use the helpers for assertions // Verify file is moved in repository - _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "moved", "dashboard1.json") + _, err := helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "moved", "dashboard1.json") require.NoError(t, err, "file should exist at new location in repository") // Verify original file is gone from repository @@ -76,57 +64,36 @@ func TestIntegrationProvisioning_MoveJob(t *testing.T) { require.NoError(t, err, "nested files should still exist") // Verify dashboard still exists in Grafana after sync - // Use eventually to let unified storage reflect the changes in dashboards. - // FIXME: investigate this - require.EventuallyWithT(t, func(collect *assert.CollectT) { - dashboards, err := helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) - assert.NoError(collect, err) - assert.Len(collect, dashboards.Items, 3, "should still have 3 dashboards after move") - // Verify that dashboards have the correct source paths - foundPaths := make(map[string]bool) - for _, dashboard := range dashboards.Items { - sourcePath := dashboard.GetAnnotations()["grafana.app/sourcePath"] - foundPaths[sourcePath] = true - } + dashboards, err := helper.DashboardsV1.Resource.List(ctx, metav1.ListOptions{}) + require.NoError(t, err) + require.Len(t, dashboards.Items, 3, "should still have 3 dashboards after move") + // Verify that dashboards have the correct source paths + foundPaths := make(map[string]bool) + for _, dashboard := range dashboards.Items { + sourcePath := dashboard.GetAnnotations()["grafana.app/sourcePath"] + foundPaths[sourcePath] = true + } - assert.True(t, foundPaths["moved/dashboard1.json"], "should have dashboard with moved source path") - assert.True(t, foundPaths["dashboard2.json"], "should have dashboard2 in original location") - assert.True(t, foundPaths["folder/dashboard3.json"], "should have dashboard3 in original nested location") - }, time.Second*10, time.Millisecond*100, "Expected to eventually have 3 dashboards after move") + require.True(t, foundPaths["moved/dashboard1.json"], "should have dashboard with moved source path") + require.True(t, foundPaths["dashboard2.json"], "should have dashboard2 in original location") + require.True(t, foundPaths["folder/dashboard3.json"], "should have dashboard3 in original nested location") }) t.Run("move multiple files and folder", func(t *testing.T) { // Create move job for multiple files including a folder - result := helper.AdminREST.Post(). - Namespace("default"). - Resource("repositories"). - Name(repo). - SubResource("jobs"). - Body(asJSON(&provisioning.JobSpec{ - Action: provisioning.JobActionMove, - Move: &provisioning.MoveJobOptions{ - Paths: []string{"dashboard2.json", "folder/"}, - TargetPath: "archived/", - }, - })). - SetHeader("Content-Type", "application/json"). - Do(ctx) - require.NoError(t, result.Error(), "should be able to create move job") - - raw, err := result.Raw() - require.NoError(t, err) - obj := &unstructured.Unstructured{} - err = json.Unmarshal(raw, obj) - require.NoError(t, err) - - // Wait for job to complete - helper.AwaitJobSuccess(t, ctx, obj) - + spec := provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"dashboard2.json", "folder/"}, + TargetPath: "archived/", + }, + } + helper.TriggerJobAndWaitForSuccess(t, repo, spec) // TODO: This additional sync should not be necessary - the move job should handle sync properly helper.SyncAndWait(t, repo, nil) // Verify files are moved in repository - _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "archived", "dashboard2.json") + _, err := helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "archived", "dashboard2.json") require.NoError(t, err, "dashboard2.json should exist at new location") _, err = helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "archived", "folder", "dashboard3.json") require.NoError(t, err, "folder/dashboard3.json should exist at new nested location") @@ -160,107 +127,52 @@ func TestIntegrationProvisioning_MoveJob(t *testing.T) { }) t.Run("move non-existent file", func(t *testing.T) { - // Create move job for non-existent file - result := helper.AdminREST.Post(). - Namespace("default"). - Resource("repositories"). - Name(repo). - SubResource("jobs"). - Body(asJSON(&provisioning.JobSpec{ - Action: provisioning.JobActionMove, - Move: &provisioning.MoveJobOptions{ - Paths: []string{"non-existent.json"}, - TargetPath: "moved/", + spec := provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"non-existent.json"}, + TargetPath: "moved/", + }, + } + + job := helper.TriggerJobAndWaitForComplete(t, repo, spec) + state := mustNestedString(job.Object, "status", "state") + require.Equal(t, "error", state, "move job should have failed due to non-existent file") + }) + + t.Run("move non-existent uid", func(t *testing.T) { + spec := provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + TargetPath: "moved-nonexistent/", + Resources: []provisioning.ResourceRef{ + { + Name: "non-existent-move-uid", + Kind: "Dashboard", + Group: "dashboard.grafana.app", + }, }, - })). - SetHeader("Content-Type", "application/json"). - Do(ctx) - require.NoError(t, result.Error(), "should be able to create move job") + }, + } - // Wait for job to complete - should fail due to strict error handling - require.EventuallyWithT(t, func(collect *assert.CollectT) { - list := &unstructured.UnstructuredList{} - err := helper.AdminREST.Get(). - Namespace("default"). - Resource("repositories"). - Name(repo). - SubResource("jobs"). - Do(ctx).Into(list) - assert.NoError(collect, err, "should be able to list jobs") - assert.NotEmpty(collect, list.Items, "expect at least one job") - - // Find the move job specifically - var moveJob *unstructured.Unstructured - for _, elem := range list.Items { - assert.Equal(collect, repo, elem.GetLabels()["provisioning.grafana.app/repository"], "should have repo label") - - action := mustNestedString(elem.Object, "spec", "action") - if action == "move" { - // Check if this is the specific job we're looking for - paths, found, err := unstructured.NestedStringSlice(elem.Object, "spec", "move", "paths") - if err == nil && found && len(paths) > 0 && paths[0] == "non-existent.json" { - moveJob = &elem - break - } - } - } - assert.NotNil(collect, moveJob, "should find a move job for non-existent file") - - state := mustNestedString(moveJob.Object, "status", "state") - assert.Equal(collect, "error", state, "move job should have failed due to non-existent file") - }, time.Second*10, time.Millisecond*100, "Expected move job to fail with error state") + job := helper.TriggerJobAndWaitForComplete(t, repo, spec) + state := mustNestedString(job.Object, "status", "state") + require.Equal(t, "error", state, "move job should have failed due to non-existent uid") }) t.Run("move without target path", func(t *testing.T) { // Create move job without target path (should fail) - result := helper.AdminREST.Post(). - Namespace("default"). - Resource("repositories"). - Name(repo). - SubResource("jobs"). - Body(asJSON(&provisioning.JobSpec{ - Action: provisioning.JobActionMove, - Move: &provisioning.MoveJobOptions{ - Paths: []string{"moved/dashboard1.json"}, - // TargetPath intentionally omitted - }, - })). - SetHeader("Content-Type", "application/json"). - Do(ctx) - require.NoError(t, result.Error(), "should be able to create move job") + spec := provisioning.JobSpec{ + Action: provisioning.JobActionMove, + Move: &provisioning.MoveJobOptions{ + Paths: []string{"moved/dashboard1.json"}, + // TargetPath intentionally omitted + }, + } - // Wait for job to complete - should fail due to missing target path - require.EventuallyWithT(t, func(collect *assert.CollectT) { - list := &unstructured.UnstructuredList{} - err := helper.AdminREST.Get(). - Namespace("default"). - Resource("repositories"). - Name(repo). - SubResource("jobs"). - Do(ctx).Into(list) - assert.NoError(collect, err, "should be able to list jobs") - assert.NotEmpty(collect, list.Items, "expect at least one job") - - // Find the move job specifically - var moveJob *unstructured.Unstructured - for _, elem := range list.Items { - assert.Equal(collect, repo, elem.GetLabels()["provisioning.grafana.app/repository"], "should have repo label") - - action := mustNestedString(elem.Object, "spec", "action") - if action == "move" { - // Check if this is the job without target path - targetPath, found, _ := unstructured.NestedString(elem.Object, "spec", "move", "targetPath") - if !found || targetPath == "" { - moveJob = &elem - break - } - } - } - assert.NotNil(collect, moveJob, "should find a move job without target path") - - state := mustNestedString(moveJob.Object, "status", "state") - assert.Equal(collect, "error", state, "move job should have failed due to missing target path") - }, time.Second*10, time.Millisecond*100, "Expected move job to fail with error state") + job := helper.TriggerJobAndWaitForComplete(t, repo, spec) + state := mustNestedString(job.Object, "status", "state") + assert.Equal(t, "error", state, "move job should have failed due to missing target path") }) t.Run("move by resource reference", func(t *testing.T) { @@ -317,7 +229,7 @@ func TestIntegrationProvisioning_MoveJob(t *testing.T) { }, } - helper.TriggerJobAndWait(t, refRepo, spec) + helper.TriggerJobAndWaitForSuccess(t, refRepo, spec) // Verify corresponding file is moved in repository _, err = helper.Repositories.Resource.Get(ctx, refRepo, metav1.GetOptions{}, "files", "moved-by-ref", "move-source-1.json") require.NoError(t, err, "file should be moved to new location in repository") @@ -351,7 +263,7 @@ func TestIntegrationProvisioning_MoveJob(t *testing.T) { }, }, } - helper.TriggerJobAndWait(t, refRepo, spec) + helper.TriggerJobAndWaitForSuccess(t, refRepo, spec) // Verify file is moved in repository _, err = helper.Repositories.Resource.Get(ctx, refRepo, metav1.GetOptions{}, "files", "archived-by-ref", "move-source-2.json") @@ -399,7 +311,7 @@ func TestIntegrationProvisioning_MoveJob(t *testing.T) { }, }, } - helper.TriggerJobAndWait(t, refRepo, spec) + helper.TriggerJobAndWaitForSuccess(t, refRepo, spec) // Verify both targeted resources are moved in repository _, err = helper.Repositories.Resource.Get(ctx, refRepo, metav1.GetOptions{}, "files", "mixed-target", "mixed-move-1.json") diff --git a/pkg/tests/apis/provisioning/repository_test.go b/pkg/tests/apis/provisioning/repository_test.go index 336d33a2bb0..fcf1b2185d2 100644 --- a/pkg/tests/apis/provisioning/repository_test.go +++ b/pkg/tests/apis/provisioning/repository_test.go @@ -192,49 +192,19 @@ func TestIntegrationProvisioning_FailInvalidSchema(t *testing.T) { require.Error(t, err, "invalid dashboard shouldn't exist") require.True(t, apierrors.IsNotFound(err)) - var jobObj *unstructured.Unstructured - require.EventuallyWithT(t, func(collect *assert.CollectT) { - result := helper.AdminREST.Post(). - Namespace("default"). - Resource("repositories"). - Name(repo). - SubResource("jobs"). - Body(asJSON(&provisioning.JobSpec{ - Action: provisioning.JobActionPull, - Pull: &provisioning.SyncJobOptions{}, - })). - SetHeader("Content-Type", "application/json"). - Do(t.Context()) - require.NoError(collect, result.Error()) - job, err := result.Get() - require.NoError(collect, err) - var ok bool - jobObj, ok = job.(*unstructured.Unstructured) - assert.True(collect, ok, "expecting unstructured object, but got %T", job) - }, time.Second*10, time.Millisecond*10, "Expected to be able to start a sync job") + spec := provisioning.JobSpec{ + Action: provisioning.JobActionPull, + Pull: &provisioning.SyncJobOptions{}, + } - require.EventuallyWithT(t, func(collect *assert.CollectT) { - // helper.TriggerJobProcessing(t) - result, err := helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, - "jobs", string(jobObj.GetUID())) + result := helper.TriggerJobAndWaitForComplete(t, repo, spec) + job := &provisioning.Job{} + err = runtime.DefaultUnstructuredConverter.FromUnstructured(result.Object, job) + require.NoError(t, err, "should convert to Job object") - if apierrors.IsNotFound(err) { - assert.Fail(collect, "job '%s' not found yet yet", jobObj.GetName()) - return // continue trying - } - - // Can fail fast here -- the jobs are immutable - require.NoError(t, err) - require.NotNil(t, result) - - job := &provisioning.Job{} - err = runtime.DefaultUnstructuredConverter.FromUnstructured(result.Object, job) - require.NoError(t, err, "should convert to Job object") - - assert.Equal(t, provisioning.JobStateError, job.Status.State) - assert.Equal(t, job.Status.Message, "completed with errors") - assert.Equal(t, job.Status.Errors[0], "Dashboard.dashboard.grafana.app \"invalid-schema-uid\" is invalid: [spec.panels.0.repeatDirection: Invalid value: conflicting values \"h\" and \"this is not an allowed value\", spec.panels.0.repeatDirection: Invalid value: conflicting values \"v\" and \"this is not an allowed value\"]") - }, time.Second*10, time.Millisecond*10, "Expected provisioning job to conclude with the status failed") + assert.Equal(t, provisioning.JobStateError, job.Status.State) + assert.Equal(t, job.Status.Message, "completed with errors") + assert.Equal(t, job.Status.Errors[0], "Dashboard.dashboard.grafana.app \"invalid-schema-uid\" is invalid: [spec.panels.0.repeatDirection: Invalid value: conflicting values \"h\" and \"this is not an allowed value\", spec.panels.0.repeatDirection: Invalid value: conflicting values \"v\" and \"this is not an allowed value\"]") _, err = helper.DashboardsV1.Resource.Get(ctx, invalidSchemaUid, metav1.GetOptions{}) require.Error(t, err, "invalid dashboard shouldn't have been created")