Skip to content

Commit ade9ce0

Browse files
JoeWang1127JoeWang1127
authored
feat(internal/librarian): rework build container contract (#863)
Rework build command container contract - Create `build-request.json` - Mount `.librarian` to `/librarian` - Mount repo root to `/repo` Fixes: #771
1 parent ca212ae commit ade9ce0

File tree

6 files changed

+102
-32
lines changed

6 files changed

+102
-32
lines changed

‎internal/config/config.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import (
2424
)
2525

2626
const (
27+
// BuildRequest is a JSON file that describes which library to build/test.
28+
BuildRequest string = "build-request.json"
2729
DefaultPushConfig string = "[email protected],Google Cloud SDK"
2830
// GeneratorInputDir is the default directory to store files that generator
2931
// needs to regenerate libraries from an empty directory.

‎internal/docker/docker.go‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,21 @@ type GenerateRequest struct {
8484
RepoDir string
8585
}
8686

87+
// BuildRequest contains all the information required for a language
88+
// container to run the build command.
89+
type BuildRequest struct {
90+
// cfg is a pointer to the [config.Config] struct, holding general configuration
91+
// values parsed from flags or environment variables.
92+
Cfg *config.Config
93+
// state is a pointer to the [config.LibrarianState] struct, representing
94+
// the overall state of the generation and release pipeline.
95+
State *config.LibrarianState
96+
// libraryID specifies the ID of the library to build
97+
LibraryID string
98+
// RepoDir is the local root directory of the language repository.
99+
RepoDir string
100+
}
101+
87102
// New constructs a Docker instance which will invoke the specified
88103
// Docker image as required to implement language-specific commands,
89104
// providing the container with required environment variables.
@@ -105,7 +120,7 @@ func New(workRoot, image, secretsProject, uid, gid string, pipelineConfig *confi
105120
// library.
106121
func (c *Docker) Generate(ctx context.Context, request *GenerateRequest) error {
107122
jsonFilePath := filepath.Join(request.RepoDir, config.LibrarianDir, config.GenerateRequest)
108-
if err := writeGenerateRequest(request.State, request.LibraryID, jsonFilePath); err != nil {
123+
if err := writeRequest(request.State, request.LibraryID, jsonFilePath); err != nil {
109124
return err
110125
}
111126
commandArgs := []string{
@@ -129,17 +144,22 @@ func (c *Docker) Generate(ctx context.Context, request *GenerateRequest) error {
129144

130145
// Build builds the library with an ID of libraryID, as configured in
131146
// the Librarian state file for the repository with a root of repoRoot.
132-
func (c *Docker) Build(ctx context.Context, cfg *config.Config, repoRoot, libraryID string) error {
147+
func (c *Docker) Build(ctx context.Context, request *BuildRequest) error {
148+
jsonFilePath := filepath.Join(request.RepoDir, config.LibrarianDir, config.BuildRequest)
149+
if err := writeRequest(request.State, request.LibraryID, jsonFilePath); err != nil {
150+
return err
151+
}
133152
mounts := []string{
134-
fmt.Sprintf("%s:/repo", repoRoot),
153+
fmt.Sprintf("%s:/librarian:ro", config.LibrarianDir),
154+
fmt.Sprintf("%s:/repo", request.RepoDir),
135155
}
136156
commandArgs := []string{
137157
"--repo-root=/repo",
138158
"--test=true",
139-
fmt.Sprintf("--library-id=%s", libraryID),
159+
fmt.Sprintf("--library-id=%s", request.LibraryID),
140160
}
141161

142-
return c.runDocker(ctx, cfg, CommandBuild, mounts, commandArgs)
162+
return c.runDocker(ctx, request.Cfg, CommandBuild, mounts, commandArgs)
143163
}
144164

145165
// Configure configures an API within a repository, either adding it to an
@@ -228,7 +248,7 @@ func (c *Docker) runCommand(cmdName string, args ...string) error {
228248
return err
229249
}
230250

231-
func writeGenerateRequest(state *config.LibrarianState, libraryID, jsonFilePath string) error {
251+
func writeRequest(state *config.LibrarianState, libraryID, jsonFilePath string) error {
232252
if err := os.MkdirAll(filepath.Dir(jsonFilePath), 0755); err != nil {
233253
return fmt.Errorf("failed to make directory: %w", err)
234254
}

‎internal/docker/docker_test.go‎

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ func TestDockerRun(t *testing.T) {
6666
testImage = "testImage"
6767
testLibraryID = "testLibraryID"
6868
testOutput = "testOutput"
69-
testRepoRoot = "testRepoRoot"
7069
)
7170

7271
state := &config.LibrarianState{}
@@ -79,6 +78,7 @@ func TestDockerRun(t *testing.T) {
7978
docker *Docker
8079
runCommand func(ctx context.Context, d *Docker) error
8180
want []string
81+
wantErr bool
8282
}{
8383
{
8484
name: "Generate",
@@ -110,6 +110,7 @@ func TestDockerRun(t *testing.T) {
110110
"--source=/source",
111111
fmt.Sprintf("--library-id=%s", testLibraryID),
112112
},
113+
wantErr: false,
113114
},
114115
{
115116
name: "Generate runs in docker",
@@ -141,24 +142,50 @@ func TestDockerRun(t *testing.T) {
141142
"--source=/source",
142143
fmt.Sprintf("--library-id=%s", testLibraryID),
143144
},
145+
wantErr: false,
144146
},
145147
{
146148
name: "Build",
147149
docker: &Docker{
148150
Image: testImage,
149151
},
150152
runCommand: func(ctx context.Context, d *Docker) error {
151-
return d.Build(ctx, cfg, testRepoRoot, testLibraryID)
153+
buildRequest := &BuildRequest{
154+
Cfg: cfg,
155+
State: state,
156+
LibraryID: testLibraryID,
157+
RepoDir: ".",
158+
}
159+
return d.Build(ctx, buildRequest)
152160
},
153161
want: []string{
154162
"run", "--rm",
155-
"-v", fmt.Sprintf("%s:/repo", testRepoRoot),
163+
"-v", ".librarian:/librarian:ro",
164+
"-v", ".:/repo",
156165
testImage,
157166
string(CommandBuild),
158167
"--repo-root=/repo",
159168
"--test=true",
160169
fmt.Sprintf("--library-id=%s", testLibraryID),
161170
},
171+
wantErr: false,
172+
},
173+
{
174+
name: "Build with invalid repo dir",
175+
docker: &Docker{
176+
Image: testImage,
177+
},
178+
runCommand: func(ctx context.Context, d *Docker) error {
179+
buildRequest := &BuildRequest{
180+
Cfg: cfg,
181+
State: state,
182+
LibraryID: testLibraryID,
183+
RepoDir: "/non-exist-dir",
184+
}
185+
return d.Build(ctx, buildRequest)
186+
},
187+
want: []string{},
188+
wantErr: true,
162189
},
163190
{
164191
name: "Configure",
@@ -178,6 +205,7 @@ func TestDockerRun(t *testing.T) {
178205
"--.librarian/generator-input=/.librarian/generator-input",
179206
fmt.Sprintf("--api=%s", testAPIPath),
180207
},
208+
wantErr: false,
181209
},
182210
} {
183211
t.Run(test.name, func(t *testing.T) {
@@ -188,9 +216,19 @@ func TestDockerRun(t *testing.T) {
188216
return nil
189217
}
190218
ctx := t.Context()
191-
if err := test.runCommand(ctx, test.docker); err != nil {
219+
err := test.runCommand(ctx, test.docker)
220+
221+
if test.wantErr {
222+
if err == nil {
223+
t.Errorf("%s should return error", test.name)
224+
}
225+
return
226+
}
227+
228+
if err != nil {
192229
t.Fatal(err)
193230
}
231+
194232
os.RemoveAll(".librarian")
195233
os.Remove(testGenerateRequest)
196234
})
@@ -263,22 +301,22 @@ func TestToGenerateRequestJSON(t *testing.T) {
263301
tempDir := t.TempDir()
264302
if test.name == "invalid_file_name" {
265303
filePath := filepath.Join(tempDir, "my\x00file.json")
266-
err := writeGenerateRequest(test.state, "google-cloud-go", filePath)
304+
err := writeRequest(test.state, "google-cloud-go", filePath)
267305
if err == nil {
268306
t.Errorf("writeGenerateRequest() expected an error but got nil")
269307
}
270308
return
271309
} else if test.expectErr {
272310
filePath := filepath.Join("/non-exist-dir", "generate-request.json")
273-
err := writeGenerateRequest(test.state, "google-cloud-go", filePath)
311+
err := writeRequest(test.state, "google-cloud-go", filePath)
274312
if err == nil {
275313
t.Errorf("writeGenerateRequest() expected an error but got nil")
276314
}
277315
return
278316
}
279317

280318
filePath := filepath.Join(tempDir, "generate-request.json")
281-
err := writeGenerateRequest(test.state, "google-cloud-go", filePath)
319+
err := writeRequest(test.state, "google-cloud-go", filePath)
282320

283321
if err != nil {
284322
t.Fatalf("writeGenerateRequest() unexpected error: %v", err)

‎internal/librarian/generate.go‎

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,10 @@ func (r *generateRunner) run(ctx context.Context) error {
177177
return nil
178178
}
179179

180-
// runGenerateCommand checks if the library exists in the remote pipeline state, if so use GenerateLibrary command
181-
// otherwise use GenerateRaw command.
182-
// In case of non-fatal error when looking up library, we will fall back to GenerateRaw command
183-
// and log the error.
184-
// If refined generation is used, the context's languageRepo field will be populated and the
185-
// library ID will be returned; otherwise, an empty string will be returned.
180+
// runGenerateCommand attempts to perform generation for an API.
181+
//
182+
// If successful, it returns the ID of the generated library; otherwise, it
183+
// returns an empty string and an error.
186184
func (r *generateRunner) runGenerateCommand(ctx context.Context, outputDir string) (string, error) {
187185
apiRoot, err := filepath.Abs(r.cfg.Source)
188186
if err != nil {
@@ -212,23 +210,35 @@ func (r *generateRunner) runGenerateCommand(ctx context.Context, outputDir strin
212210
return "", fmt.Errorf("library not found")
213211
}
214212

213+
// runBuildCommand orchestrates the building of an API library using a containerized
214+
// environment.
215+
//
216+
// The `outputDir` parameter specifies the target directory where the built artifacts
217+
// should be placed.
215218
func (r *generateRunner) runBuildCommand(ctx context.Context, outputDir, libraryID string) error {
216219
if !r.cfg.Build {
217220
slog.Info("Build flag not specified, skipping")
218221
return nil
219222
}
220-
if libraryID != "" {
221-
slog.Info("Build requested in the context of refined generation; cleaning and copying code to the local language repo before building.")
222-
// TODO(https://github.com/googleapis/librarian/issues/775)
223-
if err := os.CopyFS(r.repo.Dir, os.DirFS(outputDir)); err != nil {
224-
return err
225-
}
226-
if err := r.containerClient.Build(ctx, r.cfg, r.repo.Dir, libraryID); err != nil {
227-
return err
228-
}
223+
if libraryID == "" {
224+
slog.Warn("Cannot perform build, missing library ID")
225+
return nil
229226
}
230-
slog.Warn("Cannot perform build, missing library ID")
231-
return nil
227+
228+
buildRequest := &docker.BuildRequest{
229+
Cfg: r.cfg,
230+
State: r.state,
231+
LibraryID: libraryID,
232+
RepoDir: r.repo.Dir,
233+
}
234+
235+
slog.Info("Build requested in the context of refined generation; cleaning and copying code to the local language repo before building.")
236+
// TODO(https://github.com/googleapis/librarian/issues/775)
237+
if err := os.CopyFS(r.repo.Dir, os.DirFS(outputDir)); err != nil {
238+
return err
239+
}
240+
241+
return r.containerClient.Build(ctx, buildRequest)
232242
}
233243

234244
// detectIfLibraryConfigured returns whether a library has been configured for

‎internal/librarian/generate_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func (m *mockContainerClient) Generate(ctx context.Context, request *docker.Gene
4040
return nil
4141
}
4242

43-
func (m *mockContainerClient) Build(ctx context.Context, cfg *config.Config, buildDir, libraryID string) error {
43+
func (m *mockContainerClient) Build(ctx context.Context, request *docker.BuildRequest) error {
4444
m.buildCalls++
4545
return nil
4646
}

‎internal/librarian/librarian.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ type GitHubClient interface {
8282
// ContainerClient is an abstraction over the Docker client.
8383
type ContainerClient interface {
8484
Generate(ctx context.Context, request *docker.GenerateRequest) error
85-
Build(ctx context.Context, cfg *config.Config, repoRoot, libraryID string) error
85+
Build(ctx context.Context, request *docker.BuildRequest) error
8686
Configure(ctx context.Context, cfg *config.Config, apiRoot, apiPath, generatorInput string) error
8787
}
8888

0 commit comments

Comments
 (0)