Provisioning: Fix flaky tests with better debugging and consistent test patterns (#109601)

* Add log after jobs

* Use the same helper to create repository in export job

* Improve the logging

* Fix eventually conditions in helpers

* Fix export job tests

* Format code

* Fix linting

* Fix the format

* Fix linting issue

* Fix innefectual assignment
This commit is contained in:
Roberto Jiménez Sánchez
2025-08-13 17:35:06 +02:00
committed by GitHub
parent 587f52cf5b
commit 6527790b64
5 changed files with 251 additions and 48 deletions
+12 -1
View File
@@ -40,6 +40,14 @@ func TestIntegrationProvisioning_DeleteJob(t *testing.T) {
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.
// Debug state before delete
helper.DebugState(t, repo, "BEFORE DELETE")
// Verify file exists in repository before attempting delete
_, err := helper.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{}, "files", "dashboard1.json")
require.NoError(t, err, "dashboard1.json should exist in repository before delete")
spec := provisioning.JobSpec{
Action: provisioning.JobActionDelete,
Delete: &provisioning.DeleteJobOptions{
@@ -49,10 +57,13 @@ func TestIntegrationProvisioning_DeleteJob(t *testing.T) {
// Create delete job for single file
helper.TriggerJobAndWaitForSuccess(t, repo, spec)
// Debug state after successful delete
helper.DebugState(t, repo, "AFTER DELETE")
// 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")
+44 -33
View File
@@ -43,13 +43,19 @@ 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")
testRepo := TestRepo{
Name: repo,
Copies: map[string]string{}, // No initial files needed for export test
ExpectedDashboards: 4, // 4 dashboards created above (v0, v1, v2alpha1, v2beta1)
ExpectedFolders: 0, // No folders expected after sync
}
helper.CreateRepo(t, testRepo)
// Now export
helper.DebugState(t, repo, "BEFORE EXPORT TO REPOSITORY")
spec := provisioning.JobSpec{
Action: provisioning.JobActionPush,
Push: &provisioning.ExportJobOptions{
Folder: "", // export entire instance
Path: "", // no prefix necessary for testing
@@ -57,6 +63,8 @@ func TestProvisioning_ExportUnifiedToRepository(t *testing.T) {
}
helper.TriggerJobAndWaitForSuccess(t, repo, spec)
helper.DebugState(t, repo, "AFTER EXPORT TO REPOSITORY")
type props struct {
title string
apiVersion string
@@ -117,27 +125,27 @@ 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
// Create the first repository with sync enabled and separate filesystem path
const repo1 = "first-repository"
repo1Path := filepath.Join(helper.ProvisioningPath, repo1)
err = os.MkdirAll(repo1Path, 0750)
require.NoError(t, err, "should be able to create repository path")
createBody1 := helper.RenderObject(t, "testdata/local-write.json.tmpl", map[string]any{
"Name": repo1,
"SyncEnabled": true,
"SyncTarget": "folder",
"Path": repo1Path,
})
_, err = helper.Repositories.Resource.Create(ctx, createBody1, metav1.CreateOptions{})
require.NoError(t, err, "should be able to create first repository")
testRepo1 := TestRepo{
Name: repo1,
Target: "folder",
Path: repo1Path,
Copies: map[string]string{}, // No initial files needed for export test
ExpectedDashboards: 2, // 2 dashboards created above (v1, v2beta1)
ExpectedFolders: 1, // One folder expected after sync
}
helper.CreateRepo(t, testRepo1)
// Print file tree before export
printFileTree(t, helper.ProvisioningPath)
// Initial export
helper.DebugState(t, repo1, "BEFORE INITIAL EXPORT")
spec := provisioning.JobSpec{
Action: provisioning.JobActionPush,
Push: &provisioning.ExportJobOptions{
Folder: "", // export entire instance
Path: "", // no prefix necessary for testing
@@ -145,6 +153,8 @@ func TestIntegrationProvisioning_SecondRepositoryOnlyExportsNewDashboards(t *tes
}
helper.TriggerJobAndWaitForSuccess(t, repo1, spec)
helper.DebugState(t, repo1, "AFTER INITIAL EXPORT")
helper.SyncAndWait(t, repo1, nil)
printFileTree(t, helper.ProvisioningPath)
@@ -157,28 +167,24 @@ 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
// Create second repository - enable sync and set different target with separate filesystem path
const repo2 = "second-repository"
repo2Path := filepath.Join(helper.ProvisioningPath, repo2)
err = os.MkdirAll(repo2Path, 0750)
require.NoError(t, err, "should be able to create seconrd repository path")
printFileTree(t, helper.ProvisioningPath)
createBody2 := helper.RenderObject(t, "testdata/local-write.json.tmpl", map[string]any{
"Name": repo2,
"SyncEnabled": true,
"SyncTarget": "folder",
"Path": repo2Path,
})
_, err = helper.Repositories.Resource.Create(ctx, createBody2, metav1.CreateOptions{})
require.NoError(t, err, "should be able to create second repository")
testRepo2 := TestRepo{
Name: repo2,
Target: "folder",
Path: repo2Path,
Copies: map[string]string{}, // No initial files needed for export test
ExpectedDashboards: 2, // 2 dashboards exist when second repo syncs
ExpectedFolders: 2, // Two folders expected after sync (repo1 + repo2)
}
helper.CreateRepo(t, testRepo2)
// Wait for second repository to sync
helper.SyncAndWait(t, repo2, nil)
printFileTree(t, helper.ProvisioningPath)
// FIXME: use helpers to check status
// Validate that folders for both repositories exist
folders, err := helper.Folders.Resource.List(ctx, metav1.ListOptions{})
@@ -214,7 +220,10 @@ func TestIntegrationProvisioning_SecondRepositoryOnlyExportsNewDashboards(t *tes
require.NoError(t, err)
// Export from second repository - this should only export the unmanaged dashboard3
helper.DebugState(t, repo2, "BEFORE SECOND EXPORT")
spec = provisioning.JobSpec{
Action: provisioning.JobActionPush,
Push: &provisioning.ExportJobOptions{
Folder: "", // export entire instance
Path: "", // no prefix necessary for testing
@@ -222,6 +231,8 @@ func TestIntegrationProvisioning_SecondRepositoryOnlyExportsNewDashboards(t *tes
}
helper.TriggerJobAndWaitForSuccess(t, repo2, spec)
helper.DebugState(t, repo2, "AFTER SECOND EXPORT")
// Wait for both repositories to sync
helper.SyncAndWait(t, repo1, nil)
helper.SyncAndWait(t, repo2, nil)
+183 -14
View File
@@ -164,8 +164,16 @@ func (h *provisioningTestHelper) AwaitJobSuccess(t *testing.T, ctx context.Conte
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")
repo := job.GetLabels()[jobs.LabelRepository]
// Debug state if job failed
if len(lastErrors) > 0 || lastState != string(provisioning.JobStateSuccess) {
h.DebugState(t, repo, fmt.Sprintf("JOB FAILED: %s", job.GetName()))
}
require.Empty(t, lastErrors, "historic job '%s' has errors: %v", job.GetName(), lastErrors)
require.Equal(t, string(provisioning.JobStateSuccess), lastState,
"historic job '%s' was not successful", job.GetName())
}
@@ -181,14 +189,13 @@ func (h *provisioningTestHelper) AwaitJob(t *testing.T, ctx context.Context, job
result, err := h.Repositories.Resource.Get(ctx, repo, metav1.GetOptions{},
"jobs", string(job.GetUID()))
if apierrors.IsNotFound(err) {
if !assert.False(collect, apierrors.IsNotFound(err)) {
collect.Errorf("job '%s' not found, still waiting for it to complete", job.GetName())
return // continue trying
return
}
assert.NoError(collect, err, "failed to get job '%s' to be found", job.GetName())
if err != nil {
collect.Errorf("failed to get job '%s': %v", job.GetName(), err)
collect.FailNow()
return
}
@@ -208,9 +215,11 @@ func (h *provisioningTestHelper) AwaitJobs(t *testing.T, repoName string) {
if assert.NoError(collect, err, "failed to list active jobs") {
for _, elem := range list.Items {
repo, _, err := unstructured.NestedString(elem.Object, "spec", "repository")
require.NoError(t, err)
if repo == repoName {
collect.Errorf("there are still remaining jobs for %s: %+v", repoName, elem)
if !assert.NoError(collect, err, "failed to get repository from job spec") {
return
}
if !assert.NotEqual(collect, repoName, repo, "there are still remaining jobs for %s: %+v", repoName, elem) {
return
}
}
@@ -243,9 +252,11 @@ func (h *provisioningTestHelper) AwaitJobsWithStates(t *testing.T, repoName stri
if assert.NoError(collect, err, "failed to list active jobs") {
for _, elem := range list.Items {
repo, _, err := unstructured.NestedString(elem.Object, "spec", "repository")
require.NoError(t, err)
if repo == repoName {
collect.Errorf("there are still remaining jobs for %s: %+v", repoName, elem)
if !assert.NoError(collect, err, "failed to get repository from job spec") {
return
}
if !assert.NotEqual(collect, repoName, repo, "there are still remaining jobs for %s: %+v", repoName, elem) {
return
}
}
@@ -312,9 +323,140 @@ func (h *provisioningTestHelper) CopyToProvisioningPath(t *testing.T, from, to s
require.NoError(t, err, "failed to write file to provisioning path")
}
// DebugState logs the current state of filesystem, repository, and Grafana resources for debugging
func (h *provisioningTestHelper) DebugState(t *testing.T, repo string, label string) {
t.Helper()
t.Logf("=== DEBUG STATE: %s ===", label)
ctx := context.Background()
// Log filesystem contents using existing tree function
printFileTree(t, h.ProvisioningPath)
// Log all repositories first
t.Logf("All repositories:")
repos, err := h.Repositories.Resource.List(ctx, metav1.ListOptions{})
if err != nil {
t.Logf(" ERROR listing repositories: %v", err)
} else {
t.Logf(" Total repositories: %d", len(repos.Items))
for i, repository := range repos.Items {
t.Logf(" Repository %d: name=%s", i+1, repository.GetName())
}
}
// Log repository files for the specific repo
t.Logf("Repository '%s' files:", repo)
h.logRepositoryFiles(t, ctx, repo, " ")
// Log files for all other repositories too
if repos != nil && len(repos.Items) > 1 {
t.Logf("Files in other repositories:")
for _, repository := range repos.Items {
if repository.GetName() != repo {
t.Logf(" Repository '%s' files:", repository.GetName())
h.logRepositoryFiles(t, ctx, repository.GetName(), " ")
}
}
}
// Log Grafana dashboards
t.Logf("Grafana dashboards:")
dashboards, err := h.DashboardsV1.Resource.List(ctx, metav1.ListOptions{})
if err != nil {
t.Logf(" ERROR listing dashboards: %v", err)
} else {
t.Logf(" Total dashboards: %d", len(dashboards.Items))
for i, dashboard := range dashboards.Items {
t.Logf(" Dashboard %d: name=%s, UID=%s", i+1, dashboard.GetName(), dashboard.GetUID())
}
}
// Log Grafana folders
t.Logf("Grafana folders:")
folders, err := h.Folders.Resource.List(ctx, metav1.ListOptions{})
if err != nil {
t.Logf(" ERROR listing folders: %v", err)
} else {
t.Logf(" Total folders: %d", len(folders.Items))
for i, folder := range folders.Items {
t.Logf(" Folder %d: name=%s", i+1, folder.GetName())
}
}
t.Logf("=== END DEBUG STATE ===")
}
// logRepositoryFiles logs repository file structure using the files API
func (h *provisioningTestHelper) logRepositoryFiles(t *testing.T, ctx context.Context, repoName string, prefix string) {
t.Helper()
// Try to list files at root level
files, err := h.Repositories.Resource.Get(ctx, repoName, metav1.GetOptions{}, "files")
if err != nil {
t.Logf("%sERROR getting repository files: %v", prefix, err)
return
}
// The API returns a structured response, we need to extract the actual file data
if files.Object != nil {
h.logRepositoryObject(t, files.Object, prefix, "")
} else {
t.Logf("%s(empty repository)", prefix)
}
}
// logRepositoryObject recursively logs repository file structure from API response
func (h *provisioningTestHelper) logRepositoryObject(t *testing.T, obj map[string]interface{}, prefix string, path string) {
t.Helper()
if obj == nil {
return
}
// Skip metadata fields and focus on actual content
for key, value := range obj {
// Skip Kubernetes metadata fields
if key == "kind" || key == "apiVersion" || key == "metadata" {
continue
}
// Calculate new path for nested objects
var newPath string
if path != "" {
newPath = path + "/" + key
} else {
newPath = key
}
switch v := value.(type) {
case map[string]interface{}:
t.Logf("%s├── %s/", prefix, key)
h.logRepositoryObject(t, v, prefix+" ", newPath)
case []interface{}:
// Handle lists (like items array)
if key == "items" && len(v) > 0 {
t.Logf("%s%d items:", prefix, len(v))
for i, item := range v {
if itemMap, ok := item.(map[string]interface{}); ok {
t.Logf("%s├── item %d:", prefix, i+1)
h.logRepositoryObject(t, itemMap, prefix+" ", newPath)
}
}
}
default:
// This could be file content or metadata
if key != "kind" && key != "apiVersion" {
t.Logf("%s├── %s", prefix, key)
}
}
}
}
type TestRepo struct {
Name string
Target string
Path string
Values map[string]any
Copies map[string]string
ExpectedDashboards int
@@ -326,22 +468,49 @@ func (h *provisioningTestHelper) CreateRepo(t *testing.T, repo TestRepo) {
repo.Target = "instance"
}
localTmp := h.RenderObject(t, "testdata/local-write.json.tmpl", map[string]any{
// Use custom path if provided, otherwise use default provisioning path
repoPath := h.ProvisioningPath
if repo.Path != "" {
repoPath = repo.Path
// Ensure the directory exists
err := os.MkdirAll(repoPath, 0750)
require.NoError(t, err, "should be able to create repository path")
}
templateVars := map[string]any{
"Name": repo.Name,
"SyncEnabled": true,
"SyncTarget": repo.Target,
})
}
if repo.Path != "" {
templateVars["Path"] = repoPath
}
localTmp := h.RenderObject(t, "testdata/local-write.json.tmpl", templateVars)
_, 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)
if repo.Path != "" {
// Copy to custom path
fullPath := path.Join(repoPath, to)
err := os.MkdirAll(path.Dir(fullPath), 0750)
require.NoError(t, err, "failed to create directories for custom path")
file := h.LoadFile(from)
err = os.WriteFile(fullPath, file, 0600)
require.NoError(t, err, "failed to write file to custom path")
} else {
h.CopyToProvisioningPath(t, from, to)
}
}
// Trigger and wait for initial sync to populate resources
h.SyncAndWait(t, repo.Name, nil)
// Debug state after initial sync
h.DebugState(t, repo.Name, "AFTER INITIAL SYNC")
// Verify initial state
dashboards, err := h.DashboardsV1.Resource.List(t.Context(), metav1.ListOptions{})
require.NoError(t, err)
@@ -36,6 +36,8 @@ func TestIntegrationProvisioning_MoveJob(t *testing.T) {
helper.CreateRepo(t, testRepo)
t.Run("move single file", func(t *testing.T) {
helper.DebugState(t, repo, "BEFORE MOVE SINGLE FILE")
spec := provisioning.JobSpec{
Action: provisioning.JobActionMove,
Move: &provisioning.MoveJobOptions{
@@ -44,6 +46,8 @@ func TestIntegrationProvisioning_MoveJob(t *testing.T) {
},
}
helper.TriggerJobAndWaitForSuccess(t, repo, spec)
helper.DebugState(t, repo, "AFTER MOVE SINGLE FILE")
// TODO: This additional sync should not be necessary - the move job should handle sync properly
helper.SyncAndWait(t, repo, nil)
@@ -80,6 +84,8 @@ func TestIntegrationProvisioning_MoveJob(t *testing.T) {
})
t.Run("move multiple files and folder", func(t *testing.T) {
helper.DebugState(t, repo, "BEFORE MOVE MULTIPLE FILES")
// Create move job for multiple files including a folder
spec := provisioning.JobSpec{
Action: provisioning.JobActionMove,
@@ -89,6 +95,8 @@ func TestIntegrationProvisioning_MoveJob(t *testing.T) {
},
}
helper.TriggerJobAndWaitForSuccess(t, repo, spec)
helper.DebugState(t, repo, "AFTER MOVE MULTIPLE FILES")
// TODO: This additional sync should not be necessary - the move job should handle sync properly
helper.SyncAndWait(t, repo, nil)
@@ -127,6 +135,8 @@ func TestIntegrationProvisioning_MoveJob(t *testing.T) {
})
t.Run("move non-existent file", func(t *testing.T) {
helper.DebugState(t, repo, "BEFORE MOVE NON-EXISTENT FILE")
spec := provisioning.JobSpec{
Action: provisioning.JobActionMove,
Move: &provisioning.MoveJobOptions{
@@ -203,6 +203,8 @@ func TestIntegrationProvisioning_FailInvalidSchema(t *testing.T) {
require.Error(t, err, "invalid dashboard shouldn't exist")
require.True(t, apierrors.IsNotFound(err))
helper.DebugState(t, repo, "BEFORE PULL JOB WITH INVALID SCHEMA")
spec := provisioning.JobSpec{
Action: provisioning.JobActionPull,
Pull: &provisioning.SyncJobOptions{},