Skip to content

Commit 7554ab4

Browse files
Add --emulator-platform flag and CLI_EMULATOR_PLATFORM variable (#374)
feat: add --emulator-platform flag and CLI_EMULATOR_PLATFORM variable (#374) ## Summary Implements container platform specification for embedded emulator with comprehensive platform detection and reporting. ## Key Changes - Added `--emulator-platform` flag to specify container platform (e.g., linux/amd64, linux/arm64) - Added `CLI_EMULATOR_PLATFORM` read-only system variable showing actual running platform - Updated spanemuboost to v0.2.11 for platform support - Implemented multi-strategy platform detection with fallbacks - Added comprehensive test coverage for platform functionality - Updated README documentation ## Platform Detection The implementation ensures accurate platform reporting: - Flag controls requested platform from Docker - System variable always shows actual running platform - Supports partial platform specs (e.g., "linux" → "linux/arm64") ## Usage ```bash # Auto-detect platform spanner-mycli --embedded-emulator # Request specific platform spanner-mycli --embedded-emulator --emulator-platform linux/amd64 # Check actual platform spanner> SHOW VARIABLE CLI_EMULATOR_PLATFORM; ``` Fixes #373 Co-authored-by: Gemini Code Assist <gemini-code-assist@google.com> EOF < /dev/null
1 parent c039b34 commit 7554ab4

File tree

9 files changed

+417
-15
lines changed

9 files changed

+417
-15
lines changed

.gemini/styleguide.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,49 @@ Reference: https://go.dev/blog/loopvar-preview
3939
- Use `t.Setenv()` instead of `os.Setenv()` for better test isolation
4040
- Avoid global state modifications in tests
4141

42+
### Global State Exceptions
43+
44+
While global state modifications should generally be avoided in tests, `slog` (structured logging) is an **accepted exception** in this project because:
45+
46+
1. Many functions throughout the codebase use the global `slog` for logging
47+
2. Refactoring all functions to accept a logger parameter would be impractical
48+
3. The save/restore pattern provides adequate test isolation
49+
50+
**When modifying slog in tests, you MUST use the save/restore pattern**:
51+
52+
```go
53+
// Save the original logger
54+
oldLogger := slog.Default()
55+
defer slog.SetDefault(oldLogger)
56+
57+
// Set up test-specific logger
58+
handler := slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{
59+
Level: slog.LevelDebug,
60+
})
61+
slog.SetDefault(slog.New(handler))
62+
```
63+
64+
This pattern ensures that global logger modifications don't affect other tests, even when running in parallel.
65+
66+
### Package-level Test Functions
67+
68+
In Go, test files within the same package share the same namespace. This means that helper functions defined in one test file (e.g., `main_flags_test.go`) are accessible from other test files in the same package (e.g., `main_platform_test.go`).
69+
70+
**DO NOT** report as missing when a test uses a helper function defined in another test file of the same package:
71+
72+
```go
73+
// In main_flags_test.go
74+
func contains(s, substr string) bool {
75+
return strings.Contains(s, substr)
76+
}
77+
78+
// In main_platform_test.go (same package)
79+
// This is valid - contains() is accessible
80+
if !contains(platform, tt.wantOS) {
81+
// ...
82+
}
83+
```
84+
4285
## Code Review Focus
4386

4487
When reviewing pull requests, please focus on:

CLAUDE.md

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ spanner-mycli is a personal fork of spanner-cli, designed as an interactive comm
3737
5. **Repository merge policy**: This repository enforces **squash merge only** via Repository Ruleset - AI assistants must use `squash` method for all automated merges
3838
6. **PR merge process**: Before merging, if additional commits have been pushed since the initial review, request an updated summary by commenting `/gemini summary`. An initial summary is generated automatically, so this is only for updates. Then, use `go tool gh-helper reviews wait` (**DO NOT** use `--request-review`)
3939
7. **Squash merge commits**: MUST include descriptive summary of PR changes in squash commit message
40+
8. **GitHub comment editing**: NEVER use `gh pr comment --edit-last` - always specify the exact comment ID to avoid editing the wrong comment
4041

4142
## Essential Commands
4243

@@ -162,26 +163,31 @@ This is a simplified guide. For detailed information, refer to:
162163
1. **ALWAYS check**: [dev-docs/architecture-guide.md](dev-docs/architecture-guide.md) - Understand system architecture
163164
2. **For system variables**: [dev-docs/patterns/system-variables.md](dev-docs/patterns/system-variables.md) - Implementation patterns
164165
3. **For client statements**: [dev-docs/architecture-guide.md#client-side-statement-system](dev-docs/architecture-guide.md#client-side-statement-system) - Statement definition patterns
166+
4. **Code documentation**: When code review feedback indicates confusion about design decisions:
167+
- Add clarifying comments directly in the source code
168+
- Document the rationale for non-obvious choices
169+
- Example: If a function returns "unknown" instead of an error, explain why in comments
165170

166171
### When working with GitHub issues/PRs:
167172
1. **ALWAYS check**: [dev-docs/issue-management.md](dev-docs/issue-management.md) - Complete GitHub workflow
168-
2. **For PR labels**: [dev-docs/issue-management.md#pull-request-labels](dev-docs/issue-management.md#pull-request-labels) - Release notes categorization
169-
3. **For insights capture**: [dev-docs/issue-management.md#knowledge-management](dev-docs/issue-management.md#knowledge-management) - PR comment best practices
170-
4. **For review analysis**: Use `go tool gh-helper reviews fetch` for comprehensive feedback analysis (prevents missing critical issues)
171-
5. **For thread replies**: Use `go tool gh-helper threads reply` - Automated thread replies
172-
6. **GitHub operation priority**: Use tools in this order: `gh-helper``gh` command → GitHub MCP (API calls)
173-
7. **Sub-issue operations**: Use `gh-helper issues` commands instead of GraphQL:
173+
2. **Language requirement**: ALL GitHub communications (PRs, issues, comments, commit messages) MUST be in English
174+
3. **For PR labels**: [dev-docs/issue-management.md#pull-request-labels](dev-docs/issue-management.md#pull-request-labels) - Release notes categorization
175+
4. **For insights capture**: [dev-docs/issue-management.md#knowledge-management](dev-docs/issue-management.md#knowledge-management) - PR comment best practices
176+
5. **For review analysis**: Use `go tool gh-helper reviews fetch` for comprehensive feedback analysis (prevents missing critical issues)
177+
6. **For thread replies**: Use `go tool gh-helper threads reply` - Automated thread replies
178+
7. **GitHub operation priority**: Use tools in this order: `gh-helper``gh` command → GitHub MCP (API calls)
179+
8. **Sub-issue operations**: Use `gh-helper issues` commands instead of GraphQL:
174180
- `issues show <parent> --include-sub` - List sub-issues and check completion
175181
- `issues edit <issue> --parent <parent>` - Link as sub-issue (replaces deprecated `link-parent`)
176182
- `issues edit <issue> --parent <new> --overwrite` - Move to different parent
177183
- `issues edit <issue> --unlink-parent` - Remove parent relationship
178184
- `issues edit <issue> --after <other>` - Reorder sub-issue after another
179185
- `issues edit <issue> --position first` - Move sub-issue to beginning
180-
8. **Safe Issue/PR content handling**: ALWAYS use stdin or variables for Issue/PR creation/updates as they commonly contain code blocks with special characters (e.g., backticks, quotes, dollar signs, parentheses)
181-
9. **GitHub GraphQL API**: [docs.github.com/en/graphql](https://docs.github.com/en/graphql) - Now only needed for:
182-
- Complex custom field selections beyond gh-helper's output
183-
- Advanced queries requiring specific field combinations
184-
10. **Schema introspection**: Use `go tool github-schema` instead of GraphQL:
186+
9. **Safe Issue/PR content handling**: ALWAYS use stdin or variables for Issue/PR creation/updates as they commonly contain code blocks with special characters (e.g., backticks, quotes, dollar signs, parentheses)
187+
10. **GitHub GraphQL API**: [docs.github.com/en/graphql](https://docs.github.com/en/graphql) - Now only needed for:
188+
- Complex custom field selections beyond gh-helper's output
189+
- Advanced queries requiring specific field combinations
190+
11. **Schema introspection**: Use `go tool github-schema` instead of GraphQL:
185191
- `go tool github-schema type <TypeName>` - Show type fields and descriptions
186192
- `go tool github-schema mutation <MutationName>` - Show mutation requirements
187193
- `go tool github-schema search <pattern>` - Search for types/fields
@@ -217,6 +223,12 @@ gh issue create --body-file tmp/issue_body.md
217223
1. **ALWAYS check**: [dev-docs/development-insights.md](dev-docs/development-insights.md) - Known patterns and solutions
218224
2. **For error handling**: [dev-docs/development-insights.md#error-handling-architecture-evolution](dev-docs/development-insights.md#error-handling-architecture-evolution)
219225
3. **For resource management**: [dev-docs/development-insights.md#resource-management-in-batch-processing](dev-docs/development-insights.md#resource-management-in-batch-processing)
226+
4. **Use `go doc` commands**: When investigating Go types, interfaces, or package APIs:
227+
- `go doc <package>` - Show package documentation
228+
- `go doc <package>.<type>` - Show specific type documentation
229+
- `go doc -all <package>` - Show ALL exported identifiers (don't miss anything!)
230+
- Example: `go doc github.com/testcontainers/testcontainers-go.GenericProvider`
231+
- Example: `go doc -all github.com/testcontainers/testcontainers-go | grep Provider`
220232

221233
### When setting up development environment:
222234
1. **Start here**: Use `make worktree-setup WORKTREE_NAME=issue-123-feature` for worktree setup

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ spanner:
121121
--insecure Skip TLS verification and permit plaintext gRPC. --skip-tls-verify is an alias.
122122
--embedded-emulator Use embedded Cloud Spanner Emulator. --project, --instance, --database, --endpoint, --insecure will be automatically configured.
123123
--emulator-image= container image for --embedded-emulator
124+
--emulator-platform= Container platform (e.g. linux/amd64, linux/arm64) for embedded emulator
124125
--output-template= Filepath of output template. (EXPERIMENTAL)
125126
--log-level=
126127
--log-grpc Show gRPC logs
@@ -820,7 +821,7 @@ mutation_count: 9
820821
spanner-mycli can launch Cloud Spanner Emulator with empty database, powered by testcontainers.
821822

822823
```
823-
$ spanner-mycli --embedded-emulator [--emulator-image= gcr.io/cloud-spanner-emulator/emulator:${VERSION}]
824+
$ spanner-mycli --embedded-emulator [--emulator-image=gcr.io/cloud-spanner-emulator/emulator:${VERSION}] [--emulator-platform=linux/amd64]
824825
> SET CLI_PROMPT="%p:%i:%d%n> ";
825826
Empty set (0.00 sec)
826827

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ require (
1414
github.com/apstndb/gsqlutils v0.0.0-20250517013444-d2334c88d6ae
1515
github.com/apstndb/lox v0.0.0-20241212132733-62f24606dc82
1616
github.com/apstndb/memebridge v0.5.0
17-
github.com/apstndb/spanemuboost v0.2.10
17+
github.com/apstndb/spanemuboost v0.2.11
1818
github.com/apstndb/spannerplan v0.1.3
1919
github.com/apstndb/spantype v0.3.8
2020
github.com/apstndb/spanvalue v0.1.6

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2471,6 +2471,8 @@ github.com/apstndb/memebridge v0.5.0 h1:WaWD0Wp3Yw1BZwyvT4bIf2XsXAA+vq9ZNxUKXXV/
24712471
github.com/apstndb/memebridge v0.5.0/go.mod h1:fPrWYKcg5/eaavD6RovT9qeq8K7bDhgLKOcGhqg6zo4=
24722472
github.com/apstndb/spanemuboost v0.2.10 h1:4L0NtAIsJidTnNvI/+rSOv2+W4qYzICYC/klMzMB0x4=
24732473
github.com/apstndb/spanemuboost v0.2.10/go.mod h1:KmsJ3/u6BZSA99E5jizrfSAjN3+6MSheruMsmGpLhTQ=
2474+
github.com/apstndb/spanemuboost v0.2.11 h1:lJgklbfLo/mYfxXnIwvHn4bTwcrnFwimr0pJudERev0=
2475+
github.com/apstndb/spanemuboost v0.2.11/go.mod h1:KmsJ3/u6BZSA99E5jizrfSAjN3+6MSheruMsmGpLhTQ=
24742476
github.com/apstndb/spannerplan v0.1.3 h1:nFzTEvRL4yMzQeFdtWp3V0VaUidNA/o3T7HyIYIGk/U=
24752477
github.com/apstndb/spannerplan v0.1.3/go.mod h1:iPN9r9R2AknoFnBFqA232cUO+2ZkHpk4Q1qeSAU9xEM=
24762478
github.com/apstndb/spantype v0.3.8 h1:xy43Cclc2Hz6SL+3tZYuozOqJv6AML4Mv8cASgYo1O0=

main.go

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ import (
3636
"cloud.google.com/go/spanner"
3737
"github.com/apstndb/spanemuboost"
3838
"github.com/olekukonko/tablewriter"
39+
"github.com/testcontainers/testcontainers-go"
40+
tcspanner "github.com/testcontainers/testcontainers-go/modules/gcloud/spanner"
3941
"github.com/olekukonko/tablewriter/renderer"
4042
"github.com/olekukonko/tablewriter/tw"
4143

@@ -93,6 +95,7 @@ type spannerOptions struct {
9395
SkipTlsVerify *bool `long:"skip-tls-verify" description:"Hidden alias of --insecure from original spanner-cli" hidden:"true" default-mask:"-"`
9496
EmbeddedEmulator bool `long:"embedded-emulator" description:"Use embedded Cloud Spanner Emulator. --project, --instance, --database, --endpoint, --insecure will be automatically configured." default-mask:"-"`
9597
EmulatorImage string `long:"emulator-image" description:"container image for --embedded-emulator" default-mask:"-"`
98+
EmulatorPlatform string `long:"emulator-platform" description:"Container platform (e.g. linux/amd64, linux/arm64) for embedded emulator" default-mask:"-"`
9699
OutputTemplate string `long:"output-template" description:"Filepath of output template. (EXPERIMENTAL)" default-mask:"-"`
97100
LogLevel string `long:"log-level" default-mask:"-"`
98101
LogGrpc bool `long:"log-grpc" description:"Show gRPC logs" default-mask:"-"`
@@ -214,6 +217,168 @@ func main() {
214217
os.Exit(exitCodeSuccess)
215218
}
216219

220+
// withPlatform creates a ContainerCustomizer that sets the container platform
221+
func withPlatform(platform string) testcontainers.ContainerCustomizer {
222+
return testcontainers.CustomizeRequest(
223+
testcontainers.GenericContainerRequest{
224+
ContainerRequest: testcontainers.ContainerRequest{
225+
ImagePlatform: platform,
226+
},
227+
},
228+
)
229+
}
230+
231+
// detectContainerPlatform detects the platform of a running container
232+
// It tries multiple methods in order of preference:
233+
// 1. ImageManifestDescriptor.Platform (most accurate but not always available)
234+
// 2. Docker API image inspect (reliable when Docker socket is accessible)
235+
// 3. Basic Platform field from container (usually just OS, e.g., "linux")
236+
// Returns "unknown" if platform detection fails - this is intentional as the function
237+
// must return a string value for the system variable, and "unknown" accurately
238+
// represents that the platform could not be determined.
239+
func detectContainerPlatform(ctx context.Context, container *tcspanner.Container) string {
240+
inspectResult, err := container.Inspect(ctx)
241+
if err != nil {
242+
slog.Warn("Failed to inspect container", "error", err)
243+
// Return "unknown" rather than empty string to indicate detection attempted but failed
244+
return "unknown"
245+
}
246+
247+
// Debug log the inspect result
248+
slog.Debug("Container inspect result",
249+
"Platform", inspectResult.Platform,
250+
"ImageManifestDescriptor", inspectResult.ImageManifestDescriptor != nil,
251+
"Image", inspectResult.Image,
252+
"ConfigNotNil", inspectResult.Config != nil,
253+
)
254+
255+
// Log config details if available
256+
if inspectResult.Config != nil {
257+
slog.Debug("Container config details",
258+
"Image", inspectResult.Config.Image,
259+
"Labels", inspectResult.Config.Labels,
260+
)
261+
}
262+
263+
// Method 1: Try ImageManifestDescriptor (OCI standard)
264+
if inspectResult.ImageManifestDescriptor != nil && inspectResult.ImageManifestDescriptor.Platform != nil {
265+
p := inspectResult.ImageManifestDescriptor.Platform
266+
slog.Debug("ImageManifestDescriptor.Platform details",
267+
"OS", p.OS,
268+
"Architecture", p.Architecture,
269+
"Variant", p.Variant,
270+
"OSVersion", p.OSVersion,
271+
"OSFeatures", p.OSFeatures,
272+
)
273+
platform := p.OS + "/" + p.Architecture
274+
if p.Variant != "" {
275+
platform += "/" + p.Variant
276+
}
277+
return platform
278+
}
279+
280+
// Method 2: Try Docker API image inspect
281+
slog.Debug("ImageManifestDescriptor not available, trying image inspect")
282+
if inspectResult.Config != nil && inspectResult.Config.Image != "" {
283+
// Get provider for image inspection.
284+
// IMPORTANT: GetProvider() creates a NEW provider instance each time, not a singleton.
285+
// We must close it to avoid resource leaks.
286+
// See: https://github.com/testcontainers/testcontainers-go/blob/main/provider.go
287+
genericProvider, err := testcontainers.ProviderDefault.GetProvider()
288+
if err != nil {
289+
slog.Debug("Failed to get testcontainers provider", "error", err)
290+
} else {
291+
defer func() {
292+
if err := genericProvider.Close(); err != nil {
293+
slog.Debug("Failed to close testcontainers provider", "error", err)
294+
}
295+
}()
296+
297+
platform := inspectImagePlatform(ctx, inspectResult.Config.Image, genericProvider)
298+
if platform != "" {
299+
return platform
300+
}
301+
}
302+
}
303+
304+
// Method 3: Fallback to basic platform field
305+
if inspectResult.Platform != "" {
306+
slog.Debug("Using basic Platform field", "Platform", inspectResult.Platform)
307+
return inspectResult.Platform
308+
}
309+
310+
// All detection methods failed - return "unknown" to indicate this state
311+
return "unknown"
312+
}
313+
314+
// inspectImagePlatform uses container runtime API (Docker/Podman) to get platform information from an image.
315+
// The provider parameter should be obtained from testcontainers.ProviderDefault.GetProvider().
316+
// The caller is responsible for managing the provider lifecycle.
317+
func inspectImagePlatform(ctx context.Context, imageName string, provider testcontainers.GenericProvider) string {
318+
slog.Debug("inspectImagePlatform called", "imageName", imageName)
319+
320+
// Both Docker and Podman providers return *DockerProvider type, even for Podman.
321+
// This is because Podman provides Docker-compatible API.
322+
dockerProvider, ok := provider.(*testcontainers.DockerProvider)
323+
if !ok {
324+
slog.Debug("Provider is not a DockerProvider", "type", fmt.Sprintf("%T", provider))
325+
return ""
326+
}
327+
328+
// The provider embeds *client.Client directly (not as a field), so we can access it
329+
// using provider.Client() which returns the embedded Docker/Podman client.
330+
dockerClient := dockerProvider.Client()
331+
slog.Debug("Using container runtime client from testcontainers provider")
332+
333+
// Use a context with a timeout for Docker API calls to prevent hangs
334+
// if the container runtime is unresponsive
335+
inspectCtx, cancel := context.WithTimeout(ctx, 10*time.Second)
336+
defer cancel()
337+
338+
// Log Docker client info
339+
info, err := dockerClient.Info(inspectCtx)
340+
if err != nil {
341+
slog.Debug("Failed to get Docker info", "error", err)
342+
} else {
343+
slog.Debug("Docker client info",
344+
"ServerVersion", info.ServerVersion,
345+
"OSType", info.OSType,
346+
"Architecture", info.Architecture)
347+
}
348+
349+
imageInspect, err := dockerClient.ImageInspect(inspectCtx, imageName)
350+
if err != nil {
351+
slog.Debug("Image inspect failed",
352+
"error", err,
353+
"imageName", imageName)
354+
return ""
355+
}
356+
357+
slog.Debug("Image inspect successful",
358+
"imageName", imageName,
359+
"Architecture", imageInspect.Architecture,
360+
"Os", imageInspect.Os,
361+
"Variant", imageInspect.Variant,
362+
"ID", imageInspect.ID,
363+
)
364+
365+
// Validate that we have the required fields
366+
if imageInspect.Os == "" || imageInspect.Architecture == "" {
367+
slog.Debug("Image inspect result missing Os or Architecture",
368+
"imageName", imageName,
369+
"Os", imageInspect.Os,
370+
"Architecture", imageInspect.Architecture)
371+
return ""
372+
}
373+
374+
platform := imageInspect.Os + "/" + imageInspect.Architecture
375+
if imageInspect.Variant != "" {
376+
platform += "/" + imageInspect.Variant
377+
}
378+
slog.Debug("Returning platform", "platform", platform)
379+
return platform
380+
}
381+
217382
// run executes the main functionality of the application.
218383
// It returns an error that may contain an exit code.
219384
// Use GetExitCode(err) to determine the appropriate exit code.
@@ -270,12 +435,25 @@ func run(ctx context.Context, opts *spannerOptions) error {
270435
emulatorOpts = append(emulatorOpts, spanemuboost.EnableInstanceAutoConfigOnly())
271436
}
272437

438+
// Add platform customizer if specified
439+
if opts.EmulatorPlatform != "" {
440+
emulatorOpts = append(emulatorOpts, spanemuboost.WithContainerCustomizers(withPlatform(opts.EmulatorPlatform)))
441+
}
442+
273443
container, teardown, err := spanemuboost.NewEmulator(ctx, emulatorOpts...)
274444
if err != nil {
275445
return fmt.Errorf("failed to start Cloud Spanner Emulator: %w", err)
276446
}
277447
defer teardown()
278448

449+
// Always detect the actual platform the container is running on
450+
// The --emulator-platform flag only controls what platform is requested,
451+
// but we want to show the actual platform in CLI_EMULATOR_PLATFORM
452+
sysVars.EmulatorPlatform = detectContainerPlatform(ctx, container)
453+
slog.Debug("Detected container platform",
454+
"requested", opts.EmulatorPlatform,
455+
"actual", sysVars.EmulatorPlatform)
456+
279457
sysVars.Endpoint = container.URI()
280458
sysVars.WithoutAuthentication = true
281459
}

0 commit comments

Comments
 (0)