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.

Sign up for GitHub

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

Jump to bottom

tests: simplify monitoringdashboard test case, add to tests-e2e-direct #1861

Merged
merged 2 commits into from
Jun 7, 2024
Merged

tests: simplify monitoringdashboard test case, add to tests-e2e-direct #1861

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ad6679
chore: update monitoringdashboard test data
justinsb May 21, 2024
066aac1
tests: add monitoringdashboard to direct tests (in anticipation)
justinsb May 21, 2024
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
2 changes: 1 addition & 1 deletion ...ngdashboard/monitoringdashboardrefs/_generated_object_monitoringdashboardrefs.golden.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ spec:
filter: metric.type="agent.googleapis.com/nginx/connections/accepted_count"
resourceNames: []
title: Widget 4
displayName: monitoringdashboard-${uniqueId}
displayName: monitoringdashboard updated
projectRef:
name: otherproject
resourceID: monitoringdashboard-${uniqueId}
Expand Down
8 changes: 4 additions & 4 deletions ...e/testdata/basic/monitoring/v1beta1/monitoringdashboard/monitoringdashboardrefs/_http.log
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -524,9 +524,9 @@ User-Agent: kcc/controller-manager DeclarativeClientLib/0.0.1
}
]
},
"displayName": "monitoringdashboard-${uniqueId}",
"displayName": "monitoringdashboard updated",
"etag": "abcdef0123A=",
"name": "projects/${projectId}/dashboards/monitoringdashboard-${uniqueId}"
"name": "projects/projects/other${uniqueId}/dashboards/monitoringdashboard-${uniqueId}"
}

200 OK
Expand Down Expand Up @@ -613,7 +613,7 @@ X-Xss-Protection: 0
}
]
},
"displayName": "monitoringdashboard-${uniqueId}",
"displayName": "monitoringdashboard updated",
"etag": "abcdef0123A=",
"name": "projects/${projectNumber}/dashboards/monitoringdashboard-${uniqueId}"
}
Expand Down Expand Up @@ -708,7 +708,7 @@ X-Xss-Protection: 0
}
]
},
"displayName": "monitoringdashboard-${uniqueId}",
"displayName": "monitoringdashboard updated",
"etag": "abcdef0123A=",
"name": "projects/${projectNumber}/dashboards/monitoringdashboard-${uniqueId}"
}
Expand Down
2 changes: 1 addition & 1 deletion ...testdata/basic/monitoring/v1beta1/monitoringdashboard/monitoringdashboardrefs/update.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ metadata:
spec:
projectRef:
name: otherproject
displayName: "monitoringdashboard-${uniqueId}"
displayName: "monitoringdashboard updated"
columnLayout:
columns:
- weight: 2
Expand Down
2 changes: 1 addition & 1 deletion scripts/github-actions/tests-e2e-direct.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ echo "Running e2e tests fixtures for LoggingLogMetric direct reconciliation..."
GOLDEN_OBJECT_CHECKS=1 \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to double check, do we want to run monitoringdashboard direct controller on the test "monitoringdashboard"? If so, I think we want to include the Kind name in KCC_USE_DIRECT_RECONCILERS or using a different command (so you can  echo the steps more granularly)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I wanted to start by adding this to the "strict" tests where we check the golden output. Then I would add it to KCC_USE_DIRECT_RECONCILERS, and we would be able to see the changes in behaviour (if any) at the http and at the KRM level.

I think what's going wrong is that we've turned off golden tests with KCC_USE_DIRECT_RECONCILERS, and I don't think we should have done that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. With #1910 , those LLM tests should no longer need the KCC_USE_DIRECT_RECONCILERS flag (and thanks for the cleanup in #1919).

When the monitoring dashboard is ready for the direct controller, you can simply remove the dcl2crd label (KCC_USE_DIRECT_RECONCILERS should also work but I think it less preferred for beta resource)

GOLDEN_REQUEST_CHECKS=1 \
E2E_KUBE_TARGET=envtest RUN_E2E=1 E2E_GCP_TARGET=mock \
go test -test.count=1 -timeout 600s -v ./tests/e2e -run 'TestAllInSeries/fixtures/explicitlogmetric|TestAllInSeries/fixtures/exponentiallogmetric|TestAllInSeries/fixtures/linearlogmetric|TestAllInSeries/fixtures/logbucketmetric'
go test -test.count=1 -timeout 600s -v ./tests/e2e -run 'TestAllInSeries/fixtures/explicitlogmetric|TestAllInSeries/fixtures/exponentiallogmetric|TestAllInSeries/fixtures/linearlogmetric|TestAllInSeries/fixtures/logbucketmetric|TestAllInSeries/fixtures/monitoringdashboard'

echo "Running scenarios tests for LoggingLogMetric direct reconciliation..."

Expand Down