[History Server] Keep directory placeholders out of ListFiles results

[stantheman0128]

Why are these changes needed?

ListFiles reports two kinds of entries, and its callers rely on telling them apart: a plain name is a file, a name ending in / is a subdirectory. ServerHandler.listFilesRecursive recurses into an entry only when it ends in a slash, and _getNodeLogs passes everything else to the log categorizer.

The s3 and aliyunoss backends break that rule. CreateDirectory writes a trailing-slash placeholder object for the directory itself, and an object listing whose prefix is that directory returns the placeholder as one of its keys. path.Base then strips the slash, so the directory reports itself as a file one level down. Feeding listFilesRecursive exactly what the pre-fix s3 backend returns for a node log directory gives:

listFilesRecursive  => [raylet.out events/events events/event_GCS.log]
categorizeLogFiles  => map[internal:[events event_GCS.log]]

That phantom events entry reaches the user through the ** glob log listing and the internal log category, and fetching it returns nothing. The event readers happen to be immune, because isValidEventFile and the event_ prefix check reject the name; ipToNodeId only wastes a lookup on it.

The same placeholder can also be picked up by the GetContent fallback, which matches on path.Base: a marker for a directory named x base-matches a request for a file named x, and the fallback then serves the empty marker instead of continuing the scan.

The gcs backend already filters these out, with a comment saying that is what it is doing:

// Exclude the placeholder object if it exists for the directory itself.
if attrs.Name != "" && !strings.HasSuffix(attrs.Name, "/") {

So the fix skips trailing-slash keys in the Contents loop of the s3 and aliyunoss _listFiles, and in both listing loops of the azureblob listBlobs. For azureblob that is defensive rather than a bug fix: its CreateDirectory deliberately writes nothing, because a marker blob shows up as <no name> in Azure Storage Explorer, so this codebase never creates one there. A marker left by another tool would still reach the callers, and after this change all four readers answer the same contract.

On the test side, gcs_handler_test.go has covered List, ListFiles, CreateDirectory and WriteFile all along. The other three backends had none of that, and aliyunoss had no test that constructed a handler at all. This PR adds those four methods per backend, driving each SDK against an httptest server so the assertions land on the requests that reach the wire, the same approach as the GetContent tests in #5075.

Related issue number

None. Found while covering GetContent for #5075.

Depends on #5075: this branch is stacked on it and reuses the test server helpers it introduces, so the diff here includes that PR's commit until it merges.

Labels

• If this PR has user-facing changes that require documentation updates at release time, I have added the doc-updates-required label.
• If this PR contains breaking changes, I have added the breaking-change label.

Checks

• I've made sure the tests are passing.
• Testing Strategy
  • Unit tests
  • Manual tests

Manual test instructions

Against a live MinIO, the same quay.io/minio/minio image config/minio.yaml deploys, using the in-tree CreateDirectory and WriteFile to lay down a node's events directory and then listing it:

docker run -d -p 9412:9000 quay.io/minio/minio:latest server /data

raw ListObjectsV2  prefix=<root>/<cluster>/session_1/logs/node123/events/  delimiter=/
  Contents:        ".../logs/node123/events/"                  <- the directory's own key
  Contents:        ".../logs/node123/events/event_GCS.log"
  Contents:        ".../logs/node123/events/event_RAYLET.log"
  CommonPrefixes:  ".../logs/node123/events/old/"

ListFiles before the fix => [events event_GCS.log event_RAYLET.log old/]
ListFiles after the fix  => [event_GCS.log event_RAYLET.log old/]

So the placeholder really does come back in Contents rather than being rolled up into CommonPrefixes, and events really does reach the caller looking like a file.

Evidence

Every new test was mutation tested. Twenty-three mutants, all killed:

Mutant	Test that caught it
s3 / aliyunoss / azureblob keep the placeholder in the listing	TestListFilesSeparatesFilesFromDirectories (per backend)
ListFiles prefix drops the root dir (x3)	same
ListFiles asks for the listing without the / delimiter (x3)	same
the listing always sends an empty delimiter (s3, aliyunoss)	same
_listFiles ignores CommonPrefixes (s3)	same
List prefix drops the root dir (x3)	TestListReadsClusterMetadataUnderRootDir (per backend)
CreateDirectory omits the trailing slash (s3, aliyunoss)	TestCreateDirectoryWritesPlaceholderWhenMissing, TestCreateDirectoryLeavesExistingDirectoryAlone
CreateDirectory ignores the existence check (s3)	TestCreateDirectoryLeavesExistingDirectoryAlone
azureblob CreateDirectory starts writing a marker	TestCreateDirectoryWritesNothing
WriteFile writes to path.Base of the key (x3)	TestWriteFileUploadsBodyToGivenKey / ...Blob
ClusterInfoList sorts oldest first	all three TestListReadsClusterMetadataUnderRootDir

• go test ./pkg/... on Windows: green, except the pre-existing failure in pkg/utils (TestGetSessionDir_FatalError_FailsFast), which fails the same way on a clean master checkout and passes on Linux.
• go test -race ./pkg/... -parallel 4, which is what make test runs in CI, on Linux: green, and A/B identical to a clean master checkout.
• go vet ./...: clean.

What was not tested

• The unit tests drive the real SDKs against an httptest stub, so on their own they pin down the requests the code sends and how it reads a response of a given shape. The MinIO run above is what backs the premise for s3. For Aliyun OSS and Azure Blob I am relying on API compatibility rather than a run against those services, and the MinIO check was a manual one, not something these tests reproduce.
• The historyserver e2e suite needs a Kubernetes cluster and was not run locally.
• No test exercises the collector-to-history-server round trip. The link comes from reading the code: logcollector.Writer.CreateDirectory and eventcollector write the marker on the same paths that ServerHandler and the event readers later list.
• The filter added to the flat-listing branch of listBlobs is unreachable today, since both callers pass "/". It is there so the two branches agree, but no test covers it.
• On an Azure account with hierarchical namespace enabled, directory entries come back without a trailing slash, so this filter would not catch them.
• MaxKeys/MaxResults and the pagination boundary (NextContinuationToken, NextMarker) are not pinned by these tests.