test(e2e): add k8s storage SP API, NATS, and cluster checks (FLPATH-4… - #44
test(e2e): add k8s storage SP API, NATS, and cluster checks (FLPATH-4…#44LinskId wants to merge 1 commit into
Conversation
PR Summary by QodoAdd storage SP API, NATS, and Kubernetes E2E coverage
AI Description
Diagram
High-Level Assessment
Files changed (9)
|
Code Review by Qodo
1. StorageClass export is lost
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
Please, can you fix the PR description |
3ade7bd to
26ed681
Compare
…434) Assisted-by: Cursor Signed-off-by: igavra <igavra@redhat.com>
26ed681 to
1613ac5
Compare
| Expect(err).NotTo(HaveOccurred()) | ||
| Expect(resp.StatusCode).To(Equal(http.StatusOK)) | ||
|
|
||
| var body map[string]interface{} |
There was a problem hiding this comment.
Please don't use unstructured data. There must be a struct in the business layer that we can reuse to unmarshal into. Using unstructured data has 2 problems:
- Harder to maintain (no automatic refactoring available)
- Does not guarantee field drift in the original structure, which means extensions to the struct would not be covered in this evaluation. Comparing against a given structure (not field by field) provides such guarantee.
There was a problem hiding this comment.
k8s-storage-service-provider already generates a client for this — pkg/client/client.gen.go (oapi-codegen, same pattern as container/ACM cluster, wired into generate-api/check-generate-api). Its GetHealthWithResponse() decodes straight into a typed Health struct (Status, Path, Type, Uptime, Version), so no hand-written struct or map[string]interface{} is needed at all:
import (
storageclient "github.com/dcm-project/k8s-storage-service-provider/pkg/client"
)
client, err := storageclient.NewClientWithResponses(storageSPBaseURL)
Expect(err).NotTo(HaveOccurred())
resp, err := client.GetHealthWithResponse(ctx)
Expect(err).NotTo(HaveOccurred())
Expect(resp.StatusCode()).To(Equal(http.StatusOK))
Expect(resp.JSON200.Status).To(Equal("healthy"))Would need adding k8s-storage-service-provider as a go.mod dep in tests/e2e/. Same fix I flagged on #47 for the container/ACM cluster SPs — worth applying consistently rather than hand-writing structs per SP.
|
|
||
| var page2 map[string]interface{} | ||
| decodeJSON(resp2, &page2) | ||
| Expect(page2).To(HaveKey("volumes")) |
There was a problem hiding this comment.
what about the key's value? There's not test value to ensure the key exists only.
|
@LinskId please review my comment about using the generate go client binding and redo the tests. Also use structured data (go structs) and not unstructured (map[string]interface{}). Import the structures from the business layer rather than duplicate it so that you can focus on testing what adds value and not on the boilerplate (http connectivity and structure consistency (e.g. does this unstructured map has this key?)). |
3 participants
FLPATH-4434
Summary
storage && registration, skipped withoutDCM_ENVIRONMENT_AGENT_URL)deploy-dcm.shwith storage-class prerequisite validation and E2E SP default wiringmake test-storage-sp/make test-storage-sp-registrationScope
SP-direct tests on
:8089— simulates catalog hints viatests/e2e/fixtures/basic-pvc-catalog-item.yaml.Full catalog → instance → SP flow is out of scope (covered in control-plane SpecBuilder tests).
Test plan
./scripts/deploy-dcm.sh --k8s-storage-service-provider --kubeconfig ~/.kube/config
make test-storage-sp